Tue, Feb 13, 2024 at 08:35:08AM CET, [email protected] wrote:
>Implement enable_rdma devlink parameter to allow user to turn RDMA
>feature on and off.
>
>It is useful when there is no enough interrupts and user doesn't need
>RDMA feature.
>
>Reviewed-by: Jan Sokolowski <[email protected]>
>Reviewed-by: Przemek Kitszel <[email protected]>
>Signed-off-by: Michal Swiatkowski <[email protected]>
>---
> drivers/net/ethernet/intel/ice/ice_devlink.c | 32 ++++++++++++++++++--
> drivers/net/ethernet/intel/ice/ice_lib.c | 8 ++++-
> drivers/net/ethernet/intel/ice/ice_main.c | 18 +++++------
> 3 files changed, 45 insertions(+), 13 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c
>b/drivers/net/ethernet/intel/ice/ice_devlink.c
>index b82ff9556a4b..4f048268db72 100644
>--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
>+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
>@@ -1675,6 +1675,19 @@ ice_devlink_msix_min_pf_validate(struct devlink
>*devlink, u32 id,
> return 0;
> }
>
>+static int ice_devlink_enable_rdma_validate(struct devlink *devlink, u32 id,
>+ union devlink_param_value val,
>+ struct netlink_ext_ack *extack)
>+{
>+ struct ice_pf *pf = devlink_priv(devlink);
>+ bool new_state = val.vbool;
>+
>+ if (new_state && !test_bit(ICE_FLAG_RDMA_ENA, pf->flags))
>+ return -EOPNOTSUPP;
>+
>+ return 0;
>+}
>+
> static const struct devlink_param ice_devlink_params[] = {
> DEVLINK_PARAM_GENERIC(ENABLE_ROCE, BIT(DEVLINK_PARAM_CMODE_RUNTIME),
> ice_devlink_enable_roce_get,
>@@ -1700,6 +1713,8 @@ static const struct devlink_param ice_devlink_params[] =
>{
> ice_devlink_msix_min_pf_get,
> ice_devlink_msix_min_pf_set,
> ice_devlink_msix_min_pf_validate),
>+ DEVLINK_PARAM_GENERIC(ENABLE_RDMA, BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
>+ NULL, NULL, ice_devlink_enable_rdma_validate),
> };
>
> static void ice_devlink_free(void *devlink_ptr)
>@@ -1776,9 +1791,22 @@ ice_devlink_set_switch_id(struct ice_pf *pf, struct
>netdev_phys_item_id *ppid)
> int ice_devlink_register_params(struct ice_pf *pf)
> {
> struct devlink *devlink = priv_to_devlink(pf);
>+ union devlink_param_value value;
>+ int err;
>+
>+ err = devlink_params_register(devlink, ice_devlink_params,
Interesting, can't you just take the lock before this and call
devl_params_register()?
Moreover, could you please fix your init/cleanup paths for hold devlink
instance lock the whole time?
pw-bot: cr
>+ ARRAY_SIZE(ice_devlink_params));
>+ if (err)
>+ return err;
>
>- return devlink_params_register(devlink, ice_devlink_params,
>- ARRAY_SIZE(ice_devlink_params));
>+ devl_lock(devlink);
>+ value.vbool = test_bit(ICE_FLAG_RDMA_ENA, pf->flags);
>+ devl_param_driverinit_value_set(devlink,
>+ DEVLINK_PARAM_GENERIC_ID_ENABLE_RDMA,
>+ value);
>+ devl_unlock(devlink);
>+
>+ return 0;
> }
>
> void ice_devlink_unregister_params(struct ice_pf *pf)
>diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
>b/drivers/net/ethernet/intel/ice/ice_lib.c
>index a30d2d2b51c1..4d5c3d699044 100644
>--- a/drivers/net/ethernet/intel/ice/ice_lib.c
>+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
>@@ -829,7 +829,13 @@ bool ice_is_safe_mode(struct ice_pf *pf)
> */
> bool ice_is_rdma_ena(struct ice_pf *pf)
> {
>- return test_bit(ICE_FLAG_RDMA_ENA, pf->flags);
>+ union devlink_param_value value;
>+ int err;
>+
>+ err = devl_param_driverinit_value_get(priv_to_devlink(pf),
>+
>DEVLINK_PARAM_GENERIC_ID_ENABLE_RDMA,
>+ &value);
>+ return err ? false : value.vbool;
> }
>
> /**
>diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
>b/drivers/net/ethernet/intel/ice/ice_main.c
>index 824732f16112..4dd59d888ec7 100644
>--- a/drivers/net/ethernet/intel/ice/ice_main.c
>+++ b/drivers/net/ethernet/intel/ice/ice_main.c
>@@ -5177,23 +5177,21 @@ ice_probe(struct pci_dev *pdev, const struct
>pci_device_id __always_unused *ent)
> if (err)
> goto err_init;
>
>+ err = ice_init_devlink(pf);
>+ if (err)
>+ goto err_init_devlink;
>+
> devl_lock(priv_to_devlink(pf));
> err = ice_load(pf);
> devl_unlock(priv_to_devlink(pf));
> if (err)
> goto err_load;
>
>- err = ice_init_devlink(pf);
>- if (err)
>- goto err_init_devlink;
>-
> return 0;
>
>-err_init_devlink:
>- devl_lock(priv_to_devlink(pf));
>- ice_unload(pf);
>- devl_unlock(priv_to_devlink(pf));
> err_load:
>+ ice_deinit_devlink(pf);
>+err_init_devlink:
> ice_deinit(pf);
> err_init:
> pci_disable_device(pdev);
>@@ -5290,12 +5288,12 @@ static void ice_remove(struct pci_dev *pdev)
> if (!ice_is_safe_mode(pf))
> ice_remove_arfs(pf);
>
>- ice_deinit_devlink(pf);
>-
> devl_lock(priv_to_devlink(pf));
> ice_unload(pf);
> devl_unlock(priv_to_devlink(pf));
>
>+ ice_deinit_devlink(pf);
>+
> ice_deinit(pf);
> ice_vsi_release_all(pf);
>
>--
>2.42.0
>
>
