Re: [dpdk-dev] [PATCH v1 3/3] ethdev: enhance the API for getting burst mode information

[Thomas Monjalon <thomas@monjalon.net>]

Thank you for trying to address comments done late.

31/10/2019 18:11, Haiyue Wang:
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> -enum rte_eth_burst_mode_option {
> -	RTE_ETH_BURST_SCALAR = (1 << 0),
> -	RTE_ETH_BURST_VECTOR = (1 << 1),
> -
> -	/**< bits[15:2] are reserved for each vector type */
> -	RTE_ETH_BURST_ALTIVEC = (1 << 2),
> -	RTE_ETH_BURST_NEON = (1 << 3),
> -	RTE_ETH_BURST_SSE = (1 << 4),
> -	RTE_ETH_BURST_AVX2 = (1 << 5),
> -	RTE_ETH_BURST_AVX512 = (1 << 6),
> -
> -	RTE_ETH_BURST_SCATTERED = (1 << 16), /**< Support scattered packets */
> -	RTE_ETH_BURST_BULK_ALLOC = (1 << 17), /**< Support mbuf bulk alloc */
> -	RTE_ETH_BURST_SIMPLE = (1 << 18),
> -
> -	RTE_ETH_BURST_PER_QUEUE = (1 << 19), /**< Support per queue burst */
> -};
> +#define RTE_ETH_BURST_SCALAR        (1ULL << 0)
> +#define RTE_ETH_BURST_VECTOR        (1ULL << 1)

Only one bit is needed: if it is not vector, it is scalar.
I think you can remove the scalar bit.

> +/**< bits[15:2] are reserved for each vector type */

Why 15:2 instead of 2:15?

> +#define RTE_ETH_BURST_ALTIVEC       (1ULL << 2)
> +#define RTE_ETH_BURST_NEON          (1ULL << 3)
> +#define RTE_ETH_BURST_SSE           (1ULL << 4)
> +#define RTE_ETH_BURST_AVX2          (1ULL << 5)
> +#define RTE_ETH_BURST_AVX512        (1ULL << 6)

Of course, I still believe that giving a special treatment
to vector instructions is wrong.
You did not justify why it needs to be defined in bits
instead of string. I am not asking again because anyway you
don't really reply. I think you are executing an order you received
and I don't want to blame you more.
I suspect a real hidden issue in Intel CPUs that you try to mitigate.
No need to reply to this comment.
Anyway I will propose to replace this API in the next release.

> +/**< Support per queue burst */
> +#define RTE_ETH_BURST_PER_QUEUE     (1ULL << 63)

This comment is meaningless.
If this bit has a usage, please explain how to use this bit
in the comment.