Re: [PATCH v10 1/1] ethdev: add support to provide link type

[Thomas Monjalon <thomas@monjalon.net>]

Hello,

The title can be simply "ethdev: add link type".


08/09/2025 10:51, skori@marvell.com:
> +* **Added ethdev API in library.*

Please be more specific, we don't need to group all ethdev changes together.
Each new API feature deserves a separate bullet.

> +
> +  * Added API to report type of link connection for a port.
> +    By default, it reports ``RTE_ETH_LINK_CONNECTOR_NONE``
> +    unless driver specifies it.

I think you need to explain what is a type (copper, fiber, etc).

[...]
> +RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_eth_link_connector_to_str, 25.11)
> +const char *
> +rte_eth_link_connector_to_str(enum rte_eth_link_connector link_connector)
> +{
> +	static const char * const link_connector_str[] = {
> +		[RTE_ETH_LINK_CONNECTOR_NONE] = "None",
> +		[RTE_ETH_LINK_CONNECTOR_TP] = "Twisted Pair",
> +		[RTE_ETH_LINK_CONNECTOR_AUI] = "Attachment Unit Interface",
> +		[RTE_ETH_LINK_CONNECTOR_MII] = "Media Independent Interface",
> +		[RTE_ETH_LINK_CONNECTOR_FIBER] = "Fiber",
> +		[RTE_ETH_LINK_CONNECTOR_BNC] = "BNC",
> +		[RTE_ETH_LINK_CONNECTOR_DAC] = "Direct Attach Copper",
> +		[RTE_ETH_LINK_CONNECTOR_SGMII] = "SGMII",
> +		[RTE_ETH_LINK_CONNECTOR_QSGMII] = "QSGMII",
> +		[RTE_ETH_LINK_CONNECTOR_XFI] = "XFI",
> +		[RTE_ETH_LINK_CONNECTOR_SFI] = "SFI",
> +		[RTE_ETH_LINK_CONNECTOR_XLAUI] = "XLAUI",
> +		[RTE_ETH_LINK_CONNECTOR_GAUI] = "GAUI",
> +		[RTE_ETH_LINK_CONNECTOR_XAUI] = "XAUI",
> +		[RTE_ETH_LINK_CONNECTOR_CAUI] = "CAUI",
> +		[RTE_ETH_LINK_CONNECTOR_LAUI] = "LAUI",
> +		[RTE_ETH_LINK_CONNECTOR_SFP] = "SFP",
> +		[RTE_ETH_LINK_CONNECTOR_SFP_DD] = "SFP-DD",
> +		[RTE_ETH_LINK_CONNECTOR_SFP_PLUS] = "SFP+",
> +		[RTE_ETH_LINK_CONNECTOR_SFP28] = "SFP28",
> +		[RTE_ETH_LINK_CONNECTOR_QSFP] = "QSFP",
> +		[RTE_ETH_LINK_CONNECTOR_QSFP_PLUS] = "QSFP+",
> +		[RTE_ETH_LINK_CONNECTOR_QSFP28] = "QSFP28",
> +		[RTE_ETH_LINK_CONNECTOR_QSFP56] = "QSFP56",
> +		[RTE_ETH_LINK_CONNECTOR_QSFP_DD] = "QSFP-DD",
> +		[RTE_ETH_LINK_CONNECTOR_OTHER] = "Other",
> +	};
> +	const char *str = NULL;
> +
> +	if (link_connector < ((enum rte_eth_link_connector)RTE_DIM(link_connector_str)))
> +		str = link_connector_str[link_connector];
> +
> +	return str;
> +}

So we say link connector, not link type as in the title and descriptions?

[...]
> +enum rte_eth_link_connector {
> +	RTE_ETH_LINK_CONNECTOR_NONE = 0,     /**< Not defined */
> +	RTE_ETH_LINK_CONNECTOR_TP,           /**< Twisted Pair */
> +	RTE_ETH_LINK_CONNECTOR_AUI,          /**< Attachment Unit Interface */
> +	RTE_ETH_LINK_CONNECTOR_MII,          /**< Media Independent Interface */
> +	RTE_ETH_LINK_CONNECTOR_FIBER,        /**< Optical Fiber Link */
> +	RTE_ETH_LINK_CONNECTOR_BNC,          /**< BNC Link type for RF connection */
> +	RTE_ETH_LINK_CONNECTOR_DAC,          /**< Direct Attach copper */
> +	RTE_ETH_LINK_CONNECTOR_SGMII,        /**< Serial Gigabit Media Independent Interface */
> +	RTE_ETH_LINK_CONNECTOR_QSGMII,       /**< Link to multiplex 4 SGMII over one serial link */
> +	RTE_ETH_LINK_CONNECTOR_XFI,          /**< 10 Gigabit Attachment Unit Interface */
> +	RTE_ETH_LINK_CONNECTOR_SFI,          /**< 10 Gigabit Serial Interface for optical network */
> +	RTE_ETH_LINK_CONNECTOR_XLAUI,        /**< 40 Gigabit Attachment Unit Interface */
> +	RTE_ETH_LINK_CONNECTOR_GAUI,         /**< Gigabit Interface for 50/100/200 Gbps */
> +	RTE_ETH_LINK_CONNECTOR_XAUI,         /**< 10 Gigabit Attachment Unit Interface */
> +	RTE_ETH_LINK_CONNECTOR_CAUI,         /**< 100 Gigabit Attachment Unit Interface */
> +	RTE_ETH_LINK_CONNECTOR_LAUI,         /**< 50 Gigabit Attachment Unit Interface */
> +	RTE_ETH_LINK_CONNECTOR_SFP,          /**< Pluggable module for 1 Gigabit */
> +	RTE_ETH_LINK_CONNECTOR_SFP_PLUS,     /**< Pluggable module for 10 Gigabit */
> +	RTE_ETH_LINK_CONNECTOR_SFP28,        /**< Pluggable module for 25 Gigabit */
> +	RTE_ETH_LINK_CONNECTOR_SFP_DD,       /**< Pluggable module for 100 Gigabit */
> +	RTE_ETH_LINK_CONNECTOR_QSFP,         /**< Module to mutiplex 4 SFP i.e. 4*1=4 Gbps */
> +	RTE_ETH_LINK_CONNECTOR_QSFP_PLUS,    /**< Module to mutiplex 4 SFP_PLUS i.e. 4*10=40 Gbps */
> +	RTE_ETH_LINK_CONNECTOR_QSFP28,       /**< Module to mutiplex 4 SFP28 i.e. 4*25=100 Gbps */
> +	RTE_ETH_LINK_CONNECTOR_QSFP56,       /**< Module to mutiplex 4 SFP56 i.e. 4*50=200 Gbps */
> +	RTE_ETH_LINK_CONNECTOR_QSFP_DD,      /**< Module to mutiplex 4 SFP_DD i.e. 4*100=400 Gbps */
> +	RTE_ETH_LINK_CONNECTOR_OTHER = 31,   /**< non-physical interfaces like virtio, ring etc.
> +					       * It also includes unknown connector types,
> +					       * i.e. physical connectors not yet defined in this
> +					       * list of connector types.

It is confusing whether we use NONE or OTHER for unknown types.