DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ori Kam <orika@nvidia.com>
To: Jack Min <jackmin@nvidia.com>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	"NBU-Contact-Thomas Monjalon (EXTERNAL)" <thomas@monjalon.net>,
	Ferruh Yigit <ferruh.yigit@xilinx.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: RE: [RFC v2 2/2] ethdev: queue-based flow aged report
Date: Mon, 6 Jun 2022 09:47:56 +0000	[thread overview]
Message-ID: <MW2PR12MB4666719111ADC1877DCC4021D6A29@MW2PR12MB4666.namprd12.prod.outlook.com> (raw)
In-Reply-To: <c4a7d98d-ae6e-34a3-0c2c-b363eb6606b6@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 8917 bytes --]

Hi,
For some reason this mail stopped being plain text.
Pleas find my comment marked with [Ori]

Best,
Ori

From: Jack Min <jackmin@nvidia.com>
Sent: Thursday, June 2, 2022 1:24 PM
To: Ori Kam <orika@nvidia.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@xilinx.com>
Cc: dev@dpdk.org
Subject: Re: [RFC v2 2/2] ethdev: queue-based flow aged report

On 6/2/22 14:10, Ori Kam wrote:

Hi,
Hello,






-----Original Message-----

From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru><mailto:andrew.rybchenko@oktetlabs.ru>

Sent: Wednesday, June 1, 2022 9:21 PM

Subject: Re: [RFC v2 2/2] ethdev: queue-based flow aged report



Again, summary must not be a statement.



On 6/1/22 10:39, Xiaoyu Min wrote:

When application use queue-based flow rule management and operate the

same flow rule on the same queue, e.g create/destroy/query, API of

querying aged flow rules should also have queue id parameter just like

other queue-based flow APIs.



By this way, PMD can work in more optimized way since resources are

isolated by queue and needn't synchronize.



If application do use queue-based flow management but configure port

without RTE_FLOW_PORT_FLAG_STRICT_QUEUE, which means application operate

a given flow rule on different queues, the queue id parameter will

be ignored.



In addition to the above change, another new API is added which help the

application get information about which queues have aged out flows after

RTE_ETH_EVENT_FLOW_AGED event received. The queried queue id can be

used in the above queue based query aged flows API.



Signed-off-by: Xiaoyu Min <jackmin@nvidia.com><mailto:jackmin@nvidia.com>

---

  lib/ethdev/rte_flow.h        | 82 ++++++++++++++++++++++++++++++++++++

  lib/ethdev/rte_flow_driver.h | 13 ++++++

  2 files changed, 95 insertions(+)



diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h

index 38439fcd1d..a12becfe3b 100644

--- a/lib/ethdev/rte_flow.h

+++ b/lib/ethdev/rte_flow.h

@@ -2810,6 +2810,7 @@ enum rte_flow_action_type {

     * See function rte_flow_get_aged_flows

     * see enum RTE_ETH_EVENT_FLOW_AGED

     * See struct rte_flow_query_age

+    * See function rte_flow_get_q_aged_flows

     */

   RTE_FLOW_ACTION_TYPE_AGE,



@@ -5624,6 +5625,87 @@ rte_flow_async_action_handle_update(uint16_t port_id,

            const void *update,

            void *user_data,

            struct rte_flow_error *error);

+

+/**

+ * @warning

+ * @b EXPERIMENTAL: this API may change without prior notice.

+ *

+ * Get flow queues which have aged out flows on a given port.

+ *

+ * The application can use this function to query which queues have aged out flows after

+ * a RTE_ETH_EVENT_FLOW_AGED event is received so the returned queue id can be used to

+ * get aged out flows on this given queue by call rte_flow_get_q_aged_flows.

+ *

+ * This function can be called from the event callback or synchronously regardless of the event.

+ *

+ * @param port_id

+ *   Port identifier of Ethernet device.

+ * @param[in, out] queue_id

+ *   Array of queue id that will be set.

+ * @param[in] nb_queue_id

+ *   Maximum number of the queue id that can be returned.

+ *   This value should be equal to the size of the queue_id array.

+ * @param[out] error

+ *   Perform verbose error reporting if not NULL. Initialized in case of

+ *   error only.

+ *

+ * @return

+ *   if nb_queue_id is 0, return the amount of all queues which have aged out flows.

+ *   if nb_queue_id is not 0 , return the amount of queues which have aged out flows

+ *   reported in the queue_id array, otherwise negative errno value.



I'm sorry, but it is unclear for me what happens if provided array is

insufficient to return all queues. IMHO, we still should provide as

much as we can. The question is how to report that we have more queues.

It looks like the only sensible way is to return value greater than

nb_queue_id.



I think that just like any other function, this function should return the max based on the requested number.

Returning bigger number may result in out of buf issues, or require extra validation step from application.

In addition as far as I can see the common practice in DPDK is to return the requested number.
Yes, it just likes other functions.




I have other concern with this function, from my understanding this function will be called on the service thread

that handels the aging event, after calling this function the application still needs to propagate the event to the

correct threads.

I think it will be better if the event itself will hold which queue triggered the aging. Or even better to get the

As discussed in v1, there seems no good place in the current callback function to pass this kind of information from driver to application.

Or you have a better idea?

[Ori] Maybe use the new queues, for example maybe application can get the notification as part of the polling function.
maybe it can even get the aged rules.



notification on the correct thread. (I know it is much more complicated but maybe it is worth the effort,

since this can be used for other cases)

Yes this will be better but I don't really know how to inform the correct thread.

Or we just let application register callback per flow queue?

Something like: rte_flow_queue_callback_register(), and PMD call rte_flow_queue_callback_process() to invoke

the callback functions registered to this queue only?

[Ori] that is a good suggestion or the one I suggested above. And we can also improve the current event, the question is what
are the advantages of any approach and what are the downsides.







+ *

+ * @see rte_flow_action_age

+ * @see RTE_ETH_EVENT_FLOW_AGED

+ */

+

+__rte_experimental

+int

+rte_flow_get_aged_queues(uint16_t port_id, uint32_t queue_id[], uint32_t nb_queue_id,

+                   struct rte_flow_error *error);

+

+/**

+ * @warning

+ * @b EXPERIMENTAL: this API may change without prior notice.

+ *

+ * Get aged-out flows of a given port on the given flow queue.

+ *

+ * RTE_ETH_EVENT_FLOW_AGED event will be triggered at least one new aged out flow was

+ * detected on any flow queue after the last call to rte_flow_get_q_aged_flows.

+ *

+ * The application can use rte_flow_get_aged_queues to query which queues have aged

+ * out flows after RTE_ETH_EVEN_FLOW_AGED event.

+ *

+ * If application configure port attribute without RTE_FLOW_PORT_FLAG_STRICT_QUEUE

+ * the @p queue_id will be ignored.

+ * This function can be called to get the aged flows asynchronously from the

+ * event callback or synchronously regardless the event.

+ *

+ * @param port_id

+ *   Port identifier of Ethernet device.

+ * @param queue_id

+ *   Flow queue to query. Ignored when RTE_FLOW_PORT_FLAG_STRICT_QUEUE not set.

+ * @param[in, out] contexts

+ *   The address of an array of pointers to the aged-out flows contexts.

+ * @param[in] nb_contexts

+ *   The length of context array pointers.

+ * @param[out] error

+ *   Perform verbose error reporting if not NULL. Initialized in case of

+ *   error only.

+ *

+ * @return

+ *   if nb_contexts is 0, return the amount of all aged contexts.

+ *   if nb_contexts is not 0 , return the amount of aged flows reported

+ *   in the context array, otherwise negative errno value.

+ *

+ * @see rte_flow_action_age

+ * @see RTE_ETH_EVENT_FLOW_AGED

+ * @see rte_flow_port_flag

+ */

+

+__rte_experimental

+int

+rte_flow_get_q_aged_flows(uint16_t port_id, uint32_t queue_id, void **contexts,

+                    uint32_t nb_contexts, struct rte_flow_error *error);

  #ifdef __cplusplus

  }

  #endif

diff --git a/lib/ethdev/rte_flow_driver.h b/lib/ethdev/rte_flow_driver.h

index 2bff732d6a..b665170bf4 100644

--- a/lib/ethdev/rte_flow_driver.h

+++ b/lib/ethdev/rte_flow_driver.h

@@ -260,6 +260,19 @@ struct rte_flow_ops {

             const void *update,

             void *user_data,

             struct rte_flow_error *error);

+   /** See rte_flow_get_aged_queues() */

+   int (*get_aged_queues)

+       (uint16_t port_id,

+            uint32_t queue_id[],

+            uint32_t nb_queue_id,

+            struct rte_flow_error *error);

+   /** See rte_flow_get_q_aged_flows() */

+   int (*get_q_aged_flows)

+       (uint16_t port_id,

+            uint32_t queue_id,

+            void **contexts,

+            uint32_t nb_contexts,

+            struct rte_flow_error *error);

  };



  /**



[-- Attachment #2: Type: text/html, Size: 17364 bytes --]

  reply	other threads:[~2022-06-06  9:48 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-07  5:30 [RFC 0/2] " Xiaoyu Min
2022-04-07  5:30 ` [RFC 1/2] ethdev: port flags for pre-configuration flow hints Xiaoyu Min
2022-04-07 11:27   ` Ori Kam
2022-04-07 13:01     ` Jack Min
2022-04-07 15:04   ` Stephen Hemminger
2022-04-08  2:35     ` Jack Min
2022-05-30 16:46   ` Thomas Monjalon
2022-05-31 10:47     ` Jack Min
2022-04-07  5:30 ` [RFC 2/2] ethdev: queue-based flow aged report Xiaoyu Min
2022-05-30 16:42   ` Thomas Monjalon
2022-05-31 11:06     ` Jack Min
2022-05-31 12:57       ` Thomas Monjalon
2022-06-01  7:39 ` [RFC 0/2] " Xiaoyu Min
2022-06-01  7:39   ` [RFC v2 1/2] ethdev: port flags for pre-configuration flow hints Xiaoyu Min
2022-06-01  9:03     ` Ori Kam
2022-06-01 18:20     ` Andrew Rybchenko
2022-06-02  5:50       ` Ori Kam
2022-06-02  9:38       ` Jack Min
2022-06-01  7:39   ` [RFC v2 2/2] ethdev: queue-based flow aged report Xiaoyu Min
2022-06-01 18:21     ` Andrew Rybchenko
2022-06-02  6:10       ` Ori Kam
2022-06-02 10:23         ` Jack Min
2022-06-06  9:47           ` Ori Kam [this message]
2022-06-02  9:39       ` Jack Min

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MW2PR12MB4666719111ADC1877DCC4021D6A29@MW2PR12MB4666.namprd12.prod.outlook.com \
    --to=orika@nvidia.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@xilinx.com \
    --cc=jackmin@nvidia.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).