[patch net-next RFC 0/7] devlink: make sure devlink port registers/unregisters only for registered instance

[patch net-next RFC 0/7] devlink: make sure devlink port registers/unregisters only for registered instance

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Currently, the devlink_register() is called as the last thing during
driver init phase. For devlink objects, this is fine as the
notifications of objects creations are withheld to be send only after
devlink instance is registered. Until devlink is registered it is not
visible to userspace.

But if a  netdev is registered before, user gets a notification with
a link to devlink, which is not visible to the user yet.
This is the event order user sees:
 * new netdev event over RT netlink
 * new devlink event over devlink netlink
 * new devlink_port event over devlink netlink

Also, this is not consistent with devlink port split or devlink reload
flows, where user gets notifications in following order:
 * new devlink event over devlink netlink
and then during port split or reload operation:
 * new devlink_port event over devlink netlink
 * new netdev event over RT netlink

In this case, devlink port and related netdev are registered on already
registered devlink instance.

Purpose of this patchset is to fix the drivers init flow so devlink port
gets registered only after devlink instance is registered.

Jiri Pirko (7):
  devlink: Reorder devlink_port_register/unregister() calls to be done
    when devlink is registered
  netdevsim: Reorder devl_port_register/unregister() calls to be done
    when devlink is registered
  mlxsw: Reorder devl_port_register/unregister() calls to be done when
    devlink is registered
  nfp: Reorder devl_port_register/unregister() calls to be done when
    devlink is registered
  mlx4: Reorder devl_port_register/unregister() calls to be done when
    devlink is registered
  mlx5: Reorder devl_port_register/unregister() calls to be done when
    devlink is registered
  devlink: assert if devl_port_register/unregister() is called on
    unregistered devlink instance

 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 29 +++++----
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  |  7 ++-
 .../ethernet/fungible/funeth/funeth_main.c    | 17 ++++--
 drivers/net/ethernet/intel/ice/ice_main.c     | 21 ++++---
 .../ethernet/marvell/prestera/prestera_main.c |  6 +-
 drivers/net/ethernet/mellanox/mlx4/main.c     | 60 ++++++++++---------
 drivers/net/ethernet/mellanox/mlx5/core/dev.c | 10 +++-
 .../net/ethernet/mellanox/mlx5/core/main.c    | 17 +++---
 .../mellanox/mlx5/core/sf/dev/driver.c        |  9 +++
 drivers/net/ethernet/mellanox/mlxsw/core.c    | 24 ++++++++
 drivers/net/ethernet/mellanox/mlxsw/core.h    |  2 +
 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 38 +++++++++---
 drivers/net/ethernet/mscc/ocelot_vsc7514.c    | 10 ++--
 .../net/ethernet/netronome/nfp/nfp_net_main.c | 22 +++++--
 .../ethernet/pensando/ionic/ionic_devlink.c   |  6 +-
 drivers/net/ethernet/ti/am65-cpsw-nuss.c      |  7 ++-
 drivers/net/netdevsim/dev.c                   | 31 ++++++++--
 net/core/devlink.c                            |  2 +
 18 files changed, 218 insertions(+), 100 deletions(-)

-- 
2.37.3

[patch net-next RFC 1/7] devlink: Reorder devlink_port_register/unregister() calls to be done when devlink is registered

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Change the drivers that use devlink_port_register/unregister() to call
these functions only in case devlink is registered.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 29 ++++++++++---------
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  |  7 +++--
 .../ethernet/fungible/funeth/funeth_main.c    | 17 +++++++----
 drivers/net/ethernet/intel/ice/ice_main.c     | 21 ++++++++------
 .../ethernet/marvell/prestera/prestera_main.c |  6 ++--
 drivers/net/ethernet/mscc/ocelot_vsc7514.c    | 10 +++----
 .../ethernet/pensando/ionic/ionic_devlink.c   |  6 ++--
 drivers/net/ethernet/ti/am65-cpsw-nuss.c      |  7 +++--
 8 files changed, 60 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 26913dc816d3..c2600ce7313c 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -1285,8 +1285,15 @@ int bnxt_dl_register(struct bnxt *bp)
 	    bp->hwrm_spec_code > 0x10803)
 		bp->eswitch_mode = DEVLINK_ESWITCH_MODE_LEGACY;
 
+	if (BNXT_PF(bp)) {
+		rc = bnxt_dl_params_register(bp);
+		if (rc)
+			goto err_dl_free;
+	}
+
+	devlink_register(dl);
 	if (!BNXT_PF(bp))
-		goto out;
+		return 0;
 
 	attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
 	attrs.phys.port_number = bp->pf.port_id;
@@ -1296,20 +1303,16 @@ int bnxt_dl_register(struct bnxt *bp)
 	rc = devlink_port_register(dl, &bp->dl_port, bp->pf.port_id);
 	if (rc) {
 		netdev_err(bp->dev, "devlink_port_register failed\n");
-		goto err_dl_free;
+		goto err_dl_unreg;
 	}
 
-	rc = bnxt_dl_params_register(bp);
-	if (rc)
-		goto err_dl_port_unreg;
-
 	devlink_set_features(dl, DEVLINK_F_RELOAD);
-out:
-	devlink_register(dl);
 	return 0;
 
-err_dl_port_unreg:
-	devlink_port_unregister(&bp->dl_port);
+err_dl_unreg:
+	devlink_unregister(dl);
+	if (BNXT_PF(bp))
+		bnxt_dl_params_unregister(bp);
 err_dl_free:
 	devlink_free(dl);
 	return rc;
@@ -1319,10 +1322,10 @@ void bnxt_dl_unregister(struct bnxt *bp)
 {
 	struct devlink *dl = bp->dl;
 
+	if (BNXT_PF(bp))
+		devlink_port_unregister(&bp->dl_port);
 	devlink_unregister(dl);
-	if (BNXT_PF(bp)) {
+	if (BNXT_PF(bp))
 		bnxt_dl_params_unregister(bp);
-		devlink_port_unregister(&bp->dl_port);
-	}
 	devlink_free(dl);
 }
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 0c35abb7d065..4e468c4c20e0 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -4954,6 +4954,8 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
 	if (err)
 		goto err_dl_trap_register;
 
+	dpaa2_eth_dl_register(priv);
+
 	err = dpaa2_eth_dl_port_add(priv);
 	if (err)
 		goto err_dl_port_add;
@@ -4968,12 +4970,12 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
 	dpaa2_dbg_add(priv);
 #endif
 
-	dpaa2_eth_dl_register(priv);
 	dev_info(dev, "Probed interface %s\n", net_dev->name);
 	return 0;
 
 err_netdev_reg:
 	dpaa2_eth_dl_port_del(priv);
+	dpaa2_eth_dl_unregister(priv);
 err_dl_port_add:
 	dpaa2_eth_dl_traps_unregister(priv);
 err_dl_trap_register:
@@ -5026,8 +5028,6 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
 	net_dev = dev_get_drvdata(dev);
 	priv = netdev_priv(net_dev);
 
-	dpaa2_eth_dl_unregister(priv);
-
 #ifdef CONFIG_DEBUG_FS
 	dpaa2_dbg_remove(priv);
 #endif
@@ -5035,6 +5035,7 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
 	unregister_netdev(net_dev);
 
 	dpaa2_eth_dl_port_del(priv);
+	dpaa2_eth_dl_unregister(priv);
 	dpaa2_eth_dl_traps_unregister(priv);
 	dpaa2_eth_dl_free(priv);
 
diff --git a/drivers/net/ethernet/fungible/funeth/funeth_main.c b/drivers/net/ethernet/fungible/funeth/funeth_main.c
index b4cce30e526a..e335071bf530 100644
--- a/drivers/net/ethernet/fungible/funeth/funeth_main.c
+++ b/drivers/net/ethernet/fungible/funeth/funeth_main.c
@@ -2018,17 +2018,22 @@ static int funeth_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		goto free_devlink;
 
+	fun_devlink_register(devlink);
+
 	rc = fun_get_res_count(fdev, FUN_ADMIN_OP_PORT);
-	if (rc > 0)
-		rc = fun_create_ports(ed, rc);
 	if (rc < 0)
-		goto disable_dev;
+		goto unregister_devlink;
+	if (rc > 0) {
+		rc = fun_create_ports(ed, rc);
+		if (rc < 0)
+			goto unregister_devlink;
+	}
 
 	fun_serv_restart(fdev);
-	fun_devlink_register(devlink);
 	return 0;
 
-disable_dev:
+unregister_devlink:
+	fun_devlink_unregister(devlink);
 	fun_dev_disable(fdev);
 free_devlink:
 	mutex_destroy(&ed->state_mutex);
@@ -2044,7 +2049,6 @@ static void funeth_remove(struct pci_dev *pdev)
 
 	ed = to_fun_ethdev(fdev);
 	devlink = priv_to_devlink(ed);
-	fun_devlink_unregister(devlink);
 
 #ifdef CONFIG_PCI_IOV
 	funeth_sriov_configure(pdev, 0);
@@ -2052,6 +2056,7 @@ static void funeth_remove(struct pci_dev *pdev)
 
 	fun_serv_stop(fdev);
 	fun_destroy_ports(ed);
+	fun_devlink_unregister(devlink);
 	fun_dev_disable(fdev);
 	mutex_destroy(&ed->state_mutex);
 
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 2b23b4714a26..f47d5b87f99b 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -4919,11 +4919,13 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 	pcie_print_link_status(pf->pdev);
 
 probe_done:
-	err = ice_register_netdev(pf);
+	err = ice_devlink_register_params(pf);
 	if (err)
-		goto err_netdev_reg;
+		goto err_devlink_reg_param;
 
-	err = ice_devlink_register_params(pf);
+	ice_devlink_register(pf);
+
+	err = ice_register_netdev(pf);
 	if (err)
 		goto err_netdev_reg;
 
@@ -4934,7 +4936,7 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 		if (pf->aux_idx < 0) {
 			dev_err(dev, "Failed to allocate device ID for AUX driver\n");
 			err = -ENOMEM;
-			goto err_devlink_reg_param;
+			goto err_ida_alloc;
 		}
 
 		err = ice_init_rdma(pf);
@@ -4947,15 +4949,16 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 		dev_warn(dev, "RDMA is not supported on this device\n");
 	}
 
-	ice_devlink_register(pf);
 	return 0;
 
 err_init_aux_unroll:
 	pf->adev = NULL;
 	ida_free(&ice_aux_ida, pf->aux_idx);
-err_devlink_reg_param:
-	ice_devlink_unregister_params(pf);
+err_ida_alloc:
 err_netdev_reg:
+	ice_devlink_unregister(pf);
+	ice_devlink_unregister_params(pf);
+err_devlink_reg_param:
 err_send_version_unroll:
 	ice_vsi_release_all(pf);
 err_alloc_sw_unroll:
@@ -5051,7 +5054,6 @@ static void ice_remove(struct pci_dev *pdev)
 	struct ice_pf *pf = pci_get_drvdata(pdev);
 	int i;
 
-	ice_devlink_unregister(pf);
 	for (i = 0; i < ICE_MAX_RESET_WAIT; i++) {
 		if (!ice_is_reset_in_progress(pf->state))
 			break;
@@ -5071,7 +5073,6 @@ static void ice_remove(struct pci_dev *pdev)
 	ice_unplug_aux_dev(pf);
 	if (pf->aux_idx >= 0)
 		ida_free(&ice_aux_ida, pf->aux_idx);
-	ice_devlink_unregister_params(pf);
 	set_bit(ICE_DOWN, pf->state);
 
 	ice_deinit_lag(pf);
@@ -5083,6 +5084,8 @@ static void ice_remove(struct pci_dev *pdev)
 		ice_remove_arfs(pf);
 	ice_setup_mc_magic_wake(pf);
 	ice_vsi_release_all(pf);
+	ice_devlink_unregister(pf);
+	ice_devlink_unregister_params(pf);
 	mutex_destroy(&(&pf->hw)->fdir_fltr_lock);
 	ice_set_wake(pf);
 	ice_free_irq_msix_misc(pf);
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index 9d504142e51a..815056a130d9 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
@@ -1421,14 +1421,16 @@ static int prestera_switch_init(struct prestera_switch *sw)
 	if (err)
 		goto err_lag_init;
 
+	prestera_devlink_register(sw);
+
 	err = prestera_create_ports(sw);
 	if (err)
 		goto err_ports_create;
 
-	prestera_devlink_register(sw);
 	return 0;
 
 err_ports_create:
+	prestera_devlink_unregister(sw);
 	prestera_lag_fini(sw);
 err_lag_init:
 	prestera_devlink_traps_unregister(sw);
@@ -1455,8 +1457,8 @@ static int prestera_switch_init(struct prestera_switch *sw)
 
 static void prestera_switch_fini(struct prestera_switch *sw)
 {
-	prestera_devlink_unregister(sw);
 	prestera_destroy_ports(sw);
+	prestera_devlink_unregister(sw);
 	prestera_lag_fini(sw);
 	prestera_devlink_traps_unregister(sw);
 	prestera_span_fini(sw);
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index b097fd4a4061..69adde1da2a0 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -562,10 +562,6 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 	if (err)
 		goto out_put_ports;
 
-	err = mscc_ocelot_init_ports(pdev, ports);
-	if (err)
-		goto out_ocelot_devlink_unregister;
-
 	if (ocelot->fdma)
 		ocelot_fdma_start(ocelot);
 
@@ -589,6 +585,10 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 	of_node_put(ports);
 	devlink_register(devlink);
 
+	err = mscc_ocelot_init_ports(pdev, ports);
+	if (err)
+		goto out_ocelot_devlink_unregister;
+
 	dev_info(&pdev->dev, "Ocelot switch probed\n");
 
 	return 0;
@@ -611,10 +611,10 @@ static int mscc_ocelot_remove(struct platform_device *pdev)
 
 	if (ocelot->fdma)
 		ocelot_fdma_deinit(ocelot);
+	mscc_ocelot_release_ports(ocelot);
 	devlink_unregister(ocelot->devlink);
 	ocelot_deinit_timestamp(ocelot);
 	ocelot_devlink_sb_unregister(ocelot);
-	mscc_ocelot_release_ports(ocelot);
 	mscc_ocelot_teardown_devlink_ports(ocelot);
 	ocelot_deinit(ocelot);
 	unregister_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
index e6ff757895ab..06670343f90b 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
@@ -78,16 +78,18 @@ int ionic_devlink_register(struct ionic *ionic)
 	struct devlink_port_attrs attrs = {};
 	int err;
 
+	devlink_register(dl);
+
 	attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
 	devlink_port_attrs_set(&ionic->dl_port, &attrs);
 	err = devlink_port_register(dl, &ionic->dl_port, 0);
 	if (err) {
 		dev_err(ionic->dev, "devlink_port_register failed: %d\n", err);
+		devlink_unregister(dl);
 		return err;
 	}
 
 	SET_NETDEV_DEVLINK_PORT(ionic->lif->netdev, &ionic->dl_port);
-	devlink_register(dl);
 	return 0;
 }
 
@@ -95,6 +97,6 @@ void ionic_devlink_unregister(struct ionic *ionic)
 {
 	struct devlink *dl = priv_to_devlink(ionic);
 
-	devlink_unregister(dl);
 	devlink_port_unregister(&ionic->dl_port);
+	devlink_unregister(dl);
 }
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 51c37e99d086..4ac4b7ada890 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -2522,6 +2522,8 @@ static int am65_cpsw_nuss_register_devlink(struct am65_cpsw_common *common)
 		}
 	}
 
+	devlink_register(common->devlink);
+
 	for (i = 1; i <= common->port_num; i++) {
 		port = am65_common_get_port(common, i);
 		dl_port = &port->devlink_port;
@@ -2542,7 +2544,6 @@ static int am65_cpsw_nuss_register_devlink(struct am65_cpsw_common *common)
 			goto dl_port_unreg;
 		}
 	}
-	devlink_register(common->devlink);
 	return ret;
 
 dl_port_unreg:
@@ -2552,6 +2553,7 @@ static int am65_cpsw_nuss_register_devlink(struct am65_cpsw_common *common)
 
 		devlink_port_unregister(dl_port);
 	}
+	devlink_unregister(common->devlink);
 dl_unreg:
 	devlink_free(common->devlink);
 	return ret;
@@ -2563,14 +2565,13 @@ static void am65_cpsw_unregister_devlink(struct am65_cpsw_common *common)
 	struct am65_cpsw_port *port;
 	int i;
 
-	devlink_unregister(common->devlink);
-
 	for (i = 1; i <= common->port_num; i++) {
 		port = am65_common_get_port(common, i);
 		dl_port = &port->devlink_port;
 
 		devlink_port_unregister(dl_port);
 	}
+	devlink_unregister(common->devlink);
 
 	if (!AM65_CPSW_IS_CPSW2G(common) &&
 	    IS_ENABLED(CONFIG_TI_K3_AM65_CPSW_SWITCHDEV))
-- 
2.37.3

[patch net-next RFC 2/7] netdevsim: Reorder devl_port_register/unregister() calls to be done when devlink is registered

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Move the code so devl_port_register/unregister() are called only
then devlink is registered.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/netdevsim/dev.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index b962fc8e1397..8ed235ac986f 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -949,6 +949,7 @@ static void nsim_dev_traps_exit(struct devlink *devlink)
 
 static int nsim_dev_reload_create(struct nsim_dev *nsim_dev,
 				  struct netlink_ext_ack *extack);
+static void nsim_dev_reload_ports_destroy(struct nsim_dev *nsim_dev);
 static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev);
 
 static int nsim_dev_reload_down(struct devlink *devlink, bool netns_change,
@@ -965,6 +966,7 @@ static int nsim_dev_reload_down(struct devlink *devlink, bool netns_change,
 		return -EOPNOTSUPP;
 	}
 
+	nsim_dev_reload_ports_destroy(nsim_dev);
 	nsim_dev_reload_destroy(nsim_dev);
 	return 0;
 }
@@ -1600,17 +1602,22 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
 	if (err)
 		goto err_psample_exit;
 
-	err = nsim_dev_port_add_all(nsim_dev, nsim_bus_dev->port_count);
-	if (err)
-		goto err_hwstats_exit;
-
 	nsim_dev->esw_mode = DEVLINK_ESWITCH_MODE_LEGACY;
 	devlink_set_features(devlink, DEVLINK_F_RELOAD);
 	devl_unlock(devlink);
 	devlink_register(devlink);
+
+	devl_lock(devlink);
+	err = nsim_dev_port_add_all(nsim_dev, nsim_bus_dev->port_count);
+	devl_unlock(devlink);
+	if (err)
+		goto err_devlink_unregister;
+
 	return 0;
 
-err_hwstats_exit:
+err_devlink_unregister:
+	devlink_unregister(devlink);
+	devl_lock(devlink);
 	nsim_dev_hwstats_exit(nsim_dev);
 err_psample_exit:
 	nsim_dev_psample_exit(nsim_dev);
@@ -1640,6 +1647,15 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
 	return err;
 }
 
+static void nsim_dev_reload_ports_destroy(struct nsim_dev *nsim_dev)
+{
+	struct devlink *devlink = priv_to_devlink(nsim_dev);
+
+	if (devlink_is_reload_failed(devlink))
+		return;
+	nsim_dev_port_del_all(nsim_dev);
+}
+
 static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev)
 {
 	struct devlink *devlink = priv_to_devlink(nsim_dev);
@@ -1654,7 +1670,6 @@ static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev)
 			nsim_esw_legacy_enable(nsim_dev, NULL);
 	}
 
-	nsim_dev_port_del_all(nsim_dev);
 	nsim_dev_hwstats_exit(nsim_dev);
 	nsim_dev_psample_exit(nsim_dev);
 	nsim_dev_health_exit(nsim_dev);
@@ -1668,6 +1683,10 @@ void nsim_drv_remove(struct nsim_bus_dev *nsim_bus_dev)
 	struct nsim_dev *nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
 	struct devlink *devlink = priv_to_devlink(nsim_dev);
 
+	devl_lock(devlink);
+	nsim_dev_reload_ports_destroy(nsim_dev);
+	devl_unlock(devlink);
+
 	devlink_unregister(devlink);
 	devl_lock(devlink);
 	nsim_dev_reload_destroy(nsim_dev);
-- 
2.37.3

[patch net-next RFC 3/7] mlxsw: Reorder devl_port_register/unregister() calls to be done when devlink is registered

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Move the code so devl_port_register/unregister() are called only
then devlink is registered.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c    | 24 ++++++++++++
 drivers/net/ethernet/mellanox/mlxsw/core.h    |  2 +
 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 38 ++++++++++++++-----
 3 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index a0a06e2eff82..9908fb0f2d8a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -2215,8 +2215,24 @@ __mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 		devl_unlock(devlink);
 		devlink_register(devlink);
 	}
+
+	if (mlxsw_driver->ports_init) {
+		if (!reload)
+			devl_lock(devlink);
+		err = mlxsw_driver->ports_init(mlxsw_core);
+		if (!reload)
+			devl_unlock(devlink);
+		if (err)
+			goto err_driver_ports_init;
+	}
+
 	return 0;
 
+err_driver_ports_init:
+	if (!reload) {
+		devlink_unregister(devlink);
+		devl_lock(devlink);
+	}
 err_driver_init:
 	mlxsw_env_fini(mlxsw_core->env);
 err_env_init:
@@ -2284,6 +2300,14 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core,
 {
 	struct devlink *devlink = priv_to_devlink(mlxsw_core);
 
+	if (mlxsw_core->driver->ports_fini) {
+		if (!reload)
+			devl_lock(devlink);
+		mlxsw_core->driver->ports_fini(mlxsw_core);
+		if (!reload)
+			devl_unlock(devlink);
+	}
+
 	if (!reload) {
 		devlink_unregister(devlink);
 		devl_lock(devlink);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index e0a6fcbbcb19..a41ca92318e8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -348,6 +348,8 @@ struct mlxsw_driver {
 		    const struct mlxsw_bus_info *mlxsw_bus_info,
 		    struct netlink_ext_ack *extack);
 	void (*fini)(struct mlxsw_core *mlxsw_core);
+	int (*ports_init)(struct mlxsw_core *mlxsw_core);
+	void (*ports_fini)(struct mlxsw_core *mlxsw_core);
 	int (*port_split)(struct mlxsw_core *mlxsw_core, u16 local_port,
 			  unsigned int count, struct netlink_ext_ack *extack);
 	int (*port_unsplit)(struct mlxsw_core *mlxsw_core, u16 local_port,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index f5b2d965d476..b216c7dd8419 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3251,16 +3251,8 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
 		goto err_sample_trigger_init;
 	}
 
-	err = mlxsw_sp_ports_create(mlxsw_sp);
-	if (err) {
-		dev_err(mlxsw_sp->bus_info->dev, "Failed to create ports\n");
-		goto err_ports_create;
-	}
-
 	return 0;
 
-err_ports_create:
-	rhashtable_destroy(&mlxsw_sp->sample_trigger_ht);
 err_sample_trigger_init:
 	mlxsw_sp_port_module_info_fini(mlxsw_sp);
 err_port_module_info_init:
@@ -3445,11 +3437,24 @@ static int mlxsw_sp4_init(struct mlxsw_core *mlxsw_core,
 	return mlxsw_sp_init(mlxsw_core, mlxsw_bus_info, extack);
 }
 
+static int mlxsw_sp_ports_init(struct mlxsw_core *mlxsw_core)
+{
+	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
+	int err;
+
+	err = mlxsw_sp_ports_create(mlxsw_sp);
+	if (err) {
+		dev_err(mlxsw_sp->bus_info->dev, "Failed to create ports\n");
+		return err;
+	}
+
+	return 0;
+}
+
 static void mlxsw_sp_fini(struct mlxsw_core *mlxsw_core)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
 
-	mlxsw_sp_ports_remove(mlxsw_sp);
 	rhashtable_destroy(&mlxsw_sp->sample_trigger_ht);
 	mlxsw_sp_port_module_info_fini(mlxsw_sp);
 	mlxsw_sp_dpipe_fini(mlxsw_sp);
@@ -3478,6 +3483,13 @@ static void mlxsw_sp_fini(struct mlxsw_core *mlxsw_core)
 	mlxsw_sp_parsing_fini(mlxsw_sp);
 }
 
+static void mlxsw_sp_ports_fini(struct mlxsw_core *mlxsw_core)
+{
+	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
+
+	mlxsw_sp_ports_remove(mlxsw_sp);
+}
+
 static const struct mlxsw_config_profile mlxsw_sp1_config_profile = {
 	.used_flood_mode                = 1,
 	.flood_mode                     = MLXSW_CMD_MBOX_CONFIG_PROFILE_FLOOD_MODE_CONTROLLED,
@@ -3934,6 +3946,8 @@ static struct mlxsw_driver mlxsw_sp1_driver = {
 	.fw_filename			= MLXSW_SP1_FW_FILENAME,
 	.init				= mlxsw_sp1_init,
 	.fini				= mlxsw_sp_fini,
+	.ports_init			= mlxsw_sp_ports_init,
+	.ports_fini			= mlxsw_sp_ports_fini,
 	.port_split			= mlxsw_sp_port_split,
 	.port_unsplit			= mlxsw_sp_port_unsplit,
 	.sb_pool_get			= mlxsw_sp_sb_pool_get,
@@ -3971,6 +3985,8 @@ static struct mlxsw_driver mlxsw_sp2_driver = {
 	.fw_filename			= MLXSW_SP2_FW_FILENAME,
 	.init				= mlxsw_sp2_init,
 	.fini				= mlxsw_sp_fini,
+	.ports_init			= mlxsw_sp_ports_init,
+	.ports_fini			= mlxsw_sp_ports_fini,
 	.port_split			= mlxsw_sp_port_split,
 	.port_unsplit			= mlxsw_sp_port_unsplit,
 	.ports_remove_selected		= mlxsw_sp_ports_remove_selected,
@@ -4010,6 +4026,8 @@ static struct mlxsw_driver mlxsw_sp3_driver = {
 	.fw_filename			= MLXSW_SP3_FW_FILENAME,
 	.init				= mlxsw_sp3_init,
 	.fini				= mlxsw_sp_fini,
+	.ports_init			= mlxsw_sp_ports_init,
+	.ports_fini			= mlxsw_sp_ports_fini,
 	.port_split			= mlxsw_sp_port_split,
 	.port_unsplit			= mlxsw_sp_port_unsplit,
 	.ports_remove_selected		= mlxsw_sp_ports_remove_selected,
@@ -4047,6 +4065,8 @@ static struct mlxsw_driver mlxsw_sp4_driver = {
 	.priv_size			= sizeof(struct mlxsw_sp),
 	.init				= mlxsw_sp4_init,
 	.fini				= mlxsw_sp_fini,
+	.ports_init			= mlxsw_sp_ports_init,
+	.ports_fini			= mlxsw_sp_ports_fini,
 	.port_split			= mlxsw_sp_port_split,
 	.port_unsplit			= mlxsw_sp_port_unsplit,
 	.ports_remove_selected		= mlxsw_sp_ports_remove_selected,
-- 
2.37.3

[patch net-next RFC 4/7] nfp: Reorder devl_port_register/unregister() calls to be done when devlink is registered

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Move the code so devl_port_register/unregister() are called only
then devlink is registered.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 .../net/ethernet/netronome/nfp/nfp_net_main.c | 22 ++++++++++++++-----
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index abfe788d558f..9b4c48defd5c 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -752,7 +752,7 @@ int nfp_net_pci_probe(struct nfp_pf *pf)
 
 	err = nfp_shared_buf_register(pf);
 	if (err)
-		goto err_devlink_unreg;
+		goto err_clean_app;
 
 	err = nfp_devlink_params_register(pf);
 	if (err)
@@ -769,23 +769,29 @@ int nfp_net_pci_probe(struct nfp_pf *pf)
 	err = nfp_net_pf_alloc_irqs(pf);
 	if (err)
 		goto err_free_vnics;
+	devl_unlock(devlink);
 
+	devlink_register(devlink);
+
+	devl_lock(devlink);
 	err = nfp_net_pf_app_start(pf);
 	if (err)
-		goto err_free_irqs;
+		goto err_devlink_unreg;
 
 	err = nfp_net_pf_init_vnics(pf);
 	if (err)
 		goto err_stop_app;
 
 	devl_unlock(devlink);
-	devlink_register(devlink);
 
 	return 0;
 
 err_stop_app:
 	nfp_net_pf_app_stop(pf);
-err_free_irqs:
+	devl_unlock(devlink);
+err_devlink_unreg:
+	devlink_unregister(devlink);
+	devl_lock(devlink);
 	nfp_net_pf_free_irqs(pf);
 err_free_vnics:
 	nfp_net_pf_free_vnics(pf);
@@ -795,7 +801,7 @@ int nfp_net_pci_probe(struct nfp_pf *pf)
 	nfp_devlink_params_unregister(pf);
 err_shared_buf_unreg:
 	nfp_shared_buf_unregister(pf);
-err_devlink_unreg:
+err_clean_app:
 	cancel_work_sync(&pf->port_refresh_work);
 	nfp_net_pf_app_clean(pf);
 err_unmap:
@@ -808,7 +814,6 @@ void nfp_net_pci_remove(struct nfp_pf *pf)
 	struct devlink *devlink = priv_to_devlink(pf);
 	struct nfp_net *nn, *next;
 
-	devlink_unregister(priv_to_devlink(pf));
 	devl_lock(devlink);
 	list_for_each_entry_safe(nn, next, &pf->vnics, vnic_list) {
 		if (!nfp_net_is_data_vnic(nn))
@@ -818,6 +823,11 @@ void nfp_net_pci_remove(struct nfp_pf *pf)
 	}
 
 	nfp_net_pf_app_stop(pf);
+	devl_unlock(devlink);
+
+	devlink_unregister(priv_to_devlink(pf));
+
+	devl_lock(devlink);
 	/* stop app first, to avoid double free of ctrl vNIC's ddir */
 	nfp_net_debugfs_dir_clean(&pf->ddir);
 
-- 
2.37.3

[patch net-next RFC 5/7] mlx4: Reorder devl_port_register/unregister() calls to be done when devlink is registered

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Move the code so devl_port_register/unregister() are called only
then devlink is registered.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 60 +++++++++++++----------
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 3ae246391549..14f1c76a50eb 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3730,14 +3730,13 @@ static int mlx4_load_one(struct pci_dev *pdev, int pci_dev_data,
 }
 
 static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data,
-			   struct mlx4_priv *priv)
+			   unsigned int *total_vfs,
+			   int *nvfs, struct mlx4_priv *priv)
 {
 	int err;
-	int nvfs[MLX4_MAX_PORTS + 1] = {0, 0, 0};
 	int prb_vf[MLX4_MAX_PORTS + 1] = {0, 0, 0};
 	const int param_map[MLX4_MAX_PORTS + 1][MLX4_MAX_PORTS + 1] = {
 		{2, 0, 0}, {0, 1, 2}, {0, 1, 2} };
-	unsigned total_vfs = 0;
 	unsigned int i;
 
 	pr_info(DRV_NAME ": Initializing %s\n", pci_name(pdev));
@@ -3752,8 +3751,8 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data,
 	 * per port, we must limit the number of VFs to 63 (since their are
 	 * 128 MACs)
 	 */
-	for (i = 0; i < ARRAY_SIZE(nvfs) && i < num_vfs_argc;
-	     total_vfs += nvfs[param_map[num_vfs_argc - 1][i]], i++) {
+	for (i = 0; i <= MLX4_MAX_PORTS && i < num_vfs_argc;
+	     *total_vfs += nvfs[param_map[num_vfs_argc - 1][i]], i++) {
 		nvfs[param_map[num_vfs_argc - 1][i]] = num_vfs[i];
 		if (nvfs[i] < 0) {
 			dev_err(&pdev->dev, "num_vfs module parameter cannot be negative\n");
@@ -3770,10 +3769,10 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data,
 			goto err_disable_pdev;
 		}
 	}
-	if (total_vfs > MLX4_MAX_NUM_VF) {
+	if (*total_vfs > MLX4_MAX_NUM_VF) {
 		dev_err(&pdev->dev,
 			"Requested more VF's (%d) than allowed by hw (%d)\n",
-			total_vfs, MLX4_MAX_NUM_VF);
+			*total_vfs, MLX4_MAX_NUM_VF);
 		err = -EINVAL;
 		goto err_disable_pdev;
 	}
@@ -3828,14 +3827,14 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data,
 		/* When acting as pf, we normally skip vfs unless explicitly
 		 * requested to probe them.
 		 */
-		if (total_vfs) {
+		if (*total_vfs) {
 			unsigned vfs_offset = 0;
 
-			for (i = 0; i < ARRAY_SIZE(nvfs) &&
+			for (i = 0; i <= MLX4_MAX_PORTS &&
 			     vfs_offset + nvfs[i] < extended_func_num(pdev);
 			     vfs_offset += nvfs[i], i++)
 				;
-			if (i == ARRAY_SIZE(nvfs)) {
+			if (i == MLX4_MAX_PORTS + 1) {
 				err = -ENODEV;
 				goto err_release_regions;
 			}
@@ -3857,15 +3856,8 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data,
 	if (err)
 		goto err_crdump;
 
-	err = mlx4_load_one(pdev, pci_dev_data, total_vfs, nvfs, priv, 0);
-	if (err)
-		goto err_catas;
-
 	return 0;
 
-err_catas:
-	mlx4_catas_end(&priv->dev);
-
 err_crdump:
 	mlx4_crdump_end(&priv->dev);
 
@@ -3994,6 +3986,8 @@ static const struct devlink_ops mlx4_devlink_ops = {
 
 static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 {
+	int nvfs[MLX4_MAX_PORTS + 1] = {0, 0, 0};
+	unsigned int total_vfs = 0;
 	struct devlink *devlink;
 	struct mlx4_priv *priv;
 	struct mlx4_dev *dev;
@@ -4024,9 +4018,9 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	ret = devlink_params_register(devlink, mlx4_devlink_params,
 				      ARRAY_SIZE(mlx4_devlink_params));
 	if (ret)
-		goto err_devlink_unregister;
+		goto err_persist_free;
 	mlx4_devlink_set_params_init_values(devlink);
-	ret =  __mlx4_init_one(pdev, id->driver_data, priv);
+	ret =  __mlx4_init_one(pdev, id->driver_data, &total_vfs, nvfs, priv);
 	if (ret)
 		goto err_params_unregister;
 
@@ -4034,12 +4028,21 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	devlink_set_features(devlink, DEVLINK_F_RELOAD);
 	devl_unlock(devlink);
 	devlink_register(devlink);
+	devl_lock(devlink);
+	ret = mlx4_load_one(pdev, priv->pci_dev_data, total_vfs, nvfs, priv, 0);
+	devl_unlock(devlink);
+	if (ret)
+		goto err_devlink_unregister;
+
 	return 0;
 
+err_devlink_unregister:
+	devlink_unregister(devlink);
+	devl_lock(devlink);
 err_params_unregister:
 	devlink_params_unregister(devlink, mlx4_devlink_params,
 				  ARRAY_SIZE(mlx4_devlink_params));
-err_devlink_unregister:
+err_persist_free:
 	kfree(dev->persist);
 err_devlink_free:
 	devl_unlock(devlink);
@@ -4146,6 +4149,16 @@ static void mlx4_remove_one(struct pci_dev *pdev)
 	struct devlink *devlink = priv_to_devlink(priv);
 	int active_vfs = 0;
 
+	/* device marked to be under deletion running now without the lock
+	 * letting other tasks to be terminated
+	 */
+	devl_lock(devlink);
+	if (persist->interface_state & MLX4_INTERFACE_STATE_UP)
+		mlx4_unload_one(pdev);
+	else
+		mlx4_info(dev, "%s: interface is down\n", __func__);
+	devl_unlock(devlink);
+
 	devlink_unregister(devlink);
 
 	devl_lock(devlink);
@@ -4165,13 +4178,6 @@ static void mlx4_remove_one(struct pci_dev *pdev)
 		}
 	}
 
-	/* device marked to be under deletion running now without the lock
-	 * letting other tasks to be terminated
-	 */
-	if (persist->interface_state & MLX4_INTERFACE_STATE_UP)
-		mlx4_unload_one(pdev);
-	else
-		mlx4_info(dev, "%s: interface is down\n", __func__);
 	mlx4_catas_end(dev);
 	mlx4_crdump_end(dev);
 	if (dev->flags & MLX4_FLAG_SRIOV && !active_vfs) {
-- 
2.37.3

[patch net-next RFC 6/7] mlx5: Reorder devl_port_register/unregister() calls to be done when devlink is registered

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Move the code so devl_port_register/unregister() are called only
then devlink is registered.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/dev.c   | 10 ++++++++--
 drivers/net/ethernet/mellanox/mlx5/core/main.c  | 17 ++++++++++-------
 .../ethernet/mellanox/mlx5/core/sf/dev/driver.c |  9 +++++++++
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dev.c b/drivers/net/ethernet/mellanox/mlx5/core/dev.c
index 0571e40c6ee5..dd3801198898 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/dev.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/dev.c
@@ -444,11 +444,14 @@ int mlx5_register_device(struct mlx5_core_dev *dev)
 {
 	int ret;
 
-	devl_assert_locked(priv_to_devlink(dev));
+	devl_lock(priv_to_devlink(dev));
+	mutex_lock(&dev->intf_state_mutex);
 	mutex_lock(&mlx5_intf_mutex);
 	dev->priv.flags &= ~MLX5_PRIV_FLAGS_DISABLE_ALL_ADEV;
 	ret = mlx5_rescan_drivers_locked(dev);
 	mutex_unlock(&mlx5_intf_mutex);
+	mutex_unlock(&dev->intf_state_mutex);
+	devl_unlock(priv_to_devlink(dev));
 	if (ret)
 		mlx5_unregister_device(dev);
 
@@ -457,11 +460,14 @@ int mlx5_register_device(struct mlx5_core_dev *dev)
 
 void mlx5_unregister_device(struct mlx5_core_dev *dev)
 {
-	devl_assert_locked(priv_to_devlink(dev));
+	devl_lock(priv_to_devlink(dev));
+	mutex_lock(&dev->intf_state_mutex);
 	mutex_lock(&mlx5_intf_mutex);
 	dev->priv.flags = MLX5_PRIV_FLAGS_DISABLE_ALL_ADEV;
 	mlx5_rescan_drivers_locked(dev);
 	mutex_unlock(&mlx5_intf_mutex);
+	mutex_unlock(&dev->intf_state_mutex);
+	devl_unlock(priv_to_devlink(dev));
 }
 
 static int add_drivers(struct mlx5_core_dev *dev)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 7f5db13e3550..f6f37289b49d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1392,16 +1392,10 @@ int mlx5_init_one(struct mlx5_core_dev *dev)
 	if (err)
 		goto err_devlink_reg;
 
-	err = mlx5_register_device(dev);
-	if (err)
-		goto err_register;
-
 	mutex_unlock(&dev->intf_state_mutex);
 	devl_unlock(devlink);
 	return 0;
 
-err_register:
-	mlx5_devlink_unregister(priv_to_devlink(dev));
 err_devlink_reg:
 	clear_bit(MLX5_INTERFACE_STATE_UP, &dev->intf_state);
 	mlx5_unload(dev);
@@ -1423,7 +1417,6 @@ void mlx5_uninit_one(struct mlx5_core_dev *dev)
 	devl_lock(devlink);
 	mutex_lock(&dev->intf_state_mutex);
 
-	mlx5_unregister_device(dev);
 	mlx5_devlink_unregister(priv_to_devlink(dev));
 
 	if (!test_bit(MLX5_INTERFACE_STATE_UP, &dev->intf_state)) {
@@ -1747,8 +1740,17 @@ static int probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	pci_save_state(pdev);
 	devlink_register(devlink);
+	err = mlx5_register_device(dev);
+	if (err) {
+		mlx5_core_err(dev, "mlx5_register_device failed with error code %d\n",
+			      err);
+		goto err_register_device;
+	}
+
 	return 0;
 
+err_register_device:
+	devlink_unregister(devlink);
 err_init_one:
 	mlx5_pci_close(dev);
 pci_init_err:
@@ -1771,6 +1773,7 @@ static void remove_one(struct pci_dev *pdev)
 	 */
 	mlx5_drain_fw_reset(dev);
 	set_bit(MLX5_BREAK_FW_WAIT, &dev->intf_state);
+	mlx5_unregister_device(dev);
 	devlink_unregister(devlink);
 	mlx5_sriov_disable(pdev);
 	mlx5_crdump_disable(dev);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/driver.c b/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/driver.c
index 7b4783ce213e..90fcb30f7481 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/driver.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/driver.c
@@ -46,9 +46,17 @@ static int mlx5_sf_dev_probe(struct auxiliary_device *adev, const struct auxilia
 		mlx5_core_warn(mdev, "mlx5_init_one err=%d\n", err);
 		goto init_one_err;
 	}
+
 	devlink_register(devlink);
+
+	err = mlx5_register_device(mdev);
+	if (err)
+		goto register_device_err;
+
 	return 0;
 
+register_device_err:
+	devlink_unregister(devlink);
 init_one_err:
 	iounmap(mdev->iseg);
 remap_err:
@@ -63,6 +71,7 @@ static void mlx5_sf_dev_remove(struct auxiliary_device *adev)
 	struct mlx5_sf_dev *sf_dev = container_of(adev, struct mlx5_sf_dev, adev);
 	struct devlink *devlink = priv_to_devlink(sf_dev->mdev);
 
+	mlx5_unregister_device(sf_dev->mdev);
 	devlink_unregister(devlink);
 	mlx5_uninit_one(sf_dev->mdev);
 	iounmap(sf_dev->mdev->iseg);
-- 
2.37.3

[patch net-next RFC 7/7] devlink: assert if devl_port_register/unregister() is called on unregistered devlink instance

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Now when all drivers do call devl_port_register/unregister() withing the
time frame during which the devlink is registered, put and assertion to
the functions to check that and avoid going back.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 net/core/devlink.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index fca3ebee97b0..c46dd7753368 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -10088,6 +10088,7 @@ int devl_port_register(struct devlink *devlink,
 		       struct devlink_port *devlink_port,
 		       unsigned int port_index)
 {
+	ASSERT_DEVLINK_REGISTERED(devlink);
 	devl_assert_locked(devlink);
 
 	if (devlink_port_index_exists(devlink, port_index))
@@ -10145,6 +10146,7 @@ EXPORT_SYMBOL_GPL(devlink_port_register);
  */
 void devl_port_unregister(struct devlink_port *devlink_port)
 {
+	ASSERT_DEVLINK_REGISTERED(devlink_port->devlink);
 	lockdep_assert_held(&devlink_port->devlink->lock);
 	WARN_ON(devlink_port->type != DEVLINK_PORT_TYPE_NOTSET);
 
-- 
2.37.3

Re: [patch net-next RFC 0/7] devlink: make sure devlink port registers/unregisters only for registered instance

[Leon Romanovsky]

On Thu, Dec 01, 2022 at 05:46:01PM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>

<...>

> Jiri Pirko (7):
>   devlink: Reorder devlink_port_register/unregister() calls to be done
>     when devlink is registered
>   netdevsim: Reorder devl_port_register/unregister() calls to be done
>     when devlink is registered
>   mlxsw: Reorder devl_port_register/unregister() calls to be done when
>     devlink is registered
>   nfp: Reorder devl_port_register/unregister() calls to be done when
>     devlink is registered
>   mlx4: Reorder devl_port_register/unregister() calls to be done when
>     devlink is registered
>   mlx5: Reorder devl_port_register/unregister() calls to be done when
>     devlink is registered
>   devlink: assert if devl_port_register/unregister() is called on
>     unregistered devlink instance

Thanks, it is more clear now what you wanted.
Everything here looks ok, but can you please find better titles for the
commit messages? They are too long.

Not related to the series, but spotted during the code review,
It will be nice if you can get rid of devlink_port->registered and rely
xarray marks for that. It will be cleaner and aligned with devlink object.

Thanks

Re: [patch net-next RFC 7/7] devlink: assert if devl_port_register/unregister() is called on unregistered devlink instance

[Leon Romanovsky]

On Thu, Dec 01, 2022 at 05:46:08PM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Now when all drivers do call devl_port_register/unregister() withing the
> time frame during which the devlink is registered, put and assertion to
> the functions to check that and avoid going back.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
>  net/core/devlink.c | 2 ++
>  1 file changed, 2 insertions(+)

You also need to remove delayed notifications from devlink_notify_register()

   9862         xa_for_each(&devlink->ports, port_index, devlink_port)
   9863                 devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);

Thanks

Re: [patch net-next RFC 7/7] devlink: assert if devl_port_register/unregister() is called on unregistered devlink instance

[Jiri Pirko]

Sun, Dec 04, 2022 at 12:40:09PM CET, leon@kernel.org wrote:
>On Thu, Dec 01, 2022 at 05:46:08PM +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>> 
>> Now when all drivers do call devl_port_register/unregister() withing the
>> time frame during which the devlink is registered, put and assertion to
>> the functions to check that and avoid going back.
>> 
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> ---
>>  net/core/devlink.c | 2 ++
>>  1 file changed, 2 insertions(+)
>
>You also need to remove delayed notifications from devlink_notify_register()
>
>   9862         xa_for_each(&devlink->ports, port_index, devlink_port)
>   9863                 devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
>

Correct, I forgot.

>Thanks

Re: [patch net-next RFC 0/7] devlink: make sure devlink port registers/unregisters only for registered instance

[Jiri Pirko]

Sun, Dec 04, 2022 at 12:35:37PM CET, leon@kernel.org wrote:
>On Thu, Dec 01, 2022 at 05:46:01PM +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>
><...>
>
>> Jiri Pirko (7):
>>   devlink: Reorder devlink_port_register/unregister() calls to be done
>>     when devlink is registered
>>   netdevsim: Reorder devl_port_register/unregister() calls to be done
>>     when devlink is registered
>>   mlxsw: Reorder devl_port_register/unregister() calls to be done when
>>     devlink is registered
>>   nfp: Reorder devl_port_register/unregister() calls to be done when
>>     devlink is registered
>>   mlx4: Reorder devl_port_register/unregister() calls to be done when
>>     devlink is registered
>>   mlx5: Reorder devl_port_register/unregister() calls to be done when
>>     devlink is registered
>>   devlink: assert if devl_port_register/unregister() is called on
>>     unregistered devlink instance
>
>Thanks, it is more clear now what you wanted.
>Everything here looks ok, but can you please find better titles for the
>commit messages? They are too long.

Okay, will try.


>
>Not related to the series, but spotted during the code review,
>It will be nice if you can get rid of devlink_port->registered and rely
>xarray marks for that. It will be cleaner and aligned with devlink object.

Okay, will look into it and address in a separate patch/patchset.


Thanks!

>
>Thanks