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

[Ivan Malov <ivan.malov@arknetworks.am>]

Hi Thomas,

On Wed, 13 Aug 2025, Thomas Monjalon wrote:

> 13/08/2025 10:43, skori@marvell.com:
>> + * Ethernet port type
>
> You mean "link port type"
>
>> + */
>> +#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.

To me, "fibre" reads more natural. Not a strong opinion either.

>
>> +#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.

+1

>
>> +#define RTE_ETH_LINK_TYPE_SFP_PLUS 19  /**< SFP_PLUS */
>
> Please add more spaces to allow a correct alignment.
>
>> +#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?

I take it this is just maximum value for 'uint16_t link_type    : 5;'.

>
> Is there a logic in the order and numbering for this list?
>
> Why not using an enum?

May be RTE_LEN2MASK() or something?

Thank you.

>
> Thanks
>
>
>
>
>
>