Message ID | 20230127155042.1846608-4-jiri@resnulli.us (mailing list archive) |
---|---|
State | Accepted |
Commit | fb8421a94c5613fee86e192bab0892ecb1d56e4c |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | devlink: fix reload notifications and remove features | expand |
On 1/27/2023 7:50 AM, Jiri Pirko wrote: > From: Jiri Pirko <jiri@nvidia.com> > > Devlink features were introduced to disallow devlink reload calls of > userspace before the devlink was fully initialized. The reason for this > workaround was the fact that devlink reload was originally called > without devlink instance lock held. > > However, with recent changes that converted devlink reload to be > performed under devlink instance lock, this is redundant so remove > devlink features entirely. > > Note that mlx5 used this to enable devlink reload conditionally only > when device didn't act as multi port slave. Move the multi port check > into mlx5_devlink_reload_down() callback alongside with the other > checks preventing the device from reload in certain states. > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > Signed-off-by: Jiri Pirko <jiri@nvidia.com> > --- > .../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 1 - > .../hisilicon/hns3/hns3pf/hclge_devlink.c | 1 - > .../hisilicon/hns3/hns3vf/hclgevf_devlink.c | 1 - > drivers/net/ethernet/intel/ice/ice_devlink.c | 1 - > drivers/net/ethernet/mellanox/mlx4/main.c | 1 - > .../net/ethernet/mellanox/mlx5/core/devlink.c | 9 +++++---- > drivers/net/ethernet/mellanox/mlxsw/core.c | 1 - > drivers/net/netdevsim/dev.c | 1 - > include/net/devlink.h | 2 +- > net/devlink/core.c | 19 ------------------- > net/devlink/devl_internal.h | 1 - > net/devlink/leftover.c | 3 --- > 12 files changed, 6 insertions(+), 35 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c > index 26913dc816d3..8b3e7697390f 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c > @@ -1303,7 +1303,6 @@ int bnxt_dl_register(struct bnxt *bp) > if (rc) > goto err_dl_port_unreg; > > - devlink_set_features(dl, DEVLINK_F_RELOAD); > out: > devlink_register(dl); > return 0; > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c > index 3d3b69605423..9a939c0b217f 100644 > --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c > @@ -114,7 +114,6 @@ int hclge_devlink_init(struct hclge_dev *hdev) > priv->hdev = hdev; > hdev->devlink = devlink; > > - devlink_set_features(devlink, DEVLINK_F_RELOAD); > devlink_register(devlink); > return 0; > } > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c > index a6c3c5e8f0ab..1b535142c65a 100644 > --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c > @@ -116,7 +116,6 @@ int hclgevf_devlink_init(struct hclgevf_dev *hdev) > priv->hdev = hdev; > hdev->devlink = devlink; > > - devlink_set_features(devlink, DEVLINK_F_RELOAD); > devlink_register(devlink); > return 0; > } > diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c > index ce753d23aba9..88497363fc4c 100644 > --- a/drivers/net/ethernet/intel/ice/ice_devlink.c > +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c > @@ -1376,7 +1376,6 @@ void ice_devlink_register(struct ice_pf *pf) > { > struct devlink *devlink = priv_to_devlink(pf); > > - devlink_set_features(devlink, DEVLINK_F_RELOAD); > devlink_register(devlink); > } > > diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c > index 6152f77dcfd8..277738c50c56 100644 > --- a/drivers/net/ethernet/mellanox/mlx4/main.c > +++ b/drivers/net/ethernet/mellanox/mlx4/main.c > @@ -4031,7 +4031,6 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id) > goto err_params_unregister; > > pci_save_state(pdev); > - devlink_set_features(devlink, DEVLINK_F_RELOAD); > devl_unlock(devlink); > devlink_register(devlink); > return 0; > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c > index 95a69544a685..63fb7912b032 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c > @@ -156,6 +156,11 @@ static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change, > return -EOPNOTSUPP; > } > > + if (mlx5_core_is_mp_slave(dev)) { > + NL_SET_ERR_MSG_MOD(extack, "reload is unsupported for multi port slave"); > + return -EOPNOTSUPP; > + } > + > if (pci_num_vf(pdev)) { > NL_SET_ERR_MSG_MOD(extack, "reload while VFs are present is unfavorable"); > } > @@ -744,7 +749,6 @@ void mlx5_devlink_traps_unregister(struct devlink *devlink) > > int mlx5_devlink_params_register(struct devlink *devlink) > { > - struct mlx5_core_dev *dev = devlink_priv(devlink); > int err; > > err = devl_params_register(devlink, mlx5_devlink_params, > @@ -762,9 +766,6 @@ int mlx5_devlink_params_register(struct devlink *devlink) > if (err) > goto max_uc_list_err; > > - if (!mlx5_core_is_mp_slave(dev)) > - devlink_set_features(devlink, DEVLINK_F_RELOAD); > - > return 0; > > max_uc_list_err: > diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c > index f8623e8388c8..42422a106433 100644 > --- a/drivers/net/ethernet/mellanox/mlxsw/core.c > +++ b/drivers/net/ethernet/mellanox/mlxsw/core.c > @@ -2285,7 +2285,6 @@ __mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info, > } > > if (!reload) { > - devlink_set_features(devlink, DEVLINK_F_RELOAD); > devl_unlock(devlink); > devlink_register(devlink); > } > diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c > index f88095b0f836..6045bece2654 100644 > --- a/drivers/net/netdevsim/dev.c > +++ b/drivers/net/netdevsim/dev.c > @@ -1609,7 +1609,6 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev) > goto err_hwstats_exit; > > nsim_dev->esw_mode = DEVLINK_ESWITCH_MODE_LEGACY; > - devlink_set_features(devlink, DEVLINK_F_RELOAD); > devl_unlock(devlink); > return 0; > > diff --git a/include/net/devlink.h b/include/net/devlink.h > index ab654cf552b8..2e85a5970a32 100644 > --- a/include/net/devlink.h > +++ b/include/net/devlink.h > @@ -1645,7 +1645,7 @@ static inline struct devlink *devlink_alloc(const struct devlink_ops *ops, > { > return devlink_alloc_ns(ops, priv_size, &init_net, dev); > } > -void devlink_set_features(struct devlink *devlink, u64 features); > + > int devl_register(struct devlink *devlink); > void devl_unregister(struct devlink *devlink); > void devlink_register(struct devlink *devlink); > diff --git a/net/devlink/core.c b/net/devlink/core.c > index 6c0e2fc57e45..aeffd1b8206d 100644 > --- a/net/devlink/core.c > +++ b/net/devlink/core.c > @@ -125,23 +125,6 @@ struct devlink *devlinks_xa_find_get(struct net *net, unsigned long *indexp) > goto retry; > } > > -/** > - * devlink_set_features - Set devlink supported features > - * > - * @devlink: devlink > - * @features: devlink support features > - * > - * This interface allows us to set reload ops separatelly from > - * the devlink_alloc. > - */ > -void devlink_set_features(struct devlink *devlink, u64 features) > -{ > - WARN_ON(features & DEVLINK_F_RELOAD && > - !devlink_reload_supported(devlink->ops)); > - devlink->features = features; > -} > -EXPORT_SYMBOL_GPL(devlink_set_features); > - > /** > * devl_register - Register devlink instance > * @devlink: devlink > @@ -303,7 +286,6 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net) > * all devlink instances from this namespace into init_net. > */ > devlinks_xa_for_each_registered_get(net, index, devlink) { > - WARN_ON(!(devlink->features & DEVLINK_F_RELOAD)); > devl_lock(devlink); > err = 0; > if (devl_is_registered(devlink)) > @@ -313,7 +295,6 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net) > &actions_performed, NULL); > devl_unlock(devlink); > devlink_put(devlink); > - > if (err && err != -EOPNOTSUPP) > pr_warn("Failed to reload devlink instance into init_net\n"); > } > diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h > index d0d889038138..ba161de4120e 100644 > --- a/net/devlink/devl_internal.h > +++ b/net/devlink/devl_internal.h > @@ -38,7 +38,6 @@ struct devlink { > struct list_head trap_policer_list; > struct list_head linecard_list; > const struct devlink_ops *ops; > - u64 features; > struct xarray snapshot_ids; > struct devlink_dev_stats stats; > struct device *dev; > diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c > index 4f78ef5a46af..92210587d349 100644 > --- a/net/devlink/leftover.c > +++ b/net/devlink/leftover.c > @@ -4387,9 +4387,6 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info) > u32 actions_performed; > int err; > > - if (!(devlink->features & DEVLINK_F_RELOAD)) > - return -EOPNOTSUPP; > - > err = devlink_resources_validate(devlink, NULL, info); > if (err) { > NL_SET_ERR_MSG_MOD(info->extack, "resources size validation failed");
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c index 26913dc816d3..8b3e7697390f 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c @@ -1303,7 +1303,6 @@ int bnxt_dl_register(struct bnxt *bp) if (rc) goto err_dl_port_unreg; - devlink_set_features(dl, DEVLINK_F_RELOAD); out: devlink_register(dl); return 0; diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c index 3d3b69605423..9a939c0b217f 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c @@ -114,7 +114,6 @@ int hclge_devlink_init(struct hclge_dev *hdev) priv->hdev = hdev; hdev->devlink = devlink; - devlink_set_features(devlink, DEVLINK_F_RELOAD); devlink_register(devlink); return 0; } diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c index a6c3c5e8f0ab..1b535142c65a 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c @@ -116,7 +116,6 @@ int hclgevf_devlink_init(struct hclgevf_dev *hdev) priv->hdev = hdev; hdev->devlink = devlink; - devlink_set_features(devlink, DEVLINK_F_RELOAD); devlink_register(devlink); return 0; } diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c index ce753d23aba9..88497363fc4c 100644 --- a/drivers/net/ethernet/intel/ice/ice_devlink.c +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c @@ -1376,7 +1376,6 @@ void ice_devlink_register(struct ice_pf *pf) { struct devlink *devlink = priv_to_devlink(pf); - devlink_set_features(devlink, DEVLINK_F_RELOAD); devlink_register(devlink); } diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c index 6152f77dcfd8..277738c50c56 100644 --- a/drivers/net/ethernet/mellanox/mlx4/main.c +++ b/drivers/net/ethernet/mellanox/mlx4/main.c @@ -4031,7 +4031,6 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id) goto err_params_unregister; pci_save_state(pdev); - devlink_set_features(devlink, DEVLINK_F_RELOAD); devl_unlock(devlink); devlink_register(devlink); return 0; diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c index 95a69544a685..63fb7912b032 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c @@ -156,6 +156,11 @@ static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change, return -EOPNOTSUPP; } + if (mlx5_core_is_mp_slave(dev)) { + NL_SET_ERR_MSG_MOD(extack, "reload is unsupported for multi port slave"); + return -EOPNOTSUPP; + } + if (pci_num_vf(pdev)) { NL_SET_ERR_MSG_MOD(extack, "reload while VFs are present is unfavorable"); } @@ -744,7 +749,6 @@ void mlx5_devlink_traps_unregister(struct devlink *devlink) int mlx5_devlink_params_register(struct devlink *devlink) { - struct mlx5_core_dev *dev = devlink_priv(devlink); int err; err = devl_params_register(devlink, mlx5_devlink_params, @@ -762,9 +766,6 @@ int mlx5_devlink_params_register(struct devlink *devlink) if (err) goto max_uc_list_err; - if (!mlx5_core_is_mp_slave(dev)) - devlink_set_features(devlink, DEVLINK_F_RELOAD); - return 0; max_uc_list_err: diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c index f8623e8388c8..42422a106433 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/core.c +++ b/drivers/net/ethernet/mellanox/mlxsw/core.c @@ -2285,7 +2285,6 @@ __mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info, } if (!reload) { - devlink_set_features(devlink, DEVLINK_F_RELOAD); devl_unlock(devlink); devlink_register(devlink); } diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c index f88095b0f836..6045bece2654 100644 --- a/drivers/net/netdevsim/dev.c +++ b/drivers/net/netdevsim/dev.c @@ -1609,7 +1609,6 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev) goto err_hwstats_exit; nsim_dev->esw_mode = DEVLINK_ESWITCH_MODE_LEGACY; - devlink_set_features(devlink, DEVLINK_F_RELOAD); devl_unlock(devlink); return 0; diff --git a/include/net/devlink.h b/include/net/devlink.h index ab654cf552b8..2e85a5970a32 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -1645,7 +1645,7 @@ static inline struct devlink *devlink_alloc(const struct devlink_ops *ops, { return devlink_alloc_ns(ops, priv_size, &init_net, dev); } -void devlink_set_features(struct devlink *devlink, u64 features); + int devl_register(struct devlink *devlink); void devl_unregister(struct devlink *devlink); void devlink_register(struct devlink *devlink); diff --git a/net/devlink/core.c b/net/devlink/core.c index 6c0e2fc57e45..aeffd1b8206d 100644 --- a/net/devlink/core.c +++ b/net/devlink/core.c @@ -125,23 +125,6 @@ struct devlink *devlinks_xa_find_get(struct net *net, unsigned long *indexp) goto retry; } -/** - * devlink_set_features - Set devlink supported features - * - * @devlink: devlink - * @features: devlink support features - * - * This interface allows us to set reload ops separatelly from - * the devlink_alloc. - */ -void devlink_set_features(struct devlink *devlink, u64 features) -{ - WARN_ON(features & DEVLINK_F_RELOAD && - !devlink_reload_supported(devlink->ops)); - devlink->features = features; -} -EXPORT_SYMBOL_GPL(devlink_set_features); - /** * devl_register - Register devlink instance * @devlink: devlink @@ -303,7 +286,6 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net) * all devlink instances from this namespace into init_net. */ devlinks_xa_for_each_registered_get(net, index, devlink) { - WARN_ON(!(devlink->features & DEVLINK_F_RELOAD)); devl_lock(devlink); err = 0; if (devl_is_registered(devlink)) @@ -313,7 +295,6 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net) &actions_performed, NULL); devl_unlock(devlink); devlink_put(devlink); - if (err && err != -EOPNOTSUPP) pr_warn("Failed to reload devlink instance into init_net\n"); } diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index d0d889038138..ba161de4120e 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -38,7 +38,6 @@ struct devlink { struct list_head trap_policer_list; struct list_head linecard_list; const struct devlink_ops *ops; - u64 features; struct xarray snapshot_ids; struct devlink_dev_stats stats; struct device *dev; diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index 4f78ef5a46af..92210587d349 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -4387,9 +4387,6 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info) u32 actions_performed; int err; - if (!(devlink->features & DEVLINK_F_RELOAD)) - return -EOPNOTSUPP; - err = devlink_resources_validate(devlink, NULL, info); if (err) { NL_SET_ERR_MSG_MOD(info->extack, "resources size validation failed");