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.yi...@xilinx.com>
Sent: Friday, June 3, 2022 06:57
To: Kevin Liu <jin....@corigine.com>; dev@dpdk.org
Cc: Niklas Soderlund <niklas.soderl...@corigine.com>; Diana Wang
<na.w...@corigine.com>; Nole Zhang <peng.zh...@corigine.com>; Chaoyong He
<chaoyong...@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....@corigine.com>
> Signed-off-by: Diana Wang <na.w...@corigine.com>
> Signed-off-by: Peng Zhang <peng.zh...@corigine.com>
> Signed-off-by: Chaoyong He <chaoyong...@corigine.com>
> Signed-off-by: Niklas Söderlund <niklas.soderl...@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)];
