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

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

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 

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://mails.dpdk.org/archives/dev/2018-November/118786.html

-- 
Adrien Mazarguil
6WIND