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

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

> 13/08/2025 10:43, skori@marvell.com:
> > + * Ethernet port type
> 
> You mean "link port type"
> 
Ack.

> > + */
> > +#define RTE_ETH_LINK_TYPE_NONE     0  /**< Not defined */
> > +#define RTE_ETH_LINK_TYPE_TP       1  /**< Twisted Pair */
> > +#define RTE_ETH_LINK_TYPE_AUI      2  /**< Attachment Unit Interface */
> > +#define RTE_ETH_LINK_TYPE_MII      3  /**< Media Independent Interface */
> > +#define RTE_ETH_LINK_TYPE_FIBRE    4  /**< Fibre */
> 
> In general we use the US word "fiber",
> but we are not very consistent, so it is not a strong opinion.
> 
Ack. I will change it.

> > +#define RTE_ETH_LINK_TYPE_BNC      5  /**< BNC */
> > +#define RTE_ETH_LINK_TYPE_DA       6  /**< Direct Attach copper */
> > +#define RTE_ETH_LINK_TYPE_SGMII    7  /**< SGMII */
> > +#define RTE_ETH_LINK_TYPE_QSGMII   8  /**< QSGMII */
> > +#define RTE_ETH_LINK_TYPE_XFI      9  /**< XFI */
> > +#define RTE_ETH_LINK_TYPE_SFI     10  /**< SFI */
> > +#define RTE_ETH_LINK_TYPE_XLAUI   11  /**< XLAUI */
> > +#define RTE_ETH_LINK_TYPE_GAUI    12  /**< GAUI */
> > +#define RTE_ETH_LINK_TYPE_XAUI    13  /**< XAUI */
> > +#define RTE_ETH_LINK_TYPE_GBASE   14  /**< GBASE */
> > +#define RTE_ETH_LINK_TYPE_CAUI    15  /**< CAUI */
> > +#define RTE_ETH_LINK_TYPE_LAUI    16  /**< LAUI */
> > +#define RTE_ETH_LINK_TYPE_SFP     17  /**< SFP */
> > +#define RTE_ETH_LINK_TYPE_SFP_DD  18  /**< SFP_DD */
> 
> You should use more full words in comments, at least for DD.
I thought that these are standard and well known. So didn't add but there is no harm in adding comments. I will add that.

> 
> > +#define RTE_ETH_LINK_TYPE_SFP_PLUS 19  /**< SFP_PLUS */
> 
> Please add more spaces to allow a correct alignment.
> 
Ack.

> > +#define RTE_ETH_LINK_TYPE_SFP28   20  /**< SFP28 */
> > +#define RTE_ETH_LINK_TYPE_QSFP    21  /**< QSFP */
> > +#define RTE_ETH_LINK_TYPE_QSFP_PLUS 22  /**< QSFP_PLUS */ #define
> > +RTE_ETH_LINK_TYPE_QSFP28  23  /**< QSFP28 */ #define
> > +RTE_ETH_LINK_TYPE_QSFP56  24  /**< QSFP56 */ #define
> > +RTE_ETH_LINK_TYPE_QSFP_DD 25  /**< QSFP_DD */ #define
> > +RTE_ETH_LINK_TYPE_OTHER  0x1F /**< Other type */
> 
> Why the last one is in hexadecimal? and why 1F?
> 
> Is there a logic in the order and numbering for this list?
> 
> Why not using an enum?
> 
- As struct rte_eth_link:: link_type is a 5 bits field. Hence used the last value as other/unknown link type. We can change it to decimal value.
- I have no reasoning for numbering.
- Yes, It be kept in enum. No strong opinion on this.

> Thanks
> 
> 
> 
>