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-boun...@dpdk.org> On Behalf Of Ophir Munk
> > Sent: Friday, November 9, 2018 10:14 AM
> > To: Yongseok Koh <ys...@mellanox.com>; Adrien Mazarguil
> > <adrien.mazarg...@6wind.com>; Andrew Rybchenko
> > <arybche...@solarflare.com>
> > Cc: Ferruh Yigit <ferruh.yi...@intel.com>; dev@dpdk.org; Thomas Monjalon
> > <tho...@monjalon.net>; Asaf Penso <as...@mellanox.com>; Shahaf Shuler
> > <shah...@mellanox.com>; Olga Shern <ol...@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 <ophi...@mellanox.com>; Adrien Mazarguil
> > > <adrien.mazarg...@6wind.com>; Andrew Rybchenko
> > > <arybche...@solarflare.com>
> > > Cc: Ferruh Yigit <ferruh.yi...@intel.com>; dev@dpdk.org; Thomas Monjalon
> > > <tho...@monjalon.net>; Asaf Penso <as...@mellanox.com>; Shahaf
> > > Shuler <shah...@mellanox.com>; Olga Shern <ol...@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
