From: Leon Romanovsky <[email protected]>
Changelog:
v1:
* Missed removal of extra WARN_ON
* Added "ops parameter to macro as Dan suggested.
v0: https://lore.kernel.org/all/[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 | 156 ++++++++++--------
net/dsa/dsa2.c | 2 +-
28 files changed, 131 insertions(+), 134 deletions(-)
--
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 9ae38128d6e1..67a846d424b7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -8906,6 +8906,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, op, name) \
+ do { \
+ if ((op)->name) \
+ if (!((ptr)->name)) \
+ (ptr)->name = (op)->name; \
+ } while (0)
+
+ /* Keep sorted */
+ SET_DEVICE_OP(dev_ops, ops, reload_actions);
+ SET_DEVICE_OP(dev_ops, ops, reload_down);
+ SET_DEVICE_OP(dev_ops, ops, reload_limits);
+ SET_DEVICE_OP(dev_ops, ops, reload_up);
+
+#undef SET_DEVICE_OP
+}
+EXPORT_SYMBOL_GPL(devlink_set_ops);
+
/**
* devlink_alloc_ns - Allocate new devlink instance resources
* in specific namespace
@@ -8926,8 +8963,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)
@@ -8942,6 +8977,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]>
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 67a846d424b7..eb6ec87adaae 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3958,9 +3958,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));
@@ -9104,49 +9101,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]>
Drop const identifier from devlink_ops declaration to allow
run-time modification of pre-declared ops.
Acked-by: Simon Horman <[email protected]> #nfp
Signed-off-by: Leon Romanovsky <[email protected]>
---
.../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 | 2 +-
.../hisilicon/hns3/hns3vf/hclgevf_devlink.c | 2 +-
.../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 | 2 +-
.../net/ethernet/mellanox/mlx5/core/devlink.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/core.c | 2 +-
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 | 2 +-
drivers/ptp/ptp_ocp.c | 2 +-
drivers/staging/qlge/qlge_main.c | 2 +-
include/net/devlink.h | 9 ++--
net/core/devlink.c | 52 +++++++++----------
net/dsa/dsa2.c | 2 +-
26 files changed, 55 insertions(+), 58 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 951c0c00cc95..0a1004d0be07 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -280,7 +280,7 @@ void bnxt_dl_health_recovery_done(struct bnxt *bp)
static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
struct netlink_ext_ack *extack);
-static const struct devlink_ops bnxt_dl_ops = {
+static struct devlink_ops bnxt_dl_ops = {
#ifdef CONFIG_BNXT_SRIOV
.eswitch_mode_set = bnxt_dl_eswitch_mode_set,
.eswitch_mode_get = bnxt_dl_eswitch_mode_get,
@@ -289,7 +289,7 @@ static const struct devlink_ops bnxt_dl_ops = {
.flash_update = bnxt_dl_flash_update,
};
-static const struct devlink_ops bnxt_vf_dl_ops;
+static struct devlink_ops bnxt_vf_dl_ops;
enum bnxt_dl_param_id {
BNXT_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
@@ -762,8 +762,8 @@ static void bnxt_dl_params_unregister(struct bnxt *bp)
int bnxt_dl_register(struct bnxt *bp)
{
- const struct devlink_ops *devlink_ops;
struct devlink_port_attrs attrs = {};
+ struct devlink_ops *devlink_ops;
struct bnxt_dl *bp_dl;
struct devlink *dl;
int rc;
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index dafc79bd34f4..6684f4127ef2 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -3167,7 +3167,7 @@ liquidio_eswitch_mode_set(struct devlink *devlink, u16 mode,
return ret;
}
-static const struct devlink_ops liquidio_devlink_ops = {
+static struct devlink_ops liquidio_devlink_ops = {
.eswitch_mode_get = liquidio_eswitch_mode_get,
.eswitch_mode_set = liquidio_eswitch_mode_set,
};
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
index 7fefe1574b6a..da6040fbb354 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
@@ -182,7 +182,7 @@ static int dpaa2_eth_dl_trap_group_action_set(struct devlink *devlink,
return 0;
}
-static const struct devlink_ops dpaa2_eth_devlink_ops = {
+static struct devlink_ops dpaa2_eth_devlink_ops = {
.info_get = dpaa2_eth_dl_info_get,
.trap_init = dpaa2_eth_dl_trap_init,
.trap_action_set = dpaa2_eth_dl_trap_action_set,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
index 59b0ae7d59e0..329b020c688d 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
@@ -97,7 +97,7 @@ static int hclge_devlink_reload_up(struct devlink *devlink,
}
}
-static const struct devlink_ops hclge_devlink_ops = {
+static struct devlink_ops hclge_devlink_ops = {
.info_get = hclge_devlink_info_get,
.reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT),
.reload_down = hclge_devlink_reload_down,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
index d60cc9426f70..1d9eecc928a5 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
@@ -98,7 +98,7 @@ static int hclgevf_devlink_reload_up(struct devlink *devlink,
}
}
-static const struct devlink_ops hclgevf_devlink_ops = {
+static struct devlink_ops hclgevf_devlink_ops = {
.info_get = hclgevf_devlink_info_get,
.reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT),
.reload_down = hclgevf_devlink_reload_down,
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_devlink.c b/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
index 60ae8bfc5f69..22365c85911f 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
@@ -289,7 +289,7 @@ static int hinic_devlink_flash_update(struct devlink *devlink,
return hinic_firmware_update(priv, params->fw, extack);
}
-static const struct devlink_ops hinic_devlink_ops = {
+static struct devlink_ops hinic_devlink_ops = {
.flash_update = hinic_devlink_flash_update,
};
diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index ab3d876fa624..ac3a66542a29 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -454,7 +454,7 @@ ice_devlink_flash_update(struct devlink *devlink,
return ice_flash_pldm_image(pf, params->fw, preservation, extack);
}
-static const struct devlink_ops ice_devlink_ops = {
+static struct devlink_ops ice_devlink_ops = {
.supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK,
.info_get = ice_devlink_info_get,
.flash_update = ice_devlink_flash_update,
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index 70bacd38a6d9..e65ae07bd7b0 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
@@ -1491,7 +1491,7 @@ static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req
return devlink_info_driver_name_put(req, DRV_NAME);
}
-static const struct devlink_ops rvu_devlink_ops = {
+static struct devlink_ops rvu_devlink_ops = {
.info_get = rvu_devlink_info_get,
.eswitch_mode_get = rvu_devlink_eswitch_mode_get,
.eswitch_mode_set = rvu_devlink_eswitch_mode_set,
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
index 06279cd6da67..8110deba9331 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
@@ -379,7 +379,7 @@ static int prestera_trap_action_set(struct devlink *devlink,
enum devlink_trap_action action,
struct netlink_ext_ack *extack);
-static const struct devlink_ops prestera_dl_ops = {
+static struct devlink_ops prestera_dl_ops = {
.info_get = prestera_dl_info_get,
.trap_init = prestera_trap_init,
.trap_action_set = prestera_trap_action_set,
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 9541f3a920c8..ab805b6f23d4 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3980,7 +3980,7 @@ static int mlx4_devlink_reload_up(struct devlink *devlink, enum devlink_reload_a
return err;
}
-static const struct devlink_ops mlx4_devlink_ops = {
+static struct devlink_ops mlx4_devlink_ops = {
.port_type_set = mlx4_devlink_port_type_set,
.reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT),
.reload_down = mlx4_devlink_reload_down,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index d7576b6fa43b..47c9f7f5bb79 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -283,7 +283,7 @@ static int mlx5_devlink_trap_action_set(struct devlink *devlink,
return err;
}
-static const struct devlink_ops mlx5_devlink_ops = {
+static struct devlink_ops mlx5_devlink_ops = {
#ifdef CONFIG_MLX5_ESWITCH
.eswitch_mode_set = mlx5_devlink_eswitch_mode_set,
.eswitch_mode_get = mlx5_devlink_eswitch_mode_get,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 9e831e8b607a..1012279008f9 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1614,7 +1614,7 @@ mlxsw_devlink_trap_policer_counter_get(struct devlink *devlink,
p_drops);
}
-static const struct devlink_ops mlxsw_devlink_ops = {
+static struct devlink_ops mlxsw_devlink_ops = {
.reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT) |
BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE),
.reload_down = mlxsw_devlink_core_bus_device_reload_down,
diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
index 1952d6a1b98a..b7e8c7caa437 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h
@@ -115,6 +115,6 @@ void ocelot_port_devlink_teardown(struct ocelot *ocelot, int port);
extern struct notifier_block ocelot_netdevice_nb;
extern struct notifier_block ocelot_switchdev_nb;
extern struct notifier_block ocelot_switchdev_blocking_nb;
-extern const struct devlink_ops ocelot_devlink_ops;
+extern struct devlink_ops ocelot_devlink_ops;
#endif
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index e54b9fb2a97a..8c08f63c4836 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -143,7 +143,7 @@ ocelot_devlink_sb_occ_tc_port_bind_get(struct devlink_port *dlp,
p_cur, p_max);
}
-const struct devlink_ops ocelot_devlink_ops = {
+struct devlink_ops ocelot_devlink_ops = {
.sb_pool_get = ocelot_devlink_sb_pool_get,
.sb_pool_set = ocelot_devlink_sb_pool_set,
.sb_port_pool_get = ocelot_devlink_sb_port_pool_get,
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index bea978df7713..0db2ce522d5d 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -336,7 +336,7 @@ nfp_devlink_flash_update(struct devlink *devlink,
return nfp_flash_update_common(devlink_priv(devlink), params->fw, extack);
}
-const struct devlink_ops nfp_devlink_ops = {
+struct devlink_ops nfp_devlink_ops = {
.port_split = nfp_devlink_port_split,
.port_unsplit = nfp_devlink_port_unsplit,
.sb_pool_get = nfp_devlink_sb_pool_get,
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.h b/drivers/net/ethernet/netronome/nfp/nfp_main.h
index a7dede946a33..2273d0e76870 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.h
@@ -145,7 +145,7 @@ struct nfp_pf {
extern struct pci_driver nfp_netvf_pci_driver;
-extern const struct devlink_ops nfp_devlink_ops;
+extern struct devlink_ops nfp_devlink_ops;
int nfp_net_pci_probe(struct nfp_pf *pf);
void nfp_net_pci_remove(struct nfp_pf *pf);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
index 2267da95640b..3c26b0a1c8dc 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
@@ -55,7 +55,7 @@ static int ionic_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
return err;
}
-static const struct devlink_ops ionic_dl_ops = {
+static struct devlink_ops ionic_dl_ops = {
.info_get = ionic_dl_info_get,
.flash_update = ionic_dl_flash_update,
};
diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.c b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
index 6bb4e165b592..fe1d21c36944 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_devlink.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
@@ -196,7 +196,7 @@ static int qed_devlink_info_get(struct devlink *devlink,
DEVLINK_INFO_VERSION_GENERIC_FW_APP, buf);
}
-static const struct devlink_ops qed_dl_ops = {
+static struct devlink_ops qed_dl_ops = {
.info_get = qed_devlink_info_get,
};
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 0de5f4a4fe08..9c686eaf798f 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -2200,7 +2200,7 @@ static void am65_cpsw_unregister_notifiers(struct am65_cpsw_common *cpsw)
unregister_netdevice_notifier(&cpsw->am65_cpsw_netdevice_nb);
}
-static const struct devlink_ops am65_cpsw_devlink_ops = {};
+static struct devlink_ops am65_cpsw_devlink_ops = {};
static void am65_cpsw_init_stp_ale_entry(struct am65_cpsw_common *cpsw)
{
diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
index 1530532748a8..ca6f78527af1 100644
--- a/drivers/net/ethernet/ti/cpsw_new.c
+++ b/drivers/net/ethernet/ti/cpsw_new.c
@@ -1604,7 +1604,7 @@ static void cpsw_unregister_notifiers(struct cpsw_common *cpsw)
unregister_netdevice_notifier(&cpsw_netdevice_nb);
}
-static const struct devlink_ops cpsw_devlink_ops = {
+static struct devlink_ops cpsw_devlink_ops = {
};
static int cpsw_dl_switch_mode_get(struct devlink *dl, u32 id,
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index cb6645012a30..466d2c27e868 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -1241,7 +1241,7 @@ nsim_dev_devlink_trap_drop_counter_get(struct devlink *devlink,
return 0;
}
-static const struct devlink_ops nsim_dev_devlink_ops = {
+static struct devlink_ops nsim_dev_devlink_ops = {
.eswitch_mode_set = nsim_devlink_eswitch_mode_set,
.eswitch_mode_get = nsim_devlink_eswitch_mode_get,
.supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_COMPONENT |
diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 34f943c8c9fd..c89be26ce4d2 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -1106,7 +1106,7 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
return 0;
}
-static const struct devlink_ops ptp_ocp_devlink_ops = {
+static struct devlink_ops ptp_ocp_devlink_ops = {
.flash_update = ptp_ocp_devlink_flash_update,
.info_get = ptp_ocp_devlink_info_get,
};
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index 1dc849378a0f..55657c10b443 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -4535,7 +4535,7 @@ static void qlge_timer(struct timer_list *t)
mod_timer(&qdev->timer, jiffies + (5 * HZ));
}
-static const struct devlink_ops qlge_devlink_ops;
+static struct devlink_ops qlge_devlink_ops;
static int qlge_probe(struct pci_dev *pdev,
const struct pci_device_id *pci_entry)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index a7852a257bf6..317b09917c41 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -46,7 +46,7 @@ struct devlink {
struct list_head trap_list;
struct list_head trap_group_list;
struct list_head trap_policer_list;
- const struct devlink_ops *ops;
+ struct devlink_ops *ops;
struct xarray snapshot_ids;
struct devlink_dev_stats stats;
struct device *dev;
@@ -1557,10 +1557,9 @@ struct net *devlink_net(const struct devlink *devlink);
*
* Drivers that operate on real HW must use devlink_alloc() instead.
*/
-struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
- size_t priv_size, struct net *net,
- struct device *dev);
-static inline struct devlink *devlink_alloc(const struct devlink_ops *ops,
+struct devlink *devlink_alloc_ns(struct devlink_ops *ops, size_t priv_size,
+ struct net *net, struct device *dev);
+static inline struct devlink *devlink_alloc(struct devlink_ops *ops,
size_t priv_size,
struct device *dev)
{
diff --git a/net/core/devlink.c b/net/core/devlink.c
index b64303085d0e..9ae38128d6e1 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -821,7 +821,7 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
return 0;
}
-static int devlink_port_fn_hw_addr_fill(const struct devlink_ops *ops,
+static int devlink_port_fn_hw_addr_fill(struct devlink_ops *ops,
struct devlink_port *port,
struct sk_buff *msg,
struct netlink_ext_ack *extack,
@@ -911,7 +911,7 @@ devlink_port_fn_opstate_valid(enum devlink_port_fn_opstate opstate)
opstate == DEVLINK_PORT_FN_OPSTATE_ATTACHED;
}
-static int devlink_port_fn_state_fill(const struct devlink_ops *ops,
+static int devlink_port_fn_state_fill(struct devlink_ops *ops,
struct devlink_port *port,
struct sk_buff *msg,
struct netlink_ext_ack *extack,
@@ -952,9 +952,9 @@ static int
devlink_nl_port_function_attrs_put(struct sk_buff *msg, struct devlink_port *port,
struct netlink_ext_ack *extack)
{
- const struct devlink_ops *ops;
struct nlattr *function_attr;
bool msg_updated = false;
+ struct devlink_ops *ops;
int err;
function_attr = nla_nest_start_noflag(msg, DEVLINK_ATTR_PORT_FUNCTION);
@@ -1329,7 +1329,7 @@ static int devlink_port_function_hw_addr_set(struct devlink_port *port,
const struct nlattr *attr,
struct netlink_ext_ack *extack)
{
- const struct devlink_ops *ops = port->devlink->ops;
+ struct devlink_ops *ops = port->devlink->ops;
const u8 *hw_addr;
int hw_addr_len;
@@ -1364,7 +1364,7 @@ static int devlink_port_fn_state_set(struct devlink_port *port,
struct netlink_ext_ack *extack)
{
enum devlink_port_fn_state state;
- const struct devlink_ops *ops;
+ struct devlink_ops *ops;
state = nla_get_u8(attr);
ops = port->devlink->ops;
@@ -1615,7 +1615,7 @@ devlink_nl_rate_parent_node_set(struct devlink_rate *devlink_rate,
{
struct devlink *devlink = devlink_rate->devlink;
const char *parent_name = nla_data(nla_parent);
- const struct devlink_ops *ops = devlink->ops;
+ struct devlink_ops *ops = devlink->ops;
size_t len = strlen(parent_name);
struct devlink_rate *parent;
int err = -EOPNOTSUPP;
@@ -1673,8 +1673,7 @@ devlink_nl_rate_parent_node_set(struct devlink_rate *devlink_rate,
}
static int devlink_nl_rate_set(struct devlink_rate *devlink_rate,
- const struct devlink_ops *ops,
- struct genl_info *info)
+ struct devlink_ops *ops, struct genl_info *info)
{
struct nlattr *nla_parent, **attrs = info->attrs;
int err = -EOPNOTSUPP;
@@ -1717,7 +1716,7 @@ static int devlink_nl_rate_set(struct devlink_rate *devlink_rate,
return 0;
}
-static bool devlink_rate_set_ops_supported(const struct devlink_ops *ops,
+static bool devlink_rate_set_ops_supported(struct devlink_ops *ops,
struct genl_info *info,
enum devlink_rate_type type)
{
@@ -1764,7 +1763,7 @@ static int devlink_nl_cmd_rate_set_doit(struct sk_buff *skb,
{
struct devlink_rate *devlink_rate = info->user_ptr[1];
struct devlink *devlink = devlink_rate->devlink;
- const struct devlink_ops *ops = devlink->ops;
+ struct devlink_ops *ops = devlink->ops;
int err;
if (!ops || !devlink_rate_set_ops_supported(ops, info, devlink_rate->type))
@@ -1782,7 +1781,7 @@ static int devlink_nl_cmd_rate_new_doit(struct sk_buff *skb,
{
struct devlink *devlink = info->user_ptr[0];
struct devlink_rate *rate_node;
- const struct devlink_ops *ops;
+ struct devlink_ops *ops;
int err;
ops = devlink->ops;
@@ -1839,7 +1838,7 @@ static int devlink_nl_cmd_rate_del_doit(struct sk_buff *skb,
{
struct devlink_rate *rate_node = info->user_ptr[1];
struct devlink *devlink = rate_node->devlink;
- const struct devlink_ops *ops = devlink->ops;
+ struct devlink_ops *ops = devlink->ops;
int err;
if (refcount_read(&rate_node->refcnt) > 1) {
@@ -2127,7 +2126,7 @@ static int devlink_sb_pool_set(struct devlink *devlink, unsigned int sb_index,
struct netlink_ext_ack *extack)
{
- const struct devlink_ops *ops = devlink->ops;
+ struct devlink_ops *ops = devlink->ops;
if (ops->sb_pool_set)
return ops->sb_pool_set(devlink, sb_index, pool_index,
@@ -2175,7 +2174,7 @@ static int devlink_nl_sb_port_pool_fill(struct sk_buff *msg,
enum devlink_command cmd,
u32 portid, u32 seq, int flags)
{
- const struct devlink_ops *ops = devlink->ops;
+ struct devlink_ops *ops = devlink->ops;
u32 threshold;
void *hdr;
int err;
@@ -2348,7 +2347,7 @@ static int devlink_sb_port_pool_set(struct devlink_port *devlink_port,
struct netlink_ext_ack *extack)
{
- const struct devlink_ops *ops = devlink_port->devlink->ops;
+ struct devlink_ops *ops = devlink_port->devlink->ops;
if (ops->sb_port_pool_set)
return ops->sb_port_pool_set(devlink_port, sb_index,
@@ -2391,7 +2390,7 @@ devlink_nl_sb_tc_pool_bind_fill(struct sk_buff *msg, struct devlink *devlink,
enum devlink_command cmd,
u32 portid, u32 seq, int flags)
{
- const struct devlink_ops *ops = devlink->ops;
+ struct devlink_ops *ops = devlink->ops;
u16 pool_index;
u32 threshold;
void *hdr;
@@ -2599,7 +2598,7 @@ static int devlink_sb_tc_pool_bind_set(struct devlink_port *devlink_port,
struct netlink_ext_ack *extack)
{
- const struct devlink_ops *ops = devlink_port->devlink->ops;
+ struct devlink_ops *ops = devlink_port->devlink->ops;
if (ops->sb_tc_pool_bind_set)
return ops->sb_tc_pool_bind_set(devlink_port, sb_index,
@@ -2651,7 +2650,7 @@ static int devlink_nl_cmd_sb_occ_snapshot_doit(struct sk_buff *skb,
struct genl_info *info)
{
struct devlink *devlink = info->user_ptr[0];
- const struct devlink_ops *ops = devlink->ops;
+ struct devlink_ops *ops = devlink->ops;
struct devlink_sb *devlink_sb;
devlink_sb = devlink_sb_get_from_info(devlink, info);
@@ -2667,7 +2666,7 @@ static int devlink_nl_cmd_sb_occ_max_clear_doit(struct sk_buff *skb,
struct genl_info *info)
{
struct devlink *devlink = info->user_ptr[0];
- const struct devlink_ops *ops = devlink->ops;
+ struct devlink_ops *ops = devlink->ops;
struct devlink_sb *devlink_sb;
devlink_sb = devlink_sb_get_from_info(devlink, info);
@@ -2683,8 +2682,8 @@ static int devlink_nl_eswitch_fill(struct sk_buff *msg, struct devlink *devlink,
enum devlink_command cmd, u32 portid,
u32 seq, int flags)
{
- const struct devlink_ops *ops = devlink->ops;
enum devlink_eswitch_encap_mode encap_mode;
+ struct devlink_ops *ops = devlink->ops;
u8 inline_mode;
void *hdr;
int err = 0;
@@ -2777,8 +2776,8 @@ static int devlink_nl_cmd_eswitch_set_doit(struct sk_buff *skb,
struct genl_info *info)
{
struct devlink *devlink = info->user_ptr[0];
- const struct devlink_ops *ops = devlink->ops;
enum devlink_eswitch_encap_mode encap_mode;
+ struct devlink_ops *ops = devlink->ops;
u8 inline_mode;
int err = 0;
u16 mode;
@@ -3879,7 +3878,7 @@ static void devlink_ns_change_notify(struct devlink *devlink,
devlink_notify(devlink, DEVLINK_CMD_DEL);
}
-static bool devlink_reload_supported(const struct devlink_ops *ops)
+static bool devlink_reload_supported(struct devlink_ops *ops)
{
return ops->reload_down && ops->reload_up;
}
@@ -8878,7 +8877,7 @@ static struct genl_family devlink_nl_family __ro_after_init = {
.n_mcgrps = ARRAY_SIZE(devlink_nl_mcgrps),
};
-static bool devlink_reload_actions_valid(const struct devlink_ops *ops)
+static bool devlink_reload_actions_valid(struct devlink_ops *ops)
{
const struct devlink_reload_combination *comb;
int i;
@@ -8919,9 +8918,8 @@ static bool devlink_reload_actions_valid(const struct devlink_ops *ops)
* Allocate new devlink instance resources, including devlink index
* and name.
*/
-struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
- size_t priv_size, struct net *net,
- struct device *dev)
+struct devlink *devlink_alloc_ns(struct devlink_ops *ops, size_t priv_size,
+ struct net *net, struct device *dev)
{
struct devlink *devlink;
static u32 last_id;
@@ -9526,7 +9524,7 @@ EXPORT_SYMBOL_GPL(devlink_rate_leaf_destroy);
void devlink_rate_nodes_destroy(struct devlink *devlink)
{
static struct devlink_rate *devlink_rate, *tmp;
- const struct devlink_ops *ops = devlink->ops;
+ struct devlink_ops *ops = devlink->ops;
mutex_lock(&devlink->lock);
list_for_each_entry(devlink_rate, &devlink->rate_list, list) {
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 8ca6a1170c9d..c07e4be3ed55 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -784,7 +784,7 @@ dsa_devlink_sb_occ_tc_port_bind_get(struct devlink_port *dlp,
p_max);
}
-static const struct devlink_ops dsa_devlink_ops = {
+static struct devlink_ops dsa_devlink_ops = {
.info_get = dsa_devlink_info_get,
.sb_pool_get = dsa_devlink_sb_pool_get,
.sb_pool_set = dsa_devlink_sb_pool_set,
--
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 03:00:44PM +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, op, name) \
> + do { \
> + if ((op)->name) \
> + if (!((ptr)->name)) \
> + (ptr)->name = (op)->name; \
> + } while (0)
> +
> + /* Keep sorted */
> + SET_DEVICE_OP(dev_ops, ops, reload_actions);
> + SET_DEVICE_OP(dev_ops, ops, reload_down);
> + SET_DEVICE_OP(dev_ops, ops, reload_limits);
> + SET_DEVICE_OP(dev_ops, ops, reload_up);
Keep sorted in what order? And why?
thanks,
greg k-h
On Wed, 29 Sep 2021 15:00:41 +0300 Leon Romanovsky wrote:
> 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.
Swapping ops is a nasty hack in my book.
And all that to avoid having two op structures in one driver.
Or to avoid having counters which are always 0?
Sorry, at the very least you need better explanation for this.
On Wed, Sep 29, 2021 at 02:25:00PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 29, 2021 at 03:00:44PM +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, op, name) \
> > + do { \
> > + if ((op)->name) \
> > + if (!((ptr)->name)) \
> > + (ptr)->name = (op)->name; \
> > + } while (0)
> > +
> > + /* Keep sorted */
> > + SET_DEVICE_OP(dev_ops, ops, reload_actions);
> > + SET_DEVICE_OP(dev_ops, ops, reload_down);
> > + SET_DEVICE_OP(dev_ops, ops, reload_limits);
> > + SET_DEVICE_OP(dev_ops, ops, reload_up);
>
> Keep sorted in what order? And why?
Sorted by name.
It simplifies future addition of new commands and removes useless fraction
where place new line.
Thanks
>
> thanks,
>
> greg k-h
On Wed, 29 Sep 2021 13:46:38 +0000 Vladimir Oltean wrote:
> On Wed, Sep 29, 2021 at 06:40:04AM -0700, Jakub Kicinski wrote:
> > Swapping ops is a nasty hack in my book.
> >
> > And all that to avoid having two op structures in one driver.
> > Or to avoid having counters which are always 0?
> >
> > Sorry, at the very least you need better explanation for this.
>
> Leon, while the discussion about this unfolds, can you please repost
> patch 1 separately? :)
Yes, please and thanks! :)
On Wed, Sep 29, 2021 at 06:55:49AM -0700, Jakub Kicinski wrote:
> On Wed, 29 Sep 2021 15:00:45 +0300 Leon Romanovsky wrote:
> > 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);
>
> Does this work? Where do you make a copy of the ops? ???? You can't modify
> the driver-global ops, to state the obvious.
devlink_ops pointer is not constant at this stage, so why can't I copy
reload_* pointers to the "main" devlink ops?
I wanted to avoid to copy all pointers.
Thanks
On Wed, Sep 29, 2021 at 07:26:31AM -0700, Jakub Kicinski wrote:
> On Wed, 29 Sep 2021 17:16:28 +0300 Leon Romanovsky wrote:
> > > > @@ -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);
> > >
> > > Does this work? Where do you make a copy of the ops? ???? You can't modify
> > > the driver-global ops, to state the obvious.
> >
> > devlink_ops pointer is not constant at this stage, so why can't I copy
> > reload_* pointers to the "main" devlink ops?
> >
> > I wanted to avoid to copy all pointers.
>
> Hm. I must be missing a key piece here. IIUC you want to have different
> ops based on some device property. But there is only one
>
> static struct devlink_ops mlx5_devlink_ops;
>
> so how can two devlink instances in the system use that and have
> different ops without a copy?
No, I have two:
* Base ops - mlx5_devlink_ops
* Extra reload commands - mlx5_devlink_reload
Thanks
On Wed, 29 Sep 2021 17:31:04 +0300 Leon Romanovsky wrote:
> On Wed, Sep 29, 2021 at 07:26:31AM -0700, Jakub Kicinski wrote:
> > On Wed, 29 Sep 2021 17:16:28 +0300 Leon Romanovsky wrote:
> > > devlink_ops pointer is not constant at this stage, so why can't I copy
> > > reload_* pointers to the "main" devlink ops?
> > >
> > > I wanted to avoid to copy all pointers.
> >
> > Hm. I must be missing a key piece here. IIUC you want to have different
> > ops based on some device property. But there is only one
> >
> > static struct devlink_ops mlx5_devlink_ops;
> >
> > so how can two devlink instances in the system use that and have
> > different ops without a copy?
>
> No, I have two:
> * Base ops - mlx5_devlink_ops
> * Extra reload commands - mlx5_devlink_reload
Still those are global for the driver, no?
What if you have multiple NICs or whatever.
On Wed, 29 Sep 2021 17:13:28 +0300 Leon Romanovsky wrote:
> On Wed, Sep 29, 2021 at 06:40:04AM -0700, Jakub Kicinski wrote:
> > On Wed, 29 Sep 2021 15:00:41 +0300 Leon Romanovsky wrote:
> > > 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.
> >
> > Swapping ops is a nasty hack in my book.
> >
> > And all that to avoid having two op structures in one driver.
> > Or to avoid having counters which are always 0?
>
> We don't need to advertise counters for feature that is not supported.
> In multiport mlx5 devices, the reload functionality is not supported, so
> this change at least make that device to behave like all other netdev
> devices that don't support devlink reload.
>
> The ops structure is set very early to make sure that internal devlink
> routines will be able access driver back during initialization (btw very
> questionable design choice)
Indeed, is this fixable? Or now that devlink_register() was moved to
the end of probe netdev can call ops before instance is registered?
> and at that stage the driver doesn't know
> yet which device type it is going to drive.
>
> So the answer is:
> 1. Can't have two structures.
I still don't understand why. To be clear - swapping full op structures
is probably acceptable if it's a pure upgrade (existing pointers match).
Poking new ops into a structure (in alphabetical order if I understand
your reply to Greg, not destructor-before-contructor) is what I deem
questionable.
> 2. Same behaviour across all netdev devices.
Unclear what this is referring to.
On Wed, Sep 29, 2021 at 06:40:04AM -0700, Jakub Kicinski wrote:
> On Wed, 29 Sep 2021 15:00:41 +0300 Leon Romanovsky wrote:
> > 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.
>
> Swapping ops is a nasty hack in my book.
>
> And all that to avoid having two op structures in one driver.
> Or to avoid having counters which are always 0?
>
> Sorry, at the very least you need better explanation for this.
Leon, while the discussion about this unfolds, can you please repost
patch 1 separately? :)
Thanks.
On Wed, 29 Sep 2021 15:00:45 +0300 Leon Romanovsky wrote:
> 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);
Does this work? Where do you make a copy of the ops? ???? You can't modify
the driver-global ops, to state the obvious.
On Wed, Sep 29, 2021 at 06:40:04AM -0700, Jakub Kicinski wrote:
> On Wed, 29 Sep 2021 15:00:41 +0300 Leon Romanovsky wrote:
> > 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.
>
> Swapping ops is a nasty hack in my book.
>
> And all that to avoid having two op structures in one driver.
> Or to avoid having counters which are always 0?
We don't need to advertise counters for feature that is not supported.
In multiport mlx5 devices, the reload functionality is not supported, so
this change at least make that device to behave like all other netdev
devices that don't support devlink reload.
The ops structure is set very early to make sure that internal devlink
routines will be able access driver back during initialization (btw very
questionable design choice), and at that stage the driver doesn't know
yet which device type it is going to drive.
So the answer is:
1. Can't have two structures.
2. Same behaviour across all netdev devices.
>
> Sorry, at the very least you need better explanation for this.
Was it better explained now?
On Wed, Sep 29, 2021 at 06:56:21AM -0700, Jakub Kicinski wrote:
> On Wed, 29 Sep 2021 13:46:38 +0000 Vladimir Oltean wrote:
> > On Wed, Sep 29, 2021 at 06:40:04AM -0700, Jakub Kicinski wrote:
> > > Swapping ops is a nasty hack in my book.
> > >
> > > And all that to avoid having two op structures in one driver.
> > > Or to avoid having counters which are always 0?
> > >
> > > Sorry, at the very least you need better explanation for this.
> >
> > Leon, while the discussion about this unfolds, can you please repost
> > patch 1 separately? :)
>
> Yes, please and thanks! :)
Done, thanks
https://lore.kernel.org/netdev/2ed1159291f2a589b013914f2b60d8172fc525c1.1632925030.git.leonro@nvidia.com/T/#u
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 | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 06edb2f1d21e..b64303085d0e 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1072,7 +1072,9 @@ static void devlink_rate_notify(struct devlink_rate *devlink_rate,
int err;
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)
@@ -5155,7 +5157,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 +8984,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 +9002,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 +9017,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 07:35:51AM -0700, Jakub Kicinski wrote:
> On Wed, 29 Sep 2021 17:31:04 +0300 Leon Romanovsky wrote:
> > On Wed, Sep 29, 2021 at 07:26:31AM -0700, Jakub Kicinski wrote:
> > > On Wed, 29 Sep 2021 17:16:28 +0300 Leon Romanovsky wrote:
> > > > devlink_ops pointer is not constant at this stage, so why can't I copy
> > > > reload_* pointers to the "main" devlink ops?
> > > >
> > > > I wanted to avoid to copy all pointers.
> > >
> > > Hm. I must be missing a key piece here. IIUC you want to have different
> > > ops based on some device property. But there is only one
> > >
> > > static struct devlink_ops mlx5_devlink_ops;
> > >
> > > so how can two devlink instances in the system use that and have
> > > different ops without a copy?
> >
> > No, I have two:
> > * Base ops - mlx5_devlink_ops
> > * Extra reload commands - mlx5_devlink_reload
>
> Still those are global for the driver, no?
Ugh, yes
>
> What if you have multiple NICs or whatever.
I missed it and always tested with one device L(.
I'll add copy-all-ops code.
Thanks
On Wed, Sep 29, 2021 at 07:39:40AM -0700, Jakub Kicinski wrote:
> On Wed, 29 Sep 2021 17:13:28 +0300 Leon Romanovsky wrote:
> > On Wed, Sep 29, 2021 at 06:40:04AM -0700, Jakub Kicinski wrote:
> > > On Wed, 29 Sep 2021 15:00:41 +0300 Leon Romanovsky wrote:
> > > > 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.
> > >
> > > Swapping ops is a nasty hack in my book.
> > >
> > > And all that to avoid having two op structures in one driver.
> > > Or to avoid having counters which are always 0?
> >
> > We don't need to advertise counters for feature that is not supported.
> > In multiport mlx5 devices, the reload functionality is not supported, so
> > this change at least make that device to behave like all other netdev
> > devices that don't support devlink reload.
> >
> > The ops structure is set very early to make sure that internal devlink
> > routines will be able access driver back during initialization (btw very
> > questionable design choice)
>
> Indeed, is this fixable? Or now that devlink_register() was moved to
> the end of probe netdev can call ops before instance is registered?
>
> > and at that stage the driver doesn't know
> > yet which device type it is going to drive.
> >
> > So the answer is:
> > 1. Can't have two structures.
>
> I still don't understand why. To be clear - swapping full op structures
> is probably acceptable if it's a pure upgrade (existing pointers match).
> Poking new ops into a structure (in alphabetical order if I understand
> your reply to Greg, not destructor-before-contructor) is what I deem
> questionable.
It is sorted simply for readability and not for any other technical
reason.
Regarding new ops, this is how we are setting callbacks in RDMA based on
actual device support. It works like a charm.
>
> > 2. Same behaviour across all netdev devices.
>
> Unclear what this is referring to.
If your device doesn't support devlink reload, it won't print any
reload counters at all. It is not the case for the multiport mlx5
device. It doesn't support, but still present these counters.
Thanks
On Wed, 29 Sep 2021 17:16:28 +0300 Leon Romanovsky wrote:
> > > @@ -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);
> >
> > Does this work? Where do you make a copy of the ops? ???? You can't modify
> > the driver-global ops, to state the obvious.
>
> devlink_ops pointer is not constant at this stage, so why can't I copy
> reload_* pointers to the "main" devlink ops?
>
> I wanted to avoid to copy all pointers.
Hm. I must be missing a key piece here. IIUC you want to have different
ops based on some device property. But there is only one
static struct devlink_ops mlx5_devlink_ops;
so how can two devlink instances in the system use that and have
different ops without a copy?
On Wed, 29 Sep 2021 18:31:51 +0300 Leon Romanovsky wrote:
> On Wed, Sep 29, 2021 at 07:39:40AM -0700, Jakub Kicinski wrote:
> > On Wed, 29 Sep 2021 17:13:28 +0300 Leon Romanovsky wrote:
> > > We don't need to advertise counters for feature that is not supported.
> > > In multiport mlx5 devices, the reload functionality is not supported, so
> > > this change at least make that device to behave like all other netdev
> > > devices that don't support devlink reload.
> > >
> > > The ops structure is set very early to make sure that internal devlink
> > > routines will be able access driver back during initialization (btw very
> > > questionable design choice)
> >
> > Indeed, is this fixable? Or now that devlink_register() was moved to
> > the end of probe netdev can call ops before instance is registered?
> >
> > > and at that stage the driver doesn't know
> > > yet which device type it is going to drive.
> > >
> > > So the answer is:
> > > 1. Can't have two structures.
> >
> > I still don't understand why. To be clear - swapping full op structures
> > is probably acceptable if it's a pure upgrade (existing pointers match).
> > Poking new ops into a structure (in alphabetical order if I understand
> > your reply to Greg, not destructor-before-contructor) is what I deem
> > questionable.
>
> It is sorted simply for readability and not for any other technical
> reason.
>
> Regarding new ops, this is how we are setting callbacks in RDMA based on
> actual device support. It works like a charm.
>
> > > 2. Same behaviour across all netdev devices.
> >
> > Unclear what this is referring to.
>
> If your device doesn't support devlink reload, it won't print any
> reload counters at all. It is not the case for the multiport mlx5
> device. It doesn't support, but still present these counters.
There's myriad ways you can hide features.
Swapping ops is heavy handed and prone to data races, I don't like it.
On Wed, Sep 29, 2021 at 10:55:37AM -0700, Jakub Kicinski wrote:
> On Wed, 29 Sep 2021 18:31:51 +0300 Leon Romanovsky wrote:
> > On Wed, Sep 29, 2021 at 07:39:40AM -0700, Jakub Kicinski wrote:
> > > On Wed, 29 Sep 2021 17:13:28 +0300 Leon Romanovsky wrote:
> > > > We don't need to advertise counters for feature that is not supported.
> > > > In multiport mlx5 devices, the reload functionality is not supported, so
> > > > this change at least make that device to behave like all other netdev
> > > > devices that don't support devlink reload.
> > > >
> > > > The ops structure is set very early to make sure that internal devlink
> > > > routines will be able access driver back during initialization (btw very
> > > > questionable design choice)
> > >
> > > Indeed, is this fixable? Or now that devlink_register() was moved to
> > > the end of probe netdev can call ops before instance is registered?
> > >
> > > > and at that stage the driver doesn't know
> > > > yet which device type it is going to drive.
> > > >
> > > > So the answer is:
> > > > 1. Can't have two structures.
> > >
> > > I still don't understand why. To be clear - swapping full op structures
> > > is probably acceptable if it's a pure upgrade (existing pointers match).
> > > Poking new ops into a structure (in alphabetical order if I understand
> > > your reply to Greg, not destructor-before-contructor) is what I deem
> > > questionable.
> >
> > It is sorted simply for readability and not for any other technical
> > reason.
> >
> > Regarding new ops, this is how we are setting callbacks in RDMA based on
> > actual device support. It works like a charm.
> >
> > > > 2. Same behaviour across all netdev devices.
> > >
> > > Unclear what this is referring to.
> >
> > If your device doesn't support devlink reload, it won't print any
> > reload counters at all. It is not the case for the multiport mlx5
> > device. It doesn't support, but still present these counters.
>
> There's myriad ways you can hide features.
>
> Swapping ops is heavy handed and prone to data races, I don't like it.
I'm not swapping, but setting only in supported devices.
Anyway, please give me a chance to present improved version of this
mechanism and we will continue from there.
Thanks