[net] nfp: only clean `sp_indiff` when application firmware is unloaded

Commit Message

Simon Horman Oct. 20, 2022, 8:14 a.m. UTC

From: Yinjun Zhang <[e-mail]>

Currently `sp_indiff` is cleaned when driver is removed. This will
cause problem in multi-PF/multi-host case, considering one PF is
removed while another is still in use.

Since `sp_indiff` is the application firmware property, it should
only be cleaned when the firmware is unloaded. Now let management
firmware to clean it when necessary, driver only set it.

Fixes: b1e4f11e426d ("nfp: refine the ABI of getting `sp_indiff` info")
Signed-off-by: Yinjun Zhang <[e-mail]>
Signed-off-by: Simon Horman <[e-mail]>
---
 drivers/net/ethernet/netronome/nfp/nfp_main.c | 38 ++++++++-----------
 1 file changed, 15 insertions(+), 23 deletions(-)

Comments

[e-mail] Oct. 22, 2022, 5:50 a.m. UTC | #1

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <[e-mail]>:

On Thu, 20 Oct 2022 09:14:11 +0100 you wrote:
> From: Yinjun Zhang <[e-mail]>
> 
> Currently `sp_indiff` is cleaned when driver is removed. This will
> cause problem in multi-PF/multi-host case, considering one PF is
> removed while another is still in use.
> 
> Since `sp_indiff` is the application firmware property, it should
> only be cleaned when the firmware is unloaded. Now let management
> firmware to clean it when necessary, driver only set it.
> 
> [...]

Here is the summary with links:
  - [net] nfp: only clean `sp_indiff` when application firmware is unloaded
    https://git.kernel.org/netdev/net/c/0bda03623e6b

You are awesome, thank you!

Patch

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c b/drivers/net/ethernet/netronome/nfp/nfp_main.c
index e66e548919d4..71301dbd8fb5 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
@@ -716,16 +716,26 @@  static u64 nfp_net_pf_get_app_cap(struct nfp_pf *pf)
 	return val;
 }
 
-static int nfp_pf_cfg_hwinfo(struct nfp_pf *pf, bool sp_indiff)
+static void nfp_pf_cfg_hwinfo(struct nfp_pf *pf)
 {
 	struct nfp_nsp *nsp;
 	char hwinfo[32];
+	bool sp_indiff;
 	int err;
 
 	nsp = nfp_nsp_open(pf->cpp);
 	if (IS_ERR(nsp))
-		return PTR_ERR(nsp);
+		return;
+
+	if (!nfp_nsp_has_hwinfo_set(nsp))
+		goto end;
 
+	sp_indiff = (nfp_net_pf_get_app_id(pf) == NFP_APP_FLOWER_NIC) ||
+		    (nfp_net_pf_get_app_cap(pf) & NFP_NET_APP_CAP_SP_INDIFF);
+
+	/* No need to clean `sp_indiff` in driver, management firmware
+	 * will do it when application firmware is unloaded.
+	 */
 	snprintf(hwinfo, sizeof(hwinfo), "sp_indiff=%d", sp_indiff);
 	err = nfp_nsp_hwinfo_set(nsp, hwinfo, sizeof(hwinfo));
 	/* Not a fatal error, no need to return error to stop driver from loading */
@@ -739,21 +749,8 @@  static int nfp_pf_cfg_hwinfo(struct nfp_pf *pf, bool sp_indiff)
 		pf->eth_tbl = __nfp_eth_read_ports(pf->cpp, nsp);
 	}
 
+end:
 	nfp_nsp_close(nsp);
-	return 0;
-}
-
-static int nfp_pf_nsp_cfg(struct nfp_pf *pf)
-{
-	bool sp_indiff = (nfp_net_pf_get_app_id(pf) == NFP_APP_FLOWER_NIC) ||
-			 (nfp_net_pf_get_app_cap(pf) & NFP_NET_APP_CAP_SP_INDIFF);
-
-	return nfp_pf_cfg_hwinfo(pf, sp_indiff);
-}
-
-static void nfp_pf_nsp_clean(struct nfp_pf *pf)
-{
-	nfp_pf_cfg_hwinfo(pf, false);
 }
 
 static int nfp_pci_probe(struct pci_dev *pdev,
@@ -856,13 +853,11 @@  static int nfp_pci_probe(struct pci_dev *pdev,
 		goto err_fw_unload;
 	}
 
-	err = nfp_pf_nsp_cfg(pf);
-	if (err)
-		goto err_fw_unload;
+	nfp_pf_cfg_hwinfo(pf);
 
 	err = nfp_net_pci_probe(pf);
 	if (err)
-		goto err_nsp_clean;
+		goto err_fw_unload;
 
 	err = nfp_hwmon_register(pf);
 	if (err) {
@@ -874,8 +869,6 @@  static int nfp_pci_probe(struct pci_dev *pdev,
 
 err_net_remove:
 	nfp_net_pci_remove(pf);
-err_nsp_clean:
-	nfp_pf_nsp_clean(pf);
 err_fw_unload:
 	kfree(pf->rtbl);
 	nfp_mip_close(pf->mip);
@@ -915,7 +908,6 @@  static void __nfp_pci_shutdown(struct pci_dev *pdev, bool unload_fw)
 
 	nfp_net_pci_remove(pf);
 
-	nfp_pf_nsp_clean(pf);
 	vfree(pf->dumpspec);
 	kfree(pf->rtbl);
 	nfp_mip_close(pf->mip);