Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

From: Thomas Monjalon <thomas@monjalon.net>
To: "lihuisong (C)" <lihuisong@huawei.com>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	"Min Hu (Connor)" <humin29@huawei.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	Qi Zhang <qi.z.zhang@intel.com>, Ori Kam <orika@nvidia.com>
Cc: Olivier Matz <olivier.matz@6wind.com>,
	Ajit Khaparde <ajit.khaparde@broadcom.com>,
	"jerinj@marvell.com" <jerinj@marvell.com>,
	Stephen Hemminger <stephen@networkplumber.org>,
	Slava Ovsiienko <viacheslavo@nvidia.com>,
	huangdaode <huangdaode@huawei.com>
Subject: Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior
Date: Wed, 02 Mar 2022 15:46:37 +0100	[thread overview]
Message-ID: <3844855.QkHrqEjB74@thomas> (raw)
In-Reply-To: <MW2PR12MB4666C3360FE0122A9D741F62D6039@MW2PR12MB4666.namprd12.prod.outlook.com>

02/03/2022 15:07, Ori Kam:
> From: lihuisong (C) <lihuisong@huawei.com>
> > 在 2022/3/1 0:42, Ferruh Yigit 写道:
> > > On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> > >> From: Huisong Li <lihuisong@huawei.com>
> > >>
> > >> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
> > >> dev_configure phase. However, if this flag isn't set, RSS can be enabled
> > >> through the ethdev ops and rte_flow API. This behavior is contrary to
> > >> each
> > >> other.
> > >>
> > >> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> > >> Cc: stable@dpdk.org
> > >>
> > >> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> > >
> > >
> > > Hi Huisong, Connor,
> > >
> > > Let's get a little more feedback for this patch, cc'ed more people.
> > >
> > >
> > > To enable RSS, multi queue mode should be set to
> > > 'RTE_ETH_MQ_RX_RSS_FLAG'.
> > >
> > > But I wonder if it is required to configure RSS via flow API,
> > 
> > I do not know the original purpose of adding the RSS configuration in
> > flow API.
> > 
> The purpose is simple, this allow to create RSS per rule and not a global one.
> For example create RSS that sends TCP to some queues while othe RSS will send
> UDP traffic to different queues.
> 
> > However, as far as I know, the hash algorithm can be configured via this
> > API,
> > 
> > but not via ethdev ops API.
> > 
> > > and if other PMDs check this configuration for flow API?
> > 
> > Some PMDs already have similar check in RSS releated ops or rte_flow API.
> > 
> > For example, hinic, axbge, bnxt, cnxk, otx2, and ena.

I suggest removing these checks.

> From my view point those are two different settings.
> The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
> rte_flow is per rule.
> 
> I think, that if a PMD needs this flag, in order to enable it also for rte_flow then
> it should be documented in the release note of the PMD.
> It is a valid use case that the default traffic will not have RSS and only rules created by
> rte_flow will have the RSS, for matching traffc.

I think RTE_ETH_MQ_RX_RSS_FLAG should not be required by any PMD
for rte_flow RSS rules.