On Tue, Feb 13, 2024 at 10:07:17AM +0100, Jiri Pirko wrote:
> 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()?
>
I mess up a locking here and also in subfunction patchset. I will follow
you suggestion and take lock for whole init/cleanup. Thanks.
> Moreover, could you please fix your init/cleanup paths for hold devlink
> instance lock the whole time?
>
You right, I will prepare patch for it.
>
> 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
> >
> >
