[PATCH v5 0/2] Add link_speed lanes support

[PATCH v5 0/2] Add link_speed lanes support

[Damodharam Ammepalli]

This patch series is a continuation of the patch set that supports configuring speed lanes.
https://patchwork.dpdk.org/project/dpdk/patch/20240708232351.491529-1-damodharam.ammepalli@broadcom.com/

The patchset consists
1) rtelib/testpmd changes (Addressing the comments). Earlier comments are available here, 
https://patchwork.dpdk.org/project/dpdk/list/?q=Add%20link_speed%20lanes%20support&archive=both&series=&submitter=&delegate=&state=*

2) Driver implementation of bnxt PMD.

v2->v3 Consolidating the testpmd and rtelib patches into a single patch as requested.
v3->v4 Addressed comments and fix help string and documentation.
v4->v5 Addressed comments given in v4 and also driver implementation of bnxt PMD.

Damodharam Ammepalli (2):
  ethdev: Add link_speed lanes support
  net/bnxt: code refactor for supporting speed lanes

 app/test-pmd/cmdline.c         | 248 ++++++++++++++++++++++++++++++++-
 app/test-pmd/config.c          |   4 +
 drivers/net/bnxt/bnxt.h        |   3 +
 drivers/net/bnxt/bnxt_ethdev.c | 182 ++++++++++++++++++++++--
 drivers/net/bnxt/bnxt_hwrm.c   |  40 +++++-
 lib/ethdev/ethdev_driver.h     |  91 ++++++++++++
 lib/ethdev/rte_ethdev.c        |  52 +++++++
 lib/ethdev/rte_ethdev.h        |  95 +++++++++++++
 lib/ethdev/version.map         |   5 +
 9 files changed, 700 insertions(+), 20 deletions(-)

-- 
2.43.5


-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

[PATCH v5 1/2] ethdev: Add link_speed lanes support

[Damodharam Ammepalli]

Update the eth_dev_ops structure with new function vectors
to get, get capabilities and set ethernet link speed lanes.
Update the testpmd to provide required config and information
display infrastructure.

The supporting ethernet controller driver will register callbacks
to avail link speed lanes config and get services. This lanes
configuration is applicable only when the nic is forced to fixed
speeds. In Autonegiation mode, the hardware automatically
negotiates the number of lanes.

These are the new commands.

testpmd> show port 0 speed_lanes capabilities

 Supported speeds         Valid lanes
-----------------------------------
 10 Gbps                  1
 25 Gbps                  1
 40 Gbps                  4
 50 Gbps                  1 2
 100 Gbps                 1 2 4
 200 Gbps                 2 4
 400 Gbps                 4 8
testpmd>

testpmd>
testpmd> port stop 0
testpmd> port config 0 speed_lanes 4
testpmd> port config 0 speed 200000 duplex full
testpmd> port start 0
testpmd>
testpmd> show port info 0

********************* Infos for port 0  *********************
MAC address: 14:23:F2:C3:BA:D2
Device name: 0000:b1:00.0
Driver name: net_bnxt
Firmware-version: 228.9.115.0
Connect to socket: 2
memory allocation on the socket: 2
Link status: up
Link speed: 200 Gbps
Active Lanes: 4
Link duplex: full-duplex
Autoneg status: Off

Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
---
 app/test-pmd/cmdline.c     | 248 ++++++++++++++++++++++++++++++++++++-
 app/test-pmd/config.c      |   4 +
 lib/ethdev/ethdev_driver.h |  91 ++++++++++++++
 lib/ethdev/rte_ethdev.c    |  52 ++++++++
 lib/ethdev/rte_ethdev.h    |  95 ++++++++++++++
 lib/ethdev/version.map     |   5 +
 6 files changed, 494 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b7759e38a8..643102032e 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -284,6 +284,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 
 			"dump_log_types\n"
 			"    Dumps the log level for all the dpdk modules\n\n"
+
+			"show port (port_id) speed_lanes capabilities"
+			"	Show speed lanes capabilities of a port.\n\n"
 		);
 	}
 
@@ -823,6 +826,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"port config (port_id) txq (queue_id) affinity (value)\n"
 			"    Map a Tx queue with an aggregated port "
 			"of the DPDK port\n\n"
+
+			"port config (port_id|all) speed_lanes (0|1|4|8)\n"
+			"    Set number of lanes for all ports or port_id for a forced speed\n\n"
 		);
 	}
 
@@ -1560,6 +1566,244 @@ static cmdline_parse_inst_t cmd_config_speed_specific = {
 	},
 };
 
+static int
+parse_speed_lanes_cfg(portid_t pid, uint32_t lanes)
+{
+	int ret;
+
+	ret = rte_eth_speed_lanes_set(pid, lanes);
+	if (ret == -ENOTSUP) {
+		fprintf(stderr, "Function not implemented\n");
+		return -1;
+	} else if (ret < 0) {
+		fprintf(stderr, "Set speed lanes failed\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+static void
+show_speed_lanes_capability(unsigned int num, struct rte_eth_speed_lanes_capa *speed_lanes_capa)
+{
+	unsigned int i;
+	uint32_t capa;
+
+	printf("\n%-15s %-10s", "Supported-speeds", "Valid-lanes");
+	printf("\n-----------------------------------\n");
+	for (i = 0; i < num; i++) {
+		printf("%-17s ",
+		       rte_eth_link_speed_to_str(speed_lanes_capa[i].speed));
+		capa = speed_lanes_capa[i].capa;
+		int s = 0;
+
+		while (capa) {
+			if (capa & 0x1)
+				printf("%-2d ", s);
+			s++;
+			capa = capa >> 1;
+		}
+		printf("\n");
+	}
+}
+
+/* *** display speed lanes per port capabilities *** */
+struct cmd_show_speed_lanes_result {
+	cmdline_fixed_string_t cmd_show;
+	cmdline_fixed_string_t cmd_port;
+	cmdline_fixed_string_t cmd_keyword;
+	portid_t cmd_pid;
+};
+
+static void
+cmd_show_speed_lanes_parsed(void *parsed_result,
+			    __rte_unused struct cmdline *cl,
+			    __rte_unused void *data)
+{
+	struct cmd_show_speed_lanes_result *res = parsed_result;
+	struct rte_eth_speed_lanes_capa *speed_lanes_capa;
+	unsigned int num;
+	int ret;
+
+	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
+		fprintf(stderr, "Invalid port id %u\n", res->cmd_pid);
+		return;
+	}
+
+	ret = rte_eth_speed_lanes_get_capability(res->cmd_pid, NULL, 0);
+	if (ret == -ENOTSUP) {
+		fprintf(stderr, "Function not implemented\n");
+		return;
+	} else if (ret < 0) {
+		fprintf(stderr, "Get speed lanes capability failed: %d\n", ret);
+		return;
+	}
+
+	num = (unsigned int)ret;
+	speed_lanes_capa = calloc(num, sizeof(*speed_lanes_capa));
+	if (speed_lanes_capa == NULL) {
+		fprintf(stderr, "Failed to alloc speed lanes capability buffer\n");
+		return;
+	}
+
+	ret = rte_eth_speed_lanes_get_capability(res->cmd_pid, speed_lanes_capa, num);
+	if (ret < 0) {
+		fprintf(stderr, "Error getting speed lanes capability: %d\n", ret);
+		goto out;
+	}
+
+	show_speed_lanes_capability(num, speed_lanes_capa);
+out:
+	free(speed_lanes_capa);
+}
+
+static cmdline_parse_token_string_t cmd_show_speed_lanes_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_show_speed_lanes_result,
+				 cmd_show, "show");
+static cmdline_parse_token_string_t cmd_show_speed_lanes_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_show_speed_lanes_result,
+				 cmd_port, "port");
+static cmdline_parse_token_num_t cmd_show_speed_lanes_pid =
+	TOKEN_NUM_INITIALIZER(struct cmd_show_speed_lanes_result,
+			      cmd_pid, RTE_UINT16);
+static cmdline_parse_token_string_t cmd_show_speed_lanes_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_show_speed_lanes_result,
+				 cmd_keyword, "speed_lanes");
+static cmdline_parse_token_string_t cmd_show_speed_lanes_cap_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_show_speed_lanes_result,
+				 cmd_keyword, "capabilities");
+
+static cmdline_parse_inst_t cmd_show_speed_lanes = {
+	.f = cmd_show_speed_lanes_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> speed_lanes capabilities",
+	.tokens = {
+		(void *)&cmd_show_speed_lanes_show,
+		(void *)&cmd_show_speed_lanes_port,
+		(void *)&cmd_show_speed_lanes_pid,
+		(void *)&cmd_show_speed_lanes_keyword,
+		(void *)&cmd_show_speed_lanes_cap_keyword,
+		NULL,
+	},
+};
+
+/* *** configure speed_lanes for all ports *** */
+struct cmd_config_speed_lanes_all {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t keyword;
+	cmdline_fixed_string_t all;
+	cmdline_fixed_string_t item;
+	uint32_t lanes;
+};
+
+static void
+cmd_config_speed_lanes_all_parsed(void *parsed_result,
+				  __rte_unused struct cmdline *cl,
+				  __rte_unused void *data)
+{
+	struct cmd_config_speed_lanes_all *res = parsed_result;
+	portid_t pid;
+
+	if (!all_ports_stopped()) {
+		fprintf(stderr, "Please stop all ports first\n");
+		return;
+	}
+
+	RTE_ETH_FOREACH_DEV(pid) {
+		if (parse_speed_lanes_cfg(pid, res->lanes))
+			return;
+	}
+
+	cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
+}
+
+static cmdline_parse_token_string_t cmd_config_speed_lanes_all_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_all, port, "port");
+static cmdline_parse_token_string_t cmd_config_speed_lanes_all_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_all, keyword,
+				 "config");
+static cmdline_parse_token_string_t cmd_config_speed_lanes_all_all =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_all, all, "all");
+static cmdline_parse_token_string_t cmd_config_speed_lanes_all_item =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_all, item,
+				 "speed_lanes");
+static cmdline_parse_token_num_t cmd_config_speed_lanes_all_lanes =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_speed_lanes_all, lanes, RTE_UINT32);
+
+static cmdline_parse_inst_t cmd_config_speed_lanes_all = {
+	.f = cmd_config_speed_lanes_all_parsed,
+	.data = NULL,
+	.help_str = "port config all speed_lanes <value>",
+	.tokens = {
+		(void *)&cmd_config_speed_lanes_all_port,
+		(void *)&cmd_config_speed_lanes_all_keyword,
+		(void *)&cmd_config_speed_lanes_all_all,
+		(void *)&cmd_config_speed_lanes_all_item,
+		(void *)&cmd_config_speed_lanes_all_lanes,
+		NULL,
+	},
+};
+
+/* *** configure speed_lanes for specific port *** */
+struct cmd_config_speed_lanes_specific {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t keyword;
+	uint16_t port_id;
+	cmdline_fixed_string_t item;
+	uint32_t lanes;
+};
+
+static void
+cmd_config_speed_lanes_specific_parsed(void *parsed_result,
+				       __rte_unused struct cmdline *cl,
+				       __rte_unused void *data)
+{
+	struct cmd_config_speed_lanes_specific *res = parsed_result;
+
+	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
+		return;
+
+	if (!port_is_stopped(res->port_id)) {
+		fprintf(stderr, "Please stop port %u first\n", res->port_id);
+		return;
+	}
+
+	if (parse_speed_lanes_cfg(res->port_id, res->lanes))
+		return;
+
+	cmd_reconfig_device_queue(res->port_id, 1, 1);
+}
+
+static cmdline_parse_token_string_t cmd_config_speed_lanes_specific_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_specific, port,
+				 "port");
+static cmdline_parse_token_string_t cmd_config_speed_lanes_specific_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_specific, keyword,
+				 "config");
+static cmdline_parse_token_num_t cmd_config_speed_lanes_specific_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_speed_lanes_specific, port_id,
+			      RTE_UINT16);
+static cmdline_parse_token_string_t cmd_config_speed_lanes_specific_item =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_specific, item,
+				 "speed_lanes");
+static cmdline_parse_token_num_t cmd_config_speed_lanes_specific_lanes =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_speed_lanes_specific, lanes,
+			      RTE_UINT32);
+
+static cmdline_parse_inst_t cmd_config_speed_lanes_specific = {
+	.f = cmd_config_speed_lanes_specific_parsed,
+	.data = NULL,
+	.help_str = "port config <port_id> speed_lanes <value>",
+	.tokens = {
+		(void *)&cmd_config_speed_lanes_specific_port,
+		(void *)&cmd_config_speed_lanes_specific_keyword,
+		(void *)&cmd_config_speed_lanes_specific_id,
+		(void *)&cmd_config_speed_lanes_specific_item,
+		(void *)&cmd_config_speed_lanes_specific_lanes,
+		NULL,
+	},
+};
+
 /* *** configure loopback for all ports *** */
 struct cmd_config_loopback_all {
 	cmdline_fixed_string_t port;
@@ -1645,7 +1889,6 @@ cmd_config_loopback_specific_parsed(void *parsed_result,
 	cmd_reconfig_device_queue(res->port_id, 1, 1);
 }
 
-
 static cmdline_parse_token_string_t cmd_config_loopback_specific_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_config_loopback_specific, port,
 								"port");
@@ -13238,6 +13481,9 @@ static cmdline_parse_ctx_t builtin_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_set_port_setup_on,
 	(cmdline_parse_inst_t *)&cmd_config_speed_all,
 	(cmdline_parse_inst_t *)&cmd_config_speed_specific,
+	(cmdline_parse_inst_t *)&cmd_config_speed_lanes_all,
+	(cmdline_parse_inst_t *)&cmd_config_speed_lanes_specific,
+	(cmdline_parse_inst_t *)&cmd_show_speed_lanes,
 	(cmdline_parse_inst_t *)&cmd_config_loopback_all,
 	(cmdline_parse_inst_t *)&cmd_config_loopback_specific,
 	(cmdline_parse_inst_t *)&cmd_config_rx_tx,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 6f0beafa27..43b3a02732 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -786,6 +786,7 @@ port_infos_display(portid_t port_id)
 	char name[RTE_ETH_NAME_MAX_LEN];
 	int ret;
 	char fw_version[ETHDEV_FWVERS_LEN];
+	uint32_t lanes;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
 		print_valid_ports();
@@ -828,6 +829,8 @@ port_infos_display(portid_t port_id)
 
 	printf("\nLink status: %s\n", (link.link_status) ? ("up") : ("down"));
 	printf("Link speed: %s\n", rte_eth_link_speed_to_str(link.link_speed));
+	if (rte_eth_speed_lanes_get(port_id, &lanes) == 0)
+		printf("Active Lanes: %d\n", lanes);
 	printf("Link duplex: %s\n", (link.link_duplex == RTE_ETH_LINK_FULL_DUPLEX) ?
 	       ("full-duplex") : ("half-duplex"));
 	printf("Autoneg status: %s\n", (link.link_autoneg == RTE_ETH_LINK_AUTONEG) ?
@@ -7244,3 +7247,4 @@ show_mcast_macs(portid_t port_id)
 		printf("  %s\n", buf);
 	}
 }
+
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 883e59a927..abed4784aa 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -339,6 +339,93 @@ typedef int (*eth_allmulticast_disable_t)(struct rte_eth_dev *dev);
 typedef int (*eth_link_update_t)(struct rte_eth_dev *dev,
 				int wait_to_complete);
 
+/**
+ * @internal
+ * Get number of current active lanes
+ *
+ * @param dev
+ *   ethdev handle of port.
+ * @param speed_lanes
+ *   Number of active lanes that the link has trained up. This information
+ *   is displayed for Autonegotiated or Fixed speed trained link.
+ * @return
+ *   Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ *   Success, get speed_lanes data success.
+ * @retval -ENOTSUP
+ *   Operation is not supported.
+ * @retval -EIO
+ *   Device is removed.
+ */
+typedef int (*eth_speed_lanes_get_t)(struct rte_eth_dev *dev, uint32_t *speed_lanes);
+
+/**
+ * @internal
+ * Set speed lanes supported by the NIC. This configuration is applicable only when
+ * fix speed is already configured and or will be configured. This api requires the
+ * port be stopped, since driver has to re-configure PHY with fixed speed and lanes.
+ * If no lanes are configured prior or after "port config X speed Y duplex Z", the
+ * driver will choose the default lane for that speed to bring up the link.
+ *
+ * @param dev
+ *   ethdev handle of port.
+ * @param speed_lanes
+ *   Non-negative number of lanes
+ *
+ * @return
+ *   Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ *   Success, set lanes success.
+ * @retval -ENOTSUP
+ *   Operation is not supported.
+ * @retval -EINVAL
+ *   Unsupported number of lanes for fixed speed requested.
+ * @retval -EIO
+ *   Device is removed.
+ */
+typedef int (*eth_speed_lanes_set_t)(struct rte_eth_dev *dev, uint32_t speed_lanes);
+
+/**
+ * @internal
+ * Get supported link speed lanes capability. The driver returns number of lanes
+ * supported per speed in the form of lanes capability bitmap per speed.
+ *
+ * @param speed_lanes_capa
+ *   A pointer to num of rte_eth_speed_lanes_capa struct array which carries the
+ *   bit map of lanes supported per speed. The number of supported speeds is the
+ *   size of this speed_lanes_capa table. In link up condition, only active supported
+ *   speeds lanes bitmap information will be displayed. In link down condition, all
+ *   the supported speeds and its supported lanes bitmap would be fetched and displayed.
+ *
+ *   This api is overloaded to fetch the size of the speed_lanes_capa array if
+ *   testpmd calls the driver with speed_lanes_capa = NULL and num = 0
+ *
+ * @param num
+ *   Number of elements in a speed_speed_lanes_capa array. This num is equal to the
+ *   number of supported speeds by the controller. This value will vary in link up
+ *   and link down condition. num is updated by the driver if speed_lanes_capa is NULL.
+ *
+ * @return
+ *   Negative errno value on error, positive value on success.
+ *
+ * @retval positive value
+ *   A non-negative value lower or equal to num: success. The return value
+ *   is the number of entries filled in the speed lanes array.
+ *   A non-negative value higher than num: error, the given speed lanes capa array
+ *   is too small. The return value corresponds to the num that should
+ *   be given to succeed. The entries in the speed lanes capa array are not valid
+ *   and shall not be used by the caller.
+ * @retval -ENOTSUP
+ *   Operation is not supported.
+ * @retval -EINVAL
+ *   *num* or *speed_lanes_capa* invalid.
+ */
+typedef int (*eth_speed_lanes_get_capability_t)(struct rte_eth_dev *dev,
+						struct rte_eth_speed_lanes_capa *speed_lanes_capa,
+						unsigned int num);
+
 /** @internal Get global I/O statistics of an Ethernet device. */
 typedef int (*eth_stats_get_t)(struct rte_eth_dev *dev,
 				struct rte_eth_stats *igb_stats);
@@ -1247,6 +1334,10 @@ struct eth_dev_ops {
 	eth_dev_close_t            dev_close;     /**< Close device */
 	eth_dev_reset_t		   dev_reset;	  /**< Reset device */
 	eth_link_update_t          link_update;   /**< Get device link state */
+	eth_speed_lanes_get_t	   speed_lanes_get;	  /**<Get link speed active lanes */
+	eth_speed_lanes_set_t      speed_lanes_set;	  /**<set the link speeds supported lanes */
+	/** Get link speed lanes capability */
+	eth_speed_lanes_get_capability_t speed_lanes_get_capa;
 	/** Check if the device was physically removed */
 	eth_is_removed_t           is_removed;
 
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index f1c658f49e..36427183d5 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1849,6 +1849,58 @@ rte_eth_dev_set_link_down(uint16_t port_id)
 	return ret;
 }
 
+int
+rte_eth_speed_lanes_get(uint16_t port_id, uint32_t *lane)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (*dev->dev_ops->speed_lanes_get == NULL)
+		return -ENOTSUP;
+	return eth_err(port_id, (*dev->dev_ops->speed_lanes_get)(dev, lane));
+}
+
+int
+rte_eth_speed_lanes_get_capability(uint16_t port_id,
+				   struct rte_eth_speed_lanes_capa *speed_lanes_capa,
+				   unsigned int num)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (*dev->dev_ops->speed_lanes_get_capa == NULL)
+		return -ENOTSUP;
+
+	if (speed_lanes_capa == NULL && num > 0) {
+		RTE_ETHDEV_LOG_LINE(ERR,
+				    "Cannot get ethdev port %u speed lanes capability to NULL when array size is non zero",
+				    port_id);
+		return -EINVAL;
+	}
+
+	ret = (*dev->dev_ops->speed_lanes_get_capa)(dev, speed_lanes_capa, num);
+
+	return ret;
+}
+
+int
+rte_eth_speed_lanes_set(uint16_t port_id, uint32_t speed_lanes_capa)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (*dev->dev_ops->speed_lanes_set == NULL)
+		return -ENOTSUP;
+	return eth_err(port_id, (*dev->dev_ops->speed_lanes_set)(dev, speed_lanes_capa));
+}
+
 int
 rte_eth_dev_close(uint16_t port_id)
 {
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 548fada1c7..9444e0a836 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -357,6 +357,27 @@ struct rte_eth_link {
 #define RTE_ETH_LINK_MAX_STR_LEN 40 /**< Max length of default link string. */
 /**@}*/
 
+/**
+ * Constants used to indicate the possible link speed lanes of an ethdev port.
+ */
+#define RTE_ETH_SPEED_LANE_UNKNOWN  0  /**< speed lanes unsupported mode or default */
+#define RTE_ETH_SPEED_LANE_1  1        /**< Link speed lane  1 */
+#define RTE_ETH_SPEED_LANE_2  2        /**< Link speed lanes 2 */
+#define RTE_ETH_SPEED_LANE_4  4        /**< Link speed lanes 4 */
+#define RTE_ETH_SPEED_LANE_8  8        /**< Link speed lanes 8 */
+
+/* Translate from link speed lanes to speed lanes capa */
+#define RTE_ETH_SPEED_LANES_TO_CAPA(x) RTE_BIT32(x)
+
+/* This macro indicates link speed lanes capa mask */
+#define RTE_ETH_SPEED_LANES_CAPA_MASK(x) RTE_BIT32(RTE_ETH_SPEED_ ## x)
+
+/* A structure used to get and set lanes capabilities per link speed */
+struct rte_eth_speed_lanes_capa {
+	uint32_t speed;
+	uint32_t capa;
+};
+
 /**
  * A structure used to configure the ring threshold registers of an Rx/Tx
  * queue for an Ethernet port.
@@ -3114,6 +3135,80 @@ __rte_experimental
 int rte_eth_link_to_str(char *str, size_t len,
 			const struct rte_eth_link *eth_link);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Get Active lanes.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param lanes
+ *   Driver updates lanes with the number of active lanes.
+ *   On a supported NIC on link up, lanes will be a non-zero value irrespective whether the
+ *   link is Autonegotiated or Fixed speed. No information is dispalyed for error.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if underlying hardware OR driver doesn't support.
+ *     that operation.
+ *   - (-EIO) if device is removed.
+ *   - (-ENODEV)  if *port_id* invalid.
+ */
+__rte_experimental
+int rte_eth_speed_lanes_get(uint16_t port_id, uint32_t *lanes);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Set speed lanes supported by the NIC.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param speed_lanes
+ *   A non-zero number of speed lanes, that will be applied to the ethernet PHY
+ *   along with the fixed speed configuration. Driver returns error if the user
+ *   lanes is not in speeds capability list.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if underlying hardware OR driver doesn't support.
+ *     that operation.
+ *   - (-EIO) if device is removed.
+ *   - (-ENODEV)  if *port_id* invalid.
+ *   - (-EINVAL)  if *lanes* count not in speeds capability list.
+ */
+__rte_experimental
+int rte_eth_speed_lanes_set(uint16_t port_id, uint32_t speed_lanes);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Get speed lanes supported by the NIC.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param speed_lanes_capa
+ *   An array of supported speed and its supported lanes.
+ * @param num
+ *   Size of the speed_lanes_capa array. The size is equal to the supported speeds list size.
+ *   Value of num is derived by calling this api with speed_lanes_capa=NULL and num=0
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if underlying hardware OR driver doesn't support.
+ *     that operation.
+ *   - (-EIO) if device is removed.
+ *   - (-ENODEV)  if *port_id* invalid.
+ *   - (-EINVAL)  if *speed_lanes* invalid
+ */
+__rte_experimental
+int rte_eth_speed_lanes_get_capability(uint16_t port_id,
+				       struct rte_eth_speed_lanes_capa *speed_lanes_capa,
+				       unsigned int num);
+
 /**
  * Retrieve the general I/O statistics of an Ethernet device.
  *
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 1669055ca5..b5ffded6f1 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -325,6 +325,11 @@ EXPERIMENTAL {
 	rte_flow_template_table_resizable;
 	rte_flow_template_table_resize;
 	rte_flow_template_table_resize_complete;
+
+	# added in 24.11
+	rte_eth_speed_lanes_get;
+	rte_eth_speed_lanes_get_capability;
+	rte_eth_speed_lanes_set;
 };
 
 INTERNAL {
-- 
2.43.5


-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

[PATCH v5 2/2] net/bnxt: code refactor for supporting speed lanes

[Damodharam Ammepalli]

Broadcom Thor2 NICs support link mode settings where user
can configure fixed speed and associated supported number of
lanes. This patch does code-refactoring to address proposed
poll mode library design updates.

Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        |   3 +
 drivers/net/bnxt/bnxt_ethdev.c | 182 ++++++++++++++++++++++++++++++---
 drivers/net/bnxt/bnxt_hwrm.c   |  40 ++++++--
 3 files changed, 206 insertions(+), 19 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index aaa7ea00cc..667fc84eb2 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -328,6 +328,7 @@ struct bnxt_link_info {
 	uint16_t                cfg_auto_link_speeds2_mask;
 	uint8_t                 active_lanes;
 	uint8_t			option_flags;
+	uint16_t                pmd_speed_lanes;
 };
 
 #define BNXT_COS_QUEUE_COUNT	8
@@ -1219,6 +1220,7 @@ extern int bnxt_logtype_driver;
 #define BNXT_LINK_SPEEDS_V2_VF(bp) (BNXT_VF((bp)) && ((bp)->link_info->option_flags))
 #define BNXT_LINK_SPEEDS_V2(bp) (((bp)->link_info) && (((bp)->link_info->support_speeds_v2) || \
 						       BNXT_LINK_SPEEDS_V2_VF((bp))))
+#define BNXT_MAX_SPEED_LANES 8
 extern const struct rte_flow_ops bnxt_ulp_rte_flow_ops;
 int32_t bnxt_ulp_port_init(struct bnxt *bp);
 void bnxt_ulp_port_deinit(struct bnxt *bp);
@@ -1244,4 +1246,5 @@ int bnxt_flow_meter_ops_get(struct rte_eth_dev *eth_dev, void *arg);
 struct bnxt_vnic_info *bnxt_get_default_vnic(struct bnxt *bp);
 struct tf *bnxt_get_tfp_session(struct bnxt *bp, enum bnxt_session_type type);
 uint64_t bnxt_eth_rss_support(struct bnxt *bp);
+uint16_t bnxt_parse_eth_link_speed_v2(struct bnxt *bp);
 #endif
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index e63febe782..9bd26c5149 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -122,6 +122,30 @@ static const char *const bnxt_dev_args[] = {
 	NULL
 };
 
+#define BNXT_SPEEDS_SUPP_SPEED_LANES (RTE_ETH_LINK_SPEED_10G | \
+				      RTE_ETH_LINK_SPEED_25G | \
+				      RTE_ETH_LINK_SPEED_40G | \
+				      RTE_ETH_LINK_SPEED_50G | \
+				      RTE_ETH_LINK_SPEED_100G | \
+				      RTE_ETH_LINK_SPEED_200G | \
+				      RTE_ETH_LINK_SPEED_400G)
+
+static const struct rte_eth_speed_lanes_capa speed_lanes_capa_tbl[] = {
+	{ RTE_ETH_SPEED_NUM_10G, RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_1) },
+	{ RTE_ETH_SPEED_NUM_25G, RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_1) },
+
+	{ RTE_ETH_SPEED_NUM_40G, RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_4) },
+	{ RTE_ETH_SPEED_NUM_50G, RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_1) |
+		RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_2) },
+	{ RTE_ETH_SPEED_NUM_100G, RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_1) |
+		RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_2) |
+			RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_4) },
+	{ RTE_ETH_SPEED_NUM_200G, RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_2) |
+		RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_4) },
+	{ RTE_ETH_SPEED_NUM_400G, RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_4) |
+		RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_8) },
+};
+
 /*
  * cqe-mode = an non-negative 8-bit number
  */
@@ -696,22 +720,50 @@ static inline bool bnxt_force_link_config(struct bnxt *bp)
 	}
 }
 
-static int bnxt_update_phy_setting(struct bnxt *bp)
+static int bnxt_validate_speed_lanes_change(struct bnxt *bp)
 {
 	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
 	struct rte_eth_link *link = &bp->eth_dev->data->dev_link;
-	struct rte_eth_link new;
 	uint32_t curr_speed_bit;
 	int rc;
 
+	/* Check if speed x lanes combo is supported */
+	if (dev_conf->link_speeds)  {
+		rc = bnxt_parse_eth_link_speed_v2(bp);
+		if (rc == 0)
+			return -EINVAL;
+	}
+
+	/* convert to speedbit flag */
+	curr_speed_bit = rte_eth_speed_bitflag((uint32_t)link->link_speed, 1);
+
+	/* check if speed and lanes have changed */
+	if (dev_conf->link_speeds != curr_speed_bit ||
+	    bp->link_info->active_lanes != bp->link_info->pmd_speed_lanes)
+		return 1;
+
+	return 0;
+}
+
+static int bnxt_update_phy_setting(struct bnxt *bp)
+{
+	struct rte_eth_link new;
+	int rc, rc1 = 0;
+
 	rc = bnxt_get_hwrm_link_config(bp, &new);
 	if (rc) {
 		PMD_DRV_LOG(ERR, "Failed to get link settings\n");
 		return rc;
 	}
 
-	/* convert to speedbit flag */
-	curr_speed_bit = rte_eth_speed_bitflag((uint32_t)link->link_speed, 1);
+	/* Validate speeds2 requirements */
+	if (BNXT_LINK_SPEEDS_V2(bp)) {
+		rc1 = bnxt_validate_speed_lanes_change(bp);
+		if (rc1 == -EINVAL) {
+			PMD_DRV_LOG(ERR, "Failed to set correct lanes\n");
+			return rc1;
+		}
+	}
 
 	/*
 	 * Device is not obliged link down in certain scenarios, even
@@ -719,8 +771,7 @@ static int bnxt_update_phy_setting(struct bnxt *bp)
 	 * to shutdown the port, bnxt_get_hwrm_link_config() call always
 	 * returns link up. Force phy update always in that case.
 	 */
-	if (!new.link_status || bnxt_force_link_config(bp) ||
-	    (BNXT_LINK_SPEEDS_V2(bp) && dev_conf->link_speeds != curr_speed_bit)) {
+	if (!new.link_status || bnxt_force_link_config(bp) || rc1 == 1) {
 		rc = bnxt_set_hwrm_link_config(bp, true);
 		if (rc) {
 			PMD_DRV_LOG(ERR, "Failed to update PHY settings\n");
@@ -1331,16 +1382,17 @@ static int bnxt_dev_configure_op(struct rte_eth_dev *eth_dev)
 void bnxt_print_link_info(struct rte_eth_dev *eth_dev)
 {
 	struct rte_eth_link *link = &eth_dev->data->dev_link;
+	struct bnxt *bp = eth_dev->data->dev_private;
 
 	if (link->link_status)
-		PMD_DRV_LOG(DEBUG, "Port %d Link Up - speed %u Mbps - %s\n",
-			eth_dev->data->port_id,
-			(uint32_t)link->link_speed,
-			(link->link_duplex == RTE_ETH_LINK_FULL_DUPLEX) ?
-			("full-duplex") : ("half-duplex\n"));
+		PMD_DRV_LOG(DEBUG, "Port %d Link Up - speed %u Mbps - %s Lanes - %d\n",
+			    eth_dev->data->port_id,
+			    (uint32_t)link->link_speed,
+			    (link->link_duplex == RTE_ETH_LINK_FULL_DUPLEX) ?
+			    ("full-duplex") : ("half-duplex\n"),
+			    (uint16_t)bp->link_info->active_lanes);
 	else
-		PMD_DRV_LOG(INFO, "Port %d Link Down\n",
-			eth_dev->data->port_id);
+		PMD_DRV_LOG(INFO, "Port %d Link Down\n", eth_dev->data->port_id);
 }
 
 /*
@@ -4191,6 +4243,105 @@ static int bnxt_get_module_eeprom(struct rte_eth_dev *dev,
 	return length ? -EINVAL : 0;
 }
 
+#if (RTE_VERSION_NUM(22, 11, 0, 0) <= RTE_VERSION)
+static int bnxt_speed_lanes_set(struct rte_eth_dev *dev, uint32_t speed_lanes)
+{
+	struct bnxt *bp = dev->data->dev_private;
+
+	if (!BNXT_LINK_SPEEDS_V2(bp))
+		return -ENOTSUP;
+
+	bp->link_info->pmd_speed_lanes = speed_lanes;
+
+	return 0;
+}
+
+static uint32_t
+bnxt_get_speed_lanes_capa(struct rte_eth_speed_lanes_capa *speed_lanes_capa,
+			  uint32_t speed_capa)
+{
+	uint32_t speed_bit;
+	uint32_t num = 0;
+	uint32_t i;
+
+	for (i = 0; i < RTE_DIM(speed_lanes_capa_tbl); i++) {
+		speed_bit =
+			rte_eth_speed_bitflag(speed_lanes_capa_tbl[i].speed,
+					      RTE_ETH_LINK_FULL_DUPLEX);
+		if ((speed_capa & speed_bit) == 0)
+			continue;
+
+		speed_lanes_capa[num].speed = speed_lanes_capa_tbl[i].speed;
+		speed_lanes_capa[num].capa = speed_lanes_capa_tbl[i].capa;
+		num++;
+	}
+
+	return num;
+}
+
+static int bnxt_speed_lanes_get_capa(struct rte_eth_dev *dev,
+				     struct rte_eth_speed_lanes_capa *speed_lanes_capa,
+				     unsigned int num)
+{
+	struct rte_eth_link *link = &dev->data->dev_link;
+	struct bnxt *bp = dev->data->dev_private;
+	unsigned int speed_num;
+	uint32_t speed_capa;
+	int rc;
+
+	rc = is_bnxt_in_error(bp);
+	if (rc)
+		return rc;
+
+	if (!BNXT_LINK_SPEEDS_V2(bp))
+		return -ENOTSUP;
+
+	/* speed_num counts number of speed capabilities.
+	 * When link is down, show the user choice all combinations of speeds x lanes
+	 */
+	if (link->link_status) {
+		speed_capa = bnxt_get_speed_capabilities_v2(bp);
+		speed_num = rte_popcount32(speed_capa & BNXT_SPEEDS_SUPP_SPEED_LANES);
+	} else {
+		speed_capa = BNXT_SPEEDS_SUPP_SPEED_LANES;
+		speed_num = rte_popcount32(BNXT_SPEEDS_SUPP_SPEED_LANES);
+	}
+	if (speed_num == 0)
+		return -ENOTSUP;
+
+	if (speed_lanes_capa == NULL)
+		return speed_num;
+
+	if (num < speed_num)
+		return -EINVAL;
+
+	return bnxt_get_speed_lanes_capa(speed_lanes_capa, speed_capa);
+}
+
+static int bnxt_speed_lanes_get(struct rte_eth_dev *dev, uint32_t *lanes)
+{
+	struct rte_eth_link *link = &dev->data->dev_link;
+	struct bnxt *bp = dev->data->dev_private;
+	int rc;
+
+	rc = is_bnxt_in_error(bp);
+	if (rc)
+		return rc;
+
+	if (!BNXT_LINK_SPEEDS_V2(bp))
+		return -ENOTSUP;
+
+	if (!link->link_status)
+		return -EINVAL;
+
+	 /* user app expects lanes 1 for zero */
+	*lanes = (bp->link_info->active_lanes) ?
+		bp->link_info->active_lanes : 1;
+	return 0;
+}
+
+#endif
+
 /*
  * Initialization
  */
@@ -4262,6 +4413,11 @@ static const struct eth_dev_ops bnxt_dev_ops = {
 	.timesync_read_rx_timestamp = bnxt_timesync_read_rx_timestamp,
 	.timesync_read_tx_timestamp = bnxt_timesync_read_tx_timestamp,
 	.mtr_ops_get = bnxt_flow_meter_ops_get,
+#if (RTE_VERSION_NUM(22, 11, 0, 0) <= RTE_VERSION)
+	.speed_lanes_get = bnxt_speed_lanes_get,
+	.speed_lanes_set = bnxt_speed_lanes_set,
+	.speed_lanes_get_capa = bnxt_speed_lanes_get_capa,
+#endif
 };
 
 static uint32_t bnxt_map_reset_regs(struct bnxt *bp, uint32_t reg)
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index fc142672f6..bb9032b4f8 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -72,6 +72,7 @@ struct link_speeds2_tbl {
 	uint32_t rte_speed_num;
 	uint16_t hwrm_speed;
 	uint16_t sig_mode;
+	uint16_t lanes;
 	const char *desc;
 } link_speeds2_tbl[] = {
 	{
@@ -81,6 +82,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_1G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_1GB,
 		BNXT_SIG_MODE_NRZ,
+		1,
 		"1Gb NRZ",
 	}, {
 		100,
@@ -89,6 +91,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_10G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_10GB,
 		BNXT_SIG_MODE_NRZ,
+		1,
 		"10Gb NRZ",
 	}, {
 		250,
@@ -97,6 +100,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_25G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_25GB,
 		BNXT_SIG_MODE_NRZ,
+		1,
 		"25Gb NRZ",
 	}, {
 		400,
@@ -105,6 +109,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_40G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_40GB,
 		BNXT_SIG_MODE_NRZ,
+		4,
 		"40Gb NRZ",
 	}, {
 		500,
@@ -113,6 +118,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_50G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_50GB,
 		BNXT_SIG_MODE_NRZ,
+		2,
 		"50Gb NRZ",
 	}, {
 		1000,
@@ -121,6 +127,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_100G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_100GB,
 		BNXT_SIG_MODE_NRZ,
+		4,
 		"100Gb NRZ",
 	}, {
 		501,
@@ -129,6 +136,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_50G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_50GB_PAM4_56,
 		BNXT_SIG_MODE_PAM4,
+		1,
 		"50Gb (PAM4-56: 50G per lane)",
 	}, {
 		1001,
@@ -137,6 +145,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_100G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_100GB_PAM4_56,
 		BNXT_SIG_MODE_PAM4,
+		2,
 		"100Gb (PAM4-56: 50G per lane)",
 	}, {
 		2001,
@@ -145,6 +154,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_200G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_200GB_PAM4_56,
 		BNXT_SIG_MODE_PAM4,
+		4,
 		"200Gb (PAM4-56: 50G per lane)",
 	}, {
 		4001,
@@ -153,6 +163,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_400G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_400GB_PAM4_56,
 		BNXT_SIG_MODE_PAM4,
+		8,
 		"400Gb (PAM4-56: 50G per lane)",
 	}, {
 		1002,
@@ -161,6 +172,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_100G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_100GB_PAM4_112,
 		BNXT_SIG_MODE_PAM4_112,
+		1,
 		"100Gb (PAM4-112: 100G per lane)",
 	}, {
 		2002,
@@ -169,6 +181,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_200G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_200GB_PAM4_112,
 		BNXT_SIG_MODE_PAM4_112,
+		2,
 		"200Gb (PAM4-112: 100G per lane)",
 	}, {
 		4002,
@@ -177,6 +190,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_400G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_400GB_PAM4_112,
 		BNXT_SIG_MODE_PAM4_112,
+		4,
 		"400Gb (PAM4-112: 100G per lane)",
 	}, {
 		0,
@@ -185,6 +199,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_NONE,	/* None matches, No speed */
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_1GB, /* Placeholder for wrong HWRM */
 		BNXT_SIG_MODE_NRZ, /* default sig */
+		0,
 		"Unknown",
 	},
 };
@@ -264,17 +279,29 @@ static const char *bnxt_get_xcvr_type(uint32_t xcvr_identifier_type_tx_lpi_timer
 /* Utility function to lookup speeds2 table and
  * return a rte to hwrm speed matching row to the client
  */
-static
-struct link_speeds2_tbl *bnxt_get_rte_hwrm_speeds2_entry(uint32_t speed)
+static struct link_speeds2_tbl *bnxt_get_rte_hwrm_speeds2_entry(struct bnxt *bp)
 {
 	int i, max;
+	uint32_t speed, lanes;
+	bool check_lanes;
+	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
+
+	speed = dev_conf->link_speeds;
+	lanes = bp->link_info->pmd_speed_lanes;
 
 	max = BNXT_SPEEDS2_TBL_SZ - 1;
 	speed &= ~RTE_ETH_LINK_SPEED_FIXED;
+	check_lanes = !(lanes == 0);
+
 	for (i = 0; i < max; i++) {
-		if (speed == link_speeds2_tbl[i].rte_speed)
+		if (speed == link_speeds2_tbl[i].rte_speed &&
+		    (lanes == link_speeds2_tbl[i].lanes || !check_lanes))
 			break;
 	}
+
+	if (!check_lanes)
+		PMD_DRV_LOG(INFO, "Given lanes %d, Configuring default lanes %d %s\n",
+			    lanes, link_speeds2_tbl[i].lanes, link_speeds2_tbl[i].desc);
 	return (struct link_speeds2_tbl *)&link_speeds2_tbl[i];
 }
 
@@ -3579,11 +3606,11 @@ static uint16_t bnxt_check_eth_link_autoneg(uint32_t conf_link)
 	return !conf_link;
 }
 
-static uint16_t bnxt_parse_eth_link_speed_v2(uint32_t conf_link_speed)
+uint16_t bnxt_parse_eth_link_speed_v2(struct bnxt *bp)
 {
 	/* get bitmap value based on speed */
 	return ((struct link_speeds2_tbl *)
-		bnxt_get_rte_hwrm_speeds2_entry(conf_link_speed))->force_val;
+		bnxt_get_rte_hwrm_speeds2_entry(bp))->force_val;
 }
 
 static uint16_t bnxt_parse_eth_link_speed(struct bnxt *bp, uint32_t conf_link_speed,
@@ -3598,7 +3625,7 @@ static uint16_t bnxt_parse_eth_link_speed(struct bnxt *bp, uint32_t conf_link_sp
 
 	/* Handle P7 chips saperately. It got enhanced phy attribs to choose from */
 	if (BNXT_LINK_SPEEDS_V2(bp))
-		return bnxt_parse_eth_link_speed_v2(conf_link_speed);
+		return bnxt_parse_eth_link_speed_v2(bp);
 
 	switch (conf_link_speed & ~RTE_ETH_LINK_SPEED_FIXED) {
 	case RTE_ETH_LINK_SPEED_100M:
@@ -3902,6 +3929,7 @@ static int bnxt_hwrm_port_phy_cfg_v2(struct bnxt *bp, struct bnxt_link_info *con
 	} else {
 		enables |= HWRM_PORT_PHY_CFG_INPUT_ENABLES_FORCE_LINK_SPEEDS2;
 		req.force_link_speeds2 = rte_cpu_to_le_16(conf->link_speed);
+		PMD_DRV_LOG(INFO, "Force speed %d\n", conf->link_speed);
 	}
 
 	/* Fill rest of the req message */
-- 
2.43.5


-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

Re: [PATCH v5 1/2] ethdev: Add link_speed lanes support

[Ferruh Yigit]

On 9/4/2024 6:50 PM, Damodharam Ammepalli wrote:
> Update the eth_dev_ops structure with new function vectors
> to get, get capabilities and set ethernet link speed lanes.
> Update the testpmd to provide required config and information
> display infrastructure.
> 
> The supporting ethernet controller driver will register callbacks
> to avail link speed lanes config and get services. This lanes
> configuration is applicable only when the nic is forced to fixed
> speeds. In Autonegiation mode, the hardware automatically
> negotiates the number of lanes.
> 
> These are the new commands.
> 
> testpmd> show port 0 speed_lanes capabilities
> 
>  Supported speeds         Valid lanes
> -----------------------------------
>  10 Gbps                  1
>  25 Gbps                  1
>  40 Gbps                  4
>  50 Gbps                  1 2
>  100 Gbps                 1 2 4
>  200 Gbps                 2 4
>  400 Gbps                 4 8
> testpmd>
> 
> testpmd>
> testpmd> port stop 0
> testpmd> port config 0 speed_lanes 4
> testpmd> port config 0 speed 200000 duplex full
> testpmd> port start 0
> testpmd>
> testpmd> show port info 0
> 
> ********************* Infos for port 0  *********************
> MAC address: 14:23:F2:C3:BA:D2
> Device name: 0000:b1:00.0
> Driver name: net_bnxt
> Firmware-version: 228.9.115.0
> Connect to socket: 2
> memory allocation on the socket: 2
> Link status: up
> Link speed: 200 Gbps
> Active Lanes: 4
> Link duplex: full-duplex
> Autoneg status: Off
> 
> Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
> ---
>  app/test-pmd/cmdline.c     | 248 ++++++++++++++++++++++++++++++++++++-
>  app/test-pmd/config.c      |   4 +
>  lib/ethdev/ethdev_driver.h |  91 ++++++++++++++
>  lib/ethdev/rte_ethdev.c    |  52 ++++++++
>  lib/ethdev/rte_ethdev.h    |  95 ++++++++++++++
>  lib/ethdev/version.map     |   5 +
>  6 files changed, 494 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index b7759e38a8..643102032e 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -284,6 +284,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>  
>  			"dump_log_types\n"
>  			"    Dumps the log level for all the dpdk modules\n\n"
> +
> +			"show port (port_id) speed_lanes capabilities"
> +			"	Show speed lanes capabilities of a port.\n\n"
>  		);
>  	}
>  
> @@ -823,6 +826,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"port config (port_id) txq (queue_id) affinity (value)\n"
>  			"    Map a Tx queue with an aggregated port "
>  			"of the DPDK port\n\n"
> +
> +			"port config (port_id|all) speed_lanes (0|1|4|8)\n"
>

This help string, and the implementation, implies there has to be fixed
lane values, like 1, 4, 8. Why not leave this part to the capability
reporting, and not limit (and worry) those values in the command, so
from command's perspective it can be only <lane number>.

> +			"    Set number of lanes for all ports or port_id for a forced speed\n\n"
>  		);
>  	}
>  
> @@ -1560,6 +1566,244 @@ static cmdline_parse_inst_t cmd_config_speed_specific = {
>  	},
>  };
>  
> +static int
> +parse_speed_lanes_cfg(portid_t pid, uint32_t lanes)
> +{
> +	int ret;
> +
> +	ret = rte_eth_speed_lanes_set(pid, lanes);
>

As a sample application usage, I think it is better if it gets the
capability and verify that 'lanes' is withing the capability and later
set it, what do you think?

<...>

> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h> index
548fada1c7..9444e0a836 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -357,6 +357,27 @@ struct rte_eth_link {
>  #define RTE_ETH_LINK_MAX_STR_LEN 40 /**< Max length of default link string. */
>  /**@}*/
>  
> +/**
> + * Constants used to indicate the possible link speed lanes of an ethdev port.
> + */
> +#define RTE_ETH_SPEED_LANE_UNKNOWN  0  /**< speed lanes unsupported mode or default */
> +#define RTE_ETH_SPEED_LANE_1  1        /**< Link speed lane  1 */
> +#define RTE_ETH_SPEED_LANE_2  2        /**< Link speed lanes 2 */
> +#define RTE_ETH_SPEED_LANE_4  4        /**< Link speed lanes 4 */
> +#define RTE_ETH_SPEED_LANE_8  8        /**< Link speed lanes 8 */
> +
> +/* Translate from link speed lanes to speed lanes capa */
> +#define RTE_ETH_SPEED_LANES_TO_CAPA(x) RTE_BIT32(x)
> +
> +/* This macro indicates link speed lanes capa mask */
> +#define RTE_ETH_SPEED_LANES_CAPA_MASK(x) RTE_BIT32(RTE_ETH_SPEED_ ## x)
> +
>

I am not clear why we need these macros, why not use lane number as
unsigned integer, instead of macro (RTE_ETH_SPEED_LANE_2), it will be
more flexible.

Probably all we need is 'RTE_ETH_SPEED_LANES_TO_CAPA' one to use as
helper in drivers.
Again not sure about the ..CAPA_MASK one, does it actually produce a
mask value?

Re: [PATCH v5 1/2] ethdev: Add link_speed lanes support

[Damodharam Ammepalli]

On Sun, Sep 22, 2024 at 10:02 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> On 9/4/2024 6:50 PM, Damodharam Ammepalli wrote:
> > Update the eth_dev_ops structure with new function vectors
> > to get, get capabilities and set ethernet link speed lanes.
> > Update the testpmd to provide required config and information
> > display infrastructure.
> >
> > The supporting ethernet controller driver will register callbacks
> > to avail link speed lanes config and get services. This lanes
> > configuration is applicable only when the nic is forced to fixed
> > speeds. In Autonegiation mode, the hardware automatically
> > negotiates the number of lanes.
> >
> > These are the new commands.
> >
> > testpmd> show port 0 speed_lanes capabilities
> >
> >  Supported speeds         Valid lanes
> > -----------------------------------
> >  10 Gbps                  1
> >  25 Gbps                  1
> >  40 Gbps                  4
> >  50 Gbps                  1 2
> >  100 Gbps                 1 2 4
> >  200 Gbps                 2 4
> >  400 Gbps                 4 8
> > testpmd>
> >
> > testpmd>
> > testpmd> port stop 0
> > testpmd> port config 0 speed_lanes 4
> > testpmd> port config 0 speed 200000 duplex full
> > testpmd> port start 0
> > testpmd>
> > testpmd> show port info 0
> >
> > ********************* Infos for port 0  *********************
> > MAC address: 14:23:F2:C3:BA:D2
> > Device name: 0000:b1:00.0
> > Driver name: net_bnxt
> > Firmware-version: 228.9.115.0
> > Connect to socket: 2
> > memory allocation on the socket: 2
> > Link status: up
> > Link speed: 200 Gbps
> > Active Lanes: 4
> > Link duplex: full-duplex
> > Autoneg status: Off
> >
> > Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
> > ---
> >  app/test-pmd/cmdline.c     | 248 ++++++++++++++++++++++++++++++++++++-
> >  app/test-pmd/config.c      |   4 +
> >  lib/ethdev/ethdev_driver.h |  91 ++++++++++++++
> >  lib/ethdev/rte_ethdev.c    |  52 ++++++++
> >  lib/ethdev/rte_ethdev.h    |  95 ++++++++++++++
> >  lib/ethdev/version.map     |   5 +
> >  6 files changed, 494 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> > index b7759e38a8..643102032e 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -284,6 +284,9 @@ static void cmd_help_long_parsed(void *parsed_result,
> >
> >                       "dump_log_types\n"
> >                       "    Dumps the log level for all the dpdk modules\n\n"
> > +
> > +                     "show port (port_id) speed_lanes capabilities"
> > +                     "       Show speed lanes capabilities of a port.\n\n"
> >               );
> >       }
> >
> > @@ -823,6 +826,9 @@ static void cmd_help_long_parsed(void *parsed_result,
> >                       "port config (port_id) txq (queue_id) affinity (value)\n"
> >                       "    Map a Tx queue with an aggregated port "
> >                       "of the DPDK port\n\n"
> > +
> > +                     "port config (port_id|all) speed_lanes (0|1|4|8)\n"
> >
>
> This help string, and the implementation, implies there has to be fixed
> lane values, like 1, 4, 8. Why not leave this part to the capability
> reporting, and not limit (and worry) those values in the command, so
> from command's perspective it can be only <lane number>.
>

ok, will update the help string to <lane number>

> > +                     "    Set number of lanes for all ports or port_id for a forced speed\n\n"
> >               );
> >       }
> >
> > @@ -1560,6 +1566,244 @@ static cmdline_parse_inst_t cmd_config_speed_specific = {
> >       },
> >  };
> >
> > +static int
> > +parse_speed_lanes_cfg(portid_t pid, uint32_t lanes)
> > +{
> > +     int ret;
> > +
> > +     ret = rte_eth_speed_lanes_set(pid, lanes);
> >
>
> As a sample application usage, I think it is better if it gets the
> capability and verify that 'lanes' is withing the capability and later
> set it, what do you think?
>
> <...>

Makes sense, will try out and get back to you soon.


> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h> index
> 548fada1c7..9444e0a836 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -357,6 +357,27 @@ struct rte_eth_link {
> >  #define RTE_ETH_LINK_MAX_STR_LEN 40 /**< Max length of default link string. */
> >  /**@}*/
> >
> > +/**
> > + * Constants used to indicate the possible link speed lanes of an ethdev port.
> > + */
> > +#define RTE_ETH_SPEED_LANE_UNKNOWN  0  /**< speed lanes unsupported mode or default */
> > +#define RTE_ETH_SPEED_LANE_1  1        /**< Link speed lane  1 */
> > +#define RTE_ETH_SPEED_LANE_2  2        /**< Link speed lanes 2 */
> > +#define RTE_ETH_SPEED_LANE_4  4        /**< Link speed lanes 4 */
> > +#define RTE_ETH_SPEED_LANE_8  8        /**< Link speed lanes 8 */
> > +
> > +/* Translate from link speed lanes to speed lanes capa */
> > +#define RTE_ETH_SPEED_LANES_TO_CAPA(x) RTE_BIT32(x)
> > +
> > +/* This macro indicates link speed lanes capa mask */
> > +#define RTE_ETH_SPEED_LANES_CAPA_MASK(x) RTE_BIT32(RTE_ETH_SPEED_ ## x)
> > +
> >
>
> I am not clear why we need these macros, why not use lane number as
> unsigned integer, instead of macro (RTE_ETH_SPEED_LANE_2), it will be
> more flexible.
>

ok, I can replace the macros with unsigned integers

> Probably all we need is 'RTE_ETH_SPEED_LANES_TO_CAPA' one to use as
> helper in drivers.
> Again not sure about the ..CAPA_MASK one, does it actually produce a
> mask value?
>
>
once replacing the LANE_xx to unsigned integers, then I don't need x_CAPA_MASK.
In the driver, I can call RTE_32(lane number) while populating the table.
EG:

        { RTE_ETH_SPEED_NUM_100G, RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_1) |
                                RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_2) |
                                RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_4) },
will become

        { RTE_ETH_SPEED_NUM_100G, RTE_BIT32(1) |
                                RTE_BIT32(2) |
                                RTE_BIT32(4) },

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

Re: [PATCH v5 1/2] ethdev: Add link_speed lanes support

[Damodharam Ammepalli]

[-- Attachment #1: Type: text/plain, Size: 8230 bytes --]

On Tue, Sep 24, 2024 at 3:59 PM Damodharam Ammepalli
<damodharam.ammepalli@broadcom.com> wrote:
>
> On Sun, Sep 22, 2024 at 10:02 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> >
> > On 9/4/2024 6:50 PM, Damodharam Ammepalli wrote:
> > > Update the eth_dev_ops structure with new function vectors
> > > to get, get capabilities and set ethernet link speed lanes.
> > > Update the testpmd to provide required config and information
> > > display infrastructure.
> > >
> > > The supporting ethernet controller driver will register callbacks
> > > to avail link speed lanes config and get services. This lanes
> > > configuration is applicable only when the nic is forced to fixed
> > > speeds. In Autonegiation mode, the hardware automatically
> > > negotiates the number of lanes.
> > >
> > > These are the new commands.
> > >
> > > testpmd> show port 0 speed_lanes capabilities
> > >
> > >  Supported speeds         Valid lanes
> > > -----------------------------------
> > >  10 Gbps                  1
> > >  25 Gbps                  1
> > >  40 Gbps                  4
> > >  50 Gbps                  1 2
> > >  100 Gbps                 1 2 4
> > >  200 Gbps                 2 4
> > >  400 Gbps                 4 8
> > > testpmd>
> > >
> > > testpmd>
> > > testpmd> port stop 0
> > > testpmd> port config 0 speed_lanes 4
> > > testpmd> port config 0 speed 200000 duplex full
> > > testpmd> port start 0
> > > testpmd>
> > > testpmd> show port info 0
> > >
> > > ********************* Infos for port 0  *********************
> > > MAC address: 14:23:F2:C3:BA:D2
> > > Device name: 0000:b1:00.0
> > > Driver name: net_bnxt
> > > Firmware-version: 228.9.115.0
> > > Connect to socket: 2
> > > memory allocation on the socket: 2
> > > Link status: up
> > > Link speed: 200 Gbps
> > > Active Lanes: 4
> > > Link duplex: full-duplex
> > > Autoneg status: Off
> > >
> > > Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
> > > ---
> > >  app/test-pmd/cmdline.c     | 248 ++++++++++++++++++++++++++++++++++++-
> > >  app/test-pmd/config.c      |   4 +
> > >  lib/ethdev/ethdev_driver.h |  91 ++++++++++++++
> > >  lib/ethdev/rte_ethdev.c    |  52 ++++++++
> > >  lib/ethdev/rte_ethdev.h    |  95 ++++++++++++++
> > >  lib/ethdev/version.map     |   5 +
> > >  6 files changed, 494 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> > > index b7759e38a8..643102032e 100644
> > > --- a/app/test-pmd/cmdline.c
> > > +++ b/app/test-pmd/cmdline.c
> > > @@ -284,6 +284,9 @@ static void cmd_help_long_parsed(void *parsed_result,
> > >
> > >                       "dump_log_types\n"
> > >                       "    Dumps the log level for all the dpdk modules\n\n"
> > > +
> > > +                     "show port (port_id) speed_lanes capabilities"
> > > +                     "       Show speed lanes capabilities of a port.\n\n"
> > >               );
> > >       }
> > >
> > > @@ -823,6 +826,9 @@ static void cmd_help_long_parsed(void *parsed_result,
> > >                       "port config (port_id) txq (queue_id) affinity (value)\n"
> > >                       "    Map a Tx queue with an aggregated port "
> > >                       "of the DPDK port\n\n"
> > > +
> > > +                     "port config (port_id|all) speed_lanes (0|1|4|8)\n"
> > >
> >
> > This help string, and the implementation, implies there has to be fixed
> > lane values, like 1, 4, 8. Why not leave this part to the capability
> > reporting, and not limit (and worry) those values in the command, so
> > from command's perspective it can be only <lane number>.
> >
>
> ok, will update the help string to <lane number>
>
> > > +                     "    Set number of lanes for all ports or port_id for a forced speed\n\n"
> > >               );
> > >       }
> > >
> > > @@ -1560,6 +1566,244 @@ static cmdline_parse_inst_t cmd_config_speed_specific = {
> > >       },
> > >  };
> > >
> > > +static int
> > > +parse_speed_lanes_cfg(portid_t pid, uint32_t lanes)
> > > +{
> > > +     int ret;
> > > +
> > > +     ret = rte_eth_speed_lanes_set(pid, lanes);
> > >
> >
> > As a sample application usage, I think it is better if it gets the
> > capability and verify that 'lanes' is withing the capability and later
> > set it, what do you think?
> >
> > <...>
>
> Makes sense, will try out and get back to you soon.
>
I guess a similar comment was discussed in the past, slipped my mind
to mention in my last response.
These are the reasons for this logic.
-  same lane number can be supported by any speed (eg: 4 Lanes
supported in 100,200,400 etc)
so, which speed row should app check against, since the user has not
configured fixed speed yet.
If the link is already up in AN mode, this lane config doesn't have
any significance.
- one approach would be to validate lanes in
parse_and_check_speed_duplex(), but in this case, the app should
already have non-default lanes value. We also need to consider the
case, if the user wants default lanes.
- In the ethtool, the driver does the validation of the speed x lanes
combo. Tried to match similar behaviour.

Let me know your thoughts.

>
> > > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h> index
> > 548fada1c7..9444e0a836 100644
> > > --- a/lib/ethdev/rte_ethdev.h
> > > +++ b/lib/ethdev/rte_ethdev.h
> > > @@ -357,6 +357,27 @@ struct rte_eth_link {
> > >  #define RTE_ETH_LINK_MAX_STR_LEN 40 /**< Max length of default link string. */
> > >  /**@}*/
> > >
> > > +/**
> > > + * Constants used to indicate the possible link speed lanes of an ethdev port.
> > > + */
> > > +#define RTE_ETH_SPEED_LANE_UNKNOWN  0  /**< speed lanes unsupported mode or default */
> > > +#define RTE_ETH_SPEED_LANE_1  1        /**< Link speed lane  1 */
> > > +#define RTE_ETH_SPEED_LANE_2  2        /**< Link speed lanes 2 */
> > > +#define RTE_ETH_SPEED_LANE_4  4        /**< Link speed lanes 4 */
> > > +#define RTE_ETH_SPEED_LANE_8  8        /**< Link speed lanes 8 */
> > > +
> > > +/* Translate from link speed lanes to speed lanes capa */
> > > +#define RTE_ETH_SPEED_LANES_TO_CAPA(x) RTE_BIT32(x)
> > > +
> > > +/* This macro indicates link speed lanes capa mask */
> > > +#define RTE_ETH_SPEED_LANES_CAPA_MASK(x) RTE_BIT32(RTE_ETH_SPEED_ ## x)
> > > +
> > >
> >
> > I am not clear why we need these macros, why not use lane number as
> > unsigned integer, instead of macro (RTE_ETH_SPEED_LANE_2), it will be
> > more flexible.
> >
>
> ok, I can replace the macros with unsigned integers
>
> > Probably all we need is 'RTE_ETH_SPEED_LANES_TO_CAPA' one to use as
> > helper in drivers.
> > Again not sure about the ..CAPA_MASK one, does it actually produce a
> > mask value?
> >
> >
> once replacing the LANE_xx to unsigned integers, then I don't need x_CAPA_MASK.
> In the driver, I can call RTE_32(lane number) while populating the table.
> EG:
>
>         { RTE_ETH_SPEED_NUM_100G, RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_1) |
>                                 RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_2) |
>                                 RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_4) },
> will become
>
>         { RTE_ETH_SPEED_NUM_100G, RTE_BIT32(1) |
>                                 RTE_BIT32(2) |
>                                 RTE_BIT32(4) },

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5457 bytes --]

Re: [PATCH v5 1/2] ethdev: Add link_speed lanes support

[Ferruh Yigit]

On 9/25/2024 4:00 PM, Damodharam Ammepalli wrote:
> On Tue, Sep 24, 2024 at 3:59 PM Damodharam Ammepalli
> <damodharam.ammepalli@broadcom.com> wrote:
>>
>> On Sun, Sep 22, 2024 at 10:02 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>>>
>>> On 9/4/2024 6:50 PM, Damodharam Ammepalli wrote:
>>>> Update the eth_dev_ops structure with new function vectors
>>>> to get, get capabilities and set ethernet link speed lanes.
>>>> Update the testpmd to provide required config and information
>>>> display infrastructure.
>>>>
>>>> The supporting ethernet controller driver will register callbacks
>>>> to avail link speed lanes config and get services. This lanes
>>>> configuration is applicable only when the nic is forced to fixed
>>>> speeds. In Autonegiation mode, the hardware automatically
>>>> negotiates the number of lanes.
>>>>
>>>> These are the new commands.
>>>>
>>>> testpmd> show port 0 speed_lanes capabilities
>>>>
>>>>  Supported speeds         Valid lanes
>>>> -----------------------------------
>>>>  10 Gbps                  1
>>>>  25 Gbps                  1
>>>>  40 Gbps                  4
>>>>  50 Gbps                  1 2
>>>>  100 Gbps                 1 2 4
>>>>  200 Gbps                 2 4
>>>>  400 Gbps                 4 8
>>>> testpmd>
>>>>
>>>> testpmd>
>>>> testpmd> port stop 0
>>>> testpmd> port config 0 speed_lanes 4
>>>> testpmd> port config 0 speed 200000 duplex full
>>>> testpmd> port start 0
>>>> testpmd>
>>>> testpmd> show port info 0
>>>>
>>>> ********************* Infos for port 0  *********************
>>>> MAC address: 14:23:F2:C3:BA:D2
>>>> Device name: 0000:b1:00.0
>>>> Driver name: net_bnxt
>>>> Firmware-version: 228.9.115.0
>>>> Connect to socket: 2
>>>> memory allocation on the socket: 2
>>>> Link status: up
>>>> Link speed: 200 Gbps
>>>> Active Lanes: 4
>>>> Link duplex: full-duplex
>>>> Autoneg status: Off
>>>>
>>>> Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
>>>> ---
>>>>  app/test-pmd/cmdline.c     | 248 ++++++++++++++++++++++++++++++++++++-
>>>>  app/test-pmd/config.c      |   4 +
>>>>  lib/ethdev/ethdev_driver.h |  91 ++++++++++++++
>>>>  lib/ethdev/rte_ethdev.c    |  52 ++++++++
>>>>  lib/ethdev/rte_ethdev.h    |  95 ++++++++++++++
>>>>  lib/ethdev/version.map     |   5 +
>>>>  6 files changed, 494 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>>> index b7759e38a8..643102032e 100644
>>>> --- a/app/test-pmd/cmdline.c
>>>> +++ b/app/test-pmd/cmdline.c
>>>> @@ -284,6 +284,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>>>>
>>>>                       "dump_log_types\n"
>>>>                       "    Dumps the log level for all the dpdk modules\n\n"
>>>> +
>>>> +                     "show port (port_id) speed_lanes capabilities"
>>>> +                     "       Show speed lanes capabilities of a port.\n\n"
>>>>               );
>>>>       }
>>>>
>>>> @@ -823,6 +826,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>>>>                       "port config (port_id) txq (queue_id) affinity (value)\n"
>>>>                       "    Map a Tx queue with an aggregated port "
>>>>                       "of the DPDK port\n\n"
>>>> +
>>>> +                     "port config (port_id|all) speed_lanes (0|1|4|8)\n"
>>>>
>>>
>>> This help string, and the implementation, implies there has to be fixed
>>> lane values, like 1, 4, 8. Why not leave this part to the capability
>>> reporting, and not limit (and worry) those values in the command, so
>>> from command's perspective it can be only <lane number>.
>>>
>>
>> ok, will update the help string to <lane number>
>>
>>>> +                     "    Set number of lanes for all ports or port_id for a forced speed\n\n"
>>>>               );
>>>>       }
>>>>
>>>> @@ -1560,6 +1566,244 @@ static cmdline_parse_inst_t cmd_config_speed_specific = {
>>>>       },
>>>>  };
>>>>
>>>> +static int
>>>> +parse_speed_lanes_cfg(portid_t pid, uint32_t lanes)
>>>> +{
>>>> +     int ret;
>>>> +
>>>> +     ret = rte_eth_speed_lanes_set(pid, lanes);
>>>>
>>>
>>> As a sample application usage, I think it is better if it gets the
>>> capability and verify that 'lanes' is withing the capability and later
>>> set it, what do you think?
>>>
>>> <...>
>>
>> Makes sense, will try out and get back to you soon.
>>
> I guess a similar comment was discussed in the past, slipped my mind
> to mention in my last response.
> These are the reasons for this logic.
> -  same lane number can be supported by any speed (eg: 4 Lanes
> supported in 100,200,400 etc)
> so, which speed row should app check against, since the user has not
> configured fixed speed yet.
> If the link is already up in AN mode, this lane config doesn't have
> any significance.
> - one approach would be to validate lanes in
> parse_and_check_speed_duplex(), but in this case, the app should
> already have non-default lanes value. We also need to consider the
> case, if the user wants default lanes.
> - In the ethtool, the driver does the validation of the speed x lanes
> combo. Tried to match similar behaviour.
> 
> Let me know your thoughts.
> 

Hi Damodharam,

You are right, we can't verify the capability because of missing fixed
link speed. This is the affect of separating speed and lane.
Please scratch the ask, lets continue with the function as it is.

>>
>>>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h> index
>>> 548fada1c7..9444e0a836 100644
>>>> --- a/lib/ethdev/rte_ethdev.h
>>>> +++ b/lib/ethdev/rte_ethdev.h
>>>> @@ -357,6 +357,27 @@ struct rte_eth_link {
>>>>  #define RTE_ETH_LINK_MAX_STR_LEN 40 /**< Max length of default link string. */
>>>>  /**@}*/
>>>>
>>>> +/**
>>>> + * Constants used to indicate the possible link speed lanes of an ethdev port.
>>>> + */
>>>> +#define RTE_ETH_SPEED_LANE_UNKNOWN  0  /**< speed lanes unsupported mode or default */
>>>> +#define RTE_ETH_SPEED_LANE_1  1        /**< Link speed lane  1 */
>>>> +#define RTE_ETH_SPEED_LANE_2  2        /**< Link speed lanes 2 */
>>>> +#define RTE_ETH_SPEED_LANE_4  4        /**< Link speed lanes 4 */
>>>> +#define RTE_ETH_SPEED_LANE_8  8        /**< Link speed lanes 8 */
>>>> +
>>>> +/* Translate from link speed lanes to speed lanes capa */
>>>> +#define RTE_ETH_SPEED_LANES_TO_CAPA(x) RTE_BIT32(x)
>>>> +
>>>> +/* This macro indicates link speed lanes capa mask */
>>>> +#define RTE_ETH_SPEED_LANES_CAPA_MASK(x) RTE_BIT32(RTE_ETH_SPEED_ ## x)
>>>> +
>>>>
>>>
>>> I am not clear why we need these macros, why not use lane number as
>>> unsigned integer, instead of macro (RTE_ETH_SPEED_LANE_2), it will be
>>> more flexible.
>>>
>>
>> ok, I can replace the macros with unsigned integers
>>
>>> Probably all we need is 'RTE_ETH_SPEED_LANES_TO_CAPA' one to use as
>>> helper in drivers.
>>> Again not sure about the ..CAPA_MASK one, does it actually produce a
>>> mask value?
>>>
>>>
>> once replacing the LANE_xx to unsigned integers, then I don't need x_CAPA_MASK.
>> In the driver, I can call RTE_32(lane number) while populating the table.
>> EG:
>>
>>         { RTE_ETH_SPEED_NUM_100G, RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_1) |
>>                                 RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_2) |
>>                                 RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_4) },
>> will become
>>
>>         { RTE_ETH_SPEED_NUM_100G, RTE_BIT32(1) |
>>                                 RTE_BIT32(2) |
>>                                 RTE_BIT32(4) },
> 

Re: [PATCH v5 1/2] ethdev: Add link_speed lanes support

[Ferruh Yigit]

On 9/24/2024 11:59 PM, Damodharam Ammepalli wrote:
> On Sun, Sep 22, 2024 at 10:02 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>>
>> On 9/4/2024 6:50 PM, Damodharam Ammepalli wrote:
>>> Update the eth_dev_ops structure with new function vectors
>>> to get, get capabilities and set ethernet link speed lanes.
>>> Update the testpmd to provide required config and information
>>> display infrastructure.
>>>
>>> The supporting ethernet controller driver will register callbacks
>>> to avail link speed lanes config and get services. This lanes
>>> configuration is applicable only when the nic is forced to fixed
>>> speeds. In Autonegiation mode, the hardware automatically
>>> negotiates the number of lanes.
>>>
>>> These are the new commands.
>>>
>>> testpmd> show port 0 speed_lanes capabilities
>>>
>>>  Supported speeds         Valid lanes
>>> -----------------------------------
>>>  10 Gbps                  1
>>>  25 Gbps                  1
>>>  40 Gbps                  4
>>>  50 Gbps                  1 2
>>>  100 Gbps                 1 2 4
>>>  200 Gbps                 2 4
>>>  400 Gbps                 4 8
>>> testpmd>
>>>
>>> testpmd>
>>> testpmd> port stop 0
>>> testpmd> port config 0 speed_lanes 4
>>> testpmd> port config 0 speed 200000 duplex full
>>> testpmd> port start 0
>>> testpmd>
>>> testpmd> show port info 0
>>>
>>> ********************* Infos for port 0  *********************
>>> MAC address: 14:23:F2:C3:BA:D2
>>> Device name: 0000:b1:00.0
>>> Driver name: net_bnxt
>>> Firmware-version: 228.9.115.0
>>> Connect to socket: 2
>>> memory allocation on the socket: 2
>>> Link status: up
>>> Link speed: 200 Gbps
>>> Active Lanes: 4
>>> Link duplex: full-duplex
>>> Autoneg status: Off
>>>
>>> Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
>>> ---
>>>  app/test-pmd/cmdline.c     | 248 ++++++++++++++++++++++++++++++++++++-
>>>  app/test-pmd/config.c      |   4 +
>>>  lib/ethdev/ethdev_driver.h |  91 ++++++++++++++
>>>  lib/ethdev/rte_ethdev.c    |  52 ++++++++
>>>  lib/ethdev/rte_ethdev.h    |  95 ++++++++++++++
>>>  lib/ethdev/version.map     |   5 +
>>>  6 files changed, 494 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>> index b7759e38a8..643102032e 100644
>>> --- a/app/test-pmd/cmdline.c
>>> +++ b/app/test-pmd/cmdline.c
>>> @@ -284,6 +284,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>>>
>>>                       "dump_log_types\n"
>>>                       "    Dumps the log level for all the dpdk modules\n\n"
>>> +
>>> +                     "show port (port_id) speed_lanes capabilities"
>>> +                     "       Show speed lanes capabilities of a port.\n\n"
>>>               );
>>>       }
>>>
>>> @@ -823,6 +826,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>>>                       "port config (port_id) txq (queue_id) affinity (value)\n"
>>>                       "    Map a Tx queue with an aggregated port "
>>>                       "of the DPDK port\n\n"
>>> +
>>> +                     "port config (port_id|all) speed_lanes (0|1|4|8)\n"
>>>
>>
>> This help string, and the implementation, implies there has to be fixed
>> lane values, like 1, 4, 8. Why not leave this part to the capability
>> reporting, and not limit (and worry) those values in the command, so
>> from command's perspective it can be only <lane number>.
>>
> 
> ok, will update the help string to <lane number>
> 
>>> +                     "    Set number of lanes for all ports or port_id for a forced speed\n\n"
>>>               );
>>>       }
>>>
>>> @@ -1560,6 +1566,244 @@ static cmdline_parse_inst_t cmd_config_speed_specific = {
>>>       },
>>>  };
>>>
>>> +static int
>>> +parse_speed_lanes_cfg(portid_t pid, uint32_t lanes)
>>> +{
>>> +     int ret;
>>> +
>>> +     ret = rte_eth_speed_lanes_set(pid, lanes);
>>>
>>
>> As a sample application usage, I think it is better if it gets the
>> capability and verify that 'lanes' is withing the capability and later
>> set it, what do you think?
>>
>> <...>
> 
> Makes sense, will try out and get back to you soon.
> 
> 
>>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h> index
>> 548fada1c7..9444e0a836 100644
>>> --- a/lib/ethdev/rte_ethdev.h
>>> +++ b/lib/ethdev/rte_ethdev.h
>>> @@ -357,6 +357,27 @@ struct rte_eth_link {
>>>  #define RTE_ETH_LINK_MAX_STR_LEN 40 /**< Max length of default link string. */
>>>  /**@}*/
>>>
>>> +/**
>>> + * Constants used to indicate the possible link speed lanes of an ethdev port.
>>> + */
>>> +#define RTE_ETH_SPEED_LANE_UNKNOWN  0  /**< speed lanes unsupported mode or default */
>>> +#define RTE_ETH_SPEED_LANE_1  1        /**< Link speed lane  1 */
>>> +#define RTE_ETH_SPEED_LANE_2  2        /**< Link speed lanes 2 */
>>> +#define RTE_ETH_SPEED_LANE_4  4        /**< Link speed lanes 4 */
>>> +#define RTE_ETH_SPEED_LANE_8  8        /**< Link speed lanes 8 */
>>> +
>>> +/* Translate from link speed lanes to speed lanes capa */
>>> +#define RTE_ETH_SPEED_LANES_TO_CAPA(x) RTE_BIT32(x)
>>> +
>>> +/* This macro indicates link speed lanes capa mask */
>>> +#define RTE_ETH_SPEED_LANES_CAPA_MASK(x) RTE_BIT32(RTE_ETH_SPEED_ ## x)
>>> +
>>>
>>
>> I am not clear why we need these macros, why not use lane number as
>> unsigned integer, instead of macro (RTE_ETH_SPEED_LANE_2), it will be
>> more flexible.
>>
> 
> ok, I can replace the macros with unsigned integers
> 
>> Probably all we need is 'RTE_ETH_SPEED_LANES_TO_CAPA' one to use as
>> helper in drivers.
>> Again not sure about the ..CAPA_MASK one, does it actually produce a
>> mask value?
>>
>>
> once replacing the LANE_xx to unsigned integers, then I don't need x_CAPA_MASK.
> In the driver, I can call RTE_32(lane number) while populating the table.
> EG:
> 
>         { RTE_ETH_SPEED_NUM_100G, RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_1) |
>                                 RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_2) |
>                                 RTE_ETH_SPEED_LANES_CAPA_MASK(LANE_4) },
> will become
> 
>         { RTE_ETH_SPEED_NUM_100G, RTE_BIT32(1) |
>                                 RTE_BIT32(2) |
>                                 RTE_BIT32(4) },
> 

ack

[PATCH v6 0/2] Add link_speed lanes support

[Damodharam Ammepalli]

This patch series is a continuation of the patch set that supports configuring speed lanes.
https://patchwork.dpdk.org/project/dpdk/patch/20240708232351.491529-1-damodharam.ammepalli@broadcom.com/

The patchset consists
1) rtelib/testpmd changes (Addressing the comments). Earlier comments are available here, 
https://patchwork.dpdk.org/project/dpdk/list/?q=Add%20link_speed%20lanes%20support&archive=both&series=&submitter=&delegate=&state=*

2) Driver implementation of bnxt PMD.

v2->v3 Consolidating the testpmd and rtelib patches into a single patch as requested.
v3->v4 Addressed comments and fix help string and documentation.
v4->v5 Addressed comments given in v4 and also driver implementation of bnxt PMD.
v5->v6 Removed LANE_xx enums and updated driver source files. Minor help
string fix.

Damodharam Ammepalli (2):
  ethdev: Add link_speed lanes support
  net/bnxt: code refactor for supporting speed lanes

 app/test-pmd/cmdline.c         | 248 ++++++++++++++++++++++++++++++++-
 app/test-pmd/config.c          |   4 +
 drivers/net/bnxt/bnxt.h        |   3 +
 drivers/net/bnxt/bnxt_ethdev.c | 176 +++++++++++++++++++++--
 drivers/net/bnxt/bnxt_hwrm.c   |  40 +++++-
 lib/ethdev/ethdev_driver.h     |  91 ++++++++++++
 lib/ethdev/rte_ethdev.c        |  52 +++++++
 lib/ethdev/rte_ethdev.h        |  83 +++++++++++
 lib/ethdev/version.map         |   5 +
 9 files changed, 682 insertions(+), 20 deletions(-)

-- 
2.43.5


-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

[PATCH v6 1/2] ethdev: Add link_speed lanes support

[Damodharam Ammepalli]

Update the eth_dev_ops structure with new function vectors
to get, get capabilities and set ethernet link speed lanes.
Update the testpmd to provide required config and information
display infrastructure.

The supporting ethernet controller driver will register callbacks
to avail link speed lanes config and get services. This lanes
configuration is applicable only when the nic is forced to fixed
speeds. In Autonegiation mode, the hardware automatically
negotiates the number of lanes.

These are the new commands.

testpmd> show port 0 speed_lanes capabilities

 Supported speeds         Valid lanes
-----------------------------------
 10 Gbps                  1
 25 Gbps                  1
 40 Gbps                  4
 50 Gbps                  1 2
 100 Gbps                 1 2 4
 200 Gbps                 2 4
 400 Gbps                 4 8
testpmd>

testpmd>
testpmd> port stop 0
testpmd> port config 0 speed_lanes 4
testpmd> port config 0 speed 200000 duplex full
testpmd> port start 0
testpmd>
testpmd> show port info 0

********************* Infos for port 0  *********************
MAC address: 14:23:F2:C3:BA:D2
Device name: 0000:b1:00.0
Driver name: net_bnxt
Firmware-version: 228.9.115.0
Connect to socket: 2
memory allocation on the socket: 2
Link status: up
Link speed: 200 Gbps
Active Lanes: 4
Link duplex: full-duplex
Autoneg status: Off

Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
---
 app/test-pmd/cmdline.c     | 248 ++++++++++++++++++++++++++++++++++++-
 app/test-pmd/config.c      |   4 +
 lib/ethdev/ethdev_driver.h |  91 ++++++++++++++
 lib/ethdev/rte_ethdev.c    |  52 ++++++++
 lib/ethdev/rte_ethdev.h    |  83 +++++++++++++
 lib/ethdev/version.map     |   5 +
 6 files changed, 482 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b7759e38a8..d616f37039 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -284,6 +284,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 
 			"dump_log_types\n"
 			"    Dumps the log level for all the dpdk modules\n\n"
+
+			"show port (port_id) speed_lanes capabilities"
+			"	Show speed lanes capabilities of a port.\n\n"
 		);
 	}
 
@@ -823,6 +826,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"port config (port_id) txq (queue_id) affinity (value)\n"
 			"    Map a Tx queue with an aggregated port "
 			"of the DPDK port\n\n"
+
+			"port config (port_id|all) speed_lanes (value)\n"
+			"    Set number of lanes for all ports or port_id for a forced speed\n\n"
 		);
 	}
 
@@ -1560,6 +1566,244 @@ static cmdline_parse_inst_t cmd_config_speed_specific = {
 	},
 };
 
+static int
+parse_speed_lanes_cfg(portid_t pid, uint32_t lanes)
+{
+	int ret;
+
+	ret = rte_eth_speed_lanes_set(pid, lanes);
+	if (ret == -ENOTSUP) {
+		fprintf(stderr, "Function not implemented\n");
+		return -1;
+	} else if (ret < 0) {
+		fprintf(stderr, "Set speed lanes failed\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+static void
+show_speed_lanes_capability(unsigned int num, struct rte_eth_speed_lanes_capa *speed_lanes_capa)
+{
+	unsigned int i;
+	uint32_t capa;
+
+	printf("\n%-15s %-10s", "Supported-speeds", "Valid-lanes");
+	printf("\n-----------------------------------\n");
+	for (i = 0; i < num; i++) {
+		printf("%-17s ",
+		       rte_eth_link_speed_to_str(speed_lanes_capa[i].speed));
+		capa = speed_lanes_capa[i].capa;
+		int s = 0;
+
+		while (capa) {
+			if (capa & 0x1)
+				printf("%-2d ", s);
+			s++;
+			capa = capa >> 1;
+		}
+		printf("\n");
+	}
+}
+
+/* *** display speed lanes per port capabilities *** */
+struct cmd_show_speed_lanes_result {
+	cmdline_fixed_string_t cmd_show;
+	cmdline_fixed_string_t cmd_port;
+	cmdline_fixed_string_t cmd_keyword;
+	portid_t cmd_pid;
+};
+
+static void
+cmd_show_speed_lanes_parsed(void *parsed_result,
+			    __rte_unused struct cmdline *cl,
+			    __rte_unused void *data)
+{
+	struct cmd_show_speed_lanes_result *res = parsed_result;
+	struct rte_eth_speed_lanes_capa *speed_lanes_capa;
+	unsigned int num;
+	int ret;
+
+	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
+		fprintf(stderr, "Invalid port id %u\n", res->cmd_pid);
+		return;
+	}
+
+	ret = rte_eth_speed_lanes_get_capability(res->cmd_pid, NULL, 0);
+	if (ret == -ENOTSUP) {
+		fprintf(stderr, "Function not implemented\n");
+		return;
+	} else if (ret < 0) {
+		fprintf(stderr, "Get speed lanes capability failed: %d\n", ret);
+		return;
+	}
+
+	num = (unsigned int)ret;
+	speed_lanes_capa = calloc(num, sizeof(*speed_lanes_capa));
+	if (speed_lanes_capa == NULL) {
+		fprintf(stderr, "Failed to alloc speed lanes capability buffer\n");
+		return;
+	}
+
+	ret = rte_eth_speed_lanes_get_capability(res->cmd_pid, speed_lanes_capa, num);
+	if (ret < 0) {
+		fprintf(stderr, "Error getting speed lanes capability: %d\n", ret);
+		goto out;
+	}
+
+	show_speed_lanes_capability(num, speed_lanes_capa);
+out:
+	free(speed_lanes_capa);
+}
+
+static cmdline_parse_token_string_t cmd_show_speed_lanes_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_show_speed_lanes_result,
+				 cmd_show, "show");
+static cmdline_parse_token_string_t cmd_show_speed_lanes_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_show_speed_lanes_result,
+				 cmd_port, "port");
+static cmdline_parse_token_num_t cmd_show_speed_lanes_pid =
+	TOKEN_NUM_INITIALIZER(struct cmd_show_speed_lanes_result,
+			      cmd_pid, RTE_UINT16);
+static cmdline_parse_token_string_t cmd_show_speed_lanes_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_show_speed_lanes_result,
+				 cmd_keyword, "speed_lanes");
+static cmdline_parse_token_string_t cmd_show_speed_lanes_cap_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_show_speed_lanes_result,
+				 cmd_keyword, "capabilities");
+
+static cmdline_parse_inst_t cmd_show_speed_lanes = {
+	.f = cmd_show_speed_lanes_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> speed_lanes capabilities",
+	.tokens = {
+		(void *)&cmd_show_speed_lanes_show,
+		(void *)&cmd_show_speed_lanes_port,
+		(void *)&cmd_show_speed_lanes_pid,
+		(void *)&cmd_show_speed_lanes_keyword,
+		(void *)&cmd_show_speed_lanes_cap_keyword,
+		NULL,
+	},
+};
+
+/* *** configure speed_lanes for all ports *** */
+struct cmd_config_speed_lanes_all {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t keyword;
+	cmdline_fixed_string_t all;
+	cmdline_fixed_string_t item;
+	uint32_t lanes;
+};
+
+static void
+cmd_config_speed_lanes_all_parsed(void *parsed_result,
+				  __rte_unused struct cmdline *cl,
+				  __rte_unused void *data)
+{
+	struct cmd_config_speed_lanes_all *res = parsed_result;
+	portid_t pid;
+
+	if (!all_ports_stopped()) {
+		fprintf(stderr, "Please stop all ports first\n");
+		return;
+	}
+
+	RTE_ETH_FOREACH_DEV(pid) {
+		if (parse_speed_lanes_cfg(pid, res->lanes))
+			return;
+	}
+
+	cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
+}
+
+static cmdline_parse_token_string_t cmd_config_speed_lanes_all_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_all, port, "port");
+static cmdline_parse_token_string_t cmd_config_speed_lanes_all_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_all, keyword,
+				 "config");
+static cmdline_parse_token_string_t cmd_config_speed_lanes_all_all =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_all, all, "all");
+static cmdline_parse_token_string_t cmd_config_speed_lanes_all_item =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_all, item,
+				 "speed_lanes");
+static cmdline_parse_token_num_t cmd_config_speed_lanes_all_lanes =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_speed_lanes_all, lanes, RTE_UINT32);
+
+static cmdline_parse_inst_t cmd_config_speed_lanes_all = {
+	.f = cmd_config_speed_lanes_all_parsed,
+	.data = NULL,
+	.help_str = "port config all speed_lanes <value>",
+	.tokens = {
+		(void *)&cmd_config_speed_lanes_all_port,
+		(void *)&cmd_config_speed_lanes_all_keyword,
+		(void *)&cmd_config_speed_lanes_all_all,
+		(void *)&cmd_config_speed_lanes_all_item,
+		(void *)&cmd_config_speed_lanes_all_lanes,
+		NULL,
+	},
+};
+
+/* *** configure speed_lanes for specific port *** */
+struct cmd_config_speed_lanes_specific {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t keyword;
+	uint16_t port_id;
+	cmdline_fixed_string_t item;
+	uint32_t lanes;
+};
+
+static void
+cmd_config_speed_lanes_specific_parsed(void *parsed_result,
+				       __rte_unused struct cmdline *cl,
+				       __rte_unused void *data)
+{
+	struct cmd_config_speed_lanes_specific *res = parsed_result;
+
+	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
+		return;
+
+	if (!port_is_stopped(res->port_id)) {
+		fprintf(stderr, "Please stop port %u first\n", res->port_id);
+		return;
+	}
+
+	if (parse_speed_lanes_cfg(res->port_id, res->lanes))
+		return;
+
+	cmd_reconfig_device_queue(res->port_id, 1, 1);
+}
+
+static cmdline_parse_token_string_t cmd_config_speed_lanes_specific_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_specific, port,
+				 "port");
+static cmdline_parse_token_string_t cmd_config_speed_lanes_specific_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_specific, keyword,
+				 "config");
+static cmdline_parse_token_num_t cmd_config_speed_lanes_specific_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_speed_lanes_specific, port_id,
+			      RTE_UINT16);
+static cmdline_parse_token_string_t cmd_config_speed_lanes_specific_item =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_specific, item,
+				 "speed_lanes");
+static cmdline_parse_token_num_t cmd_config_speed_lanes_specific_lanes =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_speed_lanes_specific, lanes,
+			      RTE_UINT32);
+
+static cmdline_parse_inst_t cmd_config_speed_lanes_specific = {
+	.f = cmd_config_speed_lanes_specific_parsed,
+	.data = NULL,
+	.help_str = "port config <port_id> speed_lanes <value>",
+	.tokens = {
+		(void *)&cmd_config_speed_lanes_specific_port,
+		(void *)&cmd_config_speed_lanes_specific_keyword,
+		(void *)&cmd_config_speed_lanes_specific_id,
+		(void *)&cmd_config_speed_lanes_specific_item,
+		(void *)&cmd_config_speed_lanes_specific_lanes,
+		NULL,
+	},
+};
+
 /* *** configure loopback for all ports *** */
 struct cmd_config_loopback_all {
 	cmdline_fixed_string_t port;
@@ -1645,7 +1889,6 @@ cmd_config_loopback_specific_parsed(void *parsed_result,
 	cmd_reconfig_device_queue(res->port_id, 1, 1);
 }
 
-
 static cmdline_parse_token_string_t cmd_config_loopback_specific_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_config_loopback_specific, port,
 								"port");
@@ -13238,6 +13481,9 @@ static cmdline_parse_ctx_t builtin_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_set_port_setup_on,
 	(cmdline_parse_inst_t *)&cmd_config_speed_all,
 	(cmdline_parse_inst_t *)&cmd_config_speed_specific,
+	(cmdline_parse_inst_t *)&cmd_config_speed_lanes_all,
+	(cmdline_parse_inst_t *)&cmd_config_speed_lanes_specific,
+	(cmdline_parse_inst_t *)&cmd_show_speed_lanes,
 	(cmdline_parse_inst_t *)&cmd_config_loopback_all,
 	(cmdline_parse_inst_t *)&cmd_config_loopback_specific,
 	(cmdline_parse_inst_t *)&cmd_config_rx_tx,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 6f0beafa27..43b3a02732 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -786,6 +786,7 @@ port_infos_display(portid_t port_id)
 	char name[RTE_ETH_NAME_MAX_LEN];
 	int ret;
 	char fw_version[ETHDEV_FWVERS_LEN];
+	uint32_t lanes;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
 		print_valid_ports();
@@ -828,6 +829,8 @@ port_infos_display(portid_t port_id)
 
 	printf("\nLink status: %s\n", (link.link_status) ? ("up") : ("down"));
 	printf("Link speed: %s\n", rte_eth_link_speed_to_str(link.link_speed));
+	if (rte_eth_speed_lanes_get(port_id, &lanes) == 0)
+		printf("Active Lanes: %d\n", lanes);
 	printf("Link duplex: %s\n", (link.link_duplex == RTE_ETH_LINK_FULL_DUPLEX) ?
 	       ("full-duplex") : ("half-duplex"));
 	printf("Autoneg status: %s\n", (link.link_autoneg == RTE_ETH_LINK_AUTONEG) ?
@@ -7244,3 +7247,4 @@ show_mcast_macs(portid_t port_id)
 		printf("  %s\n", buf);
 	}
 }
+
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 883e59a927..abed4784aa 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -339,6 +339,93 @@ typedef int (*eth_allmulticast_disable_t)(struct rte_eth_dev *dev);
 typedef int (*eth_link_update_t)(struct rte_eth_dev *dev,
 				int wait_to_complete);
 
+/**
+ * @internal
+ * Get number of current active lanes
+ *
+ * @param dev
+ *   ethdev handle of port.
+ * @param speed_lanes
+ *   Number of active lanes that the link has trained up. This information
+ *   is displayed for Autonegotiated or Fixed speed trained link.
+ * @return
+ *   Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ *   Success, get speed_lanes data success.
+ * @retval -ENOTSUP
+ *   Operation is not supported.
+ * @retval -EIO
+ *   Device is removed.
+ */
+typedef int (*eth_speed_lanes_get_t)(struct rte_eth_dev *dev, uint32_t *speed_lanes);
+
+/**
+ * @internal
+ * Set speed lanes supported by the NIC. This configuration is applicable only when
+ * fix speed is already configured and or will be configured. This api requires the
+ * port be stopped, since driver has to re-configure PHY with fixed speed and lanes.
+ * If no lanes are configured prior or after "port config X speed Y duplex Z", the
+ * driver will choose the default lane for that speed to bring up the link.
+ *
+ * @param dev
+ *   ethdev handle of port.
+ * @param speed_lanes
+ *   Non-negative number of lanes
+ *
+ * @return
+ *   Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ *   Success, set lanes success.
+ * @retval -ENOTSUP
+ *   Operation is not supported.
+ * @retval -EINVAL
+ *   Unsupported number of lanes for fixed speed requested.
+ * @retval -EIO
+ *   Device is removed.
+ */
+typedef int (*eth_speed_lanes_set_t)(struct rte_eth_dev *dev, uint32_t speed_lanes);
+
+/**
+ * @internal
+ * Get supported link speed lanes capability. The driver returns number of lanes
+ * supported per speed in the form of lanes capability bitmap per speed.
+ *
+ * @param speed_lanes_capa
+ *   A pointer to num of rte_eth_speed_lanes_capa struct array which carries the
+ *   bit map of lanes supported per speed. The number of supported speeds is the
+ *   size of this speed_lanes_capa table. In link up condition, only active supported
+ *   speeds lanes bitmap information will be displayed. In link down condition, all
+ *   the supported speeds and its supported lanes bitmap would be fetched and displayed.
+ *
+ *   This api is overloaded to fetch the size of the speed_lanes_capa array if
+ *   testpmd calls the driver with speed_lanes_capa = NULL and num = 0
+ *
+ * @param num
+ *   Number of elements in a speed_speed_lanes_capa array. This num is equal to the
+ *   number of supported speeds by the controller. This value will vary in link up
+ *   and link down condition. num is updated by the driver if speed_lanes_capa is NULL.
+ *
+ * @return
+ *   Negative errno value on error, positive value on success.
+ *
+ * @retval positive value
+ *   A non-negative value lower or equal to num: success. The return value
+ *   is the number of entries filled in the speed lanes array.
+ *   A non-negative value higher than num: error, the given speed lanes capa array
+ *   is too small. The return value corresponds to the num that should
+ *   be given to succeed. The entries in the speed lanes capa array are not valid
+ *   and shall not be used by the caller.
+ * @retval -ENOTSUP
+ *   Operation is not supported.
+ * @retval -EINVAL
+ *   *num* or *speed_lanes_capa* invalid.
+ */
+typedef int (*eth_speed_lanes_get_capability_t)(struct rte_eth_dev *dev,
+						struct rte_eth_speed_lanes_capa *speed_lanes_capa,
+						unsigned int num);
+
 /** @internal Get global I/O statistics of an Ethernet device. */
 typedef int (*eth_stats_get_t)(struct rte_eth_dev *dev,
 				struct rte_eth_stats *igb_stats);
@@ -1247,6 +1334,10 @@ struct eth_dev_ops {
 	eth_dev_close_t            dev_close;     /**< Close device */
 	eth_dev_reset_t		   dev_reset;	  /**< Reset device */
 	eth_link_update_t          link_update;   /**< Get device link state */
+	eth_speed_lanes_get_t	   speed_lanes_get;	  /**<Get link speed active lanes */
+	eth_speed_lanes_set_t      speed_lanes_set;	  /**<set the link speeds supported lanes */
+	/** Get link speed lanes capability */
+	eth_speed_lanes_get_capability_t speed_lanes_get_capa;
 	/** Check if the device was physically removed */
 	eth_is_removed_t           is_removed;
 
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index f1c658f49e..36427183d5 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1849,6 +1849,58 @@ rte_eth_dev_set_link_down(uint16_t port_id)
 	return ret;
 }
 
+int
+rte_eth_speed_lanes_get(uint16_t port_id, uint32_t *lane)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (*dev->dev_ops->speed_lanes_get == NULL)
+		return -ENOTSUP;
+	return eth_err(port_id, (*dev->dev_ops->speed_lanes_get)(dev, lane));
+}
+
+int
+rte_eth_speed_lanes_get_capability(uint16_t port_id,
+				   struct rte_eth_speed_lanes_capa *speed_lanes_capa,
+				   unsigned int num)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (*dev->dev_ops->speed_lanes_get_capa == NULL)
+		return -ENOTSUP;
+
+	if (speed_lanes_capa == NULL && num > 0) {
+		RTE_ETHDEV_LOG_LINE(ERR,
+				    "Cannot get ethdev port %u speed lanes capability to NULL when array size is non zero",
+				    port_id);
+		return -EINVAL;
+	}
+
+	ret = (*dev->dev_ops->speed_lanes_get_capa)(dev, speed_lanes_capa, num);
+
+	return ret;
+}
+
+int
+rte_eth_speed_lanes_set(uint16_t port_id, uint32_t speed_lanes_capa)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (*dev->dev_ops->speed_lanes_set == NULL)
+		return -ENOTSUP;
+	return eth_err(port_id, (*dev->dev_ops->speed_lanes_set)(dev, speed_lanes_capa));
+}
+
 int
 rte_eth_dev_close(uint16_t port_id)
 {
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 548fada1c7..ad8a2cb3ea 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -357,6 +357,15 @@ struct rte_eth_link {
 #define RTE_ETH_LINK_MAX_STR_LEN 40 /**< Max length of default link string. */
 /**@}*/
 
+/* Translate from link speed lanes to speed lanes capa */
+#define RTE_ETH_SPEED_LANES_TO_CAPA(x) RTE_BIT32(x)
+
+/* A structure used to get and set lanes capabilities per link speed */
+struct rte_eth_speed_lanes_capa {
+	uint32_t speed;
+	uint32_t capa;
+};
+
 /**
  * A structure used to configure the ring threshold registers of an Rx/Tx
  * queue for an Ethernet port.
@@ -3114,6 +3123,80 @@ __rte_experimental
 int rte_eth_link_to_str(char *str, size_t len,
 			const struct rte_eth_link *eth_link);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Get Active lanes.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param lanes
+ *   Driver updates lanes with the number of active lanes.
+ *   On a supported NIC on link up, lanes will be a non-zero value irrespective whether the
+ *   link is Autonegotiated or Fixed speed. No information is dispalyed for error.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if underlying hardware OR driver doesn't support.
+ *     that operation.
+ *   - (-EIO) if device is removed.
+ *   - (-ENODEV)  if *port_id* invalid.
+ */
+__rte_experimental
+int rte_eth_speed_lanes_get(uint16_t port_id, uint32_t *lanes);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Set speed lanes supported by the NIC.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param speed_lanes
+ *   A non-zero number of speed lanes, that will be applied to the ethernet PHY
+ *   along with the fixed speed configuration. Driver returns error if the user
+ *   lanes is not in speeds capability list.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if underlying hardware OR driver doesn't support.
+ *     that operation.
+ *   - (-EIO) if device is removed.
+ *   - (-ENODEV)  if *port_id* invalid.
+ *   - (-EINVAL)  if *lanes* count not in speeds capability list.
+ */
+__rte_experimental
+int rte_eth_speed_lanes_set(uint16_t port_id, uint32_t speed_lanes);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Get speed lanes supported by the NIC.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param speed_lanes_capa
+ *   An array of supported speed and its supported lanes.
+ * @param num
+ *   Size of the speed_lanes_capa array. The size is equal to the supported speeds list size.
+ *   Value of num is derived by calling this api with speed_lanes_capa=NULL and num=0
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if underlying hardware OR driver doesn't support.
+ *     that operation.
+ *   - (-EIO) if device is removed.
+ *   - (-ENODEV)  if *port_id* invalid.
+ *   - (-EINVAL)  if *speed_lanes* invalid
+ */
+__rte_experimental
+int rte_eth_speed_lanes_get_capability(uint16_t port_id,
+				       struct rte_eth_speed_lanes_capa *speed_lanes_capa,
+				       unsigned int num);
+
 /**
  * Retrieve the general I/O statistics of an Ethernet device.
  *
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 1669055ca5..b5ffded6f1 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -325,6 +325,11 @@ EXPERIMENTAL {
 	rte_flow_template_table_resizable;
 	rte_flow_template_table_resize;
 	rte_flow_template_table_resize_complete;
+
+	# added in 24.11
+	rte_eth_speed_lanes_get;
+	rte_eth_speed_lanes_get_capability;
+	rte_eth_speed_lanes_set;
 };
 
 INTERNAL {
-- 
2.43.5


-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

[PATCH v6 2/2] net/bnxt: code refactor for supporting speed lanes

[Damodharam Ammepalli]

Broadcom Thor2 NICs support link mode settings where user
can configure fixed speed and associated supported number of
lanes. This patch does code-refactoring to address proposed
poll mode library design updates.

Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        |   3 +
 drivers/net/bnxt/bnxt_ethdev.c | 176 ++++++++++++++++++++++++++++++---
 drivers/net/bnxt/bnxt_hwrm.c   |  40 ++++++--
 3 files changed, 200 insertions(+), 19 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index aaa7ea00cc..667fc84eb2 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -328,6 +328,7 @@ struct bnxt_link_info {
 	uint16_t                cfg_auto_link_speeds2_mask;
 	uint8_t                 active_lanes;
 	uint8_t			option_flags;
+	uint16_t                pmd_speed_lanes;
 };
 
 #define BNXT_COS_QUEUE_COUNT	8
@@ -1219,6 +1220,7 @@ extern int bnxt_logtype_driver;
 #define BNXT_LINK_SPEEDS_V2_VF(bp) (BNXT_VF((bp)) && ((bp)->link_info->option_flags))
 #define BNXT_LINK_SPEEDS_V2(bp) (((bp)->link_info) && (((bp)->link_info->support_speeds_v2) || \
 						       BNXT_LINK_SPEEDS_V2_VF((bp))))
+#define BNXT_MAX_SPEED_LANES 8
 extern const struct rte_flow_ops bnxt_ulp_rte_flow_ops;
 int32_t bnxt_ulp_port_init(struct bnxt *bp);
 void bnxt_ulp_port_deinit(struct bnxt *bp);
@@ -1244,4 +1246,5 @@ int bnxt_flow_meter_ops_get(struct rte_eth_dev *eth_dev, void *arg);
 struct bnxt_vnic_info *bnxt_get_default_vnic(struct bnxt *bp);
 struct tf *bnxt_get_tfp_session(struct bnxt *bp, enum bnxt_session_type type);
 uint64_t bnxt_eth_rss_support(struct bnxt *bp);
+uint16_t bnxt_parse_eth_link_speed_v2(struct bnxt *bp);
 #endif
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index e63febe782..c6ad764813 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -122,6 +122,24 @@ static const char *const bnxt_dev_args[] = {
 	NULL
 };
 
+#define BNXT_SPEEDS_SUPP_SPEED_LANES (RTE_ETH_LINK_SPEED_10G | \
+				      RTE_ETH_LINK_SPEED_25G | \
+				      RTE_ETH_LINK_SPEED_40G | \
+				      RTE_ETH_LINK_SPEED_50G | \
+				      RTE_ETH_LINK_SPEED_100G | \
+				      RTE_ETH_LINK_SPEED_200G | \
+				      RTE_ETH_LINK_SPEED_400G)
+
+static const struct rte_eth_speed_lanes_capa speed_lanes_capa_tbl[] = {
+	{ RTE_ETH_SPEED_NUM_10G, RTE_BIT32(1) },
+	{ RTE_ETH_SPEED_NUM_25G, RTE_BIT32(1) },
+	{ RTE_ETH_SPEED_NUM_40G, RTE_BIT32(4) },
+	{ RTE_ETH_SPEED_NUM_50G, RTE_BIT32(1) | RTE_BIT32(2) },
+	{ RTE_ETH_SPEED_NUM_100G, RTE_BIT32(1) | RTE_BIT32(2) | RTE_BIT32(4) },
+	{ RTE_ETH_SPEED_NUM_200G, RTE_BIT32(2) | RTE_BIT32(4) },
+	{ RTE_ETH_SPEED_NUM_400G, RTE_BIT32(4) | RTE_BIT32(8) },
+};
+
 /*
  * cqe-mode = an non-negative 8-bit number
  */
@@ -696,22 +714,50 @@ static inline bool bnxt_force_link_config(struct bnxt *bp)
 	}
 }
 
-static int bnxt_update_phy_setting(struct bnxt *bp)
+static int bnxt_validate_speed_lanes_change(struct bnxt *bp)
 {
 	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
 	struct rte_eth_link *link = &bp->eth_dev->data->dev_link;
-	struct rte_eth_link new;
 	uint32_t curr_speed_bit;
 	int rc;
 
+	/* Check if speed x lanes combo is supported */
+	if (dev_conf->link_speeds)  {
+		rc = bnxt_parse_eth_link_speed_v2(bp);
+		if (rc == 0)
+			return -EINVAL;
+	}
+
+	/* convert to speedbit flag */
+	curr_speed_bit = rte_eth_speed_bitflag((uint32_t)link->link_speed, 1);
+
+	/* check if speed and lanes have changed */
+	if (dev_conf->link_speeds != curr_speed_bit ||
+	    bp->link_info->active_lanes != bp->link_info->pmd_speed_lanes)
+		return 1;
+
+	return 0;
+}
+
+static int bnxt_update_phy_setting(struct bnxt *bp)
+{
+	struct rte_eth_link new;
+	int rc, rc1 = 0;
+
 	rc = bnxt_get_hwrm_link_config(bp, &new);
 	if (rc) {
 		PMD_DRV_LOG(ERR, "Failed to get link settings\n");
 		return rc;
 	}
 
-	/* convert to speedbit flag */
-	curr_speed_bit = rte_eth_speed_bitflag((uint32_t)link->link_speed, 1);
+	/* Validate speeds2 requirements */
+	if (BNXT_LINK_SPEEDS_V2(bp)) {
+		rc1 = bnxt_validate_speed_lanes_change(bp);
+		if (rc1 == -EINVAL) {
+			PMD_DRV_LOG(ERR, "Failed to set correct lanes\n");
+			return rc1;
+		}
+	}
 
 	/*
 	 * Device is not obliged link down in certain scenarios, even
@@ -719,8 +765,7 @@ static int bnxt_update_phy_setting(struct bnxt *bp)
 	 * to shutdown the port, bnxt_get_hwrm_link_config() call always
 	 * returns link up. Force phy update always in that case.
 	 */
-	if (!new.link_status || bnxt_force_link_config(bp) ||
-	    (BNXT_LINK_SPEEDS_V2(bp) && dev_conf->link_speeds != curr_speed_bit)) {
+	if (!new.link_status || bnxt_force_link_config(bp) || rc1 == 1) {
 		rc = bnxt_set_hwrm_link_config(bp, true);
 		if (rc) {
 			PMD_DRV_LOG(ERR, "Failed to update PHY settings\n");
@@ -1331,16 +1376,17 @@ static int bnxt_dev_configure_op(struct rte_eth_dev *eth_dev)
 void bnxt_print_link_info(struct rte_eth_dev *eth_dev)
 {
 	struct rte_eth_link *link = &eth_dev->data->dev_link;
+	struct bnxt *bp = eth_dev->data->dev_private;
 
 	if (link->link_status)
-		PMD_DRV_LOG(DEBUG, "Port %d Link Up - speed %u Mbps - %s\n",
-			eth_dev->data->port_id,
-			(uint32_t)link->link_speed,
-			(link->link_duplex == RTE_ETH_LINK_FULL_DUPLEX) ?
-			("full-duplex") : ("half-duplex\n"));
+		PMD_DRV_LOG(DEBUG, "Port %d Link Up - speed %u Mbps - %s Lanes - %d\n",
+			    eth_dev->data->port_id,
+			    (uint32_t)link->link_speed,
+			    (link->link_duplex == RTE_ETH_LINK_FULL_DUPLEX) ?
+			    ("full-duplex") : ("half-duplex\n"),
+			    (uint16_t)bp->link_info->active_lanes);
 	else
-		PMD_DRV_LOG(INFO, "Port %d Link Down\n",
-			eth_dev->data->port_id);
+		PMD_DRV_LOG(INFO, "Port %d Link Down\n", eth_dev->data->port_id);
 }
 
 /*
@@ -4191,6 +4237,105 @@ static int bnxt_get_module_eeprom(struct rte_eth_dev *dev,
 	return length ? -EINVAL : 0;
 }
 
+#if (RTE_VERSION_NUM(22, 11, 0, 0) <= RTE_VERSION)
+static int bnxt_speed_lanes_set(struct rte_eth_dev *dev, uint32_t speed_lanes)
+{
+	struct bnxt *bp = dev->data->dev_private;
+
+	if (!BNXT_LINK_SPEEDS_V2(bp))
+		return -ENOTSUP;
+
+	bp->link_info->pmd_speed_lanes = speed_lanes;
+
+	return 0;
+}
+
+static uint32_t
+bnxt_get_speed_lanes_capa(struct rte_eth_speed_lanes_capa *speed_lanes_capa,
+			  uint32_t speed_capa)
+{
+	uint32_t speed_bit;
+	uint32_t num = 0;
+	uint32_t i;
+
+	for (i = 0; i < RTE_DIM(speed_lanes_capa_tbl); i++) {
+		speed_bit =
+			rte_eth_speed_bitflag(speed_lanes_capa_tbl[i].speed,
+					      RTE_ETH_LINK_FULL_DUPLEX);
+		if ((speed_capa & speed_bit) == 0)
+			continue;
+
+		speed_lanes_capa[num].speed = speed_lanes_capa_tbl[i].speed;
+		speed_lanes_capa[num].capa = speed_lanes_capa_tbl[i].capa;
+		num++;
+	}
+
+	return num;
+}
+
+static int bnxt_speed_lanes_get_capa(struct rte_eth_dev *dev,
+				     struct rte_eth_speed_lanes_capa *speed_lanes_capa,
+				     unsigned int num)
+{
+	struct rte_eth_link *link = &dev->data->dev_link;
+	struct bnxt *bp = dev->data->dev_private;
+	unsigned int speed_num;
+	uint32_t speed_capa;
+	int rc;
+
+	rc = is_bnxt_in_error(bp);
+	if (rc)
+		return rc;
+
+	if (!BNXT_LINK_SPEEDS_V2(bp))
+		return -ENOTSUP;
+
+	/* speed_num counts number of speed capabilities.
+	 * When link is down, show the user choice all combinations of speeds x lanes
+	 */
+	if (link->link_status) {
+		speed_capa = bnxt_get_speed_capabilities_v2(bp);
+		speed_num = rte_popcount32(speed_capa & BNXT_SPEEDS_SUPP_SPEED_LANES);
+	} else {
+		speed_capa = BNXT_SPEEDS_SUPP_SPEED_LANES;
+		speed_num = rte_popcount32(BNXT_SPEEDS_SUPP_SPEED_LANES);
+	}
+	if (speed_num == 0)
+		return -ENOTSUP;
+
+	if (speed_lanes_capa == NULL)
+		return speed_num;
+
+	if (num < speed_num)
+		return -EINVAL;
+
+	return bnxt_get_speed_lanes_capa(speed_lanes_capa, speed_capa);
+}
+
+static int bnxt_speed_lanes_get(struct rte_eth_dev *dev, uint32_t *lanes)
+{
+	struct rte_eth_link *link = &dev->data->dev_link;
+	struct bnxt *bp = dev->data->dev_private;
+	int rc;
+
+	rc = is_bnxt_in_error(bp);
+	if (rc)
+		return rc;
+
+	if (!BNXT_LINK_SPEEDS_V2(bp))
+		return -ENOTSUP;
+
+	if (!link->link_status)
+		return -EINVAL;
+
+	 /* user app expects lanes 1 for zero */
+	*lanes = (bp->link_info->active_lanes) ?
+		bp->link_info->active_lanes : 1;
+	return 0;
+}
+
+#endif
+
 /*
  * Initialization
  */
@@ -4262,6 +4407,11 @@ static const struct eth_dev_ops bnxt_dev_ops = {
 	.timesync_read_rx_timestamp = bnxt_timesync_read_rx_timestamp,
 	.timesync_read_tx_timestamp = bnxt_timesync_read_tx_timestamp,
 	.mtr_ops_get = bnxt_flow_meter_ops_get,
+#if (RTE_VERSION_NUM(22, 11, 0, 0) <= RTE_VERSION)
+	.speed_lanes_get = bnxt_speed_lanes_get,
+	.speed_lanes_set = bnxt_speed_lanes_set,
+	.speed_lanes_get_capa = bnxt_speed_lanes_get_capa,
+#endif
 };
 
 static uint32_t bnxt_map_reset_regs(struct bnxt *bp, uint32_t reg)
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index fc142672f6..bb9032b4f8 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -72,6 +72,7 @@ struct link_speeds2_tbl {
 	uint32_t rte_speed_num;
 	uint16_t hwrm_speed;
 	uint16_t sig_mode;
+	uint16_t lanes;
 	const char *desc;
 } link_speeds2_tbl[] = {
 	{
@@ -81,6 +82,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_1G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_1GB,
 		BNXT_SIG_MODE_NRZ,
+		1,
 		"1Gb NRZ",
 	}, {
 		100,
@@ -89,6 +91,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_10G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_10GB,
 		BNXT_SIG_MODE_NRZ,
+		1,
 		"10Gb NRZ",
 	}, {
 		250,
@@ -97,6 +100,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_25G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_25GB,
 		BNXT_SIG_MODE_NRZ,
+		1,
 		"25Gb NRZ",
 	}, {
 		400,
@@ -105,6 +109,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_40G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_40GB,
 		BNXT_SIG_MODE_NRZ,
+		4,
 		"40Gb NRZ",
 	}, {
 		500,
@@ -113,6 +118,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_50G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_50GB,
 		BNXT_SIG_MODE_NRZ,
+		2,
 		"50Gb NRZ",
 	}, {
 		1000,
@@ -121,6 +127,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_100G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_100GB,
 		BNXT_SIG_MODE_NRZ,
+		4,
 		"100Gb NRZ",
 	}, {
 		501,
@@ -129,6 +136,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_50G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_50GB_PAM4_56,
 		BNXT_SIG_MODE_PAM4,
+		1,
 		"50Gb (PAM4-56: 50G per lane)",
 	}, {
 		1001,
@@ -137,6 +145,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_100G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_100GB_PAM4_56,
 		BNXT_SIG_MODE_PAM4,
+		2,
 		"100Gb (PAM4-56: 50G per lane)",
 	}, {
 		2001,
@@ -145,6 +154,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_200G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_200GB_PAM4_56,
 		BNXT_SIG_MODE_PAM4,
+		4,
 		"200Gb (PAM4-56: 50G per lane)",
 	}, {
 		4001,
@@ -153,6 +163,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_400G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_400GB_PAM4_56,
 		BNXT_SIG_MODE_PAM4,
+		8,
 		"400Gb (PAM4-56: 50G per lane)",
 	}, {
 		1002,
@@ -161,6 +172,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_100G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_100GB_PAM4_112,
 		BNXT_SIG_MODE_PAM4_112,
+		1,
 		"100Gb (PAM4-112: 100G per lane)",
 	}, {
 		2002,
@@ -169,6 +181,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_200G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_200GB_PAM4_112,
 		BNXT_SIG_MODE_PAM4_112,
+		2,
 		"200Gb (PAM4-112: 100G per lane)",
 	}, {
 		4002,
@@ -177,6 +190,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_400G,
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_400GB_PAM4_112,
 		BNXT_SIG_MODE_PAM4_112,
+		4,
 		"400Gb (PAM4-112: 100G per lane)",
 	}, {
 		0,
@@ -185,6 +199,7 @@ struct link_speeds2_tbl {
 		RTE_ETH_SPEED_NUM_NONE,	/* None matches, No speed */
 		HWRM_PORT_PHY_CFG_INPUT_FORCE_LINK_SPEEDS2_1GB, /* Placeholder for wrong HWRM */
 		BNXT_SIG_MODE_NRZ, /* default sig */
+		0,
 		"Unknown",
 	},
 };
@@ -264,17 +279,29 @@ static const char *bnxt_get_xcvr_type(uint32_t xcvr_identifier_type_tx_lpi_timer
 /* Utility function to lookup speeds2 table and
  * return a rte to hwrm speed matching row to the client
  */
-static
-struct link_speeds2_tbl *bnxt_get_rte_hwrm_speeds2_entry(uint32_t speed)
+static struct link_speeds2_tbl *bnxt_get_rte_hwrm_speeds2_entry(struct bnxt *bp)
 {
 	int i, max;
+	uint32_t speed, lanes;
+	bool check_lanes;
+	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
+
+	speed = dev_conf->link_speeds;
+	lanes = bp->link_info->pmd_speed_lanes;
 
 	max = BNXT_SPEEDS2_TBL_SZ - 1;
 	speed &= ~RTE_ETH_LINK_SPEED_FIXED;
+	check_lanes = !(lanes == 0);
+
 	for (i = 0; i < max; i++) {
-		if (speed == link_speeds2_tbl[i].rte_speed)
+		if (speed == link_speeds2_tbl[i].rte_speed &&
+		    (lanes == link_speeds2_tbl[i].lanes || !check_lanes))
 			break;
 	}
+
+	if (!check_lanes)
+		PMD_DRV_LOG(INFO, "Given lanes %d, Configuring default lanes %d %s\n",
+			    lanes, link_speeds2_tbl[i].lanes, link_speeds2_tbl[i].desc);
 	return (struct link_speeds2_tbl *)&link_speeds2_tbl[i];
 }
 
@@ -3579,11 +3606,11 @@ static uint16_t bnxt_check_eth_link_autoneg(uint32_t conf_link)
 	return !conf_link;
 }
 
-static uint16_t bnxt_parse_eth_link_speed_v2(uint32_t conf_link_speed)
+uint16_t bnxt_parse_eth_link_speed_v2(struct bnxt *bp)
 {
 	/* get bitmap value based on speed */
 	return ((struct link_speeds2_tbl *)
-		bnxt_get_rte_hwrm_speeds2_entry(conf_link_speed))->force_val;
+		bnxt_get_rte_hwrm_speeds2_entry(bp))->force_val;
 }
 
 static uint16_t bnxt_parse_eth_link_speed(struct bnxt *bp, uint32_t conf_link_speed,
@@ -3598,7 +3625,7 @@ static uint16_t bnxt_parse_eth_link_speed(struct bnxt *bp, uint32_t conf_link_sp
 
 	/* Handle P7 chips saperately. It got enhanced phy attribs to choose from */
 	if (BNXT_LINK_SPEEDS_V2(bp))
-		return bnxt_parse_eth_link_speed_v2(conf_link_speed);
+		return bnxt_parse_eth_link_speed_v2(bp);
 
 	switch (conf_link_speed & ~RTE_ETH_LINK_SPEED_FIXED) {
 	case RTE_ETH_LINK_SPEED_100M:
@@ -3902,6 +3929,7 @@ static int bnxt_hwrm_port_phy_cfg_v2(struct bnxt *bp, struct bnxt_link_info *con
 	} else {
 		enables |= HWRM_PORT_PHY_CFG_INPUT_ENABLES_FORCE_LINK_SPEEDS2;
 		req.force_link_speeds2 = rte_cpu_to_le_16(conf->link_speed);
+		PMD_DRV_LOG(INFO, "Force speed %d\n", conf->link_speed);
 	}
 
 	/* Fill rest of the req message */
-- 
2.43.5


-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

Re: [PATCH v6 1/2] ethdev: Add link_speed lanes support

[Ferruh Yigit]

On 9/26/2024 10:43 PM, Damodharam Ammepalli wrote:
> Update the eth_dev_ops structure with new function vectors
> to get, get capabilities and set ethernet link speed lanes.
> Update the testpmd to provide required config and information
> display infrastructure.
> 
> The supporting ethernet controller driver will register callbacks
> to avail link speed lanes config and get services. This lanes
> configuration is applicable only when the nic is forced to fixed
> speeds. In Autonegiation mode, the hardware automatically
> negotiates the number of lanes.
> 
> These are the new commands.
> 
> testpmd> show port 0 speed_lanes capabilities
> 
>  Supported speeds         Valid lanes
> -----------------------------------
>  10 Gbps                  1
>  25 Gbps                  1
>  40 Gbps                  4
>  50 Gbps                  1 2
>  100 Gbps                 1 2 4
>  200 Gbps                 2 4
>  400 Gbps                 4 8
> testpmd>
> 
> testpmd>
> testpmd> port stop 0
> testpmd> port config 0 speed_lanes 4
> testpmd> port config 0 speed 200000 duplex full
> testpmd> port start 0
> testpmd>
> testpmd> show port info 0
> 
> ********************* Infos for port 0  *********************
> MAC address: 14:23:F2:C3:BA:D2
> Device name: 0000:b1:00.0
> Driver name: net_bnxt
> Firmware-version: 228.9.115.0
> Connect to socket: 2
> memory allocation on the socket: 2
> Link status: up
> Link speed: 200 Gbps
> Active Lanes: 4
> Link duplex: full-duplex
> Autoneg status: Off
> 
> Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
>

Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>

Suggested patch title:
ethdev: support link speed lanes

<...>

> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get Active lanes.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param lanes
> + *   Driver updates lanes with the number of active lanes.
> + *   On a supported NIC on link up, lanes will be a non-zero value irrespective whether the
> + *   link is Autonegotiated or Fixed speed. No information is dispalyed for error.
>

s/dispalyed/displayed/

I can fix while merging

Re: [PATCH v6 0/2] Add link_speed lanes support

[Ferruh Yigit]

On 9/26/2024 10:43 PM, Damodharam Ammepalli wrote:
> This patch series is a continuation of the patch set that supports configuring speed lanes.
> https://patchwork.dpdk.org/project/dpdk/patch/20240708232351.491529-1-
> damodharam.ammepalli@broadcom.com/
> 
> The patchset consists
> 1) rtelib/testpmd changes (Addressing the comments). Earlier comments are available here, 
> https://patchwork.dpdk.org/project/dpdk/list/?
> q=Add%20link_speed%20lanes%20support&archive=both&series=&submitter=&delegate=&state=*
> 
> 2) Driver implementation of bnxt PMD.
> 
> v2->v3 Consolidating the testpmd and rtelib patches into a single patch as requested.
> v3->v4 Addressed comments and fix help string and documentation.
> v4->v5 Addressed comments given in v4 and also driver implementation of bnxt PMD.
> v5->v6 Removed LANE_xx enums and updated driver source files. Minor help
> string fix.
> 
> Damodharam Ammepalli (2):
>   ethdev: Add link_speed lanes support
>   net/bnxt: code refactor for supporting speed lanes
>

Hi Ajit,

Can you please review the bnxt patch, if it is good I can progress with
the series.

Re: [PATCH v6 2/2] net/bnxt: code refactor for supporting speed lanes

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 463 bytes --]

On Thu, Sep 26, 2024 at 2:56 PM Damodharam Ammepalli
<damodharam.ammepalli@broadcom.com> wrote:
>
> Broadcom Thor2 NICs support link mode settings where user
> can configure fixed speed and associated supported number of
> lanes. This patch does code-refactoring to address proposed
> poll mode library design updates.
>
> Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH v6 1/2] ethdev: Add link_speed lanes support

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 2570 bytes --]

On Thu, Sep 26, 2024 at 5:39 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> On 9/26/2024 10:43 PM, Damodharam Ammepalli wrote:
> > Update the eth_dev_ops structure with new function vectors
> > to get, get capabilities and set ethernet link speed lanes.
> > Update the testpmd to provide required config and information
> > display infrastructure.
> >
> > The supporting ethernet controller driver will register callbacks
> > to avail link speed lanes config and get services. This lanes
> > configuration is applicable only when the nic is forced to fixed
> > speeds. In Autonegiation mode, the hardware automatically
> > negotiates the number of lanes.
> >
> > These are the new commands.
> >
> > testpmd> show port 0 speed_lanes capabilities
> >
> >  Supported speeds         Valid lanes
> > -----------------------------------
> >  10 Gbps                  1
> >  25 Gbps                  1
> >  40 Gbps                  4
> >  50 Gbps                  1 2
> >  100 Gbps                 1 2 4
> >  200 Gbps                 2 4
> >  400 Gbps                 4 8
> > testpmd>
> >
> > testpmd>
> > testpmd> port stop 0
> > testpmd> port config 0 speed_lanes 4
> > testpmd> port config 0 speed 200000 duplex full
> > testpmd> port start 0
> > testpmd>
> > testpmd> show port info 0
> >
> > ********************* Infos for port 0  *********************
> > MAC address: 14:23:F2:C3:BA:D2
> > Device name: 0000:b1:00.0
> > Driver name: net_bnxt
> > Firmware-version: 228.9.115.0
> > Connect to socket: 2
> > memory allocation on the socket: 2
> > Link status: up
> > Link speed: 200 Gbps
> > Active Lanes: 4
> > Link duplex: full-duplex
> > Autoneg status: Off
> >
> > Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
> >
>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

>
> Suggested patch title:
> ethdev: support link speed lanes
>
> <...>
>
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> > + *
> > + * Get Active lanes.
> > + *
> > + * @param port_id
> > + *   The port identifier of the Ethernet device.
> > + * @param lanes
> > + *   Driver updates lanes with the number of active lanes.
> > + *   On a supported NIC on link up, lanes will be a non-zero value irrespective whether the
> > + *   link is Autonegotiated or Fixed speed. No information is dispalyed for error.
> >
>
> s/dispalyed/displayed/
>
> I can fix while merging
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH v6 0/2] Add link_speed lanes support

[Ferruh Yigit]

On 9/26/2024 10:43 PM, Damodharam Ammepalli wrote:
> This patch series is a continuation of the patch set that supports configuring speed lanes.
> https://patchwork.dpdk.org/project/dpdk/patch/20240708232351.491529-1-
> damodharam.ammepalli@broadcom.com/
> 
> The patchset consists
> 1) rtelib/testpmd changes (Addressing the comments). Earlier comments are available here, 
> https://patchwork.dpdk.org/project/dpdk/list/?
> q=Add%20link_speed%20lanes%20support&archive=both&series=&submitter=&delegate=&state=*
> 
> 2) Driver implementation of bnxt PMD.
> 
> v2->v3 Consolidating the testpmd and rtelib patches into a single patch as requested.
> v3->v4 Addressed comments and fix help string and documentation.
> v4->v5 Addressed comments given in v4 and also driver implementation of bnxt PMD.
> v5->v6 Removed LANE_xx enums and updated driver source files. Minor help
> string fix.
> 
> Damodharam Ammepalli (2):
>   ethdev: Add link_speed lanes support
>   net/bnxt: code refactor for supporting speed lanes
>

Series applied to dpdk-next-net/main, thanks.

Re: [PATCH v6 2/2] net/bnxt: code refactor for supporting speed lanes

[David Marchand]

Hello,

On Thu, Sep 26, 2024 at 11:56 PM Damodharam Ammepalli
<damodharam.ammepalli@broadcom.com> wrote:
> @@ -1331,16 +1376,17 @@ static int bnxt_dev_configure_op(struct rte_eth_dev *eth_dev)
>  void bnxt_print_link_info(struct rte_eth_dev *eth_dev)
>  {
>         struct rte_eth_link *link = &eth_dev->data->dev_link;
> +       struct bnxt *bp = eth_dev->data->dev_private;
>
>         if (link->link_status)
> -               PMD_DRV_LOG(DEBUG, "Port %d Link Up - speed %u Mbps - %s\n",
> -                       eth_dev->data->port_id,
> -                       (uint32_t)link->link_speed,
> -                       (link->link_duplex == RTE_ETH_LINK_FULL_DUPLEX) ?
> -                       ("full-duplex") : ("half-duplex\n"));
> +               PMD_DRV_LOG(DEBUG, "Port %d Link Up - speed %u Mbps - %s Lanes - %d\n",
> +                           eth_dev->data->port_id,
> +                           (uint32_t)link->link_speed,
> +                           (link->link_duplex == RTE_ETH_LINK_FULL_DUPLEX) ?
> +                           ("full-duplex") : ("half-duplex\n"),

A \n slipped in with the "half-duplex" string.
Please send a fix against next-net.


> +                           (uint16_t)bp->link_info->active_lanes);
>         else
> -               PMD_DRV_LOG(INFO, "Port %d Link Down\n",
> -                       eth_dev->data->port_id);
> +               PMD_DRV_LOG(INFO, "Port %d Link Down\n", eth_dev->data->port_id);
>  }
>
>  /*


-- 
David Marchand

Re: [PATCH v6 2/2] net/bnxt: code refactor for supporting speed lanes

[Damodharam Ammepalli]

[-- Attachment #1: Type: text/plain, Size: 2487 bytes --]

On Fri, Oct 4, 2024 at 6:56 AM David Marchand <david.marchand@redhat.com> wrote:
>
> Hello,
>
> On Thu, Sep 26, 2024 at 11:56 PM Damodharam Ammepalli
> <damodharam.ammepalli@broadcom.com> wrote:
> > @@ -1331,16 +1376,17 @@ static int bnxt_dev_configure_op(struct rte_eth_dev *eth_dev)
> >  void bnxt_print_link_info(struct rte_eth_dev *eth_dev)
> >  {
> >         struct rte_eth_link *link = &eth_dev->data->dev_link;
> > +       struct bnxt *bp = eth_dev->data->dev_private;
> >
> >         if (link->link_status)
> > -               PMD_DRV_LOG(DEBUG, "Port %d Link Up - speed %u Mbps - %s\n",
> > -                       eth_dev->data->port_id,
> > -                       (uint32_t)link->link_speed,
> > -                       (link->link_duplex == RTE_ETH_LINK_FULL_DUPLEX) ?
> > -                       ("full-duplex") : ("half-duplex\n"));
> > +               PMD_DRV_LOG(DEBUG, "Port %d Link Up - speed %u Mbps - %s Lanes - %d\n",
> > +                           eth_dev->data->port_id,
> > +                           (uint32_t)link->link_speed,
> > +                           (link->link_duplex == RTE_ETH_LINK_FULL_DUPLEX) ?
> > +                           ("full-duplex") : ("half-duplex\n"),
>
> A \n slipped in with the "half-duplex" string.
> Please send a fix against next-net.
>
>
Ack

> > +                           (uint16_t)bp->link_info->active_lanes);
> >         else
> > -               PMD_DRV_LOG(INFO, "Port %d Link Down\n",
> > -                       eth_dev->data->port_id);
> > +               PMD_DRV_LOG(INFO, "Port %d Link Down\n", eth_dev->data->port_id);
> >  }
> >
> >  /*
>
>
> --
> David Marchand
>

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5457 bytes --]