Re: [PATCH v6 07/16] ethdev: flatten RSS configuration in flow API

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

On Fri, May 04, 2018 at 09:31:05AM +0000, Zhao1, Wei wrote:
> Hi,  Adrien Mazarguil
> 
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > Sent: Thursday, May 3, 2018 8:48 PM
> > To: Peng, Yuan <yuan.peng@intel.com>
> > Cc: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org; Xu, Qian Q
> > <qian.q.xu@intel.com>; Liu, Yu Y <yu.y.liu@intel.com>; Lu, Wenzhuo
> > <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v6 07/16] ethdev: flatten RSS configuration
> > in flow API
> > 
> > Hi Peng Yuan,
> > 
> > Apologies for the delay, I'll answer below to the entire thread.
> > 
> > On Sat, Apr 28, 2018 at 07:45:31AM +0000, Peng, Yuan wrote:
> > > Hi，Adrien Mazarguil
> > >
> > > There is a bug of queue region with 18.05-rc1 The test steps are as
> > > below:
> > > ./usertools/dpdk-devbind.py -b igb_uio 05:00.0
> > > ./x86_64-native-linuxapp-gcc/app/testpmd -c 1ffff -n 4 -- -i --rxq=16
> > > --txq=16
> > > testpmd> port config all rss all
> > > Configuration of RSS hash at ethernet port 0 failed with error (22): Invalid
> > argument.
> > 
> > I assume this issue is related rte_eth_dev_configure() which was recently
> > fixed [1], right?
> > 
> > [1] "ethdev: fix applications failure on configure"
> >     http://dpdk.org/ml/archives/dev/2018-May/099858.html
> > 
> > <snip>
> > > There is a bug present with 18.05-rci when I test the feature "Move
> > > RSS to rte_flow" in i40e NIC The test steps are as below:
> > > ./usertools/dpdk-devbind.py -b igb_uio 05:00.0 05:00.1
> > > ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x1fffe -n 4  -- -i
> > > --nb-cores=8 --rxq=8 --txq=8 --port-topology=chained
> > > testpmd> set fwd rxonly
> > > Set rxonly packet forwarding mode
> > > testpmd> set verbose 1
> > > Change verbose level from 0 to 1
> > > testpmd> start
> > > testpmd> flow create 0 ingress pattern end actions rss queues 0 4 7
> > > testpmd> end / end
> > > Caught error type 16 (specific action): cause: 0x7fff84e33658, RSS
> > > hash key too large
> > >
> > > The rss rule can be set successfully when I test it yesterday with older dpdk
> > version without this patch.
> > 
> > Regarding this issue, the testpmd flow command now requests hash key
> > length from the PMD by default [2] if left unspecified by user. This value is
> > taken from the "hash_key_size" field returned by rte_eth_dev_infos_get().
> > 
> > While most PMDs return 40 here, i40e returns:
> > 
> >  (I40E_PFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t)
> >  /* that is, (12 + 1) * 4 => 52 */
> > 
> > Is this correct and really supported by i40e? Otherwise I'd suggest to fix the
> > PMD as it may also confuse applications other than testpmd.
> > 
> > Note that you should be able to revert to the old behavior with the PMD-
> > specific default key by specifying a "key_len 0" parameter to the RSS action.
> > 
> > [2] http://dpdk.org/browse/dpdk/tree/app/test-
> > pmd/cmdline_flow.c?id=v18.05-rc2#n2780
> 
> 
> This is the root cause for this issue, although i40 return :
> 
> 	dev_info->hash_key_size = (I40E_PFQF_HKEY_MAX_INDEX + 1) *
> 						sizeof(uint32_t);
> 
> that is 52 byte, but " RTE_DIM(rss_config->key)" ， which is
> 
> 	uint8_t key[(I40E_VFQF_HKEY_MAX_INDEX > I40E_PFQF_HKEY_MAX_INDEX ?
> 		     I40E_VFQF_HKEY_MAX_INDEX : I40E_PFQF_HKEY_MAX_INDEX) + 1 *
> 		    sizeof(uint32_t)]; /* Hash key. */
> is not 52!!!
> 
> I think the code should be :
> 
> 	uint8_t key[(I40E_VFQF_HKEY_MAX_INDEX > I40E_PFQF_HKEY_MAX_INDEX ?
> 		     I40E_VFQF_HKEY_MAX_INDEX : I40E_PFQF_HKEY_MAX_INDEX + 1) *
> 		    sizeof(uint32_t)]; /* Hash key. */
> 
> If you agree and permit, I WILL commit a patch to fix this bug.
> Thank you!

Whoops, dumb mistake, nice catch. If you take care of it, please add:

Fixes: ac8d22de2394 ("ethdev: flatten RSS configuration in flow API")

> > <snip>
> > > There is another problem with ixgbe nic:
> > > ./usertools/dpdk-devbind.py -b igb_uio 07:00.0 07:00.1
> > > ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x1fffe -n 4  -- -i
> > > --nb-cores=8 --rxq=8 --txq=8 --disable-rss --port-topology=chained
> > > testpmd> flow create 0 ingress pattern end actions rss queues 5 6 7
> > > testpmd> end / end
> > > Caught error type 2 (flow rule (handle)): Failed to create flow.
> > > The rule setting command can be executed successfully with older dpdk
> > version.
> > >
> > > Could you help to check if there is a relationship between the bugs and this
> > patch?
> > 
> > Perhaps a similar issue since testpmd now provides default values for
> > unspecified fields of the RSS action, that is, a default hash key with a PMD-
> > returned default length and the global "rss_hf" value for types (here 0 due to
> > --disable-rss).
> > 
> > Try appending the following arguments to the RSS action:
> > 
> >  key_len 0 types ip udp end
> > 
> > If it works, it probably means the issue was always present, it just never
> > showed up due to the inability to validate RSS action parameters previously.
> > You should make sure ixgbe supports them.

No comment regarding ixgbe?

-- 
Adrien Mazarguil
6WIND