From: Leon Romanovsky <[email protected]>
Hi,
This series starts from the fixing the bug introduced by implementing
devlink delayed notifications logic, where I missed some of the
notifications functions.
The rest series provides a way to dynamically set devlink ops that is
needed for mlx5 multiport device and starts cleanup by removing
not-needed logic.
In the next series, we will delete various publish API, drop general
lock, annotate the code and rework logic around devlink->lock.
All this is possible because driver initialization is separated from the
user input now.
Thanks
Leon Romanovsky (5):
devlink: Add missed notifications iterators
devlink: Allow modification of devlink ops
devlink: Allow set specific ops callbacks dynamically
net/mlx5: Register separate reload devlink ops for multiport device
devlink: Delete reload enable/disable interface
.../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 6 +-
.../net/ethernet/cavium/liquidio/lio_main.c | 2 +-
.../freescale/dpaa2/dpaa2-eth-devlink.c | 2 +-
.../hisilicon/hns3/hns3pf/hclge_devlink.c | 5 +-
.../hisilicon/hns3/hns3vf/hclgevf_devlink.c | 5 +-
.../net/ethernet/huawei/hinic/hinic_devlink.c | 2 +-
drivers/net/ethernet/intel/ice/ice_devlink.c | 2 +-
.../marvell/octeontx2/af/rvu_devlink.c | 2 +-
.../marvell/prestera/prestera_devlink.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/main.c | 4 +-
.../net/ethernet/mellanox/mlx5/core/devlink.c | 15 +-
.../net/ethernet/mellanox/mlx5/core/main.c | 3 -
.../mellanox/mlx5/core/sf/dev/driver.c | 5 +-
drivers/net/ethernet/mellanox/mlxsw/core.c | 12 +-
drivers/net/ethernet/mscc/ocelot.h | 2 +-
drivers/net/ethernet/mscc/ocelot_net.c | 2 +-
.../net/ethernet/netronome/nfp/nfp_devlink.c | 2 +-
drivers/net/ethernet/netronome/nfp/nfp_main.h | 2 +-
.../ethernet/pensando/ionic/ionic_devlink.c | 2 +-
drivers/net/ethernet/qlogic/qed/qed_devlink.c | 2 +-
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 +-
drivers/net/ethernet/ti/cpsw_new.c | 2 +-
drivers/net/netdevsim/dev.c | 5 +-
drivers/ptp/ptp_ocp.c | 2 +-
drivers/staging/qlge/qlge_main.c | 2 +-
include/net/devlink.h | 15 +-
net/core/devlink.c | 155 ++++++++++--------
net/dsa/dsa2.c | 2 +-
28 files changed, 131 insertions(+), 133 deletions(-)
--
2.31.1
From: Leon Romanovsky <[email protected]>
After changes to allow dynamically set the reload_up/_down callbacks,
we ensure that properly supported devlink ops are not accessible before
devlink_register, which is last command in the initialization sequence.
It makes devlink_reload_enable/_disable not relevant anymore and can be
safely deleted.
Signed-off-by: Leon Romanovsky <[email protected]>
---
.../hisilicon/hns3/hns3pf/hclge_devlink.c | 3 --
.../hisilicon/hns3/hns3vf/hclgevf_devlink.c | 3 --
drivers/net/ethernet/mellanox/mlx4/main.c | 2 -
.../net/ethernet/mellanox/mlx5/core/main.c | 3 --
.../mellanox/mlx5/core/sf/dev/driver.c | 5 +--
drivers/net/ethernet/mellanox/mlxsw/core.c | 10 ++---
drivers/net/netdevsim/dev.c | 3 --
include/net/devlink.h | 5 +--
net/core/devlink.c | 40 -------------------
9 files changed, 5 insertions(+), 69 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
index 329b020c688d..63fab1cd33d7 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
@@ -120,7 +120,6 @@ int hclge_devlink_init(struct hclge_dev *hdev)
hdev->devlink = devlink;
devlink_register(devlink);
- devlink_reload_enable(devlink);
return 0;
}
@@ -128,8 +127,6 @@ void hclge_devlink_uninit(struct hclge_dev *hdev)
{
struct devlink *devlink = hdev->devlink;
- devlink_reload_disable(devlink);
-
devlink_unregister(devlink);
devlink_free(devlink);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
index 1d9eecc928a5..26f4d20de40d 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
@@ -122,7 +122,6 @@ int hclgevf_devlink_init(struct hclgevf_dev *hdev)
hdev->devlink = devlink;
devlink_register(devlink);
- devlink_reload_enable(devlink);
return 0;
}
@@ -130,8 +129,6 @@ void hclgevf_devlink_uninit(struct hclgevf_dev *hdev)
{
struct devlink *devlink = hdev->devlink;
- devlink_reload_disable(devlink);
-
devlink_unregister(devlink);
devlink_free(devlink);
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index ab805b6f23d4..8389845d5c9e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -4026,7 +4026,6 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
pci_save_state(pdev);
devlink_register(devlink);
- devlink_reload_enable(devlink);
return 0;
err_params_unregister:
@@ -4135,7 +4134,6 @@ static void mlx4_remove_one(struct pci_dev *pdev)
struct devlink *devlink = priv_to_devlink(priv);
int active_vfs = 0;
- devlink_reload_disable(devlink);
devlink_unregister(devlink);
if (mlx4_is_slave(dev))
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 92b08fa07efa..261f18d57916 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1538,8 +1538,6 @@ static int probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
pci_save_state(pdev);
devlink_register(devlink);
- if (!mlx5_core_is_mp_slave(dev))
- devlink_reload_enable(devlink);
return 0;
err_init_one:
@@ -1559,7 +1557,6 @@ static void remove_one(struct pci_dev *pdev)
struct mlx5_core_dev *dev = pci_get_drvdata(pdev);
struct devlink *devlink = priv_to_devlink(dev);
- devlink_reload_disable(devlink);
devlink_unregister(devlink);
mlx5_crdump_disable(dev);
mlx5_drain_health_wq(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 3cf272fa2164..7b4783ce213e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/driver.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/driver.c
@@ -47,7 +47,6 @@ static int mlx5_sf_dev_probe(struct auxiliary_device *adev, const struct auxilia
goto init_one_err;
}
devlink_register(devlink);
- devlink_reload_enable(devlink);
return 0;
init_one_err:
@@ -62,10 +61,8 @@ static int mlx5_sf_dev_probe(struct auxiliary_device *adev, const struct auxilia
static void mlx5_sf_dev_remove(struct auxiliary_device *adev)
{
struct mlx5_sf_dev *sf_dev = container_of(adev, struct mlx5_sf_dev, adev);
- struct devlink *devlink;
+ struct devlink *devlink = priv_to_devlink(sf_dev->mdev);
- 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);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 1012279008f9..efbcee8d5ea9 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -2007,11 +2007,8 @@ __mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
goto err_driver_init;
}
- if (!reload) {
+ if (!reload)
devlink_register(devlink);
- devlink_reload_enable(devlink);
- }
-
return 0;
err_driver_init:
@@ -2075,10 +2072,9 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core,
{
struct devlink *devlink = priv_to_devlink(mlxsw_core);
- if (!reload) {
- devlink_reload_disable(devlink);
+ if (!reload)
devlink_unregister(devlink);
- }
+
if (devlink_is_reload_failed(devlink)) {
if (!reload)
/* Only the parts that were not de-initialized in the
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 466d2c27e868..c66c40afb19f 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -1512,7 +1512,6 @@ int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)
nsim_dev->esw_mode = DEVLINK_ESWITCH_MODE_LEGACY;
devlink_register(devlink);
- devlink_reload_enable(devlink);
return 0;
err_psample_exit:
@@ -1566,9 +1565,7 @@ void nsim_dev_remove(struct nsim_bus_dev *nsim_bus_dev)
struct nsim_dev *nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
struct devlink *devlink = priv_to_devlink(nsim_dev);
- devlink_reload_disable(devlink);
devlink_unregister(devlink);
-
nsim_dev_reload_destroy(nsim_dev);
nsim_bpf_dev_exit(nsim_dev);
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 305be548ac21..9403d13617af 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -54,8 +54,7 @@ struct devlink {
struct mutex lock; /* Serializes access to devlink instance specific objects such as
* port, sb, dpipe, resource, params, region, traps and more.
*/
- u8 reload_failed:1,
- reload_enabled:1;
+ u8 reload_failed:1;
refcount_t refcount;
struct completion comp;
char priv[0] __aligned(NETDEV_ALIGN);
@@ -1568,8 +1567,6 @@ static inline struct devlink *devlink_alloc(struct devlink_ops *ops,
void devlink_set_ops(struct devlink *devlink, struct devlink_ops *ops);
void devlink_register(struct devlink *devlink);
void devlink_unregister(struct devlink *devlink);
-void devlink_reload_enable(struct devlink *devlink);
-void devlink_reload_disable(struct devlink *devlink);
void devlink_free(struct devlink *devlink);
int devlink_port_register(struct devlink *devlink,
struct devlink_port *devlink_port,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 71d0c5671f43..acf26c34ffa7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3959,9 +3959,6 @@ static int devlink_reload(struct devlink *devlink, struct net *dest_net,
struct net *curr_net;
int err;
- if (!devlink->reload_enabled)
- return -EOPNOTSUPP;
-
memcpy(remote_reload_stats, devlink->stats.remote_reload_stats,
sizeof(remote_reload_stats));
@@ -9105,49 +9102,12 @@ void devlink_unregister(struct devlink *devlink)
wait_for_completion(&devlink->comp);
mutex_lock(&devlink_mutex);
- WARN_ON(devlink_reload_supported(devlink->ops) &&
- devlink->reload_enabled);
devlink_notify_unregister(devlink);
xa_clear_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
mutex_unlock(&devlink_mutex);
}
EXPORT_SYMBOL_GPL(devlink_unregister);
-/**
- * devlink_reload_enable - Enable reload of devlink instance
- *
- * @devlink: devlink
- *
- * Should be called at end of device initialization
- * process when reload operation is supported.
- */
-void devlink_reload_enable(struct devlink *devlink)
-{
- mutex_lock(&devlink_mutex);
- devlink->reload_enabled = true;
- mutex_unlock(&devlink_mutex);
-}
-EXPORT_SYMBOL_GPL(devlink_reload_enable);
-
-/**
- * devlink_reload_disable - Disable reload of devlink instance
- *
- * @devlink: devlink
- *
- * Should be called at the beginning of device cleanup
- * process when reload operation is supported.
- */
-void devlink_reload_disable(struct devlink *devlink)
-{
- mutex_lock(&devlink_mutex);
- /* Mutex is taken which ensures that no reload operation is in
- * progress while setting up forbidded flag.
- */
- devlink->reload_enabled = false;
- mutex_unlock(&devlink_mutex);
-}
-EXPORT_SYMBOL_GPL(devlink_reload_disable);
-
/**
* devlink_free - Free devlink instance resources
*
--
2.31.1
From: Leon Romanovsky <[email protected]>
Introduce new devlink call to set specific ops callback during
device initialization phase after devlink_alloc() is already
called.
This allows us to set reload_* specific ops based on device property
which sometimes is known almost at the end of driver initialization.
For the sake of simplicity, this API lacks any type of locking and
needs to be called before devlink_register() to make sure that no
parallel access to the ops is possible at this stage.
Signed-off-by: Leon Romanovsky <[email protected]>
---
include/net/devlink.h | 1 +
net/core/devlink.c | 41 +++++++++++++++++++++++++++++++++++++++--
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 317b09917c41..305be548ac21 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1565,6 +1565,7 @@ static inline struct devlink *devlink_alloc(struct devlink_ops *ops,
{
return devlink_alloc_ns(ops, priv_size, &init_net, dev);
}
+void devlink_set_ops(struct devlink *devlink, struct devlink_ops *ops);
void devlink_register(struct devlink *devlink);
void devlink_unregister(struct devlink *devlink);
void devlink_reload_enable(struct devlink *devlink);
diff --git a/net/core/devlink.c b/net/core/devlink.c
index cee2be47c40e..71d0c5671f43 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -8907,6 +8907,43 @@ static bool devlink_reload_actions_valid(struct devlink_ops *ops)
return true;
}
+/**
+ * devlink_set_ops - Set devlink ops dynamically
+ *
+ * @devlink: devlink
+ * @ops: devlink ops to set
+ *
+ * This interface allows us to set ops based on device property
+ * which is known after devlink_alloc() was already called. For now,
+ * it is applicable for reload_* assignments only and all other
+ * callbacks are ignored.
+ *
+ * It should be called before devlink_register(), so doesn't have any
+ * protection from concurent access.
+ */
+void devlink_set_ops(struct devlink *devlink, struct devlink_ops *ops)
+{
+ struct devlink_ops *dev_ops = devlink->ops;
+
+ WARN_ON(!devlink_reload_actions_valid(ops));
+
+#define SET_DEVICE_OP(ptr, name) \
+ do { \
+ if (ops->name) \
+ if (!((ptr)->name)) \
+ (ptr)->name = ops->name; \
+ } while (0)
+
+ /* Keep sorted */
+ SET_DEVICE_OP(dev_ops, reload_actions);
+ SET_DEVICE_OP(dev_ops, reload_down);
+ SET_DEVICE_OP(dev_ops, reload_limits);
+ SET_DEVICE_OP(dev_ops, reload_up);
+
+#undef SET_DEVICE_OP
+}
+EXPORT_SYMBOL_GPL(devlink_set_ops);
+
/**
* devlink_alloc_ns - Allocate new devlink instance resources
* in specific namespace
@@ -8927,8 +8964,6 @@ struct devlink *devlink_alloc_ns(struct devlink_ops *ops, size_t priv_size,
int ret;
WARN_ON(!ops || !dev);
- if (!devlink_reload_actions_valid(ops))
- return NULL;
devlink = kzalloc(sizeof(*devlink) + priv_size, GFP_KERNEL);
if (!devlink)
@@ -8943,6 +8978,8 @@ struct devlink *devlink_alloc_ns(struct devlink_ops *ops, size_t priv_size,
devlink->dev = dev;
devlink->ops = ops;
+ /* To check validity of reload actions */
+ devlink_set_ops(devlink, ops);
xa_init_flags(&devlink->snapshot_ids, XA_FLAGS_ALLOC);
write_pnet(&devlink->_net, net);
INIT_LIST_HEAD(&devlink->port_list);
--
2.31.1
From: Leon Romanovsky <[email protected]>
Mulitport slave device doesn't support devlink reload, so instead of
complicating initialization flow with devlink_reload_enable() which
will be removed in next patch, set specialized devlink ops callbacks
for reload operations.
This fixes an error when reload counters exposed (and equal zero) for
the mode that is not supported at all.
Fixes: d89ddaae1766 ("net/mlx5: Disable devlink reload for multi port slave device")
Signed-off-by: Leon Romanovsky <[email protected]>
---
drivers/net/ethernet/mellanox/mlx5/core/devlink.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 47c9f7f5bb79..e85eca6976a9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -309,14 +309,17 @@ static struct devlink_ops mlx5_devlink_ops = {
#endif
.flash_update = mlx5_devlink_flash_update,
.info_get = mlx5_devlink_info_get,
+ .trap_init = mlx5_devlink_trap_init,
+ .trap_fini = mlx5_devlink_trap_fini,
+ .trap_action_set = mlx5_devlink_trap_action_set,
+};
+
+static struct devlink_ops mlx5_devlink_reload = {
.reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT) |
BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE),
.reload_limits = BIT(DEVLINK_RELOAD_LIMIT_NO_RESET),
.reload_down = mlx5_devlink_reload_down,
.reload_up = mlx5_devlink_reload_up,
- .trap_init = mlx5_devlink_trap_init,
- .trap_fini = mlx5_devlink_trap_fini,
- .trap_action_set = mlx5_devlink_trap_action_set,
};
void mlx5_devlink_trap_report(struct mlx5_core_dev *dev, int trap_id, struct sk_buff *skb,
@@ -791,6 +794,7 @@ static void mlx5_devlink_traps_unregister(struct devlink *devlink)
int mlx5_devlink_register(struct devlink *devlink)
{
+ struct mlx5_core_dev *dev = devlink_priv(devlink);
int err;
err = devlink_params_register(devlink, mlx5_devlink_params,
@@ -808,6 +812,9 @@ int mlx5_devlink_register(struct devlink *devlink)
if (err)
goto traps_reg_err;
+ if (!mlx5_core_is_mp_slave(dev))
+ devlink_set_ops(devlink, &mlx5_devlink_reload);
+
return 0;
traps_reg_err:
--
2.31.1
On Wed, Sep 29, 2021 at 01:16:37PM +0300, Leon Romanovsky wrote:
> +void devlink_set_ops(struct devlink *devlink, struct devlink_ops *ops)
> +{
> + struct devlink_ops *dev_ops = devlink->ops;
> +
> + WARN_ON(!devlink_reload_actions_valid(ops));
> +
> +#define SET_DEVICE_OP(ptr, name) \
> + do { \
> + if (ops->name) \
Could you make "ops" a parameter of the macro instead of hard coding it?
regards,
dan carpenter
> + if (!((ptr)->name)) \
> + (ptr)->name = ops->name; \
> + } while (0)
> +
> + /* Keep sorted */
> + SET_DEVICE_OP(dev_ops, reload_actions);
> + SET_DEVICE_OP(dev_ops, reload_down);
> + SET_DEVICE_OP(dev_ops, reload_limits);
> + SET_DEVICE_OP(dev_ops, reload_up);
> +
> +#undef SET_DEVICE_OP
> +}
> +EXPORT_SYMBOL_GPL(devlink_set_ops);
On Wed, Sep 29, 2021 at 01:16:34PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <[email protected]>
>
> Hi,
>
> This series starts from the fixing the bug introduced by implementing
> devlink delayed notifications logic, where I missed some of the
> notifications functions.
>
> The rest series provides a way to dynamically set devlink ops that is
> needed for mlx5 multiport device and starts cleanup by removing
> not-needed logic.
>
> In the next series, we will delete various publish API, drop general
> lock, annotate the code and rework logic around devlink->lock.
>
> All this is possible because driver initialization is separated from the
> user input now.
>
> Thanks
>
> Leon Romanovsky (5):
> devlink: Add missed notifications iterators
I missed WARN_ON(), I'm resending the series.
Thanks
From: Leon Romanovsky <[email protected]>
The commit mentioned in Fixes line missed a couple of notifications that
were registered before devlink_register() and should be delayed too.
As such, the too early placed WARN_ON() check spotted it.
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
Fixes: 474053c980a0 ("devlink: Notify users when objects are accessible")
Reported-by: Eric Dumazet <[email protected]>
Signed-off-by: Leon Romanovsky <[email protected]>
---
net/core/devlink.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 06edb2f1d21e..35c74c8e5e2f 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1074,6 +1074,9 @@ static void devlink_rate_notify(struct devlink_rate *devlink_rate,
WARN_ON(cmd != DEVLINK_CMD_RATE_NEW && cmd != DEVLINK_CMD_RATE_DEL);
WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));
+ if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
+ return;
+
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!msg)
return;
@@ -5155,7 +5158,8 @@ static void devlink_nl_region_notify(struct devlink_region *region,
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));
+ if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
+ return;
msg = devlink_nl_region_notify_build(region, snapshot, cmd, 0, 0);
if (IS_ERR(msg))
@@ -8981,6 +8985,8 @@ static void devlink_notify_register(struct devlink *devlink)
struct devlink_trap_group_item *group_item;
struct devlink_trap_item *trap_item;
struct devlink_port *devlink_port;
+ struct devlink_rate *rate_node;
+ struct devlink_region *region;
devlink_notify(devlink, DEVLINK_CMD_NEW);
list_for_each_entry(devlink_port, &devlink->port_list, list)
@@ -8997,6 +9003,12 @@ static void devlink_notify_register(struct devlink *devlink)
list_for_each_entry(trap_item, &devlink->trap_list, list)
devlink_trap_notify(devlink, trap_item, DEVLINK_CMD_TRAP_NEW);
+ list_for_each_entry(rate_node, &devlink->rate_list, list)
+ devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW);
+
+ list_for_each_entry(region, &devlink->region_list, list)
+ devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_NEW);
+
devlink_params_publish(devlink);
}
@@ -9006,9 +9018,17 @@ static void devlink_notify_unregister(struct devlink *devlink)
struct devlink_trap_group_item *group_item;
struct devlink_trap_item *trap_item;
struct devlink_port *devlink_port;
+ struct devlink_rate *rate_node;
+ struct devlink_region *region;
devlink_params_unpublish(devlink);
+ list_for_each_entry_reverse(region, &devlink->region_list, list)
+ devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_DEL);
+
+ list_for_each_entry_reverse(rate_node, &devlink->rate_list, list)
+ devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_DEL);
+
list_for_each_entry_reverse(trap_item, &devlink->trap_list, list)
devlink_trap_notify(devlink, trap_item, DEVLINK_CMD_TRAP_DEL);
--
2.31.1
On Wed, Sep 29, 2021 at 01:38:23PM +0300, Dan Carpenter wrote:
> On Wed, Sep 29, 2021 at 01:16:37PM +0300, Leon Romanovsky wrote:
> > +void devlink_set_ops(struct devlink *devlink, struct devlink_ops *ops)
> > +{
> > + struct devlink_ops *dev_ops = devlink->ops;
> > +
> > + WARN_ON(!devlink_reload_actions_valid(ops));
> > +
> > +#define SET_DEVICE_OP(ptr, name) \
> > + do { \
> > + if (ops->name) \
>
> Could you make "ops" a parameter of the macro instead of hard coding it?
Sure
>
> regards,
> dan carpenter
>
> > + if (!((ptr)->name)) \
> > + (ptr)->name = ops->name; \
> > + } while (0)
> > +
> > + /* Keep sorted */
> > + SET_DEVICE_OP(dev_ops, reload_actions);
> > + SET_DEVICE_OP(dev_ops, reload_down);
> > + SET_DEVICE_OP(dev_ops, reload_limits);
> > + SET_DEVICE_OP(dev_ops, reload_up);
> > +
> > +#undef SET_DEVICE_OP
> > +}
> > +EXPORT_SYMBOL_GPL(devlink_set_ops);
>