diff mbox

[dpdk-dev,RFC,1/4] ethdev: support rss level on tunnel

Message ID 20171129173106.120828-2-xuemingl@mellanox.com (mailing list archive)
State RFC, archived
Headers

Checks

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

Commit Message

Xueming Li Nov. 29, 2017, 5:31 p.m. UTC 
  There was no RSS hash fields level definition on tunnel, implementations
default RSS on tunnel to outer or inner. Adding rss level enable users
to specifiy the tunnel level of RSS hash fields.

0:  outer most,
1:  inner,
-1: inner most(PMD auto detection if nested tunnel specified in fields)

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 lib/librte_ether/rte_flow.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Nélio Laranjeiro Nov. 30, 2017, 8:16 a.m. UTC | #1 
Hi Xueming, please see the comments below,

On Thu, Nov 30, 2017 at 01:31:03AM +0800, Xueming Li wrote:
> There was no RSS hash fields level definition on tunnel, implementations
> default RSS on tunnel to outer or inner. Adding rss level enable users
> to specifiy the tunnel level of RSS hash fields.
> 
> 0:  outer most,
> 1:  inner,
> -1: inner most(PMD auto detection if nested tunnel specified in fields)

This *inner most* is confusing, what does it mean for the following
pattern vxlan / end?
This pattern is valid for any level of the VXLAN according to the NIC
capability.  With an inner most, if the PMD support 4 levels of tunnels
it will need to create the 4 rules to match the user request.
Is this what you expect by this definition?

There is also another question, according to the possible values (0, 1,
-1), it cannot handle more than 1 level explicitly, why such limitation?

> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---
>  lib/librte_ether/rte_flow.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
> index 47c88ea..c35558c 100644
> --- a/lib/librte_ether/rte_flow.h
> +++ b/lib/librte_ether/rte_flow.h
> @@ -1078,6 +1078,7 @@ struct rte_flow_action_dup {
>   */
>  struct rte_flow_action_rss {
>  	const struct rte_eth_rss_conf *rss_conf; /**< RSS parameters. */
> +	uint8_t level; /**< RSS on tunnel level, 0:outer most, -1:inner most */
>  	uint16_t num; /**< Number of entries in queue[]. */
>  	uint16_t queue[]; /**< Queues indices to use. */
>  };
> -- 
> 1.8.3.1
> 

Thanks,
Xueming Li Nov. 30, 2017, 8:46 a.m. UTC | #2 
Thanks for feedback, comments inline:

> -----Original Message-----

> From: Nelio Laranjeiro [mailto:nelio.laranjeiro@6wind.com]

> Sent: Thursday, November 30, 2017 4:16 PM

> To: Xueming(Steven) Li <xuemingl@mellanox.com>

> Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Thomas Monjalon

> <thomas@monjalon.net>; dev@dpdk.org

> Subject: Re: [RFC 1/4] ethdev: support rss level on tunnel

> 

> Hi Xueming, please see the comments below,

> 

> On Thu, Nov 30, 2017 at 01:31:03AM +0800, Xueming Li wrote:

> > There was no RSS hash fields level definition on tunnel,

> > implementations default RSS on tunnel to outer or inner. Adding rss

> > level enable users to specifiy the tunnel level of RSS hash fields.

> >

> > 0:  outer most,

> > 1:  inner,

> > -1: inner most(PMD auto detection if nested tunnel specified in

> > fields)

> 

> This *inner most* is confusing, what does it mean for the following

> pattern vxlan / end?

> This pattern is valid for any level of the VXLAN according to the NIC

> capability.  With an inner most, if the PMD support 4 levels of tunnels it

> will need to create the 4 rules to match the user request.

> Is this what you expect by this definition?

Yes, auto detection according to tunnel spec. Users could always specify a 
concrete value, 4 for this example.

> 

> There is also another question, according to the possible values (0, 1, -

> 1), it cannot handle more than 1 level explicitly, why such limitation?

They are just typical values, any value between 0-255 is acceptable.

> 

> > Signed-off-by: Xueming Li <xuemingl@mellanox.com>

> > ---

> >  lib/librte_ether/rte_flow.h | 1 +

> >  1 file changed, 1 insertion(+)

> >

> > diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h

> > index 47c88ea..c35558c 100644

> > --- a/lib/librte_ether/rte_flow.h

> > +++ b/lib/librte_ether/rte_flow.h

> > @@ -1078,6 +1078,7 @@ struct rte_flow_action_dup {

> >   */

> >  struct rte_flow_action_rss {

> >  	const struct rte_eth_rss_conf *rss_conf; /**< RSS parameters. */

> > +	uint8_t level; /**< RSS on tunnel level, 0:outer most, -1:inner most

> > +*/

> >  	uint16_t num; /**< Number of entries in queue[]. */

> >  	uint16_t queue[]; /**< Queues indices to use. */  };

> > --

> > 1.8.3.1

> >

> 

> Thanks,

> 

> --

> Nélio Laranjeiro

> 6WIND
Nélio Laranjeiro Nov. 30, 2017, 10:14 a.m. UTC | #3 
On Thu, Nov 30, 2017 at 08:46:51AM +0000, Xueming(Steven) Li wrote:
> Thanks for feedback, comments inline:
> 
> > -----Original Message-----
> > From: Nelio Laranjeiro [mailto:nelio.laranjeiro@6wind.com]
> > Sent: Thursday, November 30, 2017 4:16 PM
> > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Thomas Monjalon
> > <thomas@monjalon.net>; dev@dpdk.org
> > Subject: Re: [RFC 1/4] ethdev: support rss level on tunnel
> > 
> > Hi Xueming, please see the comments below,
> > 
> > On Thu, Nov 30, 2017 at 01:31:03AM +0800, Xueming Li wrote:
> > > There was no RSS hash fields level definition on tunnel,
> > > implementations default RSS on tunnel to outer or inner. Adding rss
> > > level enable users to specifiy the tunnel level of RSS hash fields.
> > >
> > > 0:  outer most,
> > > 1:  inner,
> > > -1: inner most(PMD auto detection if nested tunnel specified in
> > > fields)
> > 
> > This *inner most* is confusing, what does it mean for the following
> > pattern vxlan / end?
> > This pattern is valid for any level of the VXLAN according to the NIC
> > capability.  With an inner most, if the PMD support 4 levels of tunnels it
> > will need to create the 4 rules to match the user request.
> > Is this what you expect by this definition?
> Yes, auto detection according to tunnel spec. Users could always specify a 
> concrete value, 4 for this example.

Such value must be used with care to avoid colliding rules, you should
document it.

> > 
> > There is also another question, according to the possible values (0, 1, -
> > 1), it cannot handle more than 1 level explicitly, why such limitation?
> They are just typical values, any value between 0-255 is acceptable.

Not really, according to your answer 255 is reserved for a special
definition i.e. ìnner most.  It does not mean make RSS on the 254th
tunnel.

Please update the documentation of such value with the correct possible
values and create a define for the *special* case, people implementing
such API needs to have a clear explanation on what it does.

> > > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> > > ---
> > >  lib/librte_ether/rte_flow.h | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
> > > index 47c88ea..c35558c 100644
> > > --- a/lib/librte_ether/rte_flow.h
> > > +++ b/lib/librte_ether/rte_flow.h
> > > @@ -1078,6 +1078,7 @@ struct rte_flow_action_dup {
> > >   */
> > >  struct rte_flow_action_rss {
> > >  	const struct rte_eth_rss_conf *rss_conf; /**< RSS parameters. */
> > > +	uint8_t level; /**< RSS on tunnel level, 0:outer most, -1:inner most
> > > +*/
> > >  	uint16_t num; /**< Number of entries in queue[]. */
> > >  	uint16_t queue[]; /**< Queues indices to use. */  };
> > > --
> > > 1.8.3.1
> > >
> > 
> > Thanks,
> > 
> > --
> > Nélio Laranjeiro
> > 6WIND
Xueming Li Dec. 3, 2017, 6:08 a.m. UTC | #4 
Some tunnel enhancements:
1. support GRE tunnel type
2. support L3 VXLAN tunnel type - no inner L2 header
3. introduce RSS tunnel level into rte_flow_action_rss
   RSS on inner or outer tunnel headers
4. implementation of rss tunnel level parsing in testpmd

v2:
1. Change rss default level to 0 in testpmd CLI parsing
2. Add more comments on RSS level
3. Add L3VXLAN to mbuf ptype

Xueming Li (5):
  ethdev: support rss level on tunnel
  app/testpmd: support rte_flow rss level parsing
  ethdev: support GRE and L3VXLAN tunnel type
  app/testpmd: support l3vxlan tunnel type
  mbuf: add L3 VXLAN packet type

 app/test-pmd/cmdline_flow.c      | 18 ++++++++++++++++++
 app/test-pmd/config.c            |  3 +++
 lib/librte_ether/rte_eth_ctrl.h  |  4 +++-
 lib/librte_ether/rte_ethdev.h    |  2 ++
 lib/librte_ether/rte_flow.c      |  1 +
 lib/librte_ether/rte_flow.h      | 15 +++++++++++++++
 lib/librte_mbuf/rte_mbuf_ptype.c |  1 +
 lib/librte_mbuf/rte_mbuf_ptype.h | 13 +++++++++++++
 8 files changed, 56 insertions(+), 1 deletion(-)
diff mbox

Patch

diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
index 47c88ea..c35558c 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -1078,6 +1078,7 @@  struct rte_flow_action_dup {
  */
 struct rte_flow_action_rss {
 	const struct rte_eth_rss_conf *rss_conf; /**< RSS parameters. */
+	uint8_t level; /**< RSS on tunnel level, 0:outer most, -1:inner most */
 	uint16_t num; /**< Number of entries in queue[]. */
 	uint16_t queue[]; /**< Queues indices to use. */
 };