[dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types

Xueming(Steven) Li xuemingl at mellanox.com
Wed Apr 18 16:10:45 CEST 2018



> -----Original Message-----
> From: Adrien Mazarguil <adrien.mazarguil at 6wind.com>
> Sent: Wednesday, April 18, 2018 9:26 PM
> To: Xueming(Steven) Li <xuemingl at mellanox.com>
> Cc: Shahaf Shuler <shahafs at mellanox.com>; Nelio Laranjeiro <notifications at github.com>; Wenzhuo Lu
> <wenzhuo.lu at intel.com>; Jingjing Wu <jingjing.wu at intel.com>; Thomas Monjalon <thomas at monjalon.net>;
> dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types
> 
> Hi Xueming,
> 
> On Wed, Apr 18, 2018 at 07:06:48PM +0800, Xueming Li wrote:
> > "port config all rss all" command will fail on PMD that not support
> > any of hard coding RSS hash types. This patch changed hard coding hash
> > types to supported types retrieved from device info.
> >
> > Signed-off-by: Xueming Li <xuemingl at mellanox.com>
> 
> Problem is that this patch removes the ability to request "all" RSS types regardless of PMD support.
> 
> Users will expect "all" to behave as documented, however doing so will only result in the limited set
> of types reported by PMDs. For instance at least
> mlx4 doesn't update the flow_type_rss_offloads field since it has never implemented configuration
> support for the legacy RSS API.

Keeping it "all" as it is will result in error for PMDs not completely support "all" RSS types and this 
always confuse people.
How about adding a check, if flow_type_rss_offloads not set, ignore and set default RSS "all" types?

> 
> You should add an argument with a different name (e.g. "supported" or
> "default") for that and keep the original meaning for "all".
> 
> Testpmd documentation has to be updated accordingly.
> 
> One more nit below.
> 
> > ---
> >  app/test-pmd/cmdline.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 512e3b55e..d357de7e6 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -1998,6 +1998,7 @@ cmd_config_rss_parsed(void *parsed_result,  {
> >  	struct cmd_config_rss *res = parsed_result;
> >  	struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
> > +	struct rte_eth_dev_info dev_info = {0};
> 
> This could be declared in the new block where it's used. Also note that "{0}" is nonstandard syntax,
> use memset() or initialize a field like rss_key_len above.
> 
> >  	int diag;
> >  	uint8_t i;
> >
> > @@ -2034,6 +2035,12 @@ cmd_config_rss_parsed(void *parsed_result,
> >  	}
> >  	rss_conf.rss_key = NULL;
> >  	for (i = 0; i < rte_eth_dev_count(); i++) {
> > +		if (!strcmp(res->value, "all")) {
> > +			rte_eth_dev_info_get(i, &dev_info);
> > +			if (dev_info.flow_type_rss_offloads)
> > +				rss_conf.rss_hf =
> > +					dev_info.flow_type_rss_offloads;
> > +		}
> >  		diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
> >  		if (diag < 0)
> >  			printf("Configuration of RSS hash at ethernet port %d "
> > --
> > 2.13.3
> >
> 
> 
> --
> Adrien Mazarguil
> 6WIND


More information about the dev mailing list