[RFC 0/2] queue-based flow aged report

[RFC 0/2] queue-based flow aged report

[Xiaoyu Min]

For the queue-based flow managemnt, application can operate the same flow
on the same queue.

For example, application create flow with age action, query aged flow,
and destroy aged flow. All operations of flow happens on the same queue.

In this case, the PMD could be more optimized since all operations of the
same flow happens in sequence.

The current flow aged report API hasn't queue parameter as same as other
queue-based flow management API.

This RFC propose to add new flow aged report API with queue parameter.
Keep the old one for compatibility.

Xiaoyu Min (2):
  ethdev: port flags for pre-configuration flow hints
  ethdev: queue-based flow aged report

 doc/guides/prog_guide/rte_flow.rst |  4 ++
 lib/ethdev/rte_flow.h              | 60 ++++++++++++++++++++++++++++++
 lib/ethdev/rte_flow_driver.h       |  7 ++++
 3 files changed, 71 insertions(+)

-- 
2.35.1

[RFC 1/2] ethdev: port flags for pre-configuration flow hints

[Xiaoyu Min]

The data-path focused flow rule management can manage flow rules in more
optimized way then tranditional one by using hits provided by
application in initialization phase.

In addition to the current hints we have in port attr, more hints could
be proivded by application about it's behaviour.

One example is how the application do with the same flow:
A. create/destroy flow on same queue but query flow on different queue
   or queue-less way (i.e, counter query)
B. All flow operations will be exactly on the same queue, by which PMD
   could be in more optimized way then A because resource could be
   isolated and access based on queue, without lock for example.

This patch add flag about above situation and could be extanded to cover
more situations.

Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
---
 lib/ethdev/rte_flow.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index d8827dd184..578dd837f5 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -4875,6 +4875,17 @@ rte_flow_flex_item_release(uint16_t port_id,
 			   const struct rte_flow_item_flex_handle *handle,
 			   struct rte_flow_error *error);
 
+/**
+ * The flags of rte flow port
+ */
+enum rte_flow_port_flag {
+	/**
+	 * All flow operations for one specified flow will _strictlly_ happen
+	 * on the same queue (create/destroy/query/update).
+	 */
+	RTE_FLOW_PORT_FLAG_STRICT_QUEUE = RTE_BIT32(0),
+};
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
@@ -4972,6 +4983,11 @@ struct rte_flow_port_attr {
 	 * @see RTE_FLOW_ACTION_TYPE_METER
 	 */
 	uint32_t nb_meters;
+	/**
+	 * Port flags.
+	 * @see enum rte_flow_port_flag
+	 */
+	enum rte_flow_port_flag flags;
 };
 
 /**
-- 
2.35.1

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

[Xiaoyu Min]

When application use queue-based flow management and operate the same
flow on the same queue, e.g create/destroy/query, API for querying aged
flows should also with 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
the same flow on different queues, the queue id parameter will
be ignored.

Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst |  4 +++
 lib/ethdev/rte_flow.h              | 44 ++++++++++++++++++++++++++++++
 lib/ethdev/rte_flow_driver.h       |  7 +++++
 3 files changed, 55 insertions(+)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 588914b231..d540152d74 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2963,6 +2963,10 @@ Set ageing timeout configuration to a flow.
 Event RTE_ETH_EVENT_FLOW_AGED will be reported if
 timeout passed without any matching on the flow.
 
+If queue-based flow rule management is used, when this
+even is triggered, the ret_param is set to corresponding
+flow queue.
+
 .. _table_rte_flow_action_age:
 
 .. table:: AGE
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 578dd837f5..9394fb6965 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,
 
@@ -2935,6 +2936,11 @@ struct rte_flow_action_queue {
  *
  * The flow context and the flow handle will be reported by the
  * rte_flow_get_aged_flows API.
+ *
+ * If queue-based flow rule management is used and port configured with
+ * flag RTE_FLOW_PORT_FLAG_STRICT_QUEUE, RTE_ETH_EVENT_FLOW_AGED event
+ * is triggered with ret_param set to the corresponding flow queue when
+ * a flow queue detects new aged-out flows.
  */
 struct rte_flow_action_age {
 	uint32_t timeout:24; /**< Time in seconds. */
@@ -5629,6 +5635,44 @@ rte_flow_async_action_handle_update(uint16_t port_id,
 		const void *update,
 		void *user_data,
 		struct rte_flow_error *error);
+
+/**
+ * Get aged-out flows of a given port on the given flow queue.
+ *
+ * RTE_ETH_EVENT_FLOW_AGED event will be triggered with ret_param set to
+ * corresponding flow queue when at least one new aged out flow was detected
+ * after the last call to rte_flow_get_q_aged_flows to this flow queue.
+ * 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..f617af13f6 100644
--- a/lib/ethdev/rte_flow_driver.h
+++ b/lib/ethdev/rte_flow_driver.h
@@ -260,6 +260,13 @@ struct rte_flow_ops {
 		 const void *update,
 		 void *user_data,
 		 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);
 };
 
 /**
-- 
2.35.1

RE: [RFC 1/2] ethdev: port flags for pre-configuration flow hints

[Ori Kam]

Hi Jack,

> -----Original Message-----
> From: Jack Min <jackmin@nvidia.com>
> Sent: Thursday, April 7, 2022 8:31 AM
> Subject: [RFC 1/2] ethdev: port flags for pre-configuration flow hints
> 
> The data-path focused flow rule management can manage flow rules in more
> optimized way then tranditional one by using hits provided by
> application in initialization phase.
> 
> In addition to the current hints we have in port attr, more hints could
> be proivded by application about it's behaviour.
> 
> One example is how the application do with the same flow:
> A. create/destroy flow on same queue but query flow on different queue
>    or queue-less way (i.e, counter query)
> B. All flow operations will be exactly on the same queue, by which PMD
>    could be in more optimized way then A because resource could be
>    isolated and access based on queue, without lock for example.
> 
> This patch add flag about above situation and could be extanded to cover
> more situations.
> 
> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> ---
>  lib/ethdev/rte_flow.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index d8827dd184..578dd837f5 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -4875,6 +4875,17 @@ rte_flow_flex_item_release(uint16_t port_id,
>  			   const struct rte_flow_item_flex_handle *handle,
>  			   struct rte_flow_error *error);
> 
> +/**
> + * The flags of rte flow port
> + */
> +enum rte_flow_port_flag {
> +	/**
> +	 * All flow operations for one specified flow will _strictlly_ happen
> +	 * on the same queue (create/destroy/query/update).
> +	 */
> +	RTE_FLOW_PORT_FLAG_STRICT_QUEUE = RTE_BIT32(0),
> +};
> +
>  /**
>   * @warning
>   * @b EXPERIMENTAL: this API may change without prior notice.
> @@ -4972,6 +4983,11 @@ struct rte_flow_port_attr {
>  	 * @see RTE_FLOW_ACTION_TYPE_METER
>  	 */
>  	uint32_t nb_meters;
> +	/**
> +	 * Port flags.
> +	 * @see enum rte_flow_port_flag
> +	 */
> +	enum rte_flow_port_flag flags;

Why the use of enum and not flags?
I guess there will be more flags in future, and those flags will not be related to the strict queue.

>  };
> 
>  /**
> --
> 2.35.1

Re: [RFC 1/2] ethdev: port flags for pre-configuration flow hints

[Jack Min]

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

On 4/7/22 19:27, Ori Kam wrote:
> Hi Jack,
Hey Ori,
>
>> -----Original Message-----
>> From: Jack Min<jackmin@nvidia.com>
>> Sent: Thursday, April 7, 2022 8:31 AM
>> Subject: [RFC 1/2] ethdev: port flags for pre-configuration flow hints
>>
>> The data-path focused flow rule management can manage flow rules in more
>> optimized way then tranditional one by using hits provided by
>> application in initialization phase.
>>
>> In addition to the current hints we have in port attr, more hints could
>> be proivded by application about it's behaviour.
>>
>> One example is how the application do with the same flow:
>> A. create/destroy flow on same queue but query flow on different queue
>>     or queue-less way (i.e, counter query)
>> B. All flow operations will be exactly on the same queue, by which PMD
>>     could be in more optimized way then A because resource could be
>>     isolated and access based on queue, without lock for example.
>>
>> This patch add flag about above situation and could be extanded to cover
>> more situations.
>>
>> Signed-off-by: Xiaoyu Min<jackmin@nvidia.com>
>> ---
>>   lib/ethdev/rte_flow.h | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
>> index d8827dd184..578dd837f5 100644
>> --- a/lib/ethdev/rte_flow.h
>> +++ b/lib/ethdev/rte_flow.h
>> @@ -4875,6 +4875,17 @@ rte_flow_flex_item_release(uint16_t port_id,
>>   			   const struct rte_flow_item_flex_handle *handle,
>>   			   struct rte_flow_error *error);
>>
>> +/**
>> + * The flags of rte flow port
>> + */
>> +enum rte_flow_port_flag {
>> +	/**
>> +	 * All flow operations for one specified flow will _strictlly_ happen
>> +	 * on the same queue (create/destroy/query/update).
>> +	 */
>> +	RTE_FLOW_PORT_FLAG_STRICT_QUEUE = RTE_BIT32(0),
>> +};
>> +
>>   /**
>>    * @warning
>>    * @b EXPERIMENTAL: this API may change without prior notice.
>> @@ -4972,6 +4983,11 @@ struct rte_flow_port_attr {
>>   	 * @see RTE_FLOW_ACTION_TYPE_METER
>>   	 */
>>   	uint32_t nb_meters;
>> +	/**
>> +	 * Port flags.
>> +	 * @see enum rte_flow_port_flag
>> +	 */
>> +	enum rte_flow_port_flag flags;
> Why the use of enum and not flags?
> I guess there will be more flags in future, and those flags will not be related to the strict queue.

Yes, you are right. We will have more flags, and they will not relate to 
strict queue.

I will change it to "flags".

Thank you.

>>   };
>>
>>   /**
>> --
>> 2.35.1

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

Re: [RFC 1/2] ethdev: port flags for pre-configuration flow hints

[Stephen Hemminger]

On Thu, 7 Apr 2022 13:30:46 +0800
Xiaoyu Min <jackmin@nvidia.com> wrote:

>   * @b EXPERIMENTAL: this API may change without prior notice.
> @@ -4972,6 +4983,11 @@ struct rte_flow_port_attr {
>  	 * @see RTE_FLOW_ACTION_TYPE_METER
>  	 */
>  	uint32_t nb_meters;
> +	/**
> +	 * Port flags.
> +	 * @see enum rte_flow_port_flag
> +	 */
> +	enum rte_flow_port_flag flags;

This would have to wait until 22.11 because it is ABI breakage.
Also, how would this work with old users of API?

Re: [RFC 1/2] ethdev: port flags for pre-configuration flow hints

[Jack Min]

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

On 4/7/22 23:04, Stephen Hemminger wrote:
> On Thu, 7 Apr 2022 13:30:46 +0800
> Xiaoyu Min<jackmin@nvidia.com>  wrote:
>
>>    * @b EXPERIMENTAL: this API may change without prior notice.
>> @@ -4972,6 +4983,11 @@ struct rte_flow_port_attr {
>>   	 * @see RTE_FLOW_ACTION_TYPE_METER
>>   	 */
>>   	uint32_t nb_meters;
>> +	/**
>> +	 * Port flags.
>> +	 * @see enum rte_flow_port_flag
>> +	 */
>> +	enum rte_flow_port_flag flags;
> This would have to wait until 22.11 because it is ABI breakage.
> Also, how would this work with old users of API?

I'm not familiar with DPKD API/ABI policy,

But as my understanding this one is marked as _experimental_ and also 
all related APIs

The experimental is not considered as part of ABI, and we can change 
them anytime, no?

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

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

[Thomas Monjalon]

07/04/2022 07:30, Xiaoyu Min:
> When application use queue-based flow management and operate the same
> flow on the same queue, e.g create/destroy/query, API for querying aged
> flows should also with queue id parameter just like other queue-based
> flow APIs.

A verb is missing, I am not sure to understand.

> 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
> the same flow on different queues, the queue id parameter will
> be ignored.
> 
> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst |  4 +++
>  lib/ethdev/rte_flow.h              | 44 ++++++++++++++++++++++++++++++
>  lib/ethdev/rte_flow_driver.h       |  7 +++++
>  3 files changed, 55 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 588914b231..d540152d74 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2963,6 +2963,10 @@ Set ageing timeout configuration to a flow.
>  Event RTE_ETH_EVENT_FLOW_AGED will be reported if
>  timeout passed without any matching on the flow.
>  
> +If queue-based flow rule management is used, when this
> +even is triggered, the ret_param is set to corresponding
> +flow queue.
> +
>  .. _table_rte_flow_action_age:
>  
>  .. table:: AGE
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 578dd837f5..9394fb6965 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
>   * The flow context and the flow handle will be reported by the
>   * rte_flow_get_aged_flows API.
> + *
> + * If queue-based flow rule management is used and port configured with
> + * flag RTE_FLOW_PORT_FLAG_STRICT_QUEUE, RTE_ETH_EVENT_FLOW_AGED event
> + * is triggered with ret_param set to the corresponding flow queue when
> + * a flow queue detects new aged-out flows.

Are you sure it is a good idea to use ret_param for such data?
ret_param of an event is supposed to be used by the driver
to get a confirmation from the application.

If the application needs extra info of an event,
it is better to do a separate query like rte_flow_get_aged_flows().

Re: [RFC 1/2] ethdev: port flags for pre-configuration flow hints

[Thomas Monjalon]

We were waiting for a v2 of this patch.
More comments below.

07/04/2022 07:30, Xiaoyu Min:
> The data-path focused flow rule management can manage flow rules in more
> optimized way then tranditional one by using hits provided by

hits -> hints

> application in initialization phase.
> 
> In addition to the current hints we have in port attr, more hints could
> be proivded by application about it's behaviour.

it's -> its

> One example is how the application do with the same flow:

flow -> flow rule ?

> A. create/destroy flow on same queue but query flow on different queue
>    or queue-less way (i.e, counter query)
> B. All flow operations will be exactly on the same queue, by which PMD
>    could be in more optimized way then A because resource could be
>    isolated and access based on queue, without lock for example.
> 
> This patch add flag about above situation and could be extanded to cover

extended

> more situations.
> 
> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> ---
> +/**
> + * The flags of rte flow port
> + */
> +enum rte_flow_port_flag {

Don't use enum for bit flags.

> +	/**
> +	 * All flow operations for one specified flow will _strictlly_ happen

I guess you mean "for a given flow rule"

strictlly -> _strictly_

Re: [RFC 1/2] ethdev: port flags for pre-configuration flow hints

[Jack Min]

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

On 5/31/22 00:46, Thomas Monjalon wrote:
> We were waiting for a v2 of this patch.

Hey Thomas,

Thank you for the comments.

Yes, I'll send v2.

> More comments below.
I'll update the patch in v2 accordingly.
>
> 07/04/2022 07:30, Xiaoyu Min:
>> The data-path focused flow rule management can manage flow rules in more
>> optimized way then tranditional one by using hits provided by
> hits -> hints
>
>> application in initialization phase.
>>
>> In addition to the current hints we have in port attr, more hints could
>> be proivded by application about it's behaviour.
> it's -> its
>
>> One example is how the application do with the same flow:
> flow -> flow rule ?
>
>> A. create/destroy flow on same queue but query flow on different queue
>>     or queue-less way (i.e, counter query)
>> B. All flow operations will be exactly on the same queue, by which PMD
>>     could be in more optimized way then A because resource could be
>>     isolated and access based on queue, without lock for example.
>>
>> This patch add flag about above situation and could be extanded to cover
> extended
>
>> more situations.
>>
>> Signed-off-by: Xiaoyu Min<jackmin@nvidia.com>
>> ---
>> +/**
>> + * The flags of rte flow port
>> + */
>> +enum rte_flow_port_flag {
> Don't use enum for bit flags.
>
>> +	/**
>> +	 * All flow operations for one specified flow will _strictlly_ happen
> I guess you mean "for a given flow rule"
>
> strictlly -> _strictly_
>
>
>

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

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

[Jack Min]

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

On 5/31/22 00:42, Thomas Monjalon wrote:
> 07/04/2022 07:30, Xiaoyu Min:
>> When application use queue-based flow management and operate the same
>> flow on the same queue, e.g create/destroy/query, API for querying aged
>> flows should also with queue id parameter just like other queue-based
>> flow APIs.
> A verb is missing, I am not sure to understand.
Ok, I'll re-phrase this.
>
>> 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
>> the same flow on different queues, the queue id parameter will
>> be ignored.
>>
>> Signed-off-by: Xiaoyu Min<jackmin@nvidia.com>
>> ---
>>   doc/guides/prog_guide/rte_flow.rst |  4 +++
>>   lib/ethdev/rte_flow.h              | 44 ++++++++++++++++++++++++++++++
>>   lib/ethdev/rte_flow_driver.h       |  7 +++++
>>   3 files changed, 55 insertions(+)
>>
>> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
>> index 588914b231..d540152d74 100644
>> --- a/doc/guides/prog_guide/rte_flow.rst
>> +++ b/doc/guides/prog_guide/rte_flow.rst
>> @@ -2963,6 +2963,10 @@ Set ageing timeout configuration to a flow.
>>   Event RTE_ETH_EVENT_FLOW_AGED will be reported if
>>   timeout passed without any matching on the flow.
>>   
>> +If queue-based flow rule management is used, when this
>> +even is triggered, the ret_param is set to corresponding
>> +flow queue.
>> +
>>   .. _table_rte_flow_action_age:
>>   
>>   .. table:: AGE
>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
>> index 578dd837f5..9394fb6965 100644
>> --- a/lib/ethdev/rte_flow.h
>> +++ b/lib/ethdev/rte_flow.h
>>    * The flow context and the flow handle will be reported by the
>>    * rte_flow_get_aged_flows API.
>> + *
>> + * If queue-based flow rule management is used and port configured with
>> + * flag RTE_FLOW_PORT_FLAG_STRICT_QUEUE, RTE_ETH_EVENT_FLOW_AGED event
>> + * is triggered with ret_param set to the corresponding flow queue when
>> + * a flow queue detects new aged-out flows.
> Are you sure it is a good idea to use ret_param for such data?

Well, it seems the only way to add queue information without add/change 
APIs.

> ret_param of an event is supposed to be used by the driver
> to get a confirmation from the application.
>
> If the application needs extra info of an event,
> it is better to do a separate query like rte_flow_get_aged_flows().

Ok, since the *ret_param* is supposed to be used by driver, then the 
above approach is not a good idea.

So we need a new API, something like rte_flow_get_aged_event_queues(), 
which will return

all flow queues which has the aged flows, right?

-Jack

>
>

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

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

[Thomas Monjalon]

31/05/2022 13:06, Jack Min:
> On 5/31/22 00:42, Thomas Monjalon wrote:
> > 07/04/2022 07:30, Xiaoyu Min:
> >> + * If queue-based flow rule management is used and port configured with
> >> + * flag RTE_FLOW_PORT_FLAG_STRICT_QUEUE, RTE_ETH_EVENT_FLOW_AGED event
> >> + * is triggered with ret_param set to the corresponding flow queue when
> >> + * a flow queue detects new aged-out flows.
> > 
> > Are you sure it is a good idea to use ret_param for such data?
> 
> Well, it seems the only way to add queue information without add/change 
> APIs.
> 
> > ret_param of an event is supposed to be used by the driver
> > to get a confirmation from the application.
> >
> > If the application needs extra info of an event,
> > it is better to do a separate query like rte_flow_get_aged_flows().
> 
> Ok, since the *ret_param* is supposed to be used by driver, then the 
> above approach is not a good idea.
> 
> So we need a new API, something like rte_flow_get_aged_event_queues(), 
> which will return
> 
> all flow queues which has the aged flows, right?

Yes, a new function seems required.

[RFC 0/2] queue-based flow aged report

[Xiaoyu Min]

For the queue-based flow managemnt, application can operate the same flow
on the same queue.

For example, application create flow with age action, query aged flow,
and destroy aged flow. All operations of flow happens on the same queue.

In this case, the PMD could be more optimized since all operations of the
same flow happens in sequence.

The current flow aged report API hasn't queue parameter as same as other
queue-based flow management API.

This RFC propose to add new flow aged report API with queue parameter.
Keep the old one for compatibility.

v2:
  - Add new API for querying which queues have aged out flows instead of
    passing queue id via *ret_param* in event.
  - Use bit flags instead of enum
  - Fix typo and wording

Xiaoyu Min (2):
  ethdev: port flags for pre-configuration flow hints
  ethdev: queue-based flow aged report

 lib/ethdev/rte_flow.h        | 93 ++++++++++++++++++++++++++++++++++++
 lib/ethdev/rte_flow_driver.h | 13 +++++
 2 files changed, 106 insertions(+)

-- 
2.36.1

[RFC v2 1/2] ethdev: port flags for pre-configuration flow hints

[Xiaoyu Min]

The data-path focused flow rule management can manage flow rules in more
optimized way than traditional one by using hints provided by
application in initialization phase.

In addition to the current hints we have in port attr, more hints could
be provided by application about its behaviour.

One example is how the application do with the same flow rule ?
A. create/destroy flow on same queue but query flow on different queue
   or queue-less way (i.e, counter query)
B. All flow operations will be exactly on the same queue, by which PMD
   could be in more optimized way then A because resource could be
   isolated and access based on queue, without lock, for example.

This patch add flag about above situation and could be extended to cover
more situations.

Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
---
 lib/ethdev/rte_flow.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index d8827dd184..38439fcd1d 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -4948,6 +4948,12 @@ rte_flow_info_get(uint16_t port_id,
 		  struct rte_flow_queue_info *queue_info,
 		  struct rte_flow_error *error);
 
+/**
+ * Indicate all operations for a given flow rule will _strictly_ happen
+ * on the same queue (create/destroy/query/update).
+ */
+#define RTE_FLOW_PORT_FLAG_STRICT_QUEUE RTE_BIT32(0)
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
@@ -4972,6 +4978,11 @@ struct rte_flow_port_attr {
 	 * @see RTE_FLOW_ACTION_TYPE_METER
 	 */
 	uint32_t nb_meters;
+	/**
+	 * Port flags.
+	 * @see RTE_FLOW_PORT_FLAG_STRICT_QUEUE
+	 */
+	uint32_t flags;
 };
 
 /**
-- 
2.36.1

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

[Xiaoyu Min]

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>
---
 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.
+ *
+ * @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);
 };
 
 /**
-- 
2.36.1

RE: [RFC v2 1/2] ethdev: port flags for pre-configuration flow hints

[Ori Kam]

Hi Jack,

> -----Original Message-----
> From: Jack Min <jackmin@nvidia.com>
> Sent: Wednesday, June 1, 2022 10:39 AM
> Subject: [RFC v2 1/2] ethdev: port flags for pre-configuration flow hints
> 
> The data-path focused flow rule management can manage flow rules in more
> optimized way than traditional one by using hints provided by
> application in initialization phase.
> 
> In addition to the current hints we have in port attr, more hints could
> be provided by application about its behaviour.
> 
> One example is how the application do with the same flow rule ?
> A. create/destroy flow on same queue but query flow on different queue
>    or queue-less way (i.e, counter query)
> B. All flow operations will be exactly on the same queue, by which PMD
>    could be in more optimized way then A because resource could be
>    isolated and access based on queue, without lock, for example.
> 
> This patch add flag about above situation and could be extended to cover
> more situations.
> 
> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> ---
>  lib/ethdev/rte_flow.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index d8827dd184..38439fcd1d 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -4948,6 +4948,12 @@ rte_flow_info_get(uint16_t port_id,
>  		  struct rte_flow_queue_info *queue_info,
>  		  struct rte_flow_error *error);
> 
> +/**
> + * Indicate all operations for a given flow rule will _strictly_ happen
> + * on the same queue (create/destroy/query/update).
> + */
> +#define RTE_FLOW_PORT_FLAG_STRICT_QUEUE RTE_BIT32(0)
> +
>  /**
>   * @warning
>   * @b EXPERIMENTAL: this API may change without prior notice.
> @@ -4972,6 +4978,11 @@ struct rte_flow_port_attr {
>  	 * @see RTE_FLOW_ACTION_TYPE_METER
>  	 */
>  	uint32_t nb_meters;
> +	/**
> +	 * Port flags.
> +	 * @see RTE_FLOW_PORT_FLAG_STRICT_QUEUE
> +	 */
> +	uint32_t flags;
>  };
> 
>  /**
> --
> 2.36.1


Acked-by: Ori Kam <orika@nvidia.com>
Best,
Ori

Re: [RFC v2 1/2] ethdev: port flags for pre-configuration flow hints

[Andrew Rybchenko]

Summary must not be a statement. May be:
ethdev: add strict queue to pre-configuration flow hints

On 6/1/22 10:39, Xiaoyu Min wrote:
> The data-path focused flow rule management can manage flow rules in more
> optimized way than traditional one by using hints provided by
> application in initialization phase.
> 
> In addition to the current hints we have in port attr, more hints could
> be provided by application about its behaviour.
> 
> One example is how the application do with the same flow rule ?
> A. create/destroy flow on same queue but query flow on different queue
>     or queue-less way (i.e, counter query)
> B. All flow operations will be exactly on the same queue, by which PMD
>     could be in more optimized way then A because resource could be
>     isolated and access based on queue, without lock, for example.
> 
> This patch add flag about above situation and could be extended to cover
> more situations.
> 
> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> ---
>   lib/ethdev/rte_flow.h | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index d8827dd184..38439fcd1d 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -4948,6 +4948,12 @@ rte_flow_info_get(uint16_t port_id,
>   		  struct rte_flow_queue_info *queue_info,
>   		  struct rte_flow_error *error);
>   
> +/**
> + * Indicate all operations for a given flow rule will _strictly_ happen
> + * on the same queue (create/destroy/query/update).
> + */
> +#define RTE_FLOW_PORT_FLAG_STRICT_QUEUE RTE_BIT32(0)
> +
>   /**
>    * @warning
>    * @b EXPERIMENTAL: this API may change without prior notice.
> @@ -4972,6 +4978,11 @@ struct rte_flow_port_attr {
>   	 * @see RTE_FLOW_ACTION_TYPE_METER
>   	 */
>   	uint32_t nb_meters;
> +	/**
> +	 * Port flags.
> +	 * @see RTE_FLOW_PORT_FLAG_STRICT_QUEUE

I'm not sure that it is a good idea to list flags here.
If so, it will be required to add all future flags here.
So, may be just "Port flags (RTE_FLOW_PORT_FLAG_*)"

> +	 */
> +	uint32_t flags;
>   };
>   
>   /**

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

[Andrew Rybchenko]

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>
> ---
>   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.

> + *
> + * @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);
>   };
>   
>   /**

RE: [RFC v2 1/2] ethdev: port flags for pre-configuration flow hints

[Ori Kam]

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Wednesday, June 1, 2022 9:21 PM
> Subject: Re: [RFC v2 1/2] ethdev: port flags for pre-configuration flow hints
> 
> Summary must not be a statement. May be:
> ethdev: add strict queue to pre-configuration flow hints
> 
> On 6/1/22 10:39, Xiaoyu Min wrote:
> > The data-path focused flow rule management can manage flow rules in more
> > optimized way than traditional one by using hints provided by
> > application in initialization phase.
> >
> > In addition to the current hints we have in port attr, more hints could
> > be provided by application about its behaviour.
> >
> > One example is how the application do with the same flow rule ?
> > A. create/destroy flow on same queue but query flow on different queue
> >     or queue-less way (i.e, counter query)
> > B. All flow operations will be exactly on the same queue, by which PMD
> >     could be in more optimized way then A because resource could be
> >     isolated and access based on queue, without lock, for example.
> >
> > This patch add flag about above situation and could be extended to cover
> > more situations.
> >
> > Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> > ---
> >   lib/ethdev/rte_flow.h | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
> >
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > index d8827dd184..38439fcd1d 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -4948,6 +4948,12 @@ rte_flow_info_get(uint16_t port_id,
> >   		  struct rte_flow_queue_info *queue_info,
> >   		  struct rte_flow_error *error);
> >
> > +/**
> > + * Indicate all operations for a given flow rule will _strictly_ happen
> > + * on the same queue (create/destroy/query/update).
> > + */
> > +#define RTE_FLOW_PORT_FLAG_STRICT_QUEUE RTE_BIT32(0)
> > +
> >   /**
> >    * @warning
> >    * @b EXPERIMENTAL: this API may change without prior notice.
> > @@ -4972,6 +4978,11 @@ struct rte_flow_port_attr {
> >   	 * @see RTE_FLOW_ACTION_TYPE_METER
> >   	 */
> >   	uint32_t nb_meters;
> > +	/**
> > +	 * Port flags.
> > +	 * @see RTE_FLOW_PORT_FLAG_STRICT_QUEUE
> 
> I'm not sure that it is a good idea to list flags here.
> If so, it will be required to add all future flags here.
> So, may be just "Port flags (RTE_FLOW_PORT_FLAG_*)"
> 
+1

> > +	 */
> > +	uint32_t flags;
> >   };
> >
> >   /**

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

[Ori Kam]

Hi,

> -----Original Message-----
> From: Andrew Rybchenko <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>
> > ---
> >   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.

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
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)

> > + *
> > + * @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);
> >   };
> >
> >   /**

Re: [RFC v2 1/2] ethdev: port flags for pre-configuration flow hints

[Jack Min]

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

On 6/2/22 02:20, Andrew Rybchenko wrote:
> Summary must not be a statement. May be:
> ethdev: add strict queue to pre-configuration flow hints
Ok, I'll change to this.
>
> On 6/1/22 10:39, Xiaoyu Min wrote:
>> The data-path focused flow rule management can manage flow rules in more
>> optimized way than traditional one by using hints provided by
>> application in initialization phase.
>>
>> In addition to the current hints we have in port attr, more hints could
>> be provided by application about its behaviour.
>>
>> One example is how the application do with the same flow rule ?
>> A. create/destroy flow on same queue but query flow on different queue
>>     or queue-less way (i.e, counter query)
>> B. All flow operations will be exactly on the same queue, by which PMD
>>     could be in more optimized way then A because resource could be
>>     isolated and access based on queue, without lock, for example.
>>
>> This patch add flag about above situation and could be extended to cover
>> more situations.
>>
>> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
>> ---
>>   lib/ethdev/rte_flow.h | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
>> index d8827dd184..38439fcd1d 100644
>> --- a/lib/ethdev/rte_flow.h
>> +++ b/lib/ethdev/rte_flow.h
>> @@ -4948,6 +4948,12 @@ rte_flow_info_get(uint16_t port_id,
>>             struct rte_flow_queue_info *queue_info,
>>             struct rte_flow_error *error);
>>   +/**
>> + * Indicate all operations for a given flow rule will _strictly_ happen
>> + * on the same queue (create/destroy/query/update).
>> + */
>> +#define RTE_FLOW_PORT_FLAG_STRICT_QUEUE RTE_BIT32(0)
>> +
>>   /**
>>    * @warning
>>    * @b EXPERIMENTAL: this API may change without prior notice.
>> @@ -4972,6 +4978,11 @@ struct rte_flow_port_attr {
>>        * @see RTE_FLOW_ACTION_TYPE_METER
>>        */
>>       uint32_t nb_meters;
>> +    /**
>> +     * Port flags.
>> +     * @see RTE_FLOW_PORT_FLAG_STRICT_QUEUE
>
> I'm not sure that it is a good idea to list flags here.
> If so, it will be required to add all future flags here.
Yes, it will become a lot later on.
> So, may be just "Port flags (RTE_FLOW_PORT_FLAG_*)"
Sure.
>
>> +     */
>> +    uint32_t flags;
>>   };
>>     /**
>

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

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

[Jack Min]

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

On 6/2/22 02:21, Andrew Rybchenko wrote:
> Again, summary must not be a statement.
I'l re-phrase it.
> 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>
>> ---
>>   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.
>
>> + *
>> + * @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: 11258 bytes --]

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

[Jack Min]

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

On 6/2/22 14:10, Ori Kam wrote:
> Hi,
Hello,
>
>> -----Original Message-----
>> From: Andrew Rybchenko<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>
>>> ---
>>>    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?

> 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?

>
>>> + *
>>> + * @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: 9256 bytes --]

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

[Ori Kam]

[-- 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 --]