[dpdk-dev] [V5 1/3] ethdev: introduce FEC API

[Ananyev, Konstantin]

> >>
> >> +/**
> >> + * This enum indicates the possible (forward error correction)FEC modes
> >> + * of an ethdev port.
> >> + */
> >> +enum rte_fec_mode {
> >> +	RTE_ETH_FEC_NOFEC = 0,      /**< FEC is off */
> >> +	RTE_ETH_FEC_BASER,          /**< FEC using common algorithm */
> >> +	RTE_ETH_FEC_RS,             /**< FEC using RS algorithm */
> >> +	RTE_ETH_FEC_AUTO,           /**< FEC autonegotiation modes */
> >> +	RTE_ETH_FEC_NUM
> >
> > It is better not to have RTE_ETH_FEC_NUM here:
> > Any future additions to that enum would overwrite _NUM values and would
> > considered as ABI breakage.
> > Aprart from that:
> > Acked-by: Konstantin Ananyev <konstantin.ananyev at intel.com>
> >
> HI,
>        it does not matter even though RTE_ETH_FEC_NUM value is changed
> when adding new element to that enum. RTE_ETH_FEC_NUM is used as the MAX
> vlaue of FEC modes, not one FEC mode itself.

I understand that, but when in future you'll try to add some new mode,
it will cause a change of RTE_ETH_FEC_NUM value.
As I remember, abicheck will report it as ABI breakage.
So adding new values to that enum will be really problematic.

>        One of the application scenarios is as follows,set "testpmd" as
> an example:
> show_fec_capability(uint32_t fec_cap)
> {
> 	uint32_t i;
> 
> 	if (fec_cap == 0) {
> 		printf("FEC is not supported\n");
> 		return;
> 	}
> 
> 	printf("FEC capabilities: ");
> 	for (i = RTE_ETH_FEC_BASER; i < RTE_ETH_FEC_NUM; i++) {

I think you can use RTE_DIM(fec_mode_name) here instead of RTE_ETH_FEC_NUM.

> 		if (fec_cap & 1U << i)

BTW, one more thing - as you translate from mode to capa all over the place,
I think it deserves a separate macro for it.
Something like:
#define RTE_ETH_FEC_MODE_TO_CAPA(x)	(1U << (x))

> 			printf("%s ", fec_mode_name[i].name);
> 	}
> 	printf("\n");
> }
> 
> Hope for your reply.
> 
> >> +};
> >> +
> >> +/* This indicates FEC capabilities */
> >> +#define RTE_ETH_FEC_CAPA_NOFEC  (1U << RTE_ETH_FEC_NOFEC)
> >> +#define RTE_ETH_FEC_CAPA_BASER  (1U << RTE_ETH_FEC_BASER)
> >> +#define RTE_ETH_FEC_CAPA_RS     (1U << RTE_ETH_FEC_RS)
> >> +#define RTE_ETH_FEC_CAPA_AUTO   (1U << RTE_ETH_FEC_AUTO)
> >> +
> >> +