RE: [PATCH v4 1/1] ethdev: add support to provide link type

[Sunil Kumar Kori <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?
> 

My thought process is towards the physical interfaces only. Hence considering your input to rename as "connector."
Also, for non-physical interfaces, Keeping RTE_ETH_LINK_TYPE_OTHER along with RTE_ETH_LINK_TYPE_UNKNOWN. Similar as provided by Ethtool.

> 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
Ack.

> 
> >  	};
> >  	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"
Ack.
> 
> >  		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
Ack.

> 
> > +		[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"
> 
Ack.
> > +		[RTE_ETH_LINK_TYPE_SFP_PLUS] = "SFP_PLUS",
> 
> "SFP+"
> 
Ack.
> > +		[RTE_ETH_LINK_TYPE_SFP28] = "SFP28",
> > +		[RTE_ETH_LINK_TYPE_QSFP] = "QSFP",
> > +		[RTE_ETH_LINK_TYPE_QSFP_PLUS] = "QSFP_PLUS",
> 
> "QSFP+"
> 
Ack.
> > +		[RTE_ETH_LINK_TYPE_QSFP28] = "QSFP28",
> > +		[RTE_ETH_LINK_TYPE_QSFP56] = "QSFP56",
> > +		[RTE_ETH_LINK_TYPE_QSFP_DD] = "QSFP_DD",
> 
> "QSFP-DD"
> 
Ack.

> > +		[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