DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Morten Brørup" <mb@smartsharesystems.com>
To: <skori@marvell.com>, "Thomas Monjalon" <thomas@monjalon.net>,
	"Andrew Rybchenko" <andrew.rybchenko@oktetlabs.ru>
Cc: <dev@dpdk.org>, "Nithin Dabilpuram" <ndabilpuram@marvell.com>
Subject: RE: [PATCH v4 1/1] ethdev: add support to provide link type
Date: Thu, 14 Aug 2025 11:04:18 +0200	[thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9FE35@smartserver.smartshare.dk> (raw)
In-Reply-To: <20250814081215.1904078-1-skori@marvell.com>

> From: skori@marvell.com [mailto:skori@marvell.com]
> Sent: Thursday, 14 August 2025 10.10
> 
> Adding link type parameter to provide the type
> of port like twisted pair, fibre etc.
> 
> Also added an API to convert the RTE_ETH_LINK_TYPE_XXX
> to a readable string.

Please consider if "connector" is a better name than "link type", if this is related to physical interfaces only.
E.g. is there an "LACP" link type for link aggregates (bonded links)?

Also, should a "virtual" link type (or connector) be added for TAP/Virtio/VMXNET3/ring and similar non-physical interfaces?

More comments inline below.

> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> ---
> v3..v4:
>  - Convert #define into enum.
>  - Enhance comments for each port link type.
>  - Fix test failures.
> v2..v3
>  - Extend link type list as per suggestion
>  app/test/test_ethdev_link.c      | 18 +++++++-----
>  lib/ethdev/ethdev_trace.h        |  7 +++++
>  lib/ethdev/ethdev_trace_points.c |  3 ++
>  lib/ethdev/rte_ethdev.c          | 46 ++++++++++++++++++++++++++++--
>  lib/ethdev/rte_ethdev.h          | 48 ++++++++++++++++++++++++++++++++
>  5 files changed, 113 insertions(+), 9 deletions(-)
> 
> diff --git a/app/test/test_ethdev_link.c b/app/test/test_ethdev_link.c
> index f063a5fe26..5d1a784208 100644
> --- a/app/test/test_ethdev_link.c
> +++ b/app/test/test_ethdev_link.c
> @@ -17,23 +17,25 @@ test_link_status_up_default(void)
>  		.link_speed = RTE_ETH_SPEED_NUM_2_5G,
>  		.link_status = RTE_ETH_LINK_UP,
>  		.link_autoneg = RTE_ETH_LINK_AUTONEG,
> -		.link_duplex = RTE_ETH_LINK_FULL_DUPLEX
> +		.link_duplex = RTE_ETH_LINK_FULL_DUPLEX,
> +		.link_type = RTE_ETH_LINK_TYPE_NONE

RTE_ETH_LINK_TYPE_UNKNOWN

>  	};
>  	char text[RTE_ETH_LINK_MAX_STR_LEN];
> 
>  	ret = rte_eth_link_to_str(text, sizeof(text), &link_status);
>  	RTE_TEST_ASSERT(ret > 0, "Failed to format default string\n");
>  	printf("Default link up #1: %s\n", text);
> -	TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at 2.5 Gbps FDX Autoneg",
> +	TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at 2.5 Gbps FDX Autoneg None",

"Link up at 2.5 Gbps FDX Autoneg Unknown"

>  		text, strlen(text), "Invalid default link status string");
> 
>  	link_status.link_duplex = RTE_ETH_LINK_HALF_DUPLEX;
>  	link_status.link_autoneg = RTE_ETH_LINK_FIXED;
>  	link_status.link_speed = RTE_ETH_SPEED_NUM_10M;
> +	link_status.link_type = RTE_ETH_LINK_TYPE_SGMII;
>  	ret = rte_eth_link_to_str(text, sizeof(text), &link_status);
>  	printf("Default link up #2: %s\n", text);
>  	RTE_TEST_ASSERT(ret > 0, "Failed to format default string\n");
> -	TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at 10 Mbps HDX Fixed",
> +	TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at 10 Mbps HDX Fixed SGMII",
>  		text, strlen(text), "Invalid default link status "
>  		"string with HDX");
> 
> @@ -41,7 +43,7 @@ test_link_status_up_default(void)
>  	ret = rte_eth_link_to_str(text, sizeof(text), &link_status);
>  	printf("Default link up #3: %s\n", text);
>  	RTE_TEST_ASSERT(ret > 0, "Failed to format default string\n");
> -	TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at Unknown HDX Fixed",
> +	TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at Unknown HDX Fixed SGMII",
>  		text, strlen(text), "Invalid default link status "
>  		"string with HDX");
> 
> @@ -49,7 +51,7 @@ test_link_status_up_default(void)
>  	ret = rte_eth_link_to_str(text, sizeof(text), &link_status);
>  	printf("Default link up #3: %s\n", text);
>  	RTE_TEST_ASSERT(ret > 0, "Failed to format default string\n");
> -	TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at None HDX Fixed",
> +	TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at None HDX Fixed SGMII",
>  		text, strlen(text), "Invalid default link status "
>  		"string with HDX");
> 
> @@ -57,6 +59,7 @@ test_link_status_up_default(void)
>  	link_status.link_speed = RTE_ETH_SPEED_NUM_400G;
>  	link_status.link_duplex = RTE_ETH_LINK_HALF_DUPLEX;
>  	link_status.link_autoneg = RTE_ETH_LINK_AUTONEG;
> +	link_status.link_type = RTE_ETH_LINK_TYPE_GAUI;
>  	ret = rte_eth_link_to_str(text, sizeof(text), &link_status);
>  	printf("Default link up #4:len = %d, %s\n", ret, text);
>  	RTE_TEST_ASSERT(ret < RTE_ETH_LINK_MAX_STR_LEN,
> @@ -92,7 +95,8 @@ test_link_status_invalid(void)
>  		.link_speed = 55555,
>  		.link_status = RTE_ETH_LINK_UP,
>  		.link_autoneg = RTE_ETH_LINK_AUTONEG,
> -		.link_duplex = RTE_ETH_LINK_FULL_DUPLEX
> +		.link_duplex = RTE_ETH_LINK_FULL_DUPLEX,
> +		.link_type = RTE_ETH_LINK_TYPE_UNKNOWN
>  	};
>  	char text[RTE_ETH_LINK_MAX_STR_LEN];
> 
> @@ -100,7 +104,7 @@ test_link_status_invalid(void)
>  	RTE_TEST_ASSERT(ret < RTE_ETH_LINK_MAX_STR_LEN,
>  		"Failed to format invalid string\n");
>  	printf("invalid link up #1: len=%d %s\n", ret, text);
> -	TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at Invalid FDX Autoneg",
> +	TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at Invalid FDX Autoneg Unknown",
>  		text, strlen(text), "Incorrect invalid link status string");
> 
>  	return TEST_SUCCESS;
> diff --git a/lib/ethdev/ethdev_trace.h b/lib/ethdev/ethdev_trace.h
> index 482befc209..c5cba0edba 100644
> --- a/lib/ethdev/ethdev_trace.h
> +++ b/lib/ethdev/ethdev_trace.h
> @@ -2110,6 +2110,13 @@ RTE_TRACE_POINT_FP(
>  	rte_trace_point_emit_string(ret);
>  )
> 
> +RTE_TRACE_POINT_FP(
> +	rte_eth_trace_link_type_to_str,
> +	RTE_TRACE_POINT_ARGS(uint8_t link_type, const char *ret),
> +	rte_trace_point_emit_u8(link_type);
> +	rte_trace_point_emit_string(ret);
> +);
> +
>  /* Called in loop in examples/bond and examples/ethtool */
>  RTE_TRACE_POINT_FP(
>  	rte_eth_trace_macaddr_get,
> diff --git a/lib/ethdev/ethdev_trace_points.c
> b/lib/ethdev/ethdev_trace_points.c
> index 071c508327..706acdf356 100644
> --- a/lib/ethdev/ethdev_trace_points.c
> +++ b/lib/ethdev/ethdev_trace_points.c
> @@ -206,6 +206,9 @@ RTE_TRACE_POINT_REGISTER(rte_eth_trace_link_speed_to_str,
>  RTE_TRACE_POINT_REGISTER(rte_eth_trace_link_to_str,
>  	lib.ethdev.link_to_str)
> 
> +RTE_TRACE_POINT_REGISTER(rte_eth_trace_link_type_to_str,
> +	lib.ethdev.link_type_to_str)
> +
>  RTE_TRACE_POINT_REGISTER(rte_eth_trace_stats_get,
>  	lib.ethdev.stats_get)
> 
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index dd7c00bc94..db496d70c4 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -3285,18 +3285,60 @@ rte_eth_link_to_str(char *str, size_t len, const
> struct rte_eth_link *eth_link)
>  	if (eth_link->link_status == RTE_ETH_LINK_DOWN)
>  		ret = snprintf(str, len, "Link down");
>  	else
> -		ret = snprintf(str, len, "Link up at %s %s %s",
> +		ret = snprintf(str, len, "Link up at %s %s %s %s",
>  			rte_eth_link_speed_to_str(eth_link->link_speed),
>  			(eth_link->link_duplex == RTE_ETH_LINK_FULL_DUPLEX) ?
>  			"FDX" : "HDX",
>  			(eth_link->link_autoneg == RTE_ETH_LINK_AUTONEG) ?
> -			"Autoneg" : "Fixed");
> +			"Autoneg" : "Fixed",
> +			rte_eth_link_type_to_str(eth_link->link_type));
> 
>  	rte_eth_trace_link_to_str(len, eth_link, str, ret);
> 
>  	return ret;
>  }
> 
> +RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_eth_link_type_to_str, 25.11)
> +const char *
> +rte_eth_link_type_to_str(enum rte_eth_link_type link_type)
> +{
> +	static const char * const link_type_str[] = {
> +		[RTE_ETH_LINK_TYPE_NONE] = "None",
> +		[RTE_ETH_LINK_TYPE_TP] = "Twisted Pair",
> +		[RTE_ETH_LINK_TYPE_AUI] = "Attachment Unit Interface",
> +		[RTE_ETH_LINK_TYPE_MII] = "Media Independent Interface",
> +		[RTE_ETH_LINK_TYPE_FIBER] = "Fiber",
> +		[RTE_ETH_LINK_TYPE_BNC] = "BNC",
> +		[RTE_ETH_LINK_TYPE_DA] = "Direct Attach Copper",

RTE_ETH_LINK_TYPE_DAC

> +		[RTE_ETH_LINK_TYPE_SGMII] = "SGMII",
> +		[RTE_ETH_LINK_TYPE_QSGMII] = "QSGMII",
> +		[RTE_ETH_LINK_TYPE_XFI] = "XFI",
> +		[RTE_ETH_LINK_TYPE_SFI] = "SFI",
> +		[RTE_ETH_LINK_TYPE_XLAUI] = "XLAUI",
> +		[RTE_ETH_LINK_TYPE_GAUI] = "GAUI",
> +		[RTE_ETH_LINK_TYPE_XAUI] = "XAUI",
> +		[RTE_ETH_LINK_TYPE_CAUI] = "CAUI",
> +		[RTE_ETH_LINK_TYPE_LAUI] = "LAUI",
> +		[RTE_ETH_LINK_TYPE_SFP] = "SFP",
> +		[RTE_ETH_LINK_TYPE_SFP_DD] = "SFP_DD",

"SFP-DD"

> +		[RTE_ETH_LINK_TYPE_SFP_PLUS] = "SFP_PLUS",

"SFP+"

> +		[RTE_ETH_LINK_TYPE_SFP28] = "SFP28",
> +		[RTE_ETH_LINK_TYPE_QSFP] = "QSFP",
> +		[RTE_ETH_LINK_TYPE_QSFP_PLUS] = "QSFP_PLUS",

"QSFP+"

> +		[RTE_ETH_LINK_TYPE_QSFP28] = "QSFP28",
> +		[RTE_ETH_LINK_TYPE_QSFP56] = "QSFP56",
> +		[RTE_ETH_LINK_TYPE_QSFP_DD] = "QSFP_DD",

"QSFP-DD"

> +		[RTE_ETH_LINK_TYPE_UNKNOWN] = "Unknown"
> +	};
> +	const char *str = NULL;
> +
> +	if ((link_type < RTE_DIM(link_type_str)) && link_type_str[link_type])
> +		str = link_type_str[link_type];
> +
> +	rte_eth_trace_link_type_to_str(link_type, str);
> +	return str;
> +}
> +
>  RTE_EXPORT_SYMBOL(rte_eth_stats_get)
>  int
>  rte_eth_stats_get(uint16_t port_id, struct rte_eth_stats *stats)
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index f9fb6ae549..92d1ded51f 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -329,6 +329,39 @@ struct rte_eth_stats {
>  #define RTE_ETH_SPEED_NUM_UNKNOWN UINT32_MAX /**< Unknown */
>  /**@}*/
> 
> +/**@{@name PORT type
> + * Ethernet port link type
> + */
> +enum rte_eth_link_type {
> +	RTE_ETH_LINK_TYPE_NONE = 0, /**< Not defined */
> +	RTE_ETH_LINK_TYPE_TP,       /**< Twisted Pair */
> +	RTE_ETH_LINK_TYPE_AUI,      /**< Attachment Unit Interface */
> +	RTE_ETH_LINK_TYPE_MII,      /**< Media Independent Interface */
> +	RTE_ETH_LINK_TYPE_FIBER,    /**< Optical Fiber Link */
> +	RTE_ETH_LINK_TYPE_BNC,      /**< BNC Link type for RF connection */
> +	RTE_ETH_LINK_TYPE_DA,       /**< Direct Attach copper */
> +	RTE_ETH_LINK_TYPE_SGMII,    /**< Serial Gigabit Media Independent
> Interface */
> +	RTE_ETH_LINK_TYPE_QSGMII,   /**< Link to multiplex 4 SGMII over one
> serial link */
> +	RTE_ETH_LINK_TYPE_XFI,      /**< 10 Gigabit Attachment Unit Interface */
> +	RTE_ETH_LINK_TYPE_SFI,      /**< 10 Gigabit Serial Interface for optical
> networking */
> +	RTE_ETH_LINK_TYPE_XLAUI,    /**< 40 Gigabit Attachment Unit Interface */
> +	RTE_ETH_LINK_TYPE_GAUI,     /**< Gigabit Interface for bandwidth like
> 50/100/200 Gbps */
> +	RTE_ETH_LINK_TYPE_XAUI,     /**< 10 Gigabit Attachment Unit Interface */
> +	RTE_ETH_LINK_TYPE_CAUI,     /**< 100 Gigabit Attachment Unit Interface
> */
> +	RTE_ETH_LINK_TYPE_LAUI,     /**< 50 Gigabit Attachment Unit Interface */
> +	RTE_ETH_LINK_TYPE_SFP,      /**< Pluggable module for 1 Gigabit */
> +	RTE_ETH_LINK_TYPE_SFP_PLUS, /**< Pluggable module for 10 Gigabit */
> +	RTE_ETH_LINK_TYPE_SFP28,    /**< Pluggable module for 25 Gigabit */
> +	RTE_ETH_LINK_TYPE_SFP_DD,   /**< Pluggable module for 100 Gigabit */
> +	RTE_ETH_LINK_TYPE_QSFP,     /**< Module to mutiplex 4 SFP i.e. 4*1=4
> Gigabit */
> +	RTE_ETH_LINK_TYPE_QSFP_PLUS,/**< Module to mutiplex 4 SFP_PLUS i.e.
> 4*10=40 Gigabit */
> +	RTE_ETH_LINK_TYPE_QSFP28,   /**< Module to mutiplex 4 SFP28 i.e.
> 4*25=100 Gigabit */
> +	RTE_ETH_LINK_TYPE_QSFP56,   /**< Module to mutiplex 4 SFP56 i.e.
> 4*50=200 Gigabit */
> +	RTE_ETH_LINK_TYPE_QSFP_DD,  /**< Module to mutiplex 4 SFP_DD i.e.
> 4*100=400 Gigabit */
> +	RTE_ETH_LINK_TYPE_UNKNOWN = 31, /**< Unknown Link Type */

For reference, we do have both RTE_ETH_SPEED_NUM_NONE and RTE_ETH_SPEED_NUM_UNKNOWN, where RTE_ETH_SPEED_NUM_NONE represents "link down, zero speed", and RTE_ETH_SPEED_NUM_UNKNOWN represents "link up, unknown speed".

But I doubt we need both RTE_ETH_LINK_TYPE_NONE and RTE_ETH_LINK_TYPE_UNKNOWN...
I agree that we need a value to represent "connector type unknown", but do we also need a value to represent "connector not present"? When can a port have no connector?

> +};
> +/**@}*/
> +
>  /**
>   * A structure used to retrieve link-level information of an Ethernet port.
>   */
> @@ -341,6 +374,7 @@ struct rte_eth_link {
>  			uint16_t link_duplex  : 1;  /**<
> RTE_ETH_LINK_[HALF/FULL]_DUPLEX */
>  			uint16_t link_autoneg : 1;  /**<
> RTE_ETH_LINK_[AUTONEG/FIXED] */
>  			uint16_t link_status  : 1;  /**< RTE_ETH_LINK_[DOWN/UP] */
> +			uint16_t link_type    : 5;  /**< RTE_ETH_LINK_TYPE_XXX */
>  		};
>  	};
>  };
> @@ -3116,6 +3150,20 @@ int rte_eth_link_get_nowait(uint16_t port_id, struct
> rte_eth_link *link)
>  __rte_experimental
>  const char *rte_eth_link_speed_to_str(uint32_t link_speed);
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * This function converts an Ethernet link type to a string.
> + *
> + * @param link_type
> + *   The link type to convert.
> + * @return
> + *   The string representation of the link type.
> + */
> +__rte_experimental
> +const char *rte_eth_link_type_to_str(enum rte_eth_link_type link_type);
> +
>  /**
>   * @warning
>   * @b EXPERIMENTAL: this API may change without prior notice.
> --
> 2.43.0


      reply	other threads:[~2025-08-14  9:04 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-05 11:31 [PATCH] " skori
2025-06-05 15:26 ` Stephen Hemminger
2025-06-06  9:28 ` [PATCH v2 1/1] " skori
2025-06-06  9:54   ` Morten Brørup
2025-06-06 15:23     ` Stephen Hemminger
2025-06-10  5:02       ` [EXTERNAL] " Sunil Kumar Kori
2025-06-10  6:45         ` Morten Brørup
2025-08-13  7:42           ` Sunil Kumar Kori
2025-08-13  8:42   ` [PATCH] " skori
2025-08-13  8:43   ` [PATCH v3 1/1] " skori
2025-08-13 10:25     ` Thomas Monjalon
2025-08-13 12:16       ` Ivan Malov
2025-08-13 14:17         ` Stephen Hemminger
2025-08-14  5:09       ` [EXTERNAL] " Sunil Kumar Kori
2025-08-13 15:04     ` Stephen Hemminger
2025-08-14  8:10     ` [PATCH v4 " skori
2025-08-14  9:04       ` Morten Brørup [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=98CBD80474FA8B44BF855DF32C47DC35E9FE35@smartserver.smartshare.dk \
    --to=mb@smartsharesystems.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=ndabilpuram@marvell.com \
    --cc=skori@marvell.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).