On 04.06.2024 11:09, Wojciech Drewek wrote:
> 
> 
> On 04.06.2024 07:56, Hariprasad Kelam wrote:
>>
>>
>>> From: Pawel Kaminski <pawel.kamin...@intel.com>
>>>
>>> Add support for driver-specific devlink local_forwarding param.
>>> Supported values are "enabled", "disabled" and "prioritized".
>>> Default configuration is set to "enabled".
>>>
>>> Add documentation in networking/devlink/ice.rst.
>>>
>>> In previous generations of Intel NICs the transmit scheduler was only 
>>> limited
>>> by PCIe bandwidth when scheduling/assigning hairpin-badwidth between
>>> VFs. Changes to E810 HW design introduced scheduler limitation, so that
>>> available hairpin-bandwidth is bound to external port speed.
>>> In order to address this limitation and enable NFV services such as "service
>>> chaining" a knob to adjust the scheduler config was created.
>>> Driver can send a configuration message to the FW over admin queue and
>>> internal FW logic will reconfigure HW to prioritize and add more BW to VF to
>>> VF traffic. As end result for example 10G port will no longer limit hairpin-
>>> badwith to 10G and much higher speeds can be achieved.
>>>
>>> Devlink local_forwarding param set to "prioritized" enables higher hairpin-
>>> badwitdh on related PFs. Configuration is applicable only to 8x10G and 4x25G
>>> cards.
>>>
>>> Changing local_forwarding configuration will trigger CORER reset in order to
>>> take effect.
>>>
>>> Example command to change current value:
>>> devlink dev param set pci/0000:b2:00.3 name local_forwarding \
>>>         value prioritized \
>>>         cmode runtime
>>>
>>> Co-developed-by: Michal Wilczynski <michal.wilczyn...@intel.com>
>>> Signed-off-by: Michal Wilczynski <michal.wilczyn...@intel.com>
>>> Reviewed-by: Przemek Kitszel <przemyslaw.kits...@intel.com>
>>> Signed-off-by: Pawel Kaminski <pawel.kamin...@intel.com>
>>> Signed-off-by: Wojciech Drewek <wojciech.dre...@intel.com>
>>> ---
>>> v2: Extend documentation
>>> v3: rename loopback to local_forwarding
>>> ---
>>>  Documentation/networking/devlink/ice.rst      |  23 ++++
>>>  .../net/ethernet/intel/ice/devlink/devlink.c  | 126 ++++++++++++++++++
>>>  .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  11 +-
>>>  drivers/net/ethernet/intel/ice/ice_common.c   |   4 +
>>>  drivers/net/ethernet/intel/ice/ice_type.h     |   1 +
>>>  5 files changed, 164 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/networking/devlink/ice.rst
>>> b/Documentation/networking/devlink/ice.rst
>>> index 830c04354222..0eb64bd4710f 100644
>>> --- a/Documentation/networking/devlink/ice.rst
>>> +++ b/Documentation/networking/devlink/ice.rst
>>> @@ -11,6 +11,7 @@ Parameters
>>>  ==========
>>>
>>>  .. list-table:: Generic parameters implemented
>>> +   :widths: 5 5 90
>>>
>>>     * - Name
>>>       - Mode
>>> @@ -68,6 +69,28 @@ Parameters
>>>
>>>         To verify that value has been set:
>>>         $ devlink dev param show pci/0000:16:00.0 name tx_scheduling_layers
>>> +.. list-table:: Driver specific parameters implemented
>>> +    :widths: 5 5 90
>>> +
>>> +    * - Name
>>> +      - Mode
>>> +      - Description
>>> +    * - ``local_forwarding``
>>> +      - runtime
>>> +      - Controls loopback behavior by tuning scheduler bandwidth.
>>> +        Supported values are:
>>> +
>>> +        ``enabled`` - VF to VF traffic is allowed on port
>>> +
>>> +        ``disabled`` - VF to VF traffic is not allowed on this port
>>> +
>>> +        ``prioritized`` - VF to VF traffic is prioritized on this port
>>> +
>>> +        Default value of ``local_forwarding`` parameter is ``enabled``.
>>> +        ``prioritized`` provides ability to adjust VF to VF traffic rate 
>>> to increase
>>> +        one port capacity at cost of the another. User needs to disable
>>> +        local forwarding on one of the ports in order have increased 
>>> capacity
>>> +        on the ``prioritized`` port.
>>>
>>>  Info versions
>>>  =============
>>> diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c
>>> b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>>> index f774781ab514..810a901d7afd 100644
>>> --- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
>>> +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>>> @@ -1381,9 +1381,129 @@ ice_devlink_enable_iw_validate(struct devlink
>>> *devlink, u32 id,
>>>     return 0;
>>>  }
>>>
>>> +#define DEVLINK_LOCAL_FWD_DISABLED_STR "disabled"
>>> +#define DEVLINK_LOCAL_FWD_ENABLED_STR "enabled"
>>> +#define DEVLINK_LOCAL_FWD_PRIORITIZED_STR "prioritized"
>>> +
>>> +/**
>>> + * ice_devlink_local_fwd_mode_to_str - Get string for local_fwd mode.
>>> + * @mode: local forwarding for mode used in port_info struct.
>>> + *
>>> + * Return: Mode respective string or "Invalid".
>>> + */
>>> +static const char *
>>> +ice_devlink_local_fwd_mode_to_str(enum ice_local_fwd_mode mode) {
>>> +   switch (mode) {
>>> +   case ICE_LOCAL_FWD_MODE_ENABLED:
>>> +           return DEVLINK_LOCAL_FWD_ENABLED_STR;
>>> +   case ICE_LOCAL_FWD_MODE_PRIORITIZED:
>>> +           return DEVLINK_LOCAL_FWD_PRIORITIZED_STR;
>>> +   case ICE_LOCAL_FWD_MODE_DISABLED:
>>> +           return DEVLINK_LOCAL_FWD_DISABLED_STR;
>>> +   }
>>> +
>>> +   return "Invalid";
>>> +}
>>> +
>>> +/**
>>> + * ice_devlink_local_fwd_str_to_mode - Get local_fwd mode from string
>>> name.
>>> + * @mode_str: local forwarding mode string.
>>> + *
>>> + * Return: Mode value or negative number if invalid.
>>> + */
>>> +static int ice_devlink_local_fwd_str_to_mode(const char *mode_str) {
>>> +   if (!strcmp(mode_str, DEVLINK_LOCAL_FWD_ENABLED_STR))
>>> +           return ICE_LOCAL_FWD_MODE_ENABLED;
>>> +   else if (!strcmp(mode_str, DEVLINK_LOCAL_FWD_PRIORITIZED_STR))
>>> +           return ICE_LOCAL_FWD_MODE_PRIORITIZED;
>>> +   else if (!strcmp(mode_str, DEVLINK_LOCAL_FWD_DISABLED_STR))
>>> +           return ICE_LOCAL_FWD_MODE_DISABLED;
>>> +
>>> +   return -EINVAL;
>>> +}
>>> +
>>> +/**
>>> + * ice_devlink_local_fwd_get - Get local_fwd parameter.
>>> + * @devlink: Pointer to the devlink instance.
>>> + * @id: The parameter ID to set.
>>> + * @ctx: Context to store the parameter value.
>>> + *
>>> + * Return: Zero.
>>> + */
>>> +static int ice_devlink_local_fwd_get(struct devlink *devlink, u32 id,
>>> +                                struct devlink_param_gset_ctx *ctx) {
>>> +   struct ice_pf *pf = devlink_priv(devlink);
>>> +   struct ice_port_info *pi;
>>> +   const char *mode_str;
>>> +
>>> +   pi = pf->hw.port_info;
>>> +   mode_str = ice_devlink_local_fwd_mode_to_str(pi-
>>>> local_fwd_mode);
>>> +   snprintf(ctx->val.vstr, sizeof(ctx->val.vstr), "%s", mode_str);
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +/**
>>> + * ice_devlink_local_fwd_set - Set local_fwd parameter.
>>> + * @devlink: Pointer to the devlink instance.
>>> + * @id: The parameter ID to set.
>>> + * @ctx: Context to get the parameter value.
>>> + * @extack: Netlink extended ACK structure.
>>> + *
>>> + * Return: Zero.
>>> + */
>>> +static int ice_devlink_local_fwd_set(struct devlink *devlink, u32 id,
>>> +                                struct devlink_param_gset_ctx *ctx,
>>> +                                struct netlink_ext_ack *extack) {
>>> +   int new_local_fwd_mode = ice_devlink_local_fwd_str_to_mode(ctx-
>>>> val.vstr);
>>> +   struct ice_pf *pf = devlink_priv(devlink);
>>> +   struct device *dev = ice_pf_to_dev(pf);
>>> +   struct ice_port_info *pi;
>>> +
>>> +   pi = pf->hw.port_info;
>>> +   if (pi->local_fwd_mode != new_local_fwd_mode) {
>>           This check seems redundant, as devlink calls set API only if there 
>> is change in value.
>>
>> Thanks,
>> Hariprasad k
> 
> Sure, I'll fix that.

I tried to look for this check in devlink API and I can't find it.
Could you show me the exact place where this check is done?

> 
>>> +           pi->local_fwd_mode = new_local_fwd_mode;
>>> +           dev_info(dev, "Setting local_fwd to %s\n", ctx->val.vstr);
>>> +           ice_schedule_reset(pf, ICE_RESET_CORER);
>>> +   }
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +/**
>>> + * ice_devlink_local_fwd_validate - Validate passed local_fwd parameter
>>> value.
>>> + * @devlink: Unused pointer to devlink instance.
>>> + * @id: The parameter ID to validate.
>>> + * @val: Value to validate.
>>> + * @extack: Netlink extended ACK structure.
>>> + *
>>> + * Supported values are:
>>> + * "enabled" - local_fwd is enabled, "disabled" - local_fwd is disabled
>>> + * "prioritized" - local_fwd traffic is prioritized in scheduling.
>>> + *
>>> + * Return: Zero when passed parameter value is supported. Negative
>>> +value on
>>> + * error.
>>> + */
>>> +static int ice_devlink_local_fwd_validate(struct devlink *devlink, u32 id,
>>> +                                     union devlink_param_value val,
>>> +                                     struct netlink_ext_ack *extack)
>>> +{
>>> +   if (ice_devlink_local_fwd_str_to_mode(val.vstr) < 0) {
>>> +           NL_SET_ERR_MSG_MOD(extack, "Error: Requested value is
>>> not supported.");
>>> +           return -EINVAL;
>>> +   }
>>> +
>>> +   return 0;
>>> +}
>>> +
>>>  enum ice_param_id {
>>>     ICE_DEVLINK_PARAM_ID_BASE =
>>> DEVLINK_PARAM_GENERIC_ID_MAX,
>>>     ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS,
>>> +   ICE_DEVLINK_PARAM_ID_LOCAL_FWD,
>>>  };
>>>
>>>  static const struct devlink_param ice_dvl_rdma_params[] = { @@ -1405,6
>>> +1525,12 @@ static const struct devlink_param ice_dvl_sched_params[] = {
>>>                          ice_devlink_tx_sched_layers_get,
>>>                          ice_devlink_tx_sched_layers_set,
>>>                          ice_devlink_tx_sched_layers_validate),
>>> +   DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_LOCAL_FWD,
>>> +                        "local_forwarding",
>>> DEVLINK_PARAM_TYPE_STRING,
>>> +                        BIT(DEVLINK_PARAM_CMODE_RUNTIME),
>>> +                        ice_devlink_local_fwd_get,
>>> +                        ice_devlink_local_fwd_set,
>>> +                        ice_devlink_local_fwd_validate),
>>>  };
>>>
>>>  static void ice_devlink_free(void *devlink_ptr) diff --git
>>> a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
>>> b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
>>> index 621a2ca7093e..9683842f8880 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
>>> +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
>>> @@ -232,6 +232,13 @@ struct ice_aqc_get_sw_cfg_resp_elem {
>>>  #define ICE_AQC_GET_SW_CONF_RESP_IS_VF             BIT(15)
>>>  };
>>>
>>> +/* Loopback port parameter mode values. */ enum ice_local_fwd_mode {
>>> +   ICE_LOCAL_FWD_MODE_ENABLED = 0,
>>> +   ICE_LOCAL_FWD_MODE_DISABLED = 1,
>>> +   ICE_LOCAL_FWD_MODE_PRIORITIZED = 2,
>>> +};
>>> +
>>>  /* Set Port parameters, (direct, 0x0203) */  struct 
>>> ice_aqc_set_port_params {
>>>     __le16 cmd_flags;
>>> @@ -240,7 +247,9 @@ struct ice_aqc_set_port_params {
>>>     __le16 swid;
>>>  #define ICE_AQC_PORT_SWID_VALID                    BIT(15)
>>>  #define ICE_AQC_PORT_SWID_M                        0xFF
>>> -   u8 reserved[10];
>>> +   u8 local_fwd_mode;
>>> +#define ICE_AQC_SET_P_PARAMS_LOCAL_FWD_MODE_VALID BIT(2)
>>> +   u8 reserved[9];
>>>  };
>>>
>>>  /* These resource type defines are used for all switch resource diff --git
>>> a/drivers/net/ethernet/intel/ice/ice_common.c
>>> b/drivers/net/ethernet/intel/ice/ice_common.c
>>> index 9ae61cd8923e..60ad7774812c 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_common.c
>>> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
>>> @@ -1086,6 +1086,7 @@ int ice_init_hw(struct ice_hw *hw)
>>>             goto err_unroll_cqinit;
>>>     }
>>>
>>> +   hw->port_info->local_fwd_mode =
>>> ICE_LOCAL_FWD_MODE_ENABLED;
>>>     /* set the back pointer to HW */
>>>     hw->port_info->hw = hw;
>>>
>>> @@ -3070,6 +3071,9 @@ ice_aq_set_port_params(struct ice_port_info *pi,
>>> bool double_vlan,
>>>             cmd_flags |= ICE_AQC_SET_P_PARAMS_DOUBLE_VLAN_ENA;
>>>     cmd->cmd_flags = cpu_to_le16(cmd_flags);
>>>
>>> +   cmd->local_fwd_mode = pi->local_fwd_mode |
>>> +
>>>     ICE_AQC_SET_P_PARAMS_LOCAL_FWD_MODE_VALID;
>>> +
>>>     return ice_aq_send_cmd(hw, &desc, NULL, 0, cd);  }
>>>
>>> diff --git a/drivers/net/ethernet/intel/ice/ice_type.h
>>> b/drivers/net/ethernet/intel/ice/ice_type.h
>>> index aac59c85a911..f3e4d8030f43 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_type.h
>>> +++ b/drivers/net/ethernet/intel/ice/ice_type.h
>>> @@ -730,6 +730,7 @@ struct ice_port_info {
>>>     u16 sw_id;                      /* Initial switch ID belongs to port */
>>>     u16 pf_vf_num;
>>>     u8 port_state;
>>> +   u8 local_fwd_mode;
>>>  #define ICE_SCHED_PORT_STATE_INIT  0x0
>>>  #define ICE_SCHED_PORT_STATE_READY 0x1
>>>     u8 lport;
>>> --
>>> 2.40.1
>>>
>>

Reply via email to