[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1ee2b836-ffec-4581-b68c-60ca63d50b29@intel.com>
Date: Tue, 4 Jun 2024 11:09:42 +0200
From: Wojciech Drewek <wojciech.drewek@...el.com>
To: Hariprasad Kelam <hkelam@...vell.com>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"kuba@...nel.org" <kuba@...nel.org>, "jacob.e.keller@...el.com"
<jacob.e.keller@...el.com>
Subject: Re: [PATCH iwl-next v3] ice: Add support for devlink local_forwarding
param.
On 04.06.2024 07:56, Hariprasad Kelam wrote:
>
>
>> From: Pawel Kaminski <pawel.kaminski@...el.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.wilczynski@...el.com>
>> Signed-off-by: Michal Wilczynski <michal.wilczynski@...el.com>
>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>> Signed-off-by: Pawel Kaminski <pawel.kaminski@...el.com>
>> Signed-off-by: Wojciech Drewek <wojciech.drewek@...el.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.
>> + 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
>>
>
Powered by blists - more mailing lists