diff mbox

[dpdk-dev,RFC,2/4] app/testpmd: support rte_flow rss level parsing

Message ID 20171129173106.120828-3-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 
  Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 app/test-pmd/cmdline_flow.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Nélio Laranjeiro Nov. 30, 2017, 8:18 a.m. UTC | #1 
Hi Xueming,

On Thu, Nov 30, 2017 at 01:31:04AM +0800, Xueming Li wrote:
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---
>  app/test-pmd/cmdline_flow.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index df16d2a..9402eb7 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -194,6 +194,7 @@ enum index {
>  	ACTION_RSS,
>  	ACTION_RSS_QUEUES,
>  	ACTION_RSS_QUEUE,
> +	ACTION_RSS_LEVEL,
>  	ACTION_PF,
>  	ACTION_VF,
>  	ACTION_VF_ORIGINAL,
> @@ -640,6 +641,7 @@ struct parse_action_priv {
>  
>  static const enum index action_rss[] = {
>  	ACTION_RSS_QUEUES,
> +	ACTION_RSS_LEVEL,
>  	ACTION_NEXT,
>  	ZERO,
>  };
> @@ -1586,6 +1588,13 @@ static int comp_vc_action_rss_queue(struct context *, const struct token *,
>  		.call = parse_vc_action_rss_queue,
>  		.comp = comp_vc_action_rss_queue,
>  	},
> +	[ACTION_RSS_LEVEL] = {
> +		.name = "level",
> +		.help = "rss on tunnel level",
> +		.next = NEXT(action_rss, NEXT_ENTRY(UNSIGNED)),
> +		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_rss, level)),
> +		.call = parse_vc_conf,
> +	},
>  	[ACTION_PF] = {
>  		.name = "pf",
>  		.help = "redirect packets to physical device function",
> @@ -1887,6 +1896,7 @@ static int comp_vc_action_rss_queue(struct context *, const struct token *,
>  					       sizeof(double));
>  		if ((uint8_t *)item + sizeof(*item) > data)
>  			return -1;
> +		memset(data, 0, data_size);
>  		*item = (struct rte_flow_item){
>  			.type = priv->type,
>  		};
> @@ -1904,6 +1914,9 @@ static int comp_vc_action_rss_queue(struct context *, const struct token *,
>  					       sizeof(double));
>  		if ((uint8_t *)action + sizeof(*action) > data)
>  			return -1;
> +		memset(data, 0, data_size);
> +		if (priv->type == RTE_FLOW_ACTION_TYPE_RSS)
> +			((struct rte_flow_action_rss *)data)->level = -1;

I strongly suggest to let the level set to outer, otherwise most of the
PMD will have to refuse such rule.

>  		*action = (struct rte_flow_action){
>  			.type = priv->type,
>  		};
> @@ -1911,7 +1924,6 @@ static int comp_vc_action_rss_queue(struct context *, const struct token *,
>  		ctx->object = action;
>  		ctx->objmask = NULL;
>  	}
> -	memset(data, 0, data_size);
>  	out->args.vc.data = data;
>  	ctx->objdata = data_size;
>  	return len;
> -- 
> 1.8.3.1
>
Xueming Li Nov. 30, 2017, 8:50 a.m. UTC | #2 
> -----Original Message-----

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

> Sent: Thursday, November 30, 2017 4:18 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 2/4] app/testpmd: support rte_flow rss level parsing

> 

> Hi Xueming,

> 

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

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

> > ---

> >  app/test-pmd/cmdline_flow.c | 14 +++++++++++++-

> >  1 file changed, 13 insertions(+), 1 deletion(-)

> >

> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c

> > index df16d2a..9402eb7 100644

> > --- a/app/test-pmd/cmdline_flow.c

> > +++ b/app/test-pmd/cmdline_flow.c

> > @@ -194,6 +194,7 @@ enum index {

> >  	ACTION_RSS,

> >  	ACTION_RSS_QUEUES,

> >  	ACTION_RSS_QUEUE,

> > +	ACTION_RSS_LEVEL,

> >  	ACTION_PF,

> >  	ACTION_VF,

> >  	ACTION_VF_ORIGINAL,

> > @@ -640,6 +641,7 @@ struct parse_action_priv {

> >

> >  static const enum index action_rss[] = {

> >  	ACTION_RSS_QUEUES,

> > +	ACTION_RSS_LEVEL,

> >  	ACTION_NEXT,

> >  	ZERO,

> >  };

> > @@ -1586,6 +1588,13 @@ static int comp_vc_action_rss_queue(struct

> context *, const struct token *,

> >  		.call = parse_vc_action_rss_queue,

> >  		.comp = comp_vc_action_rss_queue,

> >  	},

> > +	[ACTION_RSS_LEVEL] = {

> > +		.name = "level",

> > +		.help = "rss on tunnel level",

> > +		.next = NEXT(action_rss, NEXT_ENTRY(UNSIGNED)),

> > +		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_rss, level)),

> > +		.call = parse_vc_conf,

> > +	},

> >  	[ACTION_PF] = {

> >  		.name = "pf",

> >  		.help = "redirect packets to physical device function", @@ -

> 1887,6

> > +1896,7 @@ static int comp_vc_action_rss_queue(struct context *, const

> struct token *,

> >  					       sizeof(double));

> >  		if ((uint8_t *)item + sizeof(*item) > data)

> >  			return -1;

> > +		memset(data, 0, data_size);

> >  		*item = (struct rte_flow_item){

> >  			.type = priv->type,

> >  		};

> > @@ -1904,6 +1914,9 @@ static int comp_vc_action_rss_queue(struct context

> *, const struct token *,

> >  					       sizeof(double));

> >  		if ((uint8_t *)action + sizeof(*action) > data)

> >  			return -1;

> > +		memset(data, 0, data_size);

> > +		if (priv->type == RTE_FLOW_ACTION_TYPE_RSS)

> > +			((struct rte_flow_action_rss *)data)->level = -1;

> 

> I strongly suggest to let the level set to outer, otherwise most of the

> PMD will have to refuse such rule.

Default to outer rss? Would be glad to hear more suggestion on this.

> 

> >  		*action = (struct rte_flow_action){

> >  			.type = priv->type,

> >  		};

> > @@ -1911,7 +1924,6 @@ static int comp_vc_action_rss_queue(struct context

> *, const struct token *,

> >  		ctx->object = action;

> >  		ctx->objmask = NULL;

> >  	}

> > -	memset(data, 0, data_size);

> >  	out->args.vc.data = data;

> >  	ctx->objdata = data_size;

> >  	return len;

> > --

> > 1.8.3.1

> >

> 

> --

> Nélio Laranjeiro

> 6WIND
Nélio Laranjeiro Nov. 30, 2017, 10:13 a.m. UTC | #3 
On Thu, Nov 30, 2017 at 08:50:13AM +0000, Xueming(Steven) Li wrote:
> 
> 
> > -----Original Message-----
> > From: Nelio Laranjeiro [mailto:nelio.laranjeiro@6wind.com]
> > Sent: Thursday, November 30, 2017 4:18 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 2/4] app/testpmd: support rte_flow rss level parsing
> > 
> > Hi Xueming,
> > 
> > On Thu, Nov 30, 2017 at 01:31:04AM +0800, Xueming Li wrote:
> > > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> > > ---
> > >  app/test-pmd/cmdline_flow.c | 14 +++++++++++++-
> > >  1 file changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > > index df16d2a..9402eb7 100644
> > > --- a/app/test-pmd/cmdline_flow.c
> > > +++ b/app/test-pmd/cmdline_flow.c
> > > @@ -194,6 +194,7 @@ enum index {
> > >  	ACTION_RSS,
> > >  	ACTION_RSS_QUEUES,
> > >  	ACTION_RSS_QUEUE,
> > > +	ACTION_RSS_LEVEL,
> > >  	ACTION_PF,
> > >  	ACTION_VF,
> > >  	ACTION_VF_ORIGINAL,
> > > @@ -640,6 +641,7 @@ struct parse_action_priv {
> > >
> > >  static const enum index action_rss[] = {
> > >  	ACTION_RSS_QUEUES,
> > > +	ACTION_RSS_LEVEL,
> > >  	ACTION_NEXT,
> > >  	ZERO,
> > >  };
> > > @@ -1586,6 +1588,13 @@ static int comp_vc_action_rss_queue(struct
> > context *, const struct token *,
> > >  		.call = parse_vc_action_rss_queue,
> > >  		.comp = comp_vc_action_rss_queue,
> > >  	},
> > > +	[ACTION_RSS_LEVEL] = {
> > > +		.name = "level",
> > > +		.help = "rss on tunnel level",
> > > +		.next = NEXT(action_rss, NEXT_ENTRY(UNSIGNED)),
> > > +		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_rss, level)),
> > > +		.call = parse_vc_conf,
> > > +	},
> > >  	[ACTION_PF] = {
> > >  		.name = "pf",
> > >  		.help = "redirect packets to physical device function", @@ -
> > 1887,6
> > > +1896,7 @@ static int comp_vc_action_rss_queue(struct context *, const
> > struct token *,
> > >  					       sizeof(double));
> > >  		if ((uint8_t *)item + sizeof(*item) > data)
> > >  			return -1;
> > > +		memset(data, 0, data_size);
> > >  		*item = (struct rte_flow_item){
> > >  			.type = priv->type,
> > >  		};
> > > @@ -1904,6 +1914,9 @@ static int comp_vc_action_rss_queue(struct context
> > *, const struct token *,
> > >  					       sizeof(double));
> > >  		if ((uint8_t *)action + sizeof(*action) > data)
> > >  			return -1;
> > > +		memset(data, 0, data_size);
> > > +		if (priv->type == RTE_FLOW_ACTION_TYPE_RSS)
> > > +			((struct rte_flow_action_rss *)data)->level = -1;
> > 
> > I strongly suggest to let the level set to outer, otherwise most of the
> > PMD will have to refuse such rule.
> Default to outer rss? Would be glad to hear more suggestion on this.

The point is, not a single NIC will react the same way with testpmd with
such value as default.
NIC with RSS inner capability will start spreading tunnel packets
differently than the other who don't have it.
Default value should be set to have the same default on all NIC.

> > >  		*action = (struct rte_flow_action){
> > >  			.type = priv->type,
> > >  		};
> > > @@ -1911,7 +1924,6 @@ static int comp_vc_action_rss_queue(struct context
> > *, const struct token *,
> > >  		ctx->object = action;
> > >  		ctx->objmask = NULL;
> > >  	}
> > > -	memset(data, 0, data_size);
> > >  	out->args.vc.data = data;
> > >  	ctx->objdata = data_size;
> > >  	return len;
> > > --
> > > 1.8.3.1
diff mbox

Patch

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index df16d2a..9402eb7 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -194,6 +194,7 @@  enum index {
 	ACTION_RSS,
 	ACTION_RSS_QUEUES,
 	ACTION_RSS_QUEUE,
+	ACTION_RSS_LEVEL,
 	ACTION_PF,
 	ACTION_VF,
 	ACTION_VF_ORIGINAL,
@@ -640,6 +641,7 @@  struct parse_action_priv {
 
 static const enum index action_rss[] = {
 	ACTION_RSS_QUEUES,
+	ACTION_RSS_LEVEL,
 	ACTION_NEXT,
 	ZERO,
 };
@@ -1586,6 +1588,13 @@  static int comp_vc_action_rss_queue(struct context *, const struct token *,
 		.call = parse_vc_action_rss_queue,
 		.comp = comp_vc_action_rss_queue,
 	},
+	[ACTION_RSS_LEVEL] = {
+		.name = "level",
+		.help = "rss on tunnel level",
+		.next = NEXT(action_rss, NEXT_ENTRY(UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_rss, level)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_PF] = {
 		.name = "pf",
 		.help = "redirect packets to physical device function",
@@ -1887,6 +1896,7 @@  static int comp_vc_action_rss_queue(struct context *, const struct token *,
 					       sizeof(double));
 		if ((uint8_t *)item + sizeof(*item) > data)
 			return -1;
+		memset(data, 0, data_size);
 		*item = (struct rte_flow_item){
 			.type = priv->type,
 		};
@@ -1904,6 +1914,9 @@  static int comp_vc_action_rss_queue(struct context *, const struct token *,
 					       sizeof(double));
 		if ((uint8_t *)action + sizeof(*action) > data)
 			return -1;
+		memset(data, 0, data_size);
+		if (priv->type == RTE_FLOW_ACTION_TYPE_RSS)
+			((struct rte_flow_action_rss *)data)->level = -1;
 		*action = (struct rte_flow_action){
 			.type = priv->type,
 		};
@@ -1911,7 +1924,6 @@  static int comp_vc_action_rss_queue(struct context *, const struct token *,
 		ctx->object = action;
 		ctx->objmask = NULL;
 	}
-	memset(data, 0, data_size);
 	out->args.vc.data = data;
 	ctx->objdata = data_size;
 	return len;