> >>
> >> +/**
> >> + * 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.anan...@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)
> >> +
> >> +

Reply via email to