[PATCH 14/14] net/nfp: modify RSS logic

[Kevin Liu]

Yes, you are right, thanks for your comment.
And I found this logic is not right, and I have update this logic as below We will use RSS2 if NIC cap support, otherwise will use RSS

		if (hw->cap & NFP_NET_CFG_CTRL_RSS2)
			new_ctrl |= NFP_NET_CFG_CTRL_RSS2;
		else
			new_ctrl |= NFP_NET_CFG_CTRL_RSS;

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit at xilinx.com> 
Sent: Friday, June 3, 2022 06:57
To: Kevin Liu <jin.liu at corigine.com>; dev at dpdk.org
Cc: Niklas Soderlund <niklas.soderlund at corigine.com>; Diana Wang <na.wang at corigine.com>; Nole Zhang <peng.zhang at corigine.com>; Chaoyong He <chaoyong.he at corigine.com>
Subject: Re: [PATCH 14/14] net/nfp: modify RSS logic

On 6/2/2022 2:53 AM, Jin Liu wrote:
> Modify RSS-related interface functions, as the NFDK firmware support 
> feature NFP_NET_CFG_CTRL_RSS2 rather than NFP_NET_CFG_CTRL_RSS.
> 
> Signed-off-by: Jin Liu <jin.liu at corigine.com>
> Signed-off-by: Diana Wang <na.wang at corigine.com>
> Signed-off-by: Peng Zhang <peng.zhang at corigine.com>
> Signed-off-by: Chaoyong He <chaoyong.he at corigine.com>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at corigine.com>

<...>

> diff --git a/drivers/net/nfp/nfp_ctrl.h b/drivers/net/nfp/nfp_ctrl.h 
> index e73715e2aa..372d537462 100644
> --- a/drivers/net/nfp/nfp_ctrl.h
> +++ b/drivers/net/nfp/nfp_ctrl.h
> @@ -119,6 +119,7 @@
>   #define NFP_NET_CFG_MACADDR             0x0024
>   
>   #define NFP_NET_CFG_CTRL_LSO_ANY (NFP_NET_CFG_CTRL_LSO | 
> NFP_NET_CFG_CTRL_LSO2)
> +#define NFP_NET_CFG_CTRL_RSS_ANY (NFP_NET_CFG_CTRL_RSS | 
> +NFP_NET_CFG_CTRL_RSS2)
>   
>   /*
>    * Read-only words (0x0030 - 0x0050):
> diff --git a/drivers/net/nfp/nfp_ethdev.c 
> b/drivers/net/nfp/nfp_ethdev.c index 238b2b5451..bd7dd30f10 100644
> --- a/drivers/net/nfp/nfp_ethdev.c
> +++ b/drivers/net/nfp/nfp_ethdev.c
> @@ -123,7 +123,17 @@ nfp_net_start(struct rte_eth_dev *dev)
>   	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS) {
>   		nfp_net_rss_config_default(dev);
>   		update |= NFP_NET_CFG_UPDATE_RSS;
> -		new_ctrl |= NFP_NET_CFG_CTRL_RSS;
> +		switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
> +		case NFP_NET_CFG_VERSION_DP_NFD3:
> +			new_ctrl |= NFP_NET_CFG_CTRL_RSS;
> +			break;
> +		case NFP_NET_CFG_VERSION_DP_NFDK:
> +			new_ctrl |= NFP_NET_CFG_CTRL_RSS2;
> +			break;
> +		default:
> +			PMD_INIT_LOG(ERR, "nfp_net: no fw version match");
> +			return -ENODEV;
> +		}
>   	}
>   
>   	/* Enable device */
> diff --git a/drivers/net/nfp/nfp_ethdev_vf.c 
> b/drivers/net/nfp/nfp_ethdev_vf.c index bb8206c4f6..8769f07be4 100644
> --- a/drivers/net/nfp/nfp_ethdev_vf.c
> +++ b/drivers/net/nfp/nfp_ethdev_vf.c
> @@ -95,7 +95,17 @@ nfp_netvf_start(struct rte_eth_dev *dev)
>   	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS) {
>   		nfp_net_rss_config_default(dev);
>   		update |= NFP_NET_CFG_UPDATE_RSS;
> -		new_ctrl |= NFP_NET_CFG_CTRL_RSS;
> +		switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
> +		case NFP_NET_CFG_VERSION_DP_NFD3:
> +			new_ctrl |= NFP_NET_CFG_CTRL_RSS;
> +			break;
> +		case NFP_NET_CFG_VERSION_DP_NFDK:
> +			new_ctrl |= NFP_NET_CFG_CTRL_RSS2;
> +			break;
> +		default:
> +			PMD_INIT_LOG(ERR, "nfp_net: no fw version match");
> +			return -ENODEV;
> +		}

As this FW specific changes crept into various locations, it can be harder to maintain the code.
I wonder if something like below can help, what do you think:

unsigned int nfp_FW_RSS[] = { // common for both PF & VF
	NFP_NET_CFG_CTRL_RSS,
	NFP_NET_CFG_CTRL_RSS2,
};

new_ctrl = nfp_FW_RSS[NFD_CFG_CLASS_VER_of(hw->ver)];