Re: [PATCH v2] ethdev: document RSS default key and types

[Ophir Munk <ophirmu@mellanox.com>]

> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Tuesday, November 13, 2018 7:15 PM
> To: Ori Kam <orika@mellanox.com>
> Cc: Ophir Munk <ophirmu@mellanox.com>; Yongseok Koh
> <yskoh@mellanox.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>; Ferruh Yigit <ferruh.yigit@intel.com>;
> dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Asaf Penso
> <asafp@mellanox.com>; Shahaf Shuler <shahafs@mellanox.com>; Olga
> Shern <olgas@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH v2] ethdev: document RSS default key and
> types
> 
> Again a bit late to the party, please see below.
> 
> On Sun, Nov 11, 2018 at 09:35:22AM +0000, Ori Kam wrote:
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Ophir Munk
> > > Sent: Friday, November 9, 2018 10:14 AM
> > > To: Yongseok Koh <yskoh@mellanox.com>; Adrien Mazarguil
> > > <adrien.mazarguil@6wind.com>; Andrew Rybchenko
> > > <arybchenko@solarflare.com>
> > > Cc: Ferruh Yigit <ferruh.yigit@intel.com>; dev@dpdk.org; Thomas
> > > Monjalon <thomas@monjalon.net>; Asaf Penso <asafp@mellanox.com>;
> > > Shahaf Shuler <shahafs@mellanox.com>; Olga Shern
> > > <olgas@mellanox.com>
> > > Subject: Re: [dpdk-dev] [PATCH v2] ethdev: document RSS default key
> > > and types
> > >
> > > > -----Original Message-----
> > > > From: Yongseok Koh
> > > > Sent: Friday, November 09, 2018 1:07 AM
> > > > To: Ophir Munk <ophirmu@mellanox.com>; Adrien Mazarguil
> > > > <adrien.mazarguil@6wind.com>; Andrew Rybchenko
> > > > <arybchenko@solarflare.com>
> > > > Cc: Ferruh Yigit <ferruh.yigit@intel.com>; dev@dpdk.org; Thomas
> > > > Monjalon <thomas@monjalon.net>; Asaf Penso
> <asafp@mellanox.com>;
> > > > Shahaf Shuler <shahafs@mellanox.com>; Olga Shern
> > > > <olgas@mellanox.com>
> > > > Subject: Re: [dpdk-dev] [PATCH v2] ethdev: document RSS default
> > > > key and types
> > > >
> <snip>
> > > >
> > > > -               if (src.rss->key_len) {
> > > > +               if (src.rss->key && src.rss->key_len) {
> > > >
> > > > but looks like we should conclude this thread first?
> > > > Or, does the fix make any sense regardless of having key_len=0 or
> > > > key=null for default key?
> > > > Having more sanity check is no harm usually...
> > > >
> > > >
> > > > Thanks,
> > > > Yongseok
> > > >
> > >
> > > The setfault is the result of commit a4391f8bae ("app/testpmd: set
> > > default RSS key as null").
> > > Reverting this commit should fix the segfault but it also means
> > > there is no way to set default key (key=NULL) with testpmd.
> > > Need to check if this is only a testpmd limitation and not all
> > > applications limitation.
> > >
> > > We should decide how an application can set default RSS without
> > > knowing anything about keys.
> > >
> >
> > I agree with Adrian that the main criteria should be the length.
> > Maybe the set default RSS in testpmd should get new parameter.
> 
> Since [1] was reverted and we seem to agree that a zero key_len should
> trigger a PMD-specific default key, this can already be requested with
> testpmd by overriding key_len, e.g.:
> 
>  flow create 1 pattern eth / end actions rss key_len 0 / end
> 

Not all agree that zero key_len should trigger default key :)
The suggestion is to require applications to set both key=NULL and key_len==0.
Individual PMDs will have the option to base default RSS on key_len, key or both.

> Using an empty string as the key would yield the same result but cannot be
> expressed on the command line yet. Note that specifying a key automatically
> overrides key_len, so key_len must be forced to 0 last to get PMD defaults:
> 
>  flow create 1 pattern eth / end actions rss key foo key_len 0 / end
> 
> Here key_len is set to testpmd's default size when parsing "rss", updated to
> 3 when parsing "key foo" and updated once again when parsing "key_len 0".
> 
> Lastly, while it would make sense for testpmd to use 0 as the default value,
> doing so yields inconsistent balancing results between vendors/devices as
> they all come with a different key. Same reason as initializing the RSS types
> field to the global rss_hf instead of 0.
> 
> [1] "app/testpmd: revert setting default RSS"
> 
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai
> ls.dpdk.org%2Farchives%2Fdev%2F2018-
> November%2F118786.html&amp;data=02%7C01%7Cophirmu%40mellanox.c
> om%7Ce765a4cb2c8b4bb1547808d6498ba2a5%7Ca652971c7d2e4d9ba6a4d
> 149256f461b%7C0%7C0%7C636777261426945159&amp;sdata=k1NXLzaQeK
> fp3yptZQsJd6w0NA%2FUVRb8rctLdkDPIv8%3D&amp;reserved=0
> 
> --
> Adrien Mazarguil
> 6WIND