RE: [PATCH v2 1/2] net/nfp: standardize the use of RSS-related codes

[Chaoyong He <chaoyong.he@corigine.com>]

> On 2/21/2023 3:55 AM, Chaoyong He wrote:
> > From: Long Wu <long.wu@corigine.com>
> >
> > The usage of RTE_ETH_MQ_RX_RSS and RTE_ETH_MQ_RX_RSS_FLAG are
> mixed in
> > nfp_net_configure(), use RTE_ETH_MQ_RX_RSS_FLAG uniformly.
> >
> > Signed-off-by: Long Wu <long.wu@corigine.com>
> > Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> > ---
> >  drivers/net/nfp/nfp_common.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/nfp/nfp_common.c
> > b/drivers/net/nfp/nfp_common.c index 907777a9e4..a545a10013 100644
> > --- a/drivers/net/nfp/nfp_common.c
> > +++ b/drivers/net/nfp/nfp_common.c
> > @@ -171,7 +171,7 @@ nfp_net_configure(struct rte_eth_dev *dev)
> >  	}
> >
> >  	/* Checking RX mode */
> > -	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS &&
> > +	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG &&
> >  	    !(hw->cap & NFP_NET_CFG_CTRL_RSS_ANY)) {
> >  		PMD_INIT_LOG(INFO, "RSS not supported");
> >  		return -EINVAL;
> 
> They are same values, but when used as a mask, flag one can be more
> proper.
> 
> BUT,
> Not sure how correct to '&' an enum element, enums supposed to be
> abstraction on underneath values, right?
> For this case user should know/care enum values which is wrong, perhaps to
> OR flags to make enum elements was a mistake on our end.
> 
> Anyway, with above usage all enum elements that has RSS_FLAG is taken
> into account, like 'RTE_ETH_MQ_RX_VMDQ_RSS' (RSS mode with VMDq),
> are you sure that is the intention, is there a chance you meant:
> 
>  if (rxmode->mq_mode == RTE_ETH_MQ_RX_RSS && ...)
> 
> 
> btw, what if mq_mode is 'RTE_ETH_MQ_RX_VMDQ_DCB', is it supported?
> 
For now, we don't support 'RTE_ETH_MQ_RX_VMDQ_DCB'.

> 
> I just recognized there are various similar usage in drivers, and most of them
> comes from same commit [1], we can discuss and fix that separately.
> 
I saw you plan drop the RFC commit [1], so what we should do about this patch?
1. Keep our modify.
2. Follow your modify.
3. We just drop this commit. (your patch series will correct it anyway).
Please give me an advice to guide my next move, thanks.

> [1]
> 73fb89dd6a00 ("drivers/net: fix RSS hash offload flag if no RSS")