Date: Mon, 3 Jun 2024 14:46:02 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Wojciech Drewek <wojciech.drewek@...el.com>
Cc: netdev@...r.kernel.org, intel-wired-lan@...ts.osuosl.org,
	kuba@...nel.org, jacob.e.keller@...el.com
Subject: Re: [PATCH iwl-next v3] ice: Add support for devlink
 local_forwarding param.

Mon, Jun 03, 2024 at 02:31:46PM CEST, wojciech.drewek@...el.com 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

Does this apply on SFs too?


>+
>+        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) {
>+		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