Re: [dpdk-dev] [PATCH 1/2] eal: add static endianness conversion macros

[Thomas Monjalon <thomas@monjalon.net>]

Hi, some comments below:

18/05/2017 12:14, Adrien Mazarguil:
> These macros resolve to constant expressions that allow developers to
> perform endianness conversion on static/const objects, even outside of
> function scope as they do not translate to function calls.
> 
> This is most useful for static initializers and constant values (whenever
> it has to be performed at compilation time). Run-time endianness conversion
> of variable values should keep using rte_*_to_*() calls for best
> performance.
> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
[...]
> +#define RTE_STATIC_BSWAP64(v) \
> +	((((uint64_t)(v) & UINT64_C(0x00000000000000ff)) << 56) | \
> +	 (((uint64_t)(v) & UINT64_C(0x000000000000ff00)) << 40) | \
> +	 (((uint64_t)(v) & UINT64_C(0x0000000000ff0000)) << 24) | \
> +	 (((uint64_t)(v) & UINT64_C(0x00000000ff000000)) << 8) | \
> +	 (((uint64_t)(v) & UINT64_C(0x000000ff00000000)) >> 8) | \
> +	 (((uint64_t)(v) & UINT64_C(0x0000ff0000000000)) >> 24) | \
> +	 (((uint64_t)(v) & UINT64_C(0x00ff000000000000)) >> 40) | \
> +	 (((uint64_t)(v) & UINT64_C(0xff00000000000000)) >> 56))

Minor nit: you could align lines by inserting a space before 8.

> +#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +#define RTE_BE16(v) (uint16_t)(v)
> +#define RTE_BE32(v) (uint32_t)(v)
> +#define RTE_BE64(v) (uint64_t)(v)
> +#define RTE_LE16(v) RTE_STATIC_BSWAP16(v)
> +#define RTE_LE32(v) RTE_STATIC_BSWAP32(v)
> +#define RTE_LE64(v) RTE_STATIC_BSWAP64(v)
> +#elif RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +#define RTE_BE16(v) RTE_STATIC_BSWAP16(v)
> +#define RTE_BE32(v) RTE_STATIC_BSWAP32(v)
> +#define RTE_BE64(v) RTE_STATIC_BSWAP64(v)
> +#define RTE_LE16(v) (uint16_t)(v)
> +#define RTE_LE32(v) (uint32_t)(v)
> +#define RTE_LE64(v) (uint64_t)(v)

This naming is confusing.
Let's take RTE_BE16() as example, it does not say wether the input value
is big endian or the output value will be big endian.
I think we should mimic the wording of run-time conversions:
	RTE_BE_TO_CPU_16()

Any other ideas?