linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v1 00/21] Move devlink_register to be last devlink command
@ 2021-09-25 11:22 Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 01/21] devlink: Notify users when objects are accessible Leon Romanovsky
                   ` (20 more replies)
  0 siblings, 21 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

This is second version of patch series
https://lore.kernel.org/netdev/cover.1628599239.git.leonro@nvidia.com/

The main change is addition of delayed notification logic that will
allowed us to delete devlink_params_publish API (future series will
remove it completely) and conversion of all drivers to have devlink_register
being last commend.

The series itself is pretty straightforward, except liquidio driver
which performs initializations in various workqueues without proper
locks. That driver doesn't hole device_lock and it is clearly broken
for any parallel driver core flows (modprobe + devlink + PCI reset will
100% crash it).

In order to annotate devlink_register() will lockdep of holding
device_lock, I added workaround in this driver.

Thanks

----------------------
From previous cover letter:
Hi Dave and Jakub,

This series prepares code to remove devlink_reload_enable/_disable API
and in order to do, we move all devlink_register() calls to be right
before devlink_reload_enable().

The best place for such a call should be right before exiting from
the probe().

This is done because devlink_register() opens devlink netlink to the
users and gives them a venue to issue commands before initialization
is finished.

1. Some drivers were aware of such "functionality" and tried to protect
themselves with extra locks, state machines and devlink_reload_enable().
Let's assume that it worked for them, but I'm personally skeptical about
it.

2. Some drivers copied that pattern, but without locks and state
machines. That protected them from reload flows, but not from any _set_
routines.

3. And all other drivers simply didn't understand the implications of early
devlink_register() and can be seen as "broken".

Thanks

Leon Romanovsky (21):
  devlink: Notify users when objects are accessible
  bnxt_en: Register devlink instance at the end devlink configuration
  liquidio: Overcome missing device lock protection in init/remove flows
  dpaa2-eth: Register devlink instance at the end of probe
  net: hinic: Open device for the user access when it is ready
  ice: Open devlink when device is ready
  octeontx2: Move devlink registration to be last devlink command
  net/prestera: Split devlink and traps registrations to separate
    routines
  net/mlx4: Move devlink_register to be the last initialization command
  net/mlx5: Accept devlink user input after driver initialization
    complete
  mlxsw: core: Register devlink instance last
  net: mscc: ocelot: delay devlink registration to the end
  nfp: Move delink_register to be last command
  ionic: Move devlink registration to be last devlink command
  qed: Move devlink registration to be last devlink command
  net: ethernet: ti: Move devlink registration to be last devlink
    command
  netdevsim: Move devlink registration to be last devlink command
  net: wwan: iosm: Move devlink_register to be last devlink command
  ptp: ocp: Move devlink registration to be last devlink command
  staging: qlge: Move devlink registration to be last devlink command
  net: dsa: Move devlink registration to be last devlink command

 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |  15 +--
 .../net/ethernet/cavium/liquidio/lio_main.c   |  19 ++--
 .../freescale/dpaa2/dpaa2-eth-devlink.c       |  14 ++-
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  |   9 +-
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.h  |   5 +-
 .../net/ethernet/huawei/hinic/hinic_hw_dev.c  |   7 +-
 drivers/net/ethernet/intel/ice/ice_main.c     |   6 +-
 .../marvell/octeontx2/af/rvu_devlink.c        |  10 +-
 .../marvell/octeontx2/nic/otx2_devlink.c      |  15 +--
 .../marvell/prestera/prestera_devlink.c       |  29 +----
 .../marvell/prestera/prestera_devlink.h       |   4 +-
 .../ethernet/marvell/prestera/prestera_main.c |   8 +-
 drivers/net/ethernet/mellanox/mlx4/main.c     |   8 +-
 .../net/ethernet/mellanox/mlx5/core/devlink.c |   9 +-
 .../net/ethernet/mellanox/mlx5/core/main.c    |   2 +
 .../mellanox/mlx5/core/sf/dev/driver.c        |   2 +
 drivers/net/ethernet/mellanox/mlxsw/core.c    |  19 +---
 drivers/net/ethernet/mscc/ocelot_vsc7514.c    |   5 +-
 .../ethernet/netronome/nfp/devlink_param.c    |   9 +-
 .../net/ethernet/netronome/nfp/nfp_net_main.c |   5 +-
 .../ethernet/pensando/ionic/ionic_devlink.c   |   4 +-
 drivers/net/ethernet/qlogic/qed/qed_devlink.c |   7 +-
 drivers/net/ethernet/ti/am65-cpsw-nuss.c      |  15 +--
 drivers/net/ethernet/ti/cpsw_new.c            |   7 +-
 drivers/net/netdevsim/dev.c                   |   8 +-
 drivers/net/wwan/iosm/iosm_ipc_devlink.c      |   7 +-
 drivers/ptp/ptp_ocp.c                         |   6 +-
 drivers/staging/qlge/qlge_main.c              |   8 +-
 net/core/devlink.c                            | 107 +++++++++++++++---
 net/dsa/dsa2.c                                |  10 +-
 30 files changed, 202 insertions(+), 177 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 01/21] devlink: Notify users when objects are accessible
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-28  2:49   ` Eric Dumazet
  2021-09-25 11:22 ` [PATCH net-next v1 02/21] bnxt_en: Register devlink instance at the end devlink configuration Leon Romanovsky
                   ` (19 subsequent siblings)
  20 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

The devlink core code notified users about add/remove objects without
relation if this object can be accessible or not. In this patch we unify
such user visible notifications in one place.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 net/core/devlink.c | 107 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 93 insertions(+), 14 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 3ea33c689790..06edb2f1d21e 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -742,6 +742,7 @@ static void devlink_notify(struct devlink *devlink, enum devlink_command cmd)
 	int err;
 
 	WARN_ON(cmd != DEVLINK_CMD_NEW && cmd != DEVLINK_CMD_DEL);
+	WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
@@ -1040,11 +1041,15 @@ static int devlink_nl_port_fill(struct sk_buff *msg,
 static void devlink_port_notify(struct devlink_port *devlink_port,
 				enum devlink_command cmd)
 {
+	struct devlink *devlink = devlink_port->devlink;
 	struct sk_buff *msg;
 	int err;
 
 	WARN_ON(cmd != DEVLINK_CMD_PORT_NEW && cmd != DEVLINK_CMD_PORT_DEL);
 
+	if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
+		return;
+
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
 		return;
@@ -1055,18 +1060,19 @@ static void devlink_port_notify(struct devlink_port *devlink_port,
 		return;
 	}
 
-	genlmsg_multicast_netns(&devlink_nl_family,
-				devlink_net(devlink_port->devlink), msg, 0,
-				DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
+	genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink), msg,
+				0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
 }
 
 static void devlink_rate_notify(struct devlink_rate *devlink_rate,
 				enum devlink_command cmd)
 {
+	struct devlink *devlink = devlink_rate->devlink;
 	struct sk_buff *msg;
 	int err;
 
 	WARN_ON(cmd != DEVLINK_CMD_RATE_NEW && cmd != DEVLINK_CMD_RATE_DEL);
+	WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
@@ -1078,9 +1084,8 @@ static void devlink_rate_notify(struct devlink_rate *devlink_rate,
 		return;
 	}
 
-	genlmsg_multicast_netns(&devlink_nl_family,
-				devlink_net(devlink_rate->devlink), msg, 0,
-				DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
+	genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink), msg,
+				0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
 }
 
 static int devlink_nl_cmd_rate_get_dumpit(struct sk_buff *msg,
@@ -4150,6 +4155,7 @@ static void __devlink_flash_update_notify(struct devlink *devlink,
 	WARN_ON(cmd != DEVLINK_CMD_FLASH_UPDATE &&
 		cmd != DEVLINK_CMD_FLASH_UPDATE_END &&
 		cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS);
+	WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
@@ -5145,17 +5151,18 @@ static void devlink_nl_region_notify(struct devlink_region *region,
 				     struct devlink_snapshot *snapshot,
 				     enum devlink_command cmd)
 {
+	struct devlink *devlink = region->devlink;
 	struct sk_buff *msg;
 
 	WARN_ON(cmd != DEVLINK_CMD_REGION_NEW && cmd != DEVLINK_CMD_REGION_DEL);
+	WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));
 
 	msg = devlink_nl_region_notify_build(region, snapshot, cmd, 0, 0);
 	if (IS_ERR(msg))
 		return;
 
-	genlmsg_multicast_netns(&devlink_nl_family,
-				devlink_net(region->devlink), msg, 0,
-				DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
+	genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink), msg,
+				0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
 }
 
 /**
@@ -6920,10 +6927,12 @@ devlink_nl_health_reporter_fill(struct sk_buff *msg,
 static void devlink_recover_notify(struct devlink_health_reporter *reporter,
 				   enum devlink_command cmd)
 {
+	struct devlink *devlink = reporter->devlink;
 	struct sk_buff *msg;
 	int err;
 
 	WARN_ON(cmd != DEVLINK_CMD_HEALTH_REPORTER_RECOVER);
+	WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
@@ -6935,9 +6944,8 @@ static void devlink_recover_notify(struct devlink_health_reporter *reporter,
 		return;
 	}
 
-	genlmsg_multicast_netns(&devlink_nl_family,
-				devlink_net(reporter->devlink),
-				msg, 0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
+	genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink), msg,
+				0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
 }
 
 void
@@ -8955,6 +8963,68 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
 }
 EXPORT_SYMBOL_GPL(devlink_alloc_ns);
 
+static void
+devlink_trap_policer_notify(struct devlink *devlink,
+			    const struct devlink_trap_policer_item *policer_item,
+			    enum devlink_command cmd);
+static void
+devlink_trap_group_notify(struct devlink *devlink,
+			  const struct devlink_trap_group_item *group_item,
+			  enum devlink_command cmd);
+static void devlink_trap_notify(struct devlink *devlink,
+				const struct devlink_trap_item *trap_item,
+				enum devlink_command cmd);
+
+static void devlink_notify_register(struct devlink *devlink)
+{
+	struct devlink_trap_policer_item *policer_item;
+	struct devlink_trap_group_item *group_item;
+	struct devlink_trap_item *trap_item;
+	struct devlink_port *devlink_port;
+
+	devlink_notify(devlink, DEVLINK_CMD_NEW);
+	list_for_each_entry(devlink_port, &devlink->port_list, list)
+		devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
+
+	list_for_each_entry(policer_item, &devlink->trap_policer_list, list)
+		devlink_trap_policer_notify(devlink, policer_item,
+					    DEVLINK_CMD_TRAP_POLICER_NEW);
+
+	list_for_each_entry(group_item, &devlink->trap_group_list, list)
+		devlink_trap_group_notify(devlink, group_item,
+					  DEVLINK_CMD_TRAP_GROUP_NEW);
+
+	list_for_each_entry(trap_item, &devlink->trap_list, list)
+		devlink_trap_notify(devlink, trap_item, DEVLINK_CMD_TRAP_NEW);
+
+	devlink_params_publish(devlink);
+}
+
+static void devlink_notify_unregister(struct devlink *devlink)
+{
+	struct devlink_trap_policer_item *policer_item;
+	struct devlink_trap_group_item *group_item;
+	struct devlink_trap_item *trap_item;
+	struct devlink_port *devlink_port;
+
+	devlink_params_unpublish(devlink);
+
+	list_for_each_entry_reverse(trap_item, &devlink->trap_list, list)
+		devlink_trap_notify(devlink, trap_item, DEVLINK_CMD_TRAP_DEL);
+
+	list_for_each_entry_reverse(group_item, &devlink->trap_group_list, list)
+		devlink_trap_group_notify(devlink, group_item,
+					  DEVLINK_CMD_TRAP_GROUP_DEL);
+	list_for_each_entry_reverse(policer_item, &devlink->trap_policer_list,
+				    list)
+		devlink_trap_policer_notify(devlink, policer_item,
+					    DEVLINK_CMD_TRAP_POLICER_DEL);
+
+	list_for_each_entry_reverse(devlink_port, &devlink->port_list, list)
+		devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_DEL);
+	devlink_notify(devlink, DEVLINK_CMD_DEL);
+}
+
 /**
  *	devlink_register - Register devlink instance
  *
@@ -8964,7 +9034,7 @@ void devlink_register(struct devlink *devlink)
 {
 	mutex_lock(&devlink_mutex);
 	xa_set_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
-	devlink_notify(devlink, DEVLINK_CMD_NEW);
+	devlink_notify_register(devlink);
 	mutex_unlock(&devlink_mutex);
 }
 EXPORT_SYMBOL_GPL(devlink_register);
@@ -8982,7 +9052,7 @@ void devlink_unregister(struct devlink *devlink)
 	mutex_lock(&devlink_mutex);
 	WARN_ON(devlink_reload_supported(devlink->ops) &&
 		devlink->reload_enabled);
-	devlink_notify(devlink, DEVLINK_CMD_DEL);
+	devlink_notify_unregister(devlink);
 	xa_clear_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
 	mutex_unlock(&devlink_mutex);
 }
@@ -10086,6 +10156,9 @@ void devlink_params_publish(struct devlink *devlink)
 {
 	struct devlink_param_item *param_item;
 
+	if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
+		return;
+
 	list_for_each_entry(param_item, &devlink->param_list, list) {
 		if (param_item->published)
 			continue;
@@ -10631,6 +10704,8 @@ devlink_trap_group_notify(struct devlink *devlink,
 
 	WARN_ON_ONCE(cmd != DEVLINK_CMD_TRAP_GROUP_NEW &&
 		     cmd != DEVLINK_CMD_TRAP_GROUP_DEL);
+	if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
+		return;
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
@@ -10672,6 +10747,8 @@ static void devlink_trap_notify(struct devlink *devlink,
 
 	WARN_ON_ONCE(cmd != DEVLINK_CMD_TRAP_NEW &&
 		     cmd != DEVLINK_CMD_TRAP_DEL);
+	if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
+		return;
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
@@ -11053,6 +11130,8 @@ devlink_trap_policer_notify(struct devlink *devlink,
 
 	WARN_ON_ONCE(cmd != DEVLINK_CMD_TRAP_POLICER_NEW &&
 		     cmd != DEVLINK_CMD_TRAP_POLICER_DEL);
+	if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
+		return;
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 02/21] bnxt_en: Register devlink instance at the end devlink configuration
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 01/21] devlink: Notify users when objects are accessible Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 03/21] liquidio: Overcome missing device lock protection in init/remove flows Leon Romanovsky
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

Move devlink_register() to be last command in devlink configuration
sequence, so no user space access will be possible till devlink instance
is fully operable. As part of this change, the devlink_params_publish
call is removed as not needed.

This change fixes forgotten devlink_params_unpublish() too.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index ed95e28d60ef..951c0c00cc95 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -745,14 +745,10 @@ static int bnxt_dl_params_register(struct bnxt *bp)
 
 	rc = devlink_params_register(bp->dl, bnxt_dl_params,
 				     ARRAY_SIZE(bnxt_dl_params));
-	if (rc) {
+	if (rc)
 		netdev_warn(bp->dev, "devlink_params_register failed. rc=%d\n",
 			    rc);
-		return rc;
-	}
-	devlink_params_publish(bp->dl);
-
-	return 0;
+	return rc;
 }
 
 static void bnxt_dl_params_unregister(struct bnxt *bp)
@@ -792,9 +788,8 @@ int bnxt_dl_register(struct bnxt *bp)
 	    bp->hwrm_spec_code > 0x10803)
 		bp->eswitch_mode = DEVLINK_ESWITCH_MODE_LEGACY;
 
-	devlink_register(dl);
 	if (!BNXT_PF(bp))
-		return 0;
+		goto out;
 
 	attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
 	attrs.phys.port_number = bp->pf.port_id;
@@ -811,6 +806,8 @@ int bnxt_dl_register(struct bnxt *bp)
 	if (rc)
 		goto err_dl_port_unreg;
 
+out:
+	devlink_register(dl);
 	return 0;
 
 err_dl_port_unreg:
@@ -824,10 +821,10 @@ void bnxt_dl_unregister(struct bnxt *bp)
 {
 	struct devlink *dl = bp->dl;
 
+	devlink_unregister(dl);
 	if (BNXT_PF(bp)) {
 		bnxt_dl_params_unregister(bp);
 		devlink_port_unregister(&bp->dl_port);
 	}
-	devlink_unregister(dl);
 	devlink_free(dl);
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 03/21] liquidio: Overcome missing device lock protection in init/remove flows
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 01/21] devlink: Notify users when objects are accessible Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 02/21] bnxt_en: Register devlink instance at the end devlink configuration Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 04/21] dpaa2-eth: Register devlink instance at the end of probe Leon Romanovsky
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

The liquidio driver is broken by design. It initialize PCI devices
in separate delayed works. It causes to the situation where device lock
is dropped during initialize and remove sequences.

That lock is part of driver/core and needed to protect from races during
init, destroy and bus invocations.

In addition to lack of locking protection, it has incorrect order of
destroy flows and very questionable synchronization scheme based on
atomic_t.

This change doesn't fix that driver but makes sure that rest of the
netdev subsystem doesn't suffer from such basic protection by adding
device_lock over devlink_*() APIs and by moving devlink_register()
to be last command in setup_nic_devices().

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 .../net/ethernet/cavium/liquidio/lio_main.c   | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index a34b3bb2dd4f..dafc79bd34f4 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -1279,6 +1279,14 @@ static int liquidio_stop_nic_module(struct octeon_device *oct)
 	struct lio *lio;
 
 	dev_dbg(&oct->pci_dev->dev, "Stopping network interfaces\n");
+	device_lock(&oct->pci_dev->dev);
+	if (oct->devlink) {
+		devlink_unregister(oct->devlink);
+		devlink_free(oct->devlink);
+		oct->devlink = NULL;
+	}
+	device_unlock(&oct->pci_dev->dev);
+
 	if (!oct->ifcount) {
 		dev_err(&oct->pci_dev->dev, "Init for Octeon was not completed\n");
 		return 1;
@@ -1300,12 +1308,6 @@ static int liquidio_stop_nic_module(struct octeon_device *oct)
 	for (i = 0; i < oct->ifcount; i++)
 		liquidio_destroy_nic_device(oct, i);
 
-	if (oct->devlink) {
-		devlink_unregister(oct->devlink);
-		devlink_free(oct->devlink);
-		oct->devlink = NULL;
-	}
-
 	dev_dbg(&oct->pci_dev->dev, "Network interfaces stopped\n");
 	return 0;
 }
@@ -3749,10 +3751,12 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
 		}
 	}
 
+	device_lock(&octeon_dev->pci_dev->dev);
 	devlink = devlink_alloc(&liquidio_devlink_ops,
 				sizeof(struct lio_devlink_priv),
 				&octeon_dev->pci_dev->dev);
 	if (!devlink) {
+		device_unlock(&octeon_dev->pci_dev->dev);
 		dev_err(&octeon_dev->pci_dev->dev, "devlink alloc failed\n");
 		goto setup_nic_dev_free;
 	}
@@ -3760,9 +3764,10 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
 	lio_devlink = devlink_priv(devlink);
 	lio_devlink->oct = octeon_dev;
 
-	devlink_register(devlink);
 	octeon_dev->devlink = devlink;
 	octeon_dev->eswitch_mode = DEVLINK_ESWITCH_MODE_LEGACY;
+	devlink_register(devlink);
+	device_unlock(&octeon_dev->pci_dev->dev);
 
 	return 0;
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 04/21] dpaa2-eth: Register devlink instance at the end of probe
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (2 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 03/21] liquidio: Overcome missing device lock protection in init/remove flows Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 05/21] net: hinic: Open device for the user access when it is ready Leon Romanovsky
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

Move devlink_register to be the last command in the initialization
sequence.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 .../ethernet/freescale/dpaa2/dpaa2-eth-devlink.c   | 14 +++++++++++---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c   |  9 ++++++---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h   |  5 ++++-
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
index 426926fb6fc6..7fefe1574b6a 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
@@ -189,7 +189,7 @@ static const struct devlink_ops dpaa2_eth_devlink_ops = {
 	.trap_group_action_set = dpaa2_eth_dl_trap_group_action_set,
 };
 
-int dpaa2_eth_dl_register(struct dpaa2_eth_priv *priv)
+int dpaa2_eth_dl_alloc(struct dpaa2_eth_priv *priv)
 {
 	struct net_device *net_dev = priv->net_dev;
 	struct device *dev = net_dev->dev.parent;
@@ -203,15 +203,23 @@ int dpaa2_eth_dl_register(struct dpaa2_eth_priv *priv)
 	}
 	dl_priv = devlink_priv(priv->devlink);
 	dl_priv->dpaa2_priv = priv;
+	return 0;
+}
+
+void dpaa2_eth_dl_free(struct dpaa2_eth_priv *priv)
+{
+	devlink_free(priv->devlink);
+}
+
 
+void dpaa2_eth_dl_register(struct dpaa2_eth_priv *priv)
+{
 	devlink_register(priv->devlink);
-	return 0;
 }
 
 void dpaa2_eth_dl_unregister(struct dpaa2_eth_priv *priv)
 {
 	devlink_unregister(priv->devlink);
-	devlink_free(priv->devlink);
 }
 
 int dpaa2_eth_dl_port_add(struct dpaa2_eth_priv *priv)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 7065c71ed7b8..03c168b1712f 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -4431,7 +4431,7 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
 	if (err)
 		goto err_connect_mac;
 
-	err = dpaa2_eth_dl_register(priv);
+	err = dpaa2_eth_dl_alloc(priv);
 	if (err)
 		goto err_dl_register;
 
@@ -4453,6 +4453,7 @@ 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;
 
@@ -4461,7 +4462,7 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
 err_dl_port_add:
 	dpaa2_eth_dl_traps_unregister(priv);
 err_dl_trap_register:
-	dpaa2_eth_dl_unregister(priv);
+	dpaa2_eth_dl_free(priv);
 err_dl_register:
 	dpaa2_eth_disconnect_mac(priv);
 err_connect_mac:
@@ -4508,6 +4509,8 @@ 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
@@ -4519,7 +4522,7 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
 
 	dpaa2_eth_dl_port_del(priv);
 	dpaa2_eth_dl_traps_unregister(priv);
-	dpaa2_eth_dl_unregister(priv);
+	dpaa2_eth_dl_free(priv);
 
 	if (priv->do_link_poll)
 		kthread_stop(priv->poll_thread);
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
index cdb623d5f2c1..628d2d45f045 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
@@ -725,7 +725,10 @@ void dpaa2_eth_set_rx_taildrop(struct dpaa2_eth_priv *priv,
 
 extern const struct dcbnl_rtnl_ops dpaa2_eth_dcbnl_ops;
 
-int dpaa2_eth_dl_register(struct dpaa2_eth_priv *priv);
+int dpaa2_eth_dl_alloc(struct dpaa2_eth_priv *priv);
+void dpaa2_eth_dl_free(struct dpaa2_eth_priv *priv);
+
+void dpaa2_eth_dl_register(struct dpaa2_eth_priv *priv);
 void dpaa2_eth_dl_unregister(struct dpaa2_eth_priv *priv);
 
 int dpaa2_eth_dl_port_add(struct dpaa2_eth_priv *priv);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 05/21] net: hinic: Open device for the user access when it is ready
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (3 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 04/21] dpaa2-eth: Register devlink instance at the end of probe Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 06/21] ice: Open devlink when device " Leon Romanovsky
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

Move devlink registration to be the last command in device activation,
so it opens the driver to accept such devlink commands from the user
when it is fully initialized.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
index b2ece3adbc72..657a15447bd0 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
@@ -754,11 +754,9 @@ static int init_pfhwdev(struct hinic_pfhwdev *pfhwdev)
 		return err;
 	}
 
-	hinic_devlink_register(hwdev->devlink_dev);
 	err = hinic_func_to_func_init(hwdev);
 	if (err) {
 		dev_err(&hwif->pdev->dev, "Failed to init mailbox\n");
-		hinic_devlink_unregister(hwdev->devlink_dev);
 		hinic_pf_to_mgmt_free(&pfhwdev->pf_to_mgmt);
 		return err;
 	}
@@ -781,7 +779,7 @@ static int init_pfhwdev(struct hinic_pfhwdev *pfhwdev)
 	}
 
 	hinic_set_pf_action(hwif, HINIC_PF_MGMT_ACTIVE);
-
+	hinic_devlink_register(hwdev->devlink_dev);
 	return 0;
 }
 
@@ -793,6 +791,7 @@ static void free_pfhwdev(struct hinic_pfhwdev *pfhwdev)
 {
 	struct hinic_hwdev *hwdev = &pfhwdev->hwdev;
 
+	hinic_devlink_unregister(hwdev->devlink_dev);
 	hinic_set_pf_action(hwdev->hwif, HINIC_PF_MGMT_INIT);
 
 	if (!HINIC_IS_VF(hwdev->hwif)) {
@@ -810,8 +809,6 @@ static void free_pfhwdev(struct hinic_pfhwdev *pfhwdev)
 
 	hinic_func_to_func_free(hwdev);
 
-	hinic_devlink_unregister(hwdev->devlink_dev);
-
 	hinic_pf_to_mgmt_free(&pfhwdev->pf_to_mgmt);
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 06/21] ice: Open devlink when device is ready
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (4 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 05/21] net: hinic: Open device for the user access when it is ready Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-27 19:47   ` Jesse Brandeburg
  2021-09-25 11:22 ` [PATCH net-next v1 07/21] octeontx2: Move devlink registration to be last devlink command Leon Romanovsky
                   ` (14 subsequent siblings)
  20 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

Move devlink_registration routine to be the last command, when the
device is fully initialized.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index aacc0b345bbe..627adf8fb89d 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -4258,8 +4258,6 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 
 	pf->msg_enable = netif_msg_init(debug, ICE_DFLT_NETIF_M);
 
-	ice_devlink_register(pf);
-
 #ifndef CONFIG_DYNAMIC_DEBUG
 	if (debug < -1)
 		hw->debug_mask = debug;
@@ -4493,6 +4491,7 @@ 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:
@@ -4516,7 +4515,6 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 	ice_devlink_destroy_regions(pf);
 	ice_deinit_hw(hw);
 err_exit_unroll:
-	ice_devlink_unregister(pf);
 	pci_disable_pcie_error_reporting(pdev);
 	pci_disable_device(pdev);
 	return err;
@@ -4593,6 +4591,7 @@ 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;
@@ -4629,7 +4628,6 @@ static void ice_remove(struct pci_dev *pdev)
 	ice_deinit_pf(pf);
 	ice_devlink_destroy_regions(pf);
 	ice_deinit_hw(&pf->hw);
-	ice_devlink_unregister(pf);
 
 	/* Issue a PFR as part of the prescribed driver unload flow.  Do not
 	 * do it via ice_schedule_reset() since there is no need to rebuild
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 07/21] octeontx2: Move devlink registration to be last devlink command
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (5 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 06/21] ice: Open devlink when device " Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 08/21] net/prestera: Split devlink and traps registrations to separate routines Leon Romanovsky
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

This change prevents from users to access device before devlink is fully
configured. This change allows us to delete call to devlink_params_publish()
and impossible check during unregister flow.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 .../ethernet/marvell/octeontx2/af/rvu_devlink.c   | 10 ++--------
 .../ethernet/marvell/octeontx2/nic/otx2_devlink.c | 15 +++------------
 2 files changed, 5 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index de9562acd04b..70bacd38a6d9 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
@@ -1510,7 +1510,6 @@ int rvu_register_dl(struct rvu *rvu)
 		return -ENOMEM;
 	}
 
-	devlink_register(dl);
 	rvu_dl = devlink_priv(dl);
 	rvu_dl->dl = dl;
 	rvu_dl->rvu = rvu;
@@ -1531,13 +1530,11 @@ int rvu_register_dl(struct rvu *rvu)
 		goto err_dl_health;
 	}
 
-	devlink_params_publish(dl);
-
+	devlink_register(dl);
 	return 0;
 
 err_dl_health:
 	rvu_health_reporters_destroy(rvu);
-	devlink_unregister(dl);
 	devlink_free(dl);
 	return err;
 }
@@ -1547,12 +1544,9 @@ void rvu_unregister_dl(struct rvu *rvu)
 	struct rvu_devlink *rvu_dl = rvu->rvu_dl;
 	struct devlink *dl = rvu_dl->dl;
 
-	if (!dl)
-		return;
-
+	devlink_unregister(dl);
 	devlink_params_unregister(dl, rvu_af_dl_params,
 				  ARRAY_SIZE(rvu_af_dl_params));
 	rvu_health_reporters_destroy(rvu);
-	devlink_unregister(dl);
 	devlink_free(dl);
 }
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
index 3de18f9433ae..777a27047c8e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
@@ -108,7 +108,6 @@ int otx2_register_dl(struct otx2_nic *pfvf)
 		return -ENOMEM;
 	}
 
-	devlink_register(dl);
 	otx2_dl = devlink_priv(dl);
 	otx2_dl->dl = dl;
 	otx2_dl->pfvf = pfvf;
@@ -122,12 +121,10 @@ int otx2_register_dl(struct otx2_nic *pfvf)
 		goto err_dl;
 	}
 
-	devlink_params_publish(dl);
-
+	devlink_register(dl);
 	return 0;
 
 err_dl:
-	devlink_unregister(dl);
 	devlink_free(dl);
 	return err;
 }
@@ -135,16 +132,10 @@ int otx2_register_dl(struct otx2_nic *pfvf)
 void otx2_unregister_dl(struct otx2_nic *pfvf)
 {
 	struct otx2_devlink *otx2_dl = pfvf->dl;
-	struct devlink *dl;
-
-	if (!otx2_dl || !otx2_dl->dl)
-		return;
-
-	dl = otx2_dl->dl;
+	struct devlink *dl = otx2_dl->dl;
 
+	devlink_unregister(dl);
 	devlink_params_unregister(dl, otx2_dl_params,
 				  ARRAY_SIZE(otx2_dl_params));
-
-	devlink_unregister(dl);
 	devlink_free(dl);
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 08/21] net/prestera: Split devlink and traps registrations to separate routines
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (6 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 07/21] octeontx2: Move devlink registration to be last devlink command Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 09/21] net/mlx4: Move devlink_register to be the last initialization command Leon Romanovsky
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

Separate devlink registrations and traps registrations so devlink will
be registered when driver is fully initialized.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 .../marvell/prestera/prestera_devlink.c       | 29 ++++---------------
 .../marvell/prestera/prestera_devlink.h       |  4 ++-
 .../ethernet/marvell/prestera/prestera_main.c |  8 +++--
 3 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
index 5cca007a3e17..06279cd6da67 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
@@ -345,8 +345,6 @@ static struct prestera_trap prestera_trap_items_arr[] = {
 	},
 };
 
-static void prestera_devlink_traps_fini(struct prestera_switch *sw);
-
 static int prestera_drop_counter_get(struct devlink *devlink,
 				     const struct devlink_trap *trap,
 				     u64 *p_drops);
@@ -381,8 +379,6 @@ static int prestera_trap_action_set(struct devlink *devlink,
 				    enum devlink_trap_action action,
 				    struct netlink_ext_ack *extack);
 
-static int prestera_devlink_traps_register(struct prestera_switch *sw);
-
 static const struct devlink_ops prestera_dl_ops = {
 	.info_get = prestera_dl_info_get,
 	.trap_init = prestera_trap_init,
@@ -407,34 +403,18 @@ void prestera_devlink_free(struct prestera_switch *sw)
 	devlink_free(dl);
 }
 
-int prestera_devlink_register(struct prestera_switch *sw)
+void prestera_devlink_register(struct prestera_switch *sw)
 {
 	struct devlink *dl = priv_to_devlink(sw);
-	int err;
 
 	devlink_register(dl);
-
-	err = prestera_devlink_traps_register(sw);
-	if (err) {
-		devlink_unregister(dl);
-		dev_err(sw->dev->dev, "devlink_traps_register failed: %d\n",
-			err);
-		return err;
-	}
-
-	return 0;
 }
 
 void prestera_devlink_unregister(struct prestera_switch *sw)
 {
-	struct prestera_trap_data *trap_data = sw->trap_data;
 	struct devlink *dl = priv_to_devlink(sw);
 
-	prestera_devlink_traps_fini(sw);
 	devlink_unregister(dl);
-
-	kfree(trap_data->trap_items_arr);
-	kfree(trap_data);
 }
 
 int prestera_devlink_port_register(struct prestera_port *port)
@@ -482,7 +462,7 @@ struct devlink_port *prestera_devlink_get_port(struct net_device *dev)
 	return &port->dl_port;
 }
 
-static int prestera_devlink_traps_register(struct prestera_switch *sw)
+int prestera_devlink_traps_register(struct prestera_switch *sw)
 {
 	const u32 groups_count = ARRAY_SIZE(prestera_trap_groups_arr);
 	const u32 traps_count = ARRAY_SIZE(prestera_trap_items_arr);
@@ -621,8 +601,9 @@ static int prestera_drop_counter_get(struct devlink *devlink,
 						 cpu_code_type, p_drops);
 }
 
-static void prestera_devlink_traps_fini(struct prestera_switch *sw)
+void prestera_devlink_traps_unregister(struct prestera_switch *sw)
 {
+	struct prestera_trap_data *trap_data = sw->trap_data;
 	struct devlink *dl = priv_to_devlink(sw);
 	const struct devlink_trap *trap;
 	int i;
@@ -634,4 +615,6 @@ static void prestera_devlink_traps_fini(struct prestera_switch *sw)
 
 	devlink_trap_groups_unregister(dl, prestera_trap_groups_arr,
 				       ARRAY_SIZE(prestera_trap_groups_arr));
+	kfree(trap_data->trap_items_arr);
+	kfree(trap_data);
 }
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_devlink.h b/drivers/net/ethernet/marvell/prestera/prestera_devlink.h
index cc34c3db13a2..b322295bad3a 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_devlink.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_devlink.h
@@ -9,7 +9,7 @@
 struct prestera_switch *prestera_devlink_alloc(struct prestera_device *dev);
 void prestera_devlink_free(struct prestera_switch *sw);
 
-int prestera_devlink_register(struct prestera_switch *sw);
+void prestera_devlink_register(struct prestera_switch *sw);
 void prestera_devlink_unregister(struct prestera_switch *sw);
 
 int prestera_devlink_port_register(struct prestera_port *port);
@@ -22,5 +22,7 @@ struct devlink_port *prestera_devlink_get_port(struct net_device *dev);
 
 void prestera_devlink_trap_report(struct prestera_port *port,
 				  struct sk_buff *skb, u8 cpu_code);
+int prestera_devlink_traps_register(struct prestera_switch *sw);
+void prestera_devlink_traps_unregister(struct prestera_switch *sw);
 
 #endif /* _PRESTERA_DEVLINK_H_ */
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index 44c670807fb3..78a7a00bce22 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
@@ -851,7 +851,7 @@ static int prestera_switch_init(struct prestera_switch *sw)
 	if (err)
 		goto err_span_init;
 
-	err = prestera_devlink_register(sw);
+	err = prestera_devlink_traps_register(sw);
 	if (err)
 		goto err_dl_register;
 
@@ -863,12 +863,13 @@ static int prestera_switch_init(struct prestera_switch *sw)
 	if (err)
 		goto err_ports_create;
 
+	prestera_devlink_register(sw);
 	return 0;
 
 err_ports_create:
 	prestera_lag_fini(sw);
 err_lag_init:
-	prestera_devlink_unregister(sw);
+	prestera_devlink_traps_unregister(sw);
 err_dl_register:
 	prestera_span_fini(sw);
 err_span_init:
@@ -888,9 +889,10 @@ 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_lag_fini(sw);
-	prestera_devlink_unregister(sw);
+	prestera_devlink_traps_unregister(sw);
 	prestera_span_fini(sw);
 	prestera_acl_fini(sw);
 	prestera_event_handlers_unregister(sw);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 09/21] net/mlx4: Move devlink_register to be the last initialization command
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (7 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 08/21] net/prestera: Split devlink and traps registrations to separate routines Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 10/21] net/mlx5: Accept devlink user input after driver initialization complete Leon Romanovsky
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

Refactor the code to make sure that devlink_register() is the last
command during initialization stage.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 27ed4694fbea..9541f3a920c8 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -4015,7 +4015,6 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	mutex_init(&dev->persist->interface_state_mutex);
 	mutex_init(&dev->persist->pci_status_mutex);
 
-	devlink_register(devlink);
 	ret = devlink_params_register(devlink, mlx4_devlink_params,
 				      ARRAY_SIZE(mlx4_devlink_params));
 	if (ret)
@@ -4025,16 +4024,15 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (ret)
 		goto err_params_unregister;
 
-	devlink_params_publish(devlink);
-	devlink_reload_enable(devlink);
 	pci_save_state(pdev);
+	devlink_register(devlink);
+	devlink_reload_enable(devlink);
 	return 0;
 
 err_params_unregister:
 	devlink_params_unregister(devlink, mlx4_devlink_params,
 				  ARRAY_SIZE(mlx4_devlink_params));
 err_devlink_unregister:
-	devlink_unregister(devlink);
 	kfree(dev->persist);
 err_devlink_free:
 	devlink_free(devlink);
@@ -4138,6 +4136,7 @@ static void mlx4_remove_one(struct pci_dev *pdev)
 	int active_vfs = 0;
 
 	devlink_reload_disable(devlink);
+	devlink_unregister(devlink);
 
 	if (mlx4_is_slave(dev))
 		persist->interface_state |= MLX4_INTERFACE_STATE_NOWAIT;
@@ -4173,7 +4172,6 @@ static void mlx4_remove_one(struct pci_dev *pdev)
 	mlx4_pci_disable_device(dev);
 	devlink_params_unregister(devlink, mlx4_devlink_params,
 				  ARRAY_SIZE(mlx4_devlink_params));
-	devlink_unregister(devlink);
 	kfree(dev->persist);
 	devlink_free(devlink);
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 10/21] net/mlx5: Accept devlink user input after driver initialization complete
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (8 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 09/21] net/mlx4: Move devlink_register to be the last initialization command Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 11/21] mlxsw: core: Register devlink instance last Leon Romanovsky
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

The change of devlink_alloc() to accept device makes sure that device
is fully initialized and device_register() does nothing except allowing
users to use that devlink instance.

Such change ensures that no user input will be usable till that point and
it eliminates the need to worry about internal locking as long as devlink_register
is called last since all accesses to the devlink are during initialization.

This change fixes the following lockdep warning.

 ======================================================
 WARNING: possible circular locking dependency detected
 5.14.0-rc2+ #27 Not tainted
 ------------------------------------------------------
 devlink/265 is trying to acquire lock:
 ffff8880133c2bc0 (&dev->intf_state_mutex){+.+.}-{3:3}, at: mlx5_unload_one+0x1e/0xa0 [mlx5_core]
 but task is already holding lock:
 ffffffff8362b468 (devlink_mutex){+.+.}-{3:3}, at: devlink_nl_pre_doit+0x2b/0x8d0
 which lock already depends on the new lock.
 the existing dependency chain (in reverse order) is:

 -> #1 (devlink_mutex){+.+.}-{3:3}:
        __mutex_lock+0x149/0x1310
        devlink_register+0xe7/0x280
        mlx5_devlink_register+0x118/0x480 [mlx5_core]
        mlx5_init_one+0x34b/0x440 [mlx5_core]
        probe_one+0x480/0x6e0 [mlx5_core]
        pci_device_probe+0x2a0/0x4a0
        really_probe+0x1cb/0xba0
        __driver_probe_device+0x18f/0x470
        driver_probe_device+0x49/0x120
        __driver_attach+0x1ce/0x400
        bus_for_each_dev+0x11e/0x1a0
        bus_add_driver+0x309/0x570
        driver_register+0x20f/0x390
        0xffffffffa04a0062
        do_one_initcall+0xd5/0x400
        do_init_module+0x1c8/0x760
        load_module+0x7d9d/0xa4b0
        __do_sys_finit_module+0x118/0x1a0
        do_syscall_64+0x3d/0x90
        entry_SYSCALL_64_after_hwframe+0x44/0xae

 -> #0 (&dev->intf_state_mutex){+.+.}-{3:3}:
        __lock_acquire+0x2999/0x5a40
        lock_acquire+0x1a9/0x4a0
        __mutex_lock+0x149/0x1310
        mlx5_unload_one+0x1e/0xa0 [mlx5_core]
        mlx5_devlink_reload_down+0x185/0x2b0 [mlx5_core]
        devlink_reload+0x1f2/0x640
        devlink_nl_cmd_reload+0x6c3/0x10d0
        genl_family_rcv_msg_doit+0x1e9/0x2f0
        genl_rcv_msg+0x27f/0x4a0
        netlink_rcv_skb+0x11e/0x340
        genl_rcv+0x24/0x40
        netlink_unicast+0x433/0x700
        netlink_sendmsg+0x6fb/0xbe0
        sock_sendmsg+0xb0/0xe0
        __sys_sendto+0x192/0x240
        __x64_sys_sendto+0xdc/0x1b0
        do_syscall_64+0x3d/0x90
        entry_SYSCALL_64_after_hwframe+0x44/0xae

 other info that might help us debug this:

  Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   lock(devlink_mutex);
                                lock(&dev->intf_state_mutex);
                                lock(devlink_mutex);
   lock(&dev->intf_state_mutex);

  *** DEADLOCK ***

 3 locks held by devlink/265:
  #0: ffffffff836371d0 (cb_lock){++++}-{3:3}, at: genl_rcv+0x15/0x40
  #1: ffffffff83637288 (genl_mutex){+.+.}-{3:3}, at: genl_rcv_msg+0x31a/0x4a0
  #2: ffffffff8362b468 (devlink_mutex){+.+.}-{3:3}, at: devlink_nl_pre_doit+0x2b/0x8d0

 stack backtrace:
 CPU: 0 PID: 265 Comm: devlink Not tainted 5.14.0-rc2+ #27
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
 Call Trace:
  dump_stack_lvl+0x45/0x59
  check_noncircular+0x268/0x310
  ? print_circular_bug+0x460/0x460
  ? __kernel_text_address+0xe/0x30
  ? alloc_chain_hlocks+0x1e6/0x5a0
  __lock_acquire+0x2999/0x5a40
  ? lockdep_hardirqs_on_prepare+0x3e0/0x3e0
  ? add_lock_to_list.constprop.0+0x6c/0x530
  lock_acquire+0x1a9/0x4a0
  ? mlx5_unload_one+0x1e/0xa0 [mlx5_core]
  ? lock_release+0x6c0/0x6c0
  ? lockdep_hardirqs_on_prepare+0x3e0/0x3e0
  ? lock_is_held_type+0x98/0x110
  __mutex_lock+0x149/0x1310
  ? mlx5_unload_one+0x1e/0xa0 [mlx5_core]
  ? lock_is_held_type+0x98/0x110
  ? mlx5_unload_one+0x1e/0xa0 [mlx5_core]
  ? find_held_lock+0x2d/0x110
  ? mutex_lock_io_nested+0x1160/0x1160
  ? mlx5_lag_is_active+0x72/0x90 [mlx5_core]
  ? lock_downgrade+0x6d0/0x6d0
  ? do_raw_spin_lock+0x12e/0x270
  ? rwlock_bug.part.0+0x90/0x90
  ? mlx5_unload_one+0x1e/0xa0 [mlx5_core]
  mlx5_unload_one+0x1e/0xa0 [mlx5_core]
  mlx5_devlink_reload_down+0x185/0x2b0 [mlx5_core]
  ? netlink_broadcast_filtered+0x308/0xac0
  ? mlx5_devlink_info_get+0x1f0/0x1f0 [mlx5_core]
  ? __build_skb_around+0x110/0x2b0
  ? __alloc_skb+0x113/0x2b0
  devlink_reload+0x1f2/0x640
  ? devlink_unregister+0x1e0/0x1e0
  ? security_capable+0x51/0x90
  devlink_nl_cmd_reload+0x6c3/0x10d0
  ? devlink_nl_cmd_get_doit+0x1e0/0x1e0
  ? devlink_nl_pre_doit+0x72/0x8d0
  genl_family_rcv_msg_doit+0x1e9/0x2f0
  ? __lock_acquire+0x15e2/0x5a40
  ? genl_family_rcv_msg_attrs_parse.constprop.0+0x240/0x240
  ? mutex_lock_io_nested+0x1160/0x1160
  ? security_capable+0x51/0x90
  genl_rcv_msg+0x27f/0x4a0
  ? genl_get_cmd+0x3c0/0x3c0
  ? lock_acquire+0x1a9/0x4a0
  ? devlink_nl_cmd_get_doit+0x1e0/0x1e0
  ? lock_release+0x6c0/0x6c0
  netlink_rcv_skb+0x11e/0x340
  ? genl_get_cmd+0x3c0/0x3c0
  ? netlink_ack+0x930/0x930
  genl_rcv+0x24/0x40
  netlink_unicast+0x433/0x700
  ? netlink_attachskb+0x750/0x750
  ? __alloc_skb+0x113/0x2b0
  netlink_sendmsg+0x6fb/0xbe0
  ? netlink_unicast+0x700/0x700
  ? netlink_unicast+0x700/0x700
  sock_sendmsg+0xb0/0xe0
  __sys_sendto+0x192/0x240
  ? __x64_sys_getpeername+0xb0/0xb0
  ? do_sys_openat2+0x10a/0x370
  ? down_write_nested+0x150/0x150
  ? do_user_addr_fault+0x215/0xd50
  ? __x64_sys_openat+0x11f/0x1d0
  ? __x64_sys_open+0x1a0/0x1a0
  __x64_sys_sendto+0xdc/0x1b0
  ? syscall_enter_from_user_mode+0x1d/0x50
  do_syscall_64+0x3d/0x90
  entry_SYSCALL_64_after_hwframe+0x44/0xae
 RIP: 0033:0x7f50b50b6b3a
 Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 41 89 ca 64 8b 04 25 18 00 00 00 85 c0 75 15 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 76 c3 0f 1f 44 00 00 55 48 83 ec 30 44 89 4c
 RSP: 002b:00007fff6c0d3f38 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
 RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00007f50b50b6b3a
 RDX: 0000000000000038 RSI: 000055763ac08440 RDI: 0000000000000003
 RBP: 000055763ac08410 R08: 00007f50b5192200 R09: 000000000000000c
 R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
 R13: 0000000000000000 R14: 000055763ac08410 R15: 000055763ac08440
 mlx5_core 0000:00:09.0: firmware version: 4.8.9999
 mlx5_core 0000:00:09.0: 0.000 Gb/s available PCIe bandwidth (8.0 GT/s PCIe x255 link)
 mlx5_core 0000:00:09.0 eth1: Link up

Fixes: a6f3b62386a0 ("net/mlx5: Move devlink registration before interfaces load")
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/devlink.c       | 9 ++-------
 drivers/net/ethernet/mellanox/mlx5/core/main.c          | 2 ++
 drivers/net/ethernet/mellanox/mlx5/core/sf/dev/driver.c | 2 ++
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index b36f721625e4..d7576b6fa43b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -793,11 +793,11 @@ int mlx5_devlink_register(struct devlink *devlink)
 {
 	int err;
 
-	devlink_register(devlink);
 	err = devlink_params_register(devlink, mlx5_devlink_params,
 				      ARRAY_SIZE(mlx5_devlink_params));
 	if (err)
-		goto params_reg_err;
+		return err;
+
 	mlx5_devlink_set_params_init_values(devlink);
 
 	err = mlx5_devlink_auxdev_params_register(devlink);
@@ -808,7 +808,6 @@ int mlx5_devlink_register(struct devlink *devlink)
 	if (err)
 		goto traps_reg_err;
 
-	devlink_params_publish(devlink);
 	return 0;
 
 traps_reg_err:
@@ -816,17 +815,13 @@ int mlx5_devlink_register(struct devlink *devlink)
 auxdev_reg_err:
 	devlink_params_unregister(devlink, mlx5_devlink_params,
 				  ARRAY_SIZE(mlx5_devlink_params));
-params_reg_err:
-	devlink_unregister(devlink);
 	return err;
 }
 
 void mlx5_devlink_unregister(struct devlink *devlink)
 {
-	devlink_params_unpublish(devlink);
 	mlx5_devlink_traps_unregister(devlink);
 	mlx5_devlink_auxdev_params_unregister(devlink);
 	devlink_params_unregister(devlink, mlx5_devlink_params,
 				  ARRAY_SIZE(mlx5_devlink_params));
-	devlink_unregister(devlink);
 }
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 79482824c64f..92b08fa07efa 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1537,6 +1537,7 @@ static int probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 		dev_err(&pdev->dev, "mlx5_crdump_enable failed with error code %d\n", err);
 
 	pci_save_state(pdev);
+	devlink_register(devlink);
 	if (!mlx5_core_is_mp_slave(dev))
 		devlink_reload_enable(devlink);
 	return 0;
@@ -1559,6 +1560,7 @@ static void remove_one(struct pci_dev *pdev)
 	struct devlink *devlink = priv_to_devlink(dev);
 
 	devlink_reload_disable(devlink);
+	devlink_unregister(devlink);
 	mlx5_crdump_disable(dev);
 	mlx5_drain_health_wq(dev);
 	mlx5_uninit_one(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 052f48068dc1..3cf272fa2164 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/driver.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/driver.c
@@ -46,6 +46,7 @@ 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);
 	devlink_reload_enable(devlink);
 	return 0;
 
@@ -65,6 +66,7 @@ static void mlx5_sf_dev_remove(struct auxiliary_device *adev)
 
 	devlink = priv_to_devlink(sf_dev->mdev);
 	devlink_reload_disable(devlink);
+	devlink_unregister(devlink);
 	mlx5_uninit_one(sf_dev->mdev);
 	iounmap(sf_dev->mdev->iseg);
 	mlx5_mdev_uninit(sf_dev->mdev);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 11/21] mlxsw: core: Register devlink instance last
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (9 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 10/21] net/mlx5: Accept devlink user input after driver initialization complete Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-26 14:56   ` Ido Schimmel
  2021-09-25 11:22 ` [PATCH net-next v1 12/21] net: mscc: ocelot: delay devlink registration to the end Leon Romanovsky
                   ` (9 subsequent siblings)
  20 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

Make sure that devlink is open to receive user input when all
parameters are initialized.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 9a570fa167b6..9e831e8b607a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1973,9 +1973,6 @@ __mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 	if (err)
 		goto err_emad_init;
 
-	if (!reload)
-		devlink_register(devlink);
-
 	if (!reload) {
 		err = mlxsw_core_params_register(mlxsw_core);
 		if (err)
@@ -2010,10 +2007,10 @@ __mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 			goto err_driver_init;
 	}
 
-	devlink_params_publish(devlink);
-
-	if (!reload)
+	if (!reload) {
+		devlink_register(devlink);
 		devlink_reload_enable(devlink);
+	}
 
 	return 0;
 
@@ -2030,8 +2027,6 @@ __mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 	if (!reload)
 		mlxsw_core_params_unregister(mlxsw_core);
 err_register_params:
-	if (!reload)
-		devlink_unregister(devlink);
 	mlxsw_emad_fini(mlxsw_core);
 err_emad_init:
 	kfree(mlxsw_core->lag.mapping);
@@ -2080,8 +2075,10 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core,
 {
 	struct devlink *devlink = priv_to_devlink(mlxsw_core);
 
-	if (!reload)
+	if (!reload) {
 		devlink_reload_disable(devlink);
+		devlink_unregister(devlink);
+	}
 	if (devlink_is_reload_failed(devlink)) {
 		if (!reload)
 			/* Only the parts that were not de-initialized in the
@@ -2092,7 +2089,6 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core,
 			return;
 	}
 
-	devlink_params_unpublish(devlink);
 	if (mlxsw_core->driver->fini)
 		mlxsw_core->driver->fini(mlxsw_core);
 	mlxsw_env_fini(mlxsw_core->env);
@@ -2101,8 +2097,6 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core,
 	mlxsw_core_health_fini(mlxsw_core);
 	if (!reload)
 		mlxsw_core_params_unregister(mlxsw_core);
-	if (!reload)
-		devlink_unregister(devlink);
 	mlxsw_emad_fini(mlxsw_core);
 	kfree(mlxsw_core->lag.mapping);
 	mlxsw_ports_fini(mlxsw_core, reload);
@@ -2116,7 +2110,6 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core,
 
 reload_fail_deinit:
 	mlxsw_core_params_unregister(mlxsw_core);
-	devlink_unregister(devlink);
 	devlink_resources_unregister(devlink, NULL);
 	devlink_free(devlink);
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 12/21] net: mscc: ocelot: delay devlink registration to the end
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (10 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 11/21] mlxsw: core: Register devlink instance last Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 13/21] nfp: Move delink_register to be last command Leon Romanovsky
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

Open access to the devlink interface when the driver fully initialized.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 2b8ea48d2fc4..5d01993f6be0 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -1134,7 +1134,6 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 	if (err)
 		goto out_put_ports;
 
-	devlink_register(devlink);
 	err = mscc_ocelot_init_ports(pdev, ports);
 	if (err)
 		goto out_ocelot_devlink_unregister;
@@ -1157,6 +1156,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 	register_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
 
 	of_node_put(ports);
+	devlink_register(devlink);
 
 	dev_info(&pdev->dev, "Ocelot switch probed\n");
 
@@ -1166,7 +1166,6 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 	mscc_ocelot_release_ports(ocelot);
 	mscc_ocelot_teardown_devlink_ports(ocelot);
 out_ocelot_devlink_unregister:
-	devlink_unregister(devlink);
 	ocelot_deinit(ocelot);
 out_put_ports:
 	of_node_put(ports);
@@ -1179,11 +1178,11 @@ static int mscc_ocelot_remove(struct platform_device *pdev)
 {
 	struct ocelot *ocelot = platform_get_drvdata(pdev);
 
+	devlink_unregister(ocelot->devlink);
 	ocelot_deinit_timestamp(ocelot);
 	ocelot_devlink_sb_unregister(ocelot);
 	mscc_ocelot_release_ports(ocelot);
 	mscc_ocelot_teardown_devlink_ports(ocelot);
-	devlink_unregister(ocelot->devlink);
 	ocelot_deinit(ocelot);
 	unregister_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
 	unregister_switchdev_notifier(&ocelot_switchdev_nb);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 13/21] nfp: Move delink_register to be last command
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (11 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 12/21] net: mscc: ocelot: delay devlink registration to the end Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-27  8:39   ` Simon Horman
  2021-09-25 11:22 ` [PATCH net-next v1 14/21] ionic: Move devlink registration to be last devlink command Leon Romanovsky
                   ` (7 subsequent siblings)
  20 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

Open user space access to the devlink after driver is probed.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/ethernet/netronome/nfp/devlink_param.c | 9 ++-------
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c  | 5 ++---
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/devlink_param.c b/drivers/net/ethernet/netronome/nfp/devlink_param.c
index 36491835ac65..db297ee4d7ad 100644
--- a/drivers/net/ethernet/netronome/nfp/devlink_param.c
+++ b/drivers/net/ethernet/netronome/nfp/devlink_param.c
@@ -233,13 +233,8 @@ int nfp_devlink_params_register(struct nfp_pf *pf)
 	if (err <= 0)
 		return err;
 
-	err = devlink_params_register(devlink, nfp_devlink_params,
-				      ARRAY_SIZE(nfp_devlink_params));
-	if (err)
-		return err;
-
-	devlink_params_publish(devlink);
-	return 0;
+	return devlink_params_register(devlink, nfp_devlink_params,
+				       ARRAY_SIZE(nfp_devlink_params));
 }
 
 void nfp_devlink_params_unregister(struct nfp_pf *pf)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index 616872928ada..5fbb7c613ff1 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -701,7 +701,6 @@ int nfp_net_pci_probe(struct nfp_pf *pf)
 	if (err)
 		goto err_unmap;
 
-	devlink_register(devlink);
 	err = nfp_shared_buf_register(pf);
 	if (err)
 		goto err_devlink_unreg;
@@ -731,6 +730,7 @@ int nfp_net_pci_probe(struct nfp_pf *pf)
 		goto err_stop_app;
 
 	mutex_unlock(&pf->lock);
+	devlink_register(devlink);
 
 	return 0;
 
@@ -748,7 +748,6 @@ int nfp_net_pci_probe(struct nfp_pf *pf)
 	nfp_shared_buf_unregister(pf);
 err_devlink_unreg:
 	cancel_work_sync(&pf->port_refresh_work);
-	devlink_unregister(devlink);
 	nfp_net_pf_app_clean(pf);
 err_unmap:
 	nfp_net_pci_unmap_mem(pf);
@@ -759,6 +758,7 @@ void nfp_net_pci_remove(struct nfp_pf *pf)
 {
 	struct nfp_net *nn, *next;
 
+	devlink_unregister(priv_to_devlink(pf));
 	mutex_lock(&pf->lock);
 	list_for_each_entry_safe(nn, next, &pf->vnics, vnic_list) {
 		if (!nfp_net_is_data_vnic(nn))
@@ -775,7 +775,6 @@ void nfp_net_pci_remove(struct nfp_pf *pf)
 
 	nfp_devlink_params_unregister(pf);
 	nfp_shared_buf_unregister(pf);
-	devlink_unregister(priv_to_devlink(pf));
 
 	nfp_net_pf_free_irqs(pf);
 	nfp_net_pf_app_clean(pf);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 14/21] ionic: Move devlink registration to be last devlink command
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (12 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 13/21] nfp: Move delink_register to be last command Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-27 16:07   ` Shannon Nelson
  2021-09-25 11:22 ` [PATCH net-next v1 15/21] qed: " Leon Romanovsky
                   ` (6 subsequent siblings)
  20 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

This change prevents from users to access device before devlink is
fully configured.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_devlink.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
index 93282394d332..2267da95640b 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
@@ -82,7 +82,6 @@ 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);
@@ -93,6 +92,7 @@ int ionic_devlink_register(struct ionic *ionic)
 	}
 
 	devlink_port_type_eth_set(&ionic->dl_port, ionic->lif->netdev);
+	devlink_register(dl);
 	return 0;
 }
 
@@ -100,6 +100,6 @@ void ionic_devlink_unregister(struct ionic *ionic)
 {
 	struct devlink *dl = priv_to_devlink(ionic);
 
-	devlink_port_unregister(&ionic->dl_port);
 	devlink_unregister(dl);
+	devlink_port_unregister(&ionic->dl_port);
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 15/21] qed: Move devlink registration to be last devlink command
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (13 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 14/21] ionic: Move devlink registration to be last devlink command Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 16/21] net: ethernet: ti: " Leon Romanovsky
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

This change prevents from users to access device before devlink is
fully configured.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/ethernet/qlogic/qed/qed_devlink.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.c b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
index c51f9590fe19..6bb4e165b592 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_devlink.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
@@ -215,7 +215,6 @@ struct devlink *qed_devlink_register(struct qed_dev *cdev)
 	qdevlink = devlink_priv(dl);
 	qdevlink->cdev = cdev;
 
-	devlink_register(dl);
 	rc = devlink_params_register(dl, qed_devlink_params,
 				     ARRAY_SIZE(qed_devlink_params));
 	if (rc)
@@ -226,15 +225,13 @@ struct devlink *qed_devlink_register(struct qed_dev *cdev)
 					   QED_DEVLINK_PARAM_ID_IWARP_CMT,
 					   value);
 
-	devlink_params_publish(dl);
 	cdev->iwarp_cmt = false;
 
 	qed_fw_reporters_create(dl);
-
+	devlink_register(dl);
 	return dl;
 
 err_unregister:
-	devlink_unregister(dl);
 	devlink_free(dl);
 
 	return ERR_PTR(rc);
@@ -245,11 +242,11 @@ void qed_devlink_unregister(struct devlink *devlink)
 	if (!devlink)
 		return;
 
+	devlink_unregister(devlink);
 	qed_fw_reporters_destroy(devlink);
 
 	devlink_params_unregister(devlink, qed_devlink_params,
 				  ARRAY_SIZE(qed_devlink_params));
 
-	devlink_unregister(devlink);
 	devlink_free(devlink);
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 16/21] net: ethernet: ti: Move devlink registration to be last devlink command
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (14 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 15/21] qed: " Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 17/21] netdevsim: " Leon Romanovsky
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

This change prevents from users to access device before devlink is
fully configured.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 15 ++++++---------
 drivers/net/ethernet/ti/cpsw_new.c       |  7 ++-----
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index c2ea53ca92b6..0de5f4a4fe08 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -2429,7 +2429,6 @@ static int am65_cpsw_nuss_register_devlink(struct am65_cpsw_common *common)
 	dl_priv = devlink_priv(common->devlink);
 	dl_priv->common = common;
 
-	devlink_register(common->devlink);
 	/* Provide devlink hook to switch mode when multiple external ports
 	 * are present NUSS switchdev driver is enabled.
 	 */
@@ -2442,7 +2441,6 @@ static int am65_cpsw_nuss_register_devlink(struct am65_cpsw_common *common)
 			dev_err(dev, "devlink params reg fail ret:%d\n", ret);
 			goto dl_unreg;
 		}
-		devlink_params_publish(common->devlink);
 	}
 
 	for (i = 1; i <= common->port_num; i++) {
@@ -2463,7 +2461,7 @@ static int am65_cpsw_nuss_register_devlink(struct am65_cpsw_common *common)
 		}
 		devlink_port_type_eth_set(dl_port, port->ndev);
 	}
-
+	devlink_register(common->devlink);
 	return ret;
 
 dl_port_unreg:
@@ -2474,7 +2472,6 @@ static int am65_cpsw_nuss_register_devlink(struct am65_cpsw_common *common)
 		devlink_port_unregister(dl_port);
 	}
 dl_unreg:
-	devlink_unregister(common->devlink);
 	devlink_free(common->devlink);
 	return ret;
 }
@@ -2485,6 +2482,8 @@ 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;
@@ -2493,13 +2492,11 @@ static void am65_cpsw_unregister_devlink(struct am65_cpsw_common *common)
 	}
 
 	if (!AM65_CPSW_IS_CPSW2G(common) &&
-	    IS_ENABLED(CONFIG_TI_K3_AM65_CPSW_SWITCHDEV)) {
-		devlink_params_unpublish(common->devlink);
-		devlink_params_unregister(common->devlink, am65_cpsw_devlink_params,
+	    IS_ENABLED(CONFIG_TI_K3_AM65_CPSW_SWITCHDEV))
+		devlink_params_unregister(common->devlink,
+					  am65_cpsw_devlink_params,
 					  ARRAY_SIZE(am65_cpsw_devlink_params));
-	}
 
-	devlink_unregister(common->devlink);
 	devlink_free(common->devlink);
 }
 
diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
index 204b4826303c..1530532748a8 100644
--- a/drivers/net/ethernet/ti/cpsw_new.c
+++ b/drivers/net/ethernet/ti/cpsw_new.c
@@ -1810,7 +1810,6 @@ static int cpsw_register_devlink(struct cpsw_common *cpsw)
 	dl_priv = devlink_priv(cpsw->devlink);
 	dl_priv->cpsw = cpsw;
 
-	devlink_register(cpsw->devlink);
 	ret = devlink_params_register(cpsw->devlink, cpsw_devlink_params,
 				      ARRAY_SIZE(cpsw_devlink_params));
 	if (ret) {
@@ -1818,21 +1817,19 @@ static int cpsw_register_devlink(struct cpsw_common *cpsw)
 		goto dl_unreg;
 	}
 
-	devlink_params_publish(cpsw->devlink);
+	devlink_register(cpsw->devlink);
 	return ret;
 
 dl_unreg:
-	devlink_unregister(cpsw->devlink);
 	devlink_free(cpsw->devlink);
 	return ret;
 }
 
 static void cpsw_unregister_devlink(struct cpsw_common *cpsw)
 {
-	devlink_params_unpublish(cpsw->devlink);
+	devlink_unregister(cpsw->devlink);
 	devlink_params_unregister(cpsw->devlink, cpsw_devlink_params,
 				  ARRAY_SIZE(cpsw_devlink_params));
-	devlink_unregister(cpsw->devlink);
 	devlink_free(cpsw->devlink);
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 17/21] netdevsim: Move devlink registration to be last devlink command
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (15 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 16/21] net: ethernet: ti: " Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 18/21] net: wwan: iosm: Move devlink_register " Leon Romanovsky
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

This change prevents from users to access device before devlink is
fully configured.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/netdevsim/dev.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index b2214bc9efe2..cb6645012a30 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -1470,7 +1470,6 @@ int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)
 	if (err)
 		goto err_devlink_free;
 
-	devlink_register(devlink);
 	err = devlink_params_register(devlink, nsim_devlink_params,
 				      ARRAY_SIZE(nsim_devlink_params));
 	if (err)
@@ -1511,9 +1510,9 @@ int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)
 	if (err)
 		goto err_psample_exit;
 
-	devlink_params_publish(devlink);
-	devlink_reload_enable(devlink);
 	nsim_dev->esw_mode = DEVLINK_ESWITCH_MODE_LEGACY;
+	devlink_register(devlink);
+	devlink_reload_enable(devlink);
 	return 0;
 
 err_psample_exit:
@@ -1534,7 +1533,6 @@ int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)
 	devlink_params_unregister(devlink, nsim_devlink_params,
 				  ARRAY_SIZE(nsim_devlink_params));
 err_dl_unregister:
-	devlink_unregister(devlink);
 	devlink_resources_unregister(devlink, NULL);
 err_devlink_free:
 	devlink_free(devlink);
@@ -1569,6 +1567,7 @@ void nsim_dev_remove(struct nsim_bus_dev *nsim_bus_dev)
 	struct devlink *devlink = priv_to_devlink(nsim_dev);
 
 	devlink_reload_disable(devlink);
+	devlink_unregister(devlink);
 
 	nsim_dev_reload_destroy(nsim_dev);
 
@@ -1576,7 +1575,6 @@ void nsim_dev_remove(struct nsim_bus_dev *nsim_bus_dev)
 	nsim_dev_debugfs_exit(nsim_dev);
 	devlink_params_unregister(devlink, nsim_devlink_params,
 				  ARRAY_SIZE(nsim_devlink_params));
-	devlink_unregister(devlink);
 	devlink_resources_unregister(devlink, NULL);
 	devlink_free(devlink);
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 18/21] net: wwan: iosm: Move devlink_register to be last devlink command
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (16 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 17/21] netdevsim: " Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:22 ` [PATCH net-next v1 19/21] ptp: ocp: Move devlink registration " Leon Romanovsky
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

This change prevents from users to access device before devlink is
fully configured. Indirectly this change fixes the commit mentioned
below where devlink_unregister() was prematurely removed.

Fixes: db4278c55fa5 ("devlink: Make devlink_register to be void")
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/net/wwan/iosm/iosm_ipc_devlink.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wwan/iosm/iosm_ipc_devlink.c b/drivers/net/wwan/iosm/iosm_ipc_devlink.c
index 42dbe7fe663c..6fe56f73011b 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_devlink.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_devlink.c
@@ -305,7 +305,6 @@ struct iosm_devlink *ipc_devlink_init(struct iosm_imem *ipc_imem)
 	ipc_devlink->devlink_ctx = devlink_ctx;
 	ipc_devlink->pcie = ipc_imem->pcie;
 	ipc_devlink->dev = ipc_imem->dev;
-	devlink_register(devlink_ctx);
 
 	rc = devlink_params_register(devlink_ctx, iosm_devlink_params,
 				     ARRAY_SIZE(iosm_devlink_params));
@@ -315,7 +314,6 @@ struct iosm_devlink *ipc_devlink_init(struct iosm_imem *ipc_imem)
 		goto param_reg_fail;
 	}
 
-	devlink_params_publish(devlink_ctx);
 	ipc_devlink->cd_file_info = list;
 
 	rc = ipc_devlink_create_region(ipc_devlink);
@@ -334,6 +332,7 @@ struct iosm_devlink *ipc_devlink_init(struct iosm_imem *ipc_imem)
 	init_completion(&ipc_devlink->devlink_sio.read_sem);
 	skb_queue_head_init(&ipc_devlink->devlink_sio.rx_list);
 
+	devlink_register(devlink_ctx);
 	dev_dbg(ipc_devlink->dev, "iosm devlink register success");
 
 	return ipc_devlink;
@@ -341,7 +340,6 @@ struct iosm_devlink *ipc_devlink_init(struct iosm_imem *ipc_imem)
 chnl_get_fail:
 	ipc_devlink_destroy_region(ipc_devlink);
 region_create_fail:
-	devlink_params_unpublish(devlink_ctx);
 	devlink_params_unregister(devlink_ctx, iosm_devlink_params,
 				  ARRAY_SIZE(iosm_devlink_params));
 param_reg_fail:
@@ -358,8 +356,8 @@ void ipc_devlink_deinit(struct iosm_devlink *ipc_devlink)
 {
 	struct devlink *devlink_ctx = ipc_devlink->devlink_ctx;
 
+	devlink_unregister(devlink_ctx);
 	ipc_devlink_destroy_region(ipc_devlink);
-	devlink_params_unpublish(devlink_ctx);
 	devlink_params_unregister(devlink_ctx, iosm_devlink_params,
 				  ARRAY_SIZE(iosm_devlink_params));
 	if (ipc_devlink->devlink_sio.devlink_read_pend) {
@@ -370,6 +368,5 @@ void ipc_devlink_deinit(struct iosm_devlink *ipc_devlink)
 		skb_queue_purge(&ipc_devlink->devlink_sio.rx_list);
 
 	ipc_imem_sys_devlink_close(ipc_devlink);
-	devlink_unregister(devlink_ctx);
 	devlink_free(devlink_ctx);
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 19/21] ptp: ocp: Move devlink registration to be last devlink command
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (17 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 18/21] net: wwan: iosm: Move devlink_register " Leon Romanovsky
@ 2021-09-25 11:22 ` Leon Romanovsky
  2021-09-25 11:23 ` [PATCH net-next v1 20/21] staging: qlge: " Leon Romanovsky
  2021-09-25 11:23 ` [PATCH net-next v1 21/21] net: dsa: " Leon Romanovsky
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:22 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

This change prevents from users to access device before devlink is
fully configured.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/ptp/ptp_ocp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 4c25467198e3..34f943c8c9fd 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -2455,7 +2455,6 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return -ENOMEM;
 	}
 
-	devlink_register(devlink);
 	err = pci_enable_device(pdev);
 	if (err) {
 		dev_err(&pdev->dev, "pci_enable_device\n");
@@ -2497,7 +2496,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto out;
 
 	ptp_ocp_info(bp);
-
+	devlink_register(devlink);
 	return 0;
 
 out:
@@ -2506,7 +2505,6 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 out_disable:
 	pci_disable_device(pdev);
 out_unregister:
-	devlink_unregister(devlink);
 	devlink_free(devlink);
 	return err;
 }
@@ -2517,11 +2515,11 @@ ptp_ocp_remove(struct pci_dev *pdev)
 	struct ptp_ocp *bp = pci_get_drvdata(pdev);
 	struct devlink *devlink = priv_to_devlink(bp);
 
+	devlink_unregister(devlink);
 	ptp_ocp_detach(bp);
 	pci_set_drvdata(pdev, NULL);
 	pci_disable_device(pdev);
 
-	devlink_unregister(devlink);
 	devlink_free(devlink);
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 20/21] staging: qlge: Move devlink registration to be last devlink command
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (18 preceding siblings ...)
  2021-09-25 11:22 ` [PATCH net-next v1 19/21] ptp: ocp: Move devlink registration " Leon Romanovsky
@ 2021-09-25 11:23 ` Leon Romanovsky
  2021-09-25 11:23 ` [PATCH net-next v1 21/21] net: dsa: " Leon Romanovsky
  20 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:23 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

This change prevents from users to access device before devlink is
fully configured.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/staging/qlge/qlge_main.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index 33539f6c254d..1dc849378a0f 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -4614,10 +4614,9 @@ static int qlge_probe(struct pci_dev *pdev,
 		goto netdev_free;
 	}
 
-	devlink_register(devlink);
 	err = qlge_health_create_reporters(qdev);
 	if (err)
-		goto devlink_unregister;
+		goto netdev_free;
 
 	/* Start up the timer to trigger EEH if
 	 * the bus goes dead
@@ -4628,10 +4627,9 @@ static int qlge_probe(struct pci_dev *pdev,
 	qlge_display_dev_info(ndev);
 	atomic_set(&qdev->lb_count, 0);
 	cards_found++;
+	devlink_register(devlink);
 	return 0;
 
-devlink_unregister:
-	devlink_unregister(devlink);
 netdev_free:
 	free_netdev(ndev);
 devlink_free:
@@ -4656,13 +4654,13 @@ static void qlge_remove(struct pci_dev *pdev)
 	struct net_device *ndev = qdev->ndev;
 	struct devlink *devlink = priv_to_devlink(qdev);
 
+	devlink_unregister(devlink);
 	del_timer_sync(&qdev->timer);
 	qlge_cancel_all_work_sync(qdev);
 	unregister_netdev(ndev);
 	qlge_release_all(pdev);
 	pci_disable_device(pdev);
 	devlink_health_reporter_destroy(qdev->reporter);
-	devlink_unregister(devlink);
 	devlink_free(devlink);
 	free_netdev(ndev);
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH net-next v1 21/21] net: dsa: Move devlink registration to be last devlink command
  2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
                   ` (19 preceding siblings ...)
  2021-09-25 11:23 ` [PATCH net-next v1 20/21] staging: qlge: " Leon Romanovsky
@ 2021-09-25 11:23 ` Leon Romanovsky
  2021-09-29 13:02   ` Vladimir Oltean
  20 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-25 11:23 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

From: Leon Romanovsky <leonro@nvidia.com>

This change prevents from users to access device before devlink
is fully configured.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 net/dsa/dsa2.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index a020339e1973..8ca6a1170c9d 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -848,7 +848,6 @@ static int dsa_switch_setup(struct dsa_switch *ds)
 	dl_priv = devlink_priv(ds->devlink);
 	dl_priv->ds = ds;
 
-	devlink_register(ds->devlink);
 	/* Setup devlink port instances now, so that the switch
 	 * setup() can register regions etc, against the ports
 	 */
@@ -874,8 +873,6 @@ static int dsa_switch_setup(struct dsa_switch *ds)
 	if (err)
 		goto teardown;
 
-	devlink_params_publish(ds->devlink);
-
 	if (!ds->slave_mii_bus && ds->ops->phy_read) {
 		ds->slave_mii_bus = mdiobus_alloc();
 		if (!ds->slave_mii_bus) {
@@ -891,7 +888,7 @@ static int dsa_switch_setup(struct dsa_switch *ds)
 	}
 
 	ds->setup = true;
-
+	devlink_register(ds->devlink);
 	return 0;
 
 free_slave_mii_bus:
@@ -906,7 +903,6 @@ static int dsa_switch_setup(struct dsa_switch *ds)
 	list_for_each_entry(dp, &ds->dst->ports, list)
 		if (dp->ds == ds)
 			dsa_port_devlink_teardown(dp);
-	devlink_unregister(ds->devlink);
 	devlink_free(ds->devlink);
 	ds->devlink = NULL;
 	return err;
@@ -919,6 +915,9 @@ static void dsa_switch_teardown(struct dsa_switch *ds)
 	if (!ds->setup)
 		return;
 
+	if (ds->devlink)
+		devlink_unregister(ds->devlink);
+
 	if (ds->slave_mii_bus && ds->ops->phy_read) {
 		mdiobus_unregister(ds->slave_mii_bus);
 		mdiobus_free(ds->slave_mii_bus);
@@ -934,7 +933,6 @@ static void dsa_switch_teardown(struct dsa_switch *ds)
 		list_for_each_entry(dp, &ds->dst->ports, list)
 			if (dp->ds == ds)
 				dsa_port_devlink_teardown(dp);
-		devlink_unregister(ds->devlink);
 		devlink_free(ds->devlink);
 		ds->devlink = NULL;
 	}
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH net-next v1 11/21] mlxsw: core: Register devlink instance last
  2021-09-25 11:22 ` [PATCH net-next v1 11/21] mlxsw: core: Register devlink instance last Leon Romanovsky
@ 2021-09-26 14:56   ` Ido Schimmel
  0 siblings, 0 replies; 32+ messages in thread
From: Ido Schimmel @ 2021-09-26 14:56 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: David S . Miller, Jakub Kicinski, Leon Romanovsky,
	Alexandre Belloni, Andrew Lunn, Ariel Elior, Bin Luo,
	Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

On Sat, Sep 25, 2021 at 02:22:51PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> Make sure that devlink is open to receive user input when all
> parameters are initialized.
> 
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Ido Schimmel <idosch@nvidia.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH net-next v1 13/21] nfp: Move delink_register to be last command
  2021-09-25 11:22 ` [PATCH net-next v1 13/21] nfp: Move delink_register to be last command Leon Romanovsky
@ 2021-09-27  8:39   ` Simon Horman
  2021-09-27 11:53     ` Leon Romanovsky
  0 siblings, 1 reply; 32+ messages in thread
From: Simon Horman @ 2021-09-27  8:39 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: David S . Miller, Jakub Kicinski, Leon Romanovsky,
	Alexandre Belloni, Andrew Lunn, Ariel Elior, Bin Luo,
	Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

On Sat, Sep 25, 2021 at 02:22:53PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> Open user space access to the devlink after driver is probed.

Hi Leon,

I think a description of why is warranted here.

> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH net-next v1 13/21] nfp: Move delink_register to be last command
  2021-09-27  8:39   ` Simon Horman
@ 2021-09-27 11:53     ` Leon Romanovsky
  2021-09-27 12:20       ` Simon Horman
  0 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-27 11:53 UTC (permalink / raw)
  To: Simon Horman
  Cc: David S . Miller, Jakub Kicinski, Alexandre Belloni, Andrew Lunn,
	Ariel Elior, Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles,
	drivers, Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

On Mon, Sep 27, 2021 at 10:39:24AM +0200, Simon Horman wrote:
> On Sat, Sep 25, 2021 at 02:22:53PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> > 
> > Open user space access to the devlink after driver is probed.
> 
> Hi Leon,
> 
> I think a description of why is warranted here.

After devlink_register(), users can send GET and SET netlink commands to
the uninitialized driver. In some cases, nothing will happen, but not in
all and hard to prove that ALL drivers are safe with such early access.

It means that local users can (in theory for some and in practice for
others) crash the system (or leverage permissions) with early devlink_register()
by accessing internal to driver pointers that are not set yet.

Like I said in the commit message, I'm not fixing all drivers.
https://lore.kernel.org/netdev/cover.1632565508.git.leonro@nvidia.com/T/#m063eb4e67389bafcc3b3ddc07197bf43181b7209

Because some of the driver authors made a wonderful job to obfuscate their
driver and write completely unmanageable code.

I do move devlink_register() to be last devlink command for all drivers,
to allow me to clean devlink core locking and API in next series.

This series should raise your eyebrow and trigger a question: "is my
driver vulnerable too?". And the answer will depend on devlink_register()
position in the .probe() call.

Thanks

> 
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH net-next v1 13/21] nfp: Move delink_register to be last command
  2021-09-27 11:53     ` Leon Romanovsky
@ 2021-09-27 12:20       ` Simon Horman
  0 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2021-09-27 12:20 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: David S . Miller, Jakub Kicinski, Alexandre Belloni, Andrew Lunn,
	Ariel Elior, Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles,
	drivers, Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

On Mon, Sep 27, 2021 at 02:53:24PM +0300, Leon Romanovsky wrote:
> On Mon, Sep 27, 2021 at 10:39:24AM +0200, Simon Horman wrote:
> > On Sat, Sep 25, 2021 at 02:22:53PM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@nvidia.com>
> > > 
> > > Open user space access to the devlink after driver is probed.
> > 
> > Hi Leon,
> > 
> > I think a description of why is warranted here.
> 
> After devlink_register(), users can send GET and SET netlink commands to
> the uninitialized driver. In some cases, nothing will happen, but not in
> all and hard to prove that ALL drivers are safe with such early access.
> 
> It means that local users can (in theory for some and in practice for
> others) crash the system (or leverage permissions) with early devlink_register()
> by accessing internal to driver pointers that are not set yet.
> 
> Like I said in the commit message, I'm not fixing all drivers.
> https://lore.kernel.org/netdev/cover.1632565508.git.leonro@nvidia.com/T/#m063eb4e67389bafcc3b3ddc07197bf43181b7209
> 
> Because some of the driver authors made a wonderful job to obfuscate their
> driver and write completely unmanageable code.
> 
> I do move devlink_register() to be last devlink command for all drivers,
> to allow me to clean devlink core locking and API in next series.
> 
> This series should raise your eyebrow and trigger a question: "is my
> driver vulnerable too?". And the answer will depend on devlink_register()
> position in the .probe() call.
> 
> Thanks

Thanks for the explanation.
And thanks for taking time to update the NFP driver.

> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

Acked-by: Simon Horman <simon.horman@corigine.com>


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH net-next v1 14/21] ionic: Move devlink registration to be last devlink command
  2021-09-25 11:22 ` [PATCH net-next v1 14/21] ionic: Move devlink registration to be last devlink command Leon Romanovsky
@ 2021-09-27 16:07   ` Shannon Nelson
  0 siblings, 0 replies; 32+ messages in thread
From: Shannon Nelson @ 2021-09-27 16:07 UTC (permalink / raw)
  To: Leon Romanovsky, David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Simon Horman, Subbaraya Sundeep, Sunil Goutham,
	Taras Chornyi, Tariq Toukan, Tony Nguyen, UNGLinuxDriver,
	Vadym Kochan, Vivien Didelot, Vladimir Oltean

On 9/25/21 4:22 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
>
> This change prevents from users to access device before devlink is
> fully configured.
>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

Thanks for the work,

Acked-by: Shannon Nelson <snelson@pensando.io>


> ---
>   drivers/net/ethernet/pensando/ionic/ionic_devlink.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> index 93282394d332..2267da95640b 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> @@ -82,7 +82,6 @@ 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);
> @@ -93,6 +92,7 @@ int ionic_devlink_register(struct ionic *ionic)
>   	}
>   
>   	devlink_port_type_eth_set(&ionic->dl_port, ionic->lif->netdev);
> +	devlink_register(dl);
>   	return 0;
>   }
>   
> @@ -100,6 +100,6 @@ void ionic_devlink_unregister(struct ionic *ionic)
>   {
>   	struct devlink *dl = priv_to_devlink(ionic);
>   
> -	devlink_port_unregister(&ionic->dl_port);
>   	devlink_unregister(dl);
> +	devlink_port_unregister(&ionic->dl_port);
>   }


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH net-next v1 06/21] ice: Open devlink when device is ready
  2021-09-25 11:22 ` [PATCH net-next v1 06/21] ice: Open devlink when device " Leon Romanovsky
@ 2021-09-27 19:47   ` Jesse Brandeburg
  0 siblings, 0 replies; 32+ messages in thread
From: Jesse Brandeburg @ 2021-09-27 19:47 UTC (permalink / raw)
  To: Leon Romanovsky, David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jiri Pirko, Jonathan Lemon,
	Linu Cherian, linux-kernel, linux-omap, linux-rdma,
	linux-staging, Loic Poulain, Manish Chopra, M Chetan Kumar,
	Michael Chan, Michael Guralnik, netdev, oss-drivers,
	Richard Cochran, Saeed Mahameed, Satanand Burla, Sergey Ryazanov,
	Shannon Nelson, Simon Horman, Subbaraya Sundeep, Sunil Goutham,
	Taras Chornyi, Tariq Toukan, Tony Nguyen, UNGLinuxDriver,
	Vadym Kochan, Vivien Didelot, Vladimir Oltean

On 9/25/2021 4:22 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> Move devlink_registration routine to be the last command, when the
> device is fully initialized.
> 
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

For the ice driver, thanks for fixing it!

Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH net-next v1 01/21] devlink: Notify users when objects are accessible
  2021-09-25 11:22 ` [PATCH net-next v1 01/21] devlink: Notify users when objects are accessible Leon Romanovsky
@ 2021-09-28  2:49   ` Eric Dumazet
  2021-09-28  7:34     ` Leon Romanovsky
  0 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2021-09-28  2:49 UTC (permalink / raw)
  To: Leon Romanovsky, David S . Miller, Jakub Kicinski
  Cc: Leon Romanovsky, Alexandre Belloni, Andrew Lunn, Ariel Elior,
	Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean



On 9/25/21 4:22 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> The devlink core code notified users about add/remove objects without
> relation if this object can be accessible or not. In this patch we unify
> such user visible notifications in one place.
> 
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> ---
>  net/core/devlink.c | 107 +++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 93 insertions(+), 14 deletions(-)
> 
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 3ea33c689790..06edb2f1d21e 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -742,6 +742,7 @@ static void devlink_notify(struct devlink *devlink, enum devlink_command cmd)
>  	int err;
>  
>  	WARN_ON(cmd != DEVLINK_CMD_NEW && cmd != DEVLINK_CMD_DEL);
> +	WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));
>  
>  	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>  	if (!msg)
> @@ -1040,11 +1041,15 @@ static int devlink_nl_port_fill(struct sk_buff *msg,
>  static void devlink_port_notify(struct devlink_port *devlink_port,
>  				enum devlink_command cmd)
>  {
> +	struct devlink *devlink = devlink_port->devlink;
>  	struct sk_buff *msg;
>  	int err;
>  
>  	WARN_ON(cmd != DEVLINK_CMD_PORT_NEW && cmd != DEVLINK_CMD_PORT_DEL);
>  
> +	if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
> +		return;
> +
>  	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>  	if (!msg)
>  		return;
> @@ -1055,18 +1060,19 @@ static void devlink_port_notify(struct devlink_port *devlink_port,
>  		return;
>  	}
>  
> -	genlmsg_multicast_netns(&devlink_nl_family,
> -				devlink_net(devlink_port->devlink), msg, 0,
> -				DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
> +	genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink), msg,
> +				0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
>  }
>  
>  static void devlink_rate_notify(struct devlink_rate *devlink_rate,
>  				enum devlink_command cmd)
>  {
> +	struct devlink *devlink = devlink_rate->devlink;
>  	struct sk_buff *msg;
>  	int err;
>  
>  	WARN_ON(cmd != DEVLINK_CMD_RATE_NEW && cmd != DEVLINK_CMD_RATE_DEL);
> +	WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));


FYI, this new warning was triggered by syzbot :

WARNING: CPU: 1 PID: 6540 at net/core/devlink.c:5158 devlink_nl_region_notify+0x184/0x1e0 net/core/devlink.c:5158
Modules linked in:
CPU: 1 PID: 6540 Comm: syz-executor.0 Not tainted 5.15.0-rc2-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:devlink_nl_region_notify+0x184/0x1e0 net/core/devlink.c:5158
Code: 38 41 b8 c0 0c 00 00 31 d2 48 89 ee 4c 89 e7 e8 72 1a 26 00 48 83 c4 08 5b 5d 41 5c 41 5d 41 5e e9 01 bd 41 fa e8 fc bc 41 fa <0f> 0b e9 f7 fe ff ff e8 f0 bc 41 fa 0f 0b eb da 4c 89 e7 e8 c4 18
RSP: 0018:ffffc90002d6f658 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffff88801f08d580 RSI: ffffffff87344e94 RDI: 0000000000000003
RBP: ffff88801ee42100 R08: 0000000000000000 R09: 0000000000000000
R10: ffffffff87344d8a R11: 0000000000000000 R12: ffff88801c1dc000
R13: 0000000000000000 R14: 000000000000002c R15: ffff88801c1dc070
FS:  0000555555e8e400(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055dd7c590310 CR3: 0000000069a09000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 devlink_region_create+0x39f/0x4c0 net/core/devlink.c:10327
 nsim_dev_dummy_region_init drivers/net/netdevsim/dev.c:481 [inline]
 nsim_dev_probe+0x5f6/0x1150 drivers/net/netdevsim/dev.c:1479
 call_driver_probe drivers/base/dd.c:517 [inline]
 really_probe+0x245/0xcc0 drivers/base/dd.c:596
 __driver_probe_device+0x338/0x4d0 drivers/base/dd.c:751
 driver_probe_device+0x4c/0x1a0 drivers/base/dd.c:781
 __device_attach_driver+0x20b/0x2f0 drivers/base/dd.c:898
 bus_for_each_drv+0x15f/0x1e0 drivers/base/bus.c:427
 __device_attach+0x228/0x4a0 drivers/base/dd.c:969
 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:487
 device_add+0xc35/0x21b0 drivers/base/core.c:3359
 nsim_bus_dev_new drivers/net/netdevsim/bus.c:435 [inline]
 new_device_store+0x48b/0x770 drivers/net/netdevsim/bus.c:302
 bus_attr_store+0x72/0xa0 drivers/base/bus.c:122
 sysfs_kf_write+0x110/0x160 fs/sysfs/file.c:139
 kernfs_fop_write_iter+0x342/0x500 fs/kernfs/file.c:296
 call_write_iter include/linux/fs.h:2163 [inline]
 new_sync_write+0x429/0x660 fs/read_write.c:507
 vfs_write+0x7cf/0xae0 fs/read_write.c:594
 ksys_write+0x12d/0x250 fs/read_write.c:647
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f328409d3ef
Code: 89 54 24 18 48 89 74 24 10 89 7c 24 08 e8 99 fd ff ff 48 8b 54 24 18 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 31 44 89 c7 48 89 44 24 08 e8 cc fd ff ff 48
RSP: 002b:00007ffdc6851140 EFLAGS: 00000293 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007f328409d3ef
RDX: 0000000000000003 RSI: 00007ffdc6851190 RDI: 0000000000000004
RBP: 0000000000000004 R08: 0000000000000000 R09: 00007ffdc68510e0
R10: 0000000000000000 R11: 0000000000000293 R12: 00007f3284144971
R13: 00007ffdc6851190 R14: 0000000000000000 R15: 00007ffdc6851860

05:42PM


>  
>  	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>  	if (!msg)
> @@ -1078,9 +1084,8 @@ static void devlink_rate_notify(struct devlink_rate *devlink_rate,
>  		return;
>  	}
>  
> -	genlmsg_multicast_netns(&devlink_nl_family,
> -				devlink_net(devlink_rate->devlink), msg, 0,
> -				DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
> +	genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink), msg,
> +				0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
>  }
>  
>  static int devlink_nl_cmd_rate_get_dumpit(struct sk_buff *msg,
> @@ -4150,6 +4155,7 @@ static void __devlink_flash_update_notify(struct devlink *devlink,
>  	WARN_ON(cmd != DEVLINK_CMD_FLASH_UPDATE &&
>  		cmd != DEVLINK_CMD_FLASH_UPDATE_END &&
>  		cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS);
> +	WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));
>  
>  	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>  	if (!msg)
> @@ -5145,17 +5151,18 @@ static void devlink_nl_region_notify(struct devlink_region *region,
>  				     struct devlink_snapshot *snapshot,
>  				     enum devlink_command cmd)
>  {
> +	struct devlink *devlink = region->devlink;
>  	struct sk_buff *msg;
>  
>  	WARN_ON(cmd != DEVLINK_CMD_REGION_NEW && cmd != DEVLINK_CMD_REGION_DEL);
> +	WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));
>  
>  	msg = devlink_nl_region_notify_build(region, snapshot, cmd, 0, 0);
>  	if (IS_ERR(msg))
>  		return;
>  
> -	genlmsg_multicast_netns(&devlink_nl_family,
> -				devlink_net(region->devlink), msg, 0,
> -				DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
> +	genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink), msg,
> +				0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
>  }
>  
>  /**
> @@ -6920,10 +6927,12 @@ devlink_nl_health_reporter_fill(struct sk_buff *msg,
>  static void devlink_recover_notify(struct devlink_health_reporter *reporter,
>  				   enum devlink_command cmd)
>  {
> +	struct devlink *devlink = reporter->devlink;
>  	struct sk_buff *msg;
>  	int err;
>  
>  	WARN_ON(cmd != DEVLINK_CMD_HEALTH_REPORTER_RECOVER);
> +	WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));
>  
>  	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>  	if (!msg)
> @@ -6935,9 +6944,8 @@ static void devlink_recover_notify(struct devlink_health_reporter *reporter,
>  		return;
>  	}
>  
> -	genlmsg_multicast_netns(&devlink_nl_family,
> -				devlink_net(reporter->devlink),
> -				msg, 0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
> +	genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink), msg,
> +				0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
>  }
>  
>  void
> @@ -8955,6 +8963,68 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
>  }
>  EXPORT_SYMBOL_GPL(devlink_alloc_ns);
>  
> +static void
> +devlink_trap_policer_notify(struct devlink *devlink,
> +			    const struct devlink_trap_policer_item *policer_item,
> +			    enum devlink_command cmd);
> +static void
> +devlink_trap_group_notify(struct devlink *devlink,
> +			  const struct devlink_trap_group_item *group_item,
> +			  enum devlink_command cmd);
> +static void devlink_trap_notify(struct devlink *devlink,
> +				const struct devlink_trap_item *trap_item,
> +				enum devlink_command cmd);
> +
> +static void devlink_notify_register(struct devlink *devlink)
> +{
> +	struct devlink_trap_policer_item *policer_item;
> +	struct devlink_trap_group_item *group_item;
> +	struct devlink_trap_item *trap_item;
> +	struct devlink_port *devlink_port;
> +
> +	devlink_notify(devlink, DEVLINK_CMD_NEW);
> +	list_for_each_entry(devlink_port, &devlink->port_list, list)
> +		devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
> +
> +	list_for_each_entry(policer_item, &devlink->trap_policer_list, list)
> +		devlink_trap_policer_notify(devlink, policer_item,
> +					    DEVLINK_CMD_TRAP_POLICER_NEW);
> +
> +	list_for_each_entry(group_item, &devlink->trap_group_list, list)
> +		devlink_trap_group_notify(devlink, group_item,
> +					  DEVLINK_CMD_TRAP_GROUP_NEW);
> +
> +	list_for_each_entry(trap_item, &devlink->trap_list, list)
> +		devlink_trap_notify(devlink, trap_item, DEVLINK_CMD_TRAP_NEW);
> +
> +	devlink_params_publish(devlink);
> +}
> +
> +static void devlink_notify_unregister(struct devlink *devlink)
> +{
> +	struct devlink_trap_policer_item *policer_item;
> +	struct devlink_trap_group_item *group_item;
> +	struct devlink_trap_item *trap_item;
> +	struct devlink_port *devlink_port;
> +
> +	devlink_params_unpublish(devlink);
> +
> +	list_for_each_entry_reverse(trap_item, &devlink->trap_list, list)
> +		devlink_trap_notify(devlink, trap_item, DEVLINK_CMD_TRAP_DEL);
> +
> +	list_for_each_entry_reverse(group_item, &devlink->trap_group_list, list)
> +		devlink_trap_group_notify(devlink, group_item,
> +					  DEVLINK_CMD_TRAP_GROUP_DEL);
> +	list_for_each_entry_reverse(policer_item, &devlink->trap_policer_list,
> +				    list)
> +		devlink_trap_policer_notify(devlink, policer_item,
> +					    DEVLINK_CMD_TRAP_POLICER_DEL);
> +
> +	list_for_each_entry_reverse(devlink_port, &devlink->port_list, list)
> +		devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_DEL);
> +	devlink_notify(devlink, DEVLINK_CMD_DEL);
> +}
> +
>  /**
>   *	devlink_register - Register devlink instance
>   *
> @@ -8964,7 +9034,7 @@ void devlink_register(struct devlink *devlink)
>  {
>  	mutex_lock(&devlink_mutex);
>  	xa_set_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
> -	devlink_notify(devlink, DEVLINK_CMD_NEW);
> +	devlink_notify_register(devlink);
>  	mutex_unlock(&devlink_mutex);
>  }
>  EXPORT_SYMBOL_GPL(devlink_register);
> @@ -8982,7 +9052,7 @@ void devlink_unregister(struct devlink *devlink)
>  	mutex_lock(&devlink_mutex);
>  	WARN_ON(devlink_reload_supported(devlink->ops) &&
>  		devlink->reload_enabled);
> -	devlink_notify(devlink, DEVLINK_CMD_DEL);
> +	devlink_notify_unregister(devlink);
>  	xa_clear_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
>  	mutex_unlock(&devlink_mutex);
>  }
> @@ -10086,6 +10156,9 @@ void devlink_params_publish(struct devlink *devlink)
>  {
>  	struct devlink_param_item *param_item;
>  
> +	if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
> +		return;
> +
>  	list_for_each_entry(param_item, &devlink->param_list, list) {
>  		if (param_item->published)
>  			continue;
> @@ -10631,6 +10704,8 @@ devlink_trap_group_notify(struct devlink *devlink,
>  
>  	WARN_ON_ONCE(cmd != DEVLINK_CMD_TRAP_GROUP_NEW &&
>  		     cmd != DEVLINK_CMD_TRAP_GROUP_DEL);
> +	if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
> +		return;
>  
>  	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>  	if (!msg)
> @@ -10672,6 +10747,8 @@ static void devlink_trap_notify(struct devlink *devlink,
>  
>  	WARN_ON_ONCE(cmd != DEVLINK_CMD_TRAP_NEW &&
>  		     cmd != DEVLINK_CMD_TRAP_DEL);
> +	if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
> +		return;
>  
>  	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>  	if (!msg)
> @@ -11053,6 +11130,8 @@ devlink_trap_policer_notify(struct devlink *devlink,
>  
>  	WARN_ON_ONCE(cmd != DEVLINK_CMD_TRAP_POLICER_NEW &&
>  		     cmd != DEVLINK_CMD_TRAP_POLICER_DEL);
> +	if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
> +		return;
>  
>  	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>  	if (!msg)
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH net-next v1 01/21] devlink: Notify users when objects are accessible
  2021-09-28  2:49   ` Eric Dumazet
@ 2021-09-28  7:34     ` Leon Romanovsky
  0 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-28  7:34 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Alexandre Belloni, Andrew Lunn,
	Ariel Elior, Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles,
	drivers, Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot, Vladimir Oltean

On Mon, Sep 27, 2021 at 07:49:18PM -0700, Eric Dumazet wrote:
> 
> 
> On 9/25/21 4:22 AM, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> > 
> > The devlink core code notified users about add/remove objects without
> > relation if this object can be accessible or not. In this patch we unify
> > such user visible notifications in one place.
> > 
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > ---
> >  net/core/devlink.c | 107 +++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 93 insertions(+), 14 deletions(-)

<...>

> >  static void devlink_rate_notify(struct devlink_rate *devlink_rate,
> >  				enum devlink_command cmd)
> >  {
> > +	struct devlink *devlink = devlink_rate->devlink;
> >  	struct sk_buff *msg;
> >  	int err;
> >  
> >  	WARN_ON(cmd != DEVLINK_CMD_RATE_NEW && cmd != DEVLINK_CMD_RATE_DEL);
> > +	WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));
> 
> 
> FYI, this new warning was triggered by syzbot :

Thanks for the report, it is combination of my rebase error and missing
loop of devlink_rate_notify in the devlink_notify_register() function.

I'll fix and resubmit.

Thanks

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH net-next v1 21/21] net: dsa: Move devlink registration to be last devlink command
  2021-09-25 11:23 ` [PATCH net-next v1 21/21] net: dsa: " Leon Romanovsky
@ 2021-09-29 13:02   ` Vladimir Oltean
  2021-09-29 13:07     ` Leon Romanovsky
  0 siblings, 1 reply; 32+ messages in thread
From: Vladimir Oltean @ 2021-09-29 13:02 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: David S . Miller, Jakub Kicinski, Leon Romanovsky,
	Alexandre Belloni, Andrew Lunn, Ariel Elior, Bin Luo,
	Claudiu Manoil, Coiby Xu, Derek Chickles, drivers,
	Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot

Hi Leon,

On Sat, Sep 25, 2021 at 02:23:01PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
>
> This change prevents from users to access device before devlink
> is fully configured.
>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> ---
>  net/dsa/dsa2.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index a020339e1973..8ca6a1170c9d 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -848,7 +848,6 @@ static int dsa_switch_setup(struct dsa_switch *ds)
>  	dl_priv = devlink_priv(ds->devlink);
>  	dl_priv->ds = ds;
>
> -	devlink_register(ds->devlink);
>  	/* Setup devlink port instances now, so that the switch
>  	 * setup() can register regions etc, against the ports
>  	 */
> @@ -874,8 +873,6 @@ static int dsa_switch_setup(struct dsa_switch *ds)
>  	if (err)
>  		goto teardown;
>
> -	devlink_params_publish(ds->devlink);
> -
>  	if (!ds->slave_mii_bus && ds->ops->phy_read) {
>  		ds->slave_mii_bus = mdiobus_alloc();
>  		if (!ds->slave_mii_bus) {
> @@ -891,7 +888,7 @@ static int dsa_switch_setup(struct dsa_switch *ds)
>  	}
>
>  	ds->setup = true;
> -
> +	devlink_register(ds->devlink);
>  	return 0;
>
>  free_slave_mii_bus:
> @@ -906,7 +903,6 @@ static int dsa_switch_setup(struct dsa_switch *ds)
>  	list_for_each_entry(dp, &ds->dst->ports, list)
>  		if (dp->ds == ds)
>  			dsa_port_devlink_teardown(dp);
> -	devlink_unregister(ds->devlink);
>  	devlink_free(ds->devlink);
>  	ds->devlink = NULL;
>  	return err;
> @@ -919,6 +915,9 @@ static void dsa_switch_teardown(struct dsa_switch *ds)
>  	if (!ds->setup)
>  		return;
>
> +	if (ds->devlink)
> +		devlink_unregister(ds->devlink);
> +
>  	if (ds->slave_mii_bus && ds->ops->phy_read) {
>  		mdiobus_unregister(ds->slave_mii_bus);
>  		mdiobus_free(ds->slave_mii_bus);
> @@ -934,7 +933,6 @@ static void dsa_switch_teardown(struct dsa_switch *ds)
>  		list_for_each_entry(dp, &ds->dst->ports, list)
>  			if (dp->ds == ds)
>  				dsa_port_devlink_teardown(dp);
> -		devlink_unregister(ds->devlink);
>  		devlink_free(ds->devlink);
>  		ds->devlink = NULL;
>  	}
> --
> 2.31.1
>

Sorry, I did not have time to review/test this change earlier.
I now see this WARN_ON being triggered when I boot a board:

[    6.731180] WARNING: CPU: 1 PID: 79 at net/core/devlink.c:5158 devlink_nl_region_notify+0xa8/0xc0
[    6.740123] Modules linked in:
[    6.743217] CPU: 1 PID: 79 Comm: kworker/u4:2 Tainted: G        W         5.15.0-rc2-07010-ga9b9500ffaac-dirty #876
[    6.759155] Workqueue: events_unbound deferred_probe_work_func
[    6.765048] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    6.772060] pc : devlink_nl_region_notify+0xa8/0xc0
[    6.776978] lr : devlink_nl_region_notify+0x3c/0xc0
[    6.781893] sp : ffff8000108b38b0
[    6.785235] x29: ffff8000108b38b0 x28: ffff6fce86e53780 x27: ffff6fce86e525c8
[    6.792453] x26: ffff6fce86e69b00 x25: ffff6fce86e57600 x24: 0000000000017528
[    6.799668] x23: 0000000000000001 x22: ffff6fce86e57400 x21: 000000000000002c
[    6.806883] x20: 0000000000000000 x19: ffff6fce86e69b00 x18: 00000000fffffff8
[    6.814098] x17: 000000000000013f x16: 000000000000017f x15: ffffffffffffffff
[    6.821313] x14: ffffff0000000000 x13: ffffffffffffffff x12: 000000000000000b
[    6.828528] x11: ffffc1141c2fce38 x10: 0000000000000005 x9 : 659dd6f91764a956
[    6.835742] x8 : ffff6fce851ce100 x7 : ffffc1141c6bf000 x6 : 000000003020805e
[    6.842956] x5 : 00ffffffffffffff x4 : ffffaebaddb5a000 x3 : 0000000000000000
[    6.850169] x2 : 0000000000000000 x1 : ffff6fce851ce100 x0 : 0000000000000000
[    6.857383] Call trace:
[    6.859854]  devlink_nl_region_notify+0xa8/0xc0
[    6.864422]  devlink_region_create+0x110/0x150
[    6.868902]  dsa_devlink_region_create+0x14/0x20
[    6.873563]  sja1105_devlink_setup+0x54/0xa0
[    6.877871]  sja1105_setup+0x144/0x1390
[    6.881742]  dsa_register_switch+0x978/0x1010
[    6.886139]  sja1105_probe+0x628/0x644
[    6.889920]  spi_probe+0x84/0xe4
[    6.893180]  really_probe.part.0+0x9c/0x31c
[    6.897402]  __driver_probe_device+0x98/0x144
[    6.901797]  driver_probe_device+0xc8/0x160
[    6.906019]  __device_attach_driver+0xb8/0x120
[    6.910501]  bus_for_each_drv+0x78/0xd0
[    6.914373]  __device_attach+0xd8/0x180
[    6.918243]  device_initial_probe+0x14/0x20
[    6.922466]  bus_probe_device+0x9c/0xa4
[    6.926337]  deferred_probe_work_func+0x88/0xc4
[    6.930906]  process_one_work+0x288/0x6f0
[    6.934952]  worker_thread+0x74/0x470
[    6.938646]  kthread+0x160/0x170
[    6.941909]  ret_from_fork+0x10/0x20
[    6.945519] irq event stamp: 64656

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH net-next v1 21/21] net: dsa: Move devlink registration to be last devlink command
  2021-09-29 13:02   ` Vladimir Oltean
@ 2021-09-29 13:07     ` Leon Romanovsky
  0 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2021-09-29 13:07 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: David S . Miller, Jakub Kicinski, Alexandre Belloni, Andrew Lunn,
	Ariel Elior, Bin Luo, Claudiu Manoil, Coiby Xu, Derek Chickles,
	drivers, Felix Manlunas, Florian Fainelli, Geetha sowjanya,
	Greg Kroah-Hartman, GR-everest-linux-l2, GR-Linux-NIC-Dev,
	hariprasad, Ido Schimmel, Intel Corporation, intel-wired-lan,
	Ioana Ciornei, Jerin Jacob, Jesse Brandeburg, Jiri Pirko,
	Jonathan Lemon, Linu Cherian, linux-kernel, linux-omap,
	linux-rdma, linux-staging, Loic Poulain, Manish Chopra,
	M Chetan Kumar, Michael Chan, Michael Guralnik, netdev,
	oss-drivers, Richard Cochran, Saeed Mahameed, Satanand Burla,
	Sergey Ryazanov, Shannon Nelson, Simon Horman, Subbaraya Sundeep,
	Sunil Goutham, Taras Chornyi, Tariq Toukan, Tony Nguyen,
	UNGLinuxDriver, Vadym Kochan, Vivien Didelot

On Wed, Sep 29, 2021 at 01:02:27PM +0000, Vladimir Oltean wrote:
> Hi Leon,
> 
> On Sat, Sep 25, 2021 at 02:23:01PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> >
> > This change prevents from users to access device before devlink
> > is fully configured.
> >
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > ---
> >  net/dsa/dsa2.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> > index a020339e1973..8ca6a1170c9d 100644
> > --- a/net/dsa/dsa2.c
> > +++ b/net/dsa/dsa2.c
> > @@ -848,7 +848,6 @@ static int dsa_switch_setup(struct dsa_switch *ds)
> >  	dl_priv = devlink_priv(ds->devlink);
> >  	dl_priv->ds = ds;
> >
> > -	devlink_register(ds->devlink);
> >  	/* Setup devlink port instances now, so that the switch
> >  	 * setup() can register regions etc, against the ports
> >  	 */
> > @@ -874,8 +873,6 @@ static int dsa_switch_setup(struct dsa_switch *ds)
> >  	if (err)
> >  		goto teardown;
> >
> > -	devlink_params_publish(ds->devlink);
> > -
> >  	if (!ds->slave_mii_bus && ds->ops->phy_read) {
> >  		ds->slave_mii_bus = mdiobus_alloc();
> >  		if (!ds->slave_mii_bus) {
> > @@ -891,7 +888,7 @@ static int dsa_switch_setup(struct dsa_switch *ds)
> >  	}
> >
> >  	ds->setup = true;
> > -
> > +	devlink_register(ds->devlink);
> >  	return 0;
> >
> >  free_slave_mii_bus:
> > @@ -906,7 +903,6 @@ static int dsa_switch_setup(struct dsa_switch *ds)
> >  	list_for_each_entry(dp, &ds->dst->ports, list)
> >  		if (dp->ds == ds)
> >  			dsa_port_devlink_teardown(dp);
> > -	devlink_unregister(ds->devlink);
> >  	devlink_free(ds->devlink);
> >  	ds->devlink = NULL;
> >  	return err;
> > @@ -919,6 +915,9 @@ static void dsa_switch_teardown(struct dsa_switch *ds)
> >  	if (!ds->setup)
> >  		return;
> >
> > +	if (ds->devlink)
> > +		devlink_unregister(ds->devlink);
> > +
> >  	if (ds->slave_mii_bus && ds->ops->phy_read) {
> >  		mdiobus_unregister(ds->slave_mii_bus);
> >  		mdiobus_free(ds->slave_mii_bus);
> > @@ -934,7 +933,6 @@ static void dsa_switch_teardown(struct dsa_switch *ds)
> >  		list_for_each_entry(dp, &ds->dst->ports, list)
> >  			if (dp->ds == ds)
> >  				dsa_port_devlink_teardown(dp);
> > -		devlink_unregister(ds->devlink);
> >  		devlink_free(ds->devlink);
> >  		ds->devlink = NULL;
> >  	}
> > --
> > 2.31.1
> >
> 
> Sorry, I did not have time to review/test this change earlier.
> I now see this WARN_ON being triggered when I boot a board:

Sorry about that, it was missed in one of my rebases.
The fix was posted here.
https://lore.kernel.org/all/2ed1159291f2a589b013914f2b60d8172fc525c1.1632916329.git.leonro@nvidia.com

Thanks

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2021-09-29 13:07 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 01/21] devlink: Notify users when objects are accessible Leon Romanovsky
2021-09-28  2:49   ` Eric Dumazet
2021-09-28  7:34     ` Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 02/21] bnxt_en: Register devlink instance at the end devlink configuration Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 03/21] liquidio: Overcome missing device lock protection in init/remove flows Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 04/21] dpaa2-eth: Register devlink instance at the end of probe Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 05/21] net: hinic: Open device for the user access when it is ready Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 06/21] ice: Open devlink when device " Leon Romanovsky
2021-09-27 19:47   ` Jesse Brandeburg
2021-09-25 11:22 ` [PATCH net-next v1 07/21] octeontx2: Move devlink registration to be last devlink command Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 08/21] net/prestera: Split devlink and traps registrations to separate routines Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 09/21] net/mlx4: Move devlink_register to be the last initialization command Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 10/21] net/mlx5: Accept devlink user input after driver initialization complete Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 11/21] mlxsw: core: Register devlink instance last Leon Romanovsky
2021-09-26 14:56   ` Ido Schimmel
2021-09-25 11:22 ` [PATCH net-next v1 12/21] net: mscc: ocelot: delay devlink registration to the end Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 13/21] nfp: Move delink_register to be last command Leon Romanovsky
2021-09-27  8:39   ` Simon Horman
2021-09-27 11:53     ` Leon Romanovsky
2021-09-27 12:20       ` Simon Horman
2021-09-25 11:22 ` [PATCH net-next v1 14/21] ionic: Move devlink registration to be last devlink command Leon Romanovsky
2021-09-27 16:07   ` Shannon Nelson
2021-09-25 11:22 ` [PATCH net-next v1 15/21] qed: " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 16/21] net: ethernet: ti: " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 17/21] netdevsim: " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 18/21] net: wwan: iosm: Move devlink_register " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 19/21] ptp: ocp: Move devlink registration " Leon Romanovsky
2021-09-25 11:23 ` [PATCH net-next v1 20/21] staging: qlge: " Leon Romanovsky
2021-09-25 11:23 ` [PATCH net-next v1 21/21] net: dsa: " Leon Romanovsky
2021-09-29 13:02   ` Vladimir Oltean
2021-09-29 13:07     ` Leon Romanovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).