Hi Tony,
> From: Tony Nguyen <anthony.l.nguyen@xxxxxxxxx>
> Sent: Tuesday, November 23, 2021 2:41 AM
>
> From: Shiraz Saleem <shiraz.saleem@xxxxxxxxx>
>
> Allow support for 'enable_iwarp' and 'enable_roce' devlink params to turn
> on/off iWARP or RoCE protocol support for E800 devices.
>
> For example, a user can turn on iWARP functionality with,
>
> devlink dev param set pci/0000:07:00.0 name enable_iwarp value true cmode
> runtime
>
> This add an iWARP auxiliary rdma device, ice.iwarp.<>, under this PF.
>
> A user request to enable both iWARP and RoCE under the same PF is rejected
> since this device does not support both protocols simultaneously on the same
> port.
>
> Signed-off-by: Shiraz Saleem <shiraz.saleem@xxxxxxxxx>
> Tested-by: Leszek Kaliszczuk <leszek.kaliszczuk@xxxxxxxxx>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@xxxxxxxxx>
> ---
> drivers/net/ethernet/intel/ice/ice.h | 1 +
> drivers/net/ethernet/intel/ice/ice_devlink.c | 144 +++++++++++++++++++
> drivers/net/ethernet/intel/ice/ice_devlink.h | 6 +
> drivers/net/ethernet/intel/ice/ice_idc.c | 4 +-
> drivers/net/ethernet/intel/ice/ice_main.c | 9 +-
> include/linux/net/intel/iidc.h | 7 +-
> 6 files changed, 166 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice.h
> b/drivers/net/ethernet/intel/ice/ice.h
> index b2db39ee5f85..b67ad51cbcc9 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -576,6 +576,7 @@ struct ice_pf {
> struct ice_hw_port_stats stats_prev;
> struct ice_hw hw;
> u8 stat_prev_loaded:1; /* has previous stats been loaded */
> + u8 rdma_mode;
This can be u8 rdma_mode: 1;
See below.
> u16 dcbx_cap;
> u32 tx_timeout_count;
> unsigned long tx_timeout_last_recovery; diff --git
> a/drivers/net/ethernet/intel/ice/ice_devlink.c
> b/drivers/net/ethernet/intel/ice/ice_devlink.c
> index b9bd9f9472f6..478412b28a76 100644
> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
> @@ -430,6 +430,120 @@ static const struct devlink_ops ice_devlink_ops = {
> .flash_update = ice_devlink_flash_update, };
>
> +static int
> +ice_devlink_enable_roce_get(struct devlink *devlink, u32 id,
> + struct devlink_param_gset_ctx *ctx) {
> + struct ice_pf *pf = devlink_priv(devlink);
> +
> + ctx->val.vbool = pf->rdma_mode & IIDC_RDMA_PROTOCOL_ROCEV2;
> +
This is logical operation, and vbool will be still zero when rdma mode is rocev2, because it is not bit 0.
Please see below. This error can be avoided by having rdma mode as Boolean.
> + return 0;
> +}
> +
> +static int
> +ice_devlink_enable_roce_set(struct devlink *devlink, u32 id,
> + struct devlink_param_gset_ctx *ctx) {
> + struct ice_pf *pf = devlink_priv(devlink);
> + bool roce_ena = ctx->val.vbool;
> + int ret;
> +
> + if (!roce_ena) {
> + ice_unplug_aux_dev(pf);
> + pf->rdma_mode &= ~IIDC_RDMA_PROTOCOL_ROCEV2;
> + return 0;
> + }
> +
> + pf->rdma_mode |= IIDC_RDMA_PROTOCOL_ROCEV2;
> + ret = ice_plug_aux_dev(pf);
> + if (ret)
> + pf->rdma_mode &= ~IIDC_RDMA_PROTOCOL_ROCEV2;
> +
> + return ret;
> +}
> +
> +static int
> +ice_devlink_enable_roce_validate(struct devlink *devlink, u32 id,
> + union devlink_param_value val,
> + struct netlink_ext_ack *extack)
> +{
> + struct ice_pf *pf = devlink_priv(devlink);
> +
> + if (!test_bit(ICE_FLAG_RDMA_ENA, pf->flags))
> + return -EOPNOTSUPP;
> +
> + if (pf->rdma_mode & IIDC_RDMA_PROTOCOL_IWARP) {
> + NL_SET_ERR_MSG_MOD(extack, "iWARP is currently enabled.
> This device cannot enable iWARP and RoCEv2 simultaneously");
Since ice driver has this mutually exclusive and whether RDMA is supported or not is already checked by above flag ICE_FLAG_RDMA_ENA,
rdma_mode can be done as Boolean.
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +ice_devlink_enable_iw_get(struct devlink *devlink, u32 id,
> + struct devlink_param_gset_ctx *ctx) {
> + struct ice_pf *pf = devlink_priv(devlink);
> +
> + ctx->val.vbool = pf->rdma_mode & IIDC_RDMA_PROTOCOL_IWARP;
> +
This works fine as this is bit 0, but not for roce. So lets just do boolean rdma_mode.
> + return 0;
> +}
> +
> +static int
> +ice_devlink_enable_iw_set(struct devlink *devlink, u32 id,
> + struct devlink_param_gset_ctx *ctx) {
> + struct ice_pf *pf = devlink_priv(devlink);
> + bool iw_ena = ctx->val.vbool;
> + int ret;
> +
> + if (!iw_ena) {
> + ice_unplug_aux_dev(pf);
> + pf->rdma_mode &= ~IIDC_RDMA_PROTOCOL_IWARP;
> + return 0;
> + }
> +
> + pf->rdma_mode |= IIDC_RDMA_PROTOCOL_IWARP;
> + ret = ice_plug_aux_dev(pf);
> + if (ret)
> + pf->rdma_mode &= ~IIDC_RDMA_PROTOCOL_IWARP;
> +
> + return ret;
> +}
> +
> +static int
> +ice_devlink_enable_iw_validate(struct devlink *devlink, u32 id,
> + union devlink_param_value val,
> + struct netlink_ext_ack *extack) {
> + struct ice_pf *pf = devlink_priv(devlink);
> +
> + if (!test_bit(ICE_FLAG_RDMA_ENA, pf->flags))
> + return -EOPNOTSUPP;
> +
> + if (pf->rdma_mode & IIDC_RDMA_PROTOCOL_ROCEV2) {
> + NL_SET_ERR_MSG_MOD(extack, "RoCEv2 is currently enabled.
> This device cannot enable iWARP and RoCEv2 simultaneously");
> + 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,
> + ice_devlink_enable_roce_set,
> + ice_devlink_enable_roce_validate),
> + DEVLINK_PARAM_GENERIC(ENABLE_IWARP,
> BIT(DEVLINK_PARAM_CMODE_RUNTIME),
> + ice_devlink_enable_iw_get,
> + ice_devlink_enable_iw_set,
> + ice_devlink_enable_iw_validate),
> +
> +};
> +
> static void ice_devlink_free(void *devlink_ptr) {
> devlink_free((struct devlink *)devlink_ptr); @@ -484,6 +598,36 @@
> void ice_devlink_unregister(struct ice_pf *pf)
> devlink_unregister(priv_to_devlink(pf));
> }
>
> +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,
> + ARRAY_SIZE(ice_devlink_params));
> + if (err)
> + return err;
> +
> + value.vbool = false;
> + devlink_param_driverinit_value_set(devlink,
> +
> DEVLINK_PARAM_GENERIC_ID_ENABLE_IWARP,
> + value);
> +
> + value.vbool = test_bit(ICE_FLAG_RDMA_ENA, pf->flags) ? true : false;
> + devlink_param_driverinit_value_set(devlink,
> +
> DEVLINK_PARAM_GENERIC_ID_ENABLE_ROCE,
> + value);
> +
> + return 0;
> +}
> +
> +void ice_devlink_unregister_params(struct ice_pf *pf) {
> + devlink_params_unregister(priv_to_devlink(pf), ice_devlink_params,
> + ARRAY_SIZE(ice_devlink_params));
> +}
> +
> /**
> * ice_devlink_create_pf_port - Create a devlink port for this PF
> * @pf: the PF to create a devlink port for diff --git
> a/drivers/net/ethernet/intel/ice/ice_devlink.h
> b/drivers/net/ethernet/intel/ice/ice_devlink.h
> index b7f9551e4fc4..faea757fcf5d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_devlink.h
> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.h
> @@ -4,10 +4,16 @@
> #ifndef _ICE_DEVLINK_H_
> #define _ICE_DEVLINK_H_
>
> +enum ice_devlink_param_id {
> + ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
> };
> +
This is unused in the patch. Please remove.
