[v5] ethdev: add flow rule group description

Message ID 20230207025732.2890157-1-rongweil@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v5] ethdev: add flow rule group description |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build fail github build: failed
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-testing fail Testing issues
ci/iol-x86_64-unit-testing fail Testing issues
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Rongwei Liu Feb. 7, 2023, 2:57 a.m. UTC
  Add more sentences to describe the group concepts
and define group 0 as root group for traffic to search a
hit rule.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 lib/ethdev/rte_flow.h | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
  

Comments

Ferruh Yigit Feb. 8, 2023, 8:28 p.m. UTC | #1
On 2/7/2023 2:57 AM, Rongwei Liu wrote:
> Add more sentences to describe the group concepts
> and define group 0 as root group for traffic to search a
> hit rule.
> 
> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>
> ---
>  lib/ethdev/rte_flow.h | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index b60987db4b..e71ac0c199 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -86,7 +86,18 @@ extern "C" {
>   * but may be valid in a few cases.
>   */
>  struct rte_flow_attr {
> -	uint32_t group; /**< Priority group. */
> +	/**
> +	 * A group is a superset of multiple rules.
> +	 * The default group is 0 and is processed for all packets.
> +	 * The group 0 of bifurcated drivers is shared with the kernel.
> +	 * Rules in other groups are processed only if the group is chained
> +	 * by a jump action from a previously matched rule.
> +	 * It means the group hierarchy is made by the flow rules,
> +	 * and the group 0 is the hierarchy root.
> +	 * Note there is no automatic dead loop protection.
> +	 * @see rte_flow_action_jump
> +	 */
> +	uint32_t group;

Hi Rongwei, Ori,

The elaborated comment looks matching with flow API documentation [1],
except there is additional information here about default group being
shared with kernel for bifurcated drivers.

Should this additional information added to the flow API documentation?



[1]
https://doc.dpdk.org/guides/prog_guide/rte_flow.html
  
Rongwei Liu Feb. 9, 2023, 2:06 a.m. UTC | #2
HI Ferruh:

BR
Rongwei

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Thursday, February 9, 2023 04:28
> To: Rongwei Liu <rongweil@nvidia.com>; dev@dpdk.org; Matan Azrad
> <matan@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Ori Kam
> <orika@nvidia.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas@monjalon.net>
> Cc: Raslan Darawsheh <rasland@nvidia.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>
> Subject: Re: [PATCH v5] ethdev: add flow rule group description
> 
> External email: Use caution opening links or attachments
> 
> 
> On 2/7/2023 2:57 AM, Rongwei Liu wrote:
> > Add more sentences to describe the group concepts and define group 0
> > as root group for traffic to search a hit rule.
> >
> > Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> > Acked-by: Ori Kam <orika@nvidia.com>
> > ---
> >  lib/ethdev/rte_flow.h | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> > b60987db4b..e71ac0c199 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -86,7 +86,18 @@ extern "C" {
> >   * but may be valid in a few cases.
> >   */
> >  struct rte_flow_attr {
> > -     uint32_t group; /**< Priority group. */
> > +     /**
> > +      * A group is a superset of multiple rules.
> > +      * The default group is 0 and is processed for all packets.
> > +      * The group 0 of bifurcated drivers is shared with the kernel.
> > +      * Rules in other groups are processed only if the group is chained
> > +      * by a jump action from a previously matched rule.
> > +      * It means the group hierarchy is made by the flow rules,
> > +      * and the group 0 is the hierarchy root.
> > +      * Note there is no automatic dead loop protection.
> > +      * @see rte_flow_action_jump
> > +      */
> > +     uint32_t group;
> 
> Hi Rongwei, Ori,
> 
> The elaborated comment looks matching with flow API documentation [1],
> except there is additional information here about default group being shared
> with kernel for bifurcated drivers.
> 
> Should this additional information added to the flow API documentation?
Agree with you, it' better to be present at NIC documents.
> 
> 
> 
> [1]
> https://doc.dpdk.org/guides/prog_guide/rte_flow.html
>
  

Patch

diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index b60987db4b..e71ac0c199 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -86,7 +86,18 @@  extern "C" {
  * but may be valid in a few cases.
  */
 struct rte_flow_attr {
-	uint32_t group; /**< Priority group. */
+	/**
+	 * A group is a superset of multiple rules.
+	 * The default group is 0 and is processed for all packets.
+	 * The group 0 of bifurcated drivers is shared with the kernel.
+	 * Rules in other groups are processed only if the group is chained
+	 * by a jump action from a previously matched rule.
+	 * It means the group hierarchy is made by the flow rules,
+	 * and the group 0 is the hierarchy root.
+	 * Note there is no automatic dead loop protection.
+	 * @see rte_flow_action_jump
+	 */
+	uint32_t group;
 	uint32_t priority; /**< Rule priority level within group. */
 	/**
 	 * The rule in question applies to ingress traffic (non-"transfer").