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

Message ID 9b673505092754dca22df7939cc009930864b45c.1649308627.git.jackmin@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series queue-based flow aged report |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Jack Min April 7, 2022, 5:30 a.m. UTC
  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(+)
  

Comments

Thomas Monjalon May 30, 2022, 4:42 p.m. UTC | #1
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().
  
Jack Min May 31, 2022, 11:06 a.m. UTC | #2
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

>
>
  
Thomas Monjalon May 31, 2022, 12:57 p.m. UTC | #3
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.
  

Patch

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