RE: [EXTERNAL] Re: [PATCH v7 1/1] ethdev: add support to provide link type

[Sunil Kumar Kori <skori@marvell.com>]

> On Wed, 20 Aug 2025 23:22:31 +0530
> <skori@marvell.com> wrote:
> 
> Patch does not apply cleanly to next-net, there is fuzz.
> 

Ack, I will rebase it on next-net.

> > diff --git a/lib/ethdev/ethdev_trace.h b/lib/ethdev/ethdev_trace.h
> > index 482befc209..bc6c3a4a02 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_connector_to_str,
> > +	RTE_TRACE_POINT_ARGS(uint8_t link_connector, const char *ret),
> > +	rte_trace_point_emit_u8(link_connector);
> > +	rte_trace_point_emit_string(ret);
> > +);
> > +
> 
> IMHO adding a trace point to a helper function like this is just extra clutter,
> remove it.
> 
Ack, I will share a separate patch to add trace in this API.

> > +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))) &&
> > +		link_connector_str[link_connector])
> > +		str = link_connector_str[link_connector];
> 
> This can be simplified to:
> 	if (link_connector < RTE_DIM(link_connector_str))
> 		str = link_connector_str[link_connector];
> 
> Since enum's act like int in C, no need for all that casting.
> 
Correct and In previous version it was like this only but on Windows machine, it was throwing error. Hence added typecasting.

> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * This function converts an Ethernet link type to a string.
> > + *
> > + * @param link_connector
> > + *   The link type to convert.
> > + * @return
> > + *   The string representation of the link type.
> > + */
> > +__rte_experimental
> > +const char *rte_eth_link_connector_to_str(enum rte_eth_link_connector
> > +link_connector);
> 
> Need to document that it returns NULL for invalid link_connector values.
Ack.