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

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

[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>
---
 .../common/include/generic/rte_byteorder.h      | 58 +++++++++++++++-----
 1 file changed, 44 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/common/include/generic/rte_byteorder.h b/lib/librte_eal/common/include/generic/rte_byteorder.h
index e00bccb..8408872 100644
--- a/lib/librte_eal/common/include/generic/rte_byteorder.h
+++ b/lib/librte_eal/common/include/generic/rte_byteorder.h
@@ -74,6 +74,47 @@
 #elif defined __LITTLE_ENDIAN__
 #define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
 #endif
+#if !defined(RTE_BYTE_ORDER)
+#error Unknown endianness.
+#endif
+
+#define RTE_STATIC_BSWAP16(v) \
+	((((uint16_t)(v) & UINT16_C(0x00ff)) << 8) | \
+	 (((uint16_t)(v) & UINT16_C(0xff00)) >> 8))
+
+#define RTE_STATIC_BSWAP32(v) \
+	((((uint32_t)(v) & UINT32_C(0x000000ff)) << 24) | \
+	 (((uint32_t)(v) & UINT32_C(0x0000ff00)) << 8) | \
+	 (((uint32_t)(v) & UINT32_C(0x00ff0000)) >> 8) | \
+	 (((uint32_t)(v) & UINT32_C(0xff000000)) >> 24))
+
+#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))
+
+#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)
+#else
+#error Unsupported endianness.
+#endif
 
 /*
  * An internal function to swap bytes in a 16-bit value.
@@ -84,8 +125,7 @@
 static inline uint16_t
 rte_constant_bswap16(uint16_t x)
 {
-	return (uint16_t)(((x & 0x00ffU) << 8) |
-		((x & 0xff00U) >> 8));
+	return RTE_STATIC_BSWAP16(x);
 }
 
 /*
@@ -97,10 +137,7 @@ rte_constant_bswap16(uint16_t x)
 static inline uint32_t
 rte_constant_bswap32(uint32_t x)
 {
-	return  ((x & 0x000000ffUL) << 24) |
-		((x & 0x0000ff00UL) << 8) |
-		((x & 0x00ff0000UL) >> 8) |
-		((x & 0xff000000UL) >> 24);
+	return RTE_STATIC_BSWAP32(x);
 }
 
 /*
@@ -112,14 +149,7 @@ rte_constant_bswap32(uint32_t x)
 static inline uint64_t
 rte_constant_bswap64(uint64_t x)
 {
-	return  ((x & 0x00000000000000ffULL) << 56) |
-		((x & 0x000000000000ff00ULL) << 40) |
-		((x & 0x0000000000ff0000ULL) << 24) |
-		((x & 0x00000000ff000000ULL) <<  8) |
-		((x & 0x000000ff00000000ULL) >>  8) |
-		((x & 0x0000ff0000000000ULL) >> 24) |
-		((x & 0x00ff000000000000ULL) >> 40) |
-		((x & 0xff00000000000000ULL) >> 56);
+	return RTE_STATIC_BSWAP64(x);
 }
 
 
-- 
2.1.4

[dpdk-dev] [PATCH 2/2] ethdev: tidy up endianness handling in flow API

[Adrien Mazarguil]

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
 lib/librte_ether/rte_flow.h | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
index c47edbc..02dcfe7 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -624,13 +624,7 @@ struct rte_flow_item_e_tag {
 /** Default mask for RTE_FLOW_ITEM_TYPE_E_TAG. */
 #ifndef __cplusplus
 static const struct rte_flow_item_e_tag rte_flow_item_e_tag_mask = {
-#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
-	.rsvd_grp_ecid_b = 0x3fff,
-#elif RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
-	.rsvd_grp_ecid_b = 0xff3f,
-#else
-#error Unsupported endianness.
-#endif
+	.rsvd_grp_ecid_b = RTE_BE16(0x3fff),
 };
 #endif
 
-- 
2.1.4

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

[Thomas Monjalon]

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?

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

[Adrien Mazarguil]

On Wed, Jun 07, 2017 at 04:16:58PM +0200, Thomas Monjalon wrote:
> 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.

I think alignment attempts past the mandatory line indentation often end up
in a failure (e.g. when grouping macros by name, one of them inevitably
happens to be longer than initially envisioned, same for structure fields
and trailing comment blocks, etc.) Since I'm not convinced it improves
readability, I tend to avoid them altogether for consistency.

It's a matter of style but I can change that if you prefer.

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

First I'd like to keep those macro names as short as possible, ideally not
much larger than simply casting the provided value to the target type for
usability and readability purposes. Think about files full of static
initializers, while there are not many examples right now, the definition of
static rte_flow rules and capability trees will need to use these macros
extensively.

The fact you suggested RTE_BE_TO_CPU_16() instead of RTE_CPU_TO_BE_16() as a
replacement for RTE_BE16() highlights the misunderstanding. However I find
"CPU_TO" overly verbose, particularly since the reverse macros won't exist,
remember these are made for static conversions of integer constants resolved
at compilation time, not variables. Users may additionally confuse
RTE_CPU_TO_BE_16() with its similarly-named inline function counterpart.

Functions and macros are typically named after their output, not their
input. In that sense and without further precision, RTE_BE16() is fine in my
opinion.

Remember this [1]? I think we could make everything clearer by perhaps
applying it and casting the results of these macros to the proper type,
e.g.:

 #define RTE_BE16(v) (rte_be16_t)(v)

I can probably modify this series to introduce the new types first, use them
in the conversion macro and then later clarify existing structure
fields. How about this?

[1] http://dpdk.org/ml/archives/dev/2016-November/050060.html

-- 
Adrien Mazarguil
6WIND

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

[Thomas Monjalon]

08/06/2017 11:14, Adrien Mazarguil:
> On Wed, Jun 07, 2017 at 04:16:58PM +0200, Thomas Monjalon wrote:
> > Hi, some comments below:
> > 
> > 18/05/2017 12:14, Adrien Mazarguil:
> > > +#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.
> 
> I think alignment attempts past the mandatory line indentation often end up
> in a failure (e.g. when grouping macros by name, one of them inevitably
> happens to be longer than initially envisioned, same for structure fields
> and trailing comment blocks, etc.) Since I'm not convinced it improves
> readability, I tend to avoid them altogether for consistency.

I agree
Here it is just adding a space in front of the single digit to make
bits numbers aligned on 2 digits :)

> It's a matter of style but I can change that if you prefer.
> 
> > > +#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?
> 
> First I'd like to keep those macro names as short as possible, ideally not
> much larger than simply casting the provided value to the target type for
> usability and readability purposes. Think about files full of static
> initializers, while there are not many examples right now, the definition of
> static rte_flow rules and capability trees will need to use these macros
> extensively.
> 
> The fact you suggested RTE_BE_TO_CPU_16() instead of RTE_CPU_TO_BE_16() as a
> replacement for RTE_BE16() highlights the misunderstanding. However I find
> "CPU_TO" overly verbose, particularly since the reverse macros won't exist,
> remember these are made for static conversions of integer constants resolved
> at compilation time, not variables. Users may additionally confuse
> RTE_CPU_TO_BE_16() with its similarly-named inline function counterpart.

You're right.
RTE_BE_TO_CPU_16 does not make sense.
I think you could add a comment like that:
RTE_XE_NN is equivalent to rte_cpu_to_Xe_NN run-time conversion

> Functions and macros are typically named after their output, not their
> input. In that sense and without further precision, RTE_BE16() is fine in my
> opinion.

Good point.

> Remember this [1]? I think we could make everything clearer by perhaps
> applying it and casting the results of these macros to the proper type,
> e.g.:
> 
>  #define RTE_BE16(v) (rte_be16_t)(v)
> 
> I can probably modify this series to introduce the new types first, use them
> in the conversion macro and then later clarify existing structure
> fields. How about this?

Yes good idea.

[dpdk-dev] [PATCH v2 1/3] eal: introduce big and little endian types

[Adrien Mazarguil]

From: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

This commit introduces new rte_{le,be}{16,32,64}_t types and updates
rte_{le,be,cpu}_to_{le,be,cpu}_*() accordingly.

These types are added for documentation purposes, mainly to clarify the
byte ordering to use for storage when not CPU order. Doing so eliminates
uncertainty and conversion mistakes.

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

---

This is a rehash of Nelio's original patch [1] that drops all changes to
protocol header structures and only focuses on adding these new types for
documentation purposes.

[1] http://dpdk.org/ml/archives/dev/2016-November/050060.html
---
 .../common/include/generic/rte_byteorder.h      | 38 +++++++++++++-------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/lib/librte_eal/common/include/generic/rte_byteorder.h b/lib/librte_eal/common/include/generic/rte_byteorder.h
index e00bccb..df8cc84 100644
--- a/lib/librte_eal/common/include/generic/rte_byteorder.h
+++ b/lib/librte_eal/common/include/generic/rte_byteorder.h
@@ -76,6 +76,20 @@
 #endif
 
 /*
+ * The following types should be used when handling values according to a
+ * specific byte ordering, which may differ from that of the host CPU.
+ *
+ * Libraries, public APIs and applications are encouraged to use them for
+ * documentation purposes.
+ */
+typedef uint16_t rte_be16_t; /**< 16-bit big-endian value. */
+typedef uint32_t rte_be32_t; /**< 32-bit big-endian value. */
+typedef uint64_t rte_be64_t; /**< 64-bit big-endian value. */
+typedef uint16_t rte_le16_t; /**< 16-bit little-endian value. */
+typedef uint32_t rte_le32_t; /**< 32-bit little-endian value. */
+typedef uint64_t rte_le64_t; /**< 64-bit little-endian value. */
+
+/*
  * An internal function to swap bytes in a 16-bit value.
  *
  * It is used by rte_bswap16() when the value is constant. Do not use
@@ -143,65 +157,65 @@ static uint64_t rte_bswap64(uint64_t x);
 /**
  * Convert a 16-bit value from CPU order to little endian.
  */
-static uint16_t rte_cpu_to_le_16(uint16_t x);
+static rte_le16_t rte_cpu_to_le_16(uint16_t x);
 
 /**
  * Convert a 32-bit value from CPU order to little endian.
  */
-static uint32_t rte_cpu_to_le_32(uint32_t x);
+static rte_le32_t rte_cpu_to_le_32(uint32_t x);
 
 /**
  * Convert a 64-bit value from CPU order to little endian.
  */
-static uint64_t rte_cpu_to_le_64(uint64_t x);
+static rte_le64_t rte_cpu_to_le_64(uint64_t x);
 
 
 /**
  * Convert a 16-bit value from CPU order to big endian.
  */
-static uint16_t rte_cpu_to_be_16(uint16_t x);
+static rte_be16_t rte_cpu_to_be_16(uint16_t x);
 
 /**
  * Convert a 32-bit value from CPU order to big endian.
  */
-static uint32_t rte_cpu_to_be_32(uint32_t x);
+static rte_be32_t rte_cpu_to_be_32(uint32_t x);
 
 /**
  * Convert a 64-bit value from CPU order to big endian.
  */
-static uint64_t rte_cpu_to_be_64(uint64_t x);
+static rte_be64_t rte_cpu_to_be_64(uint64_t x);
 
 
 /**
  * Convert a 16-bit value from little endian to CPU order.
  */
-static uint16_t rte_le_to_cpu_16(uint16_t x);
+static uint16_t rte_le_to_cpu_16(rte_le16_t x);
 
 /**
  * Convert a 32-bit value from little endian to CPU order.
  */
-static uint32_t rte_le_to_cpu_32(uint32_t x);
+static uint32_t rte_le_to_cpu_32(rte_le32_t x);
 
 /**
  * Convert a 64-bit value from little endian to CPU order.
  */
-static uint64_t rte_le_to_cpu_64(uint64_t x);
+static uint64_t rte_le_to_cpu_64(rte_le64_t x);
 
 
 /**
  * Convert a 16-bit value from big endian to CPU order.
  */
-static uint16_t rte_be_to_cpu_16(uint16_t x);
+static uint16_t rte_be_to_cpu_16(rte_be16_t x);
 
 /**
  * Convert a 32-bit value from big endian to CPU order.
  */
-static uint32_t rte_be_to_cpu_32(uint32_t x);
+static uint32_t rte_be_to_cpu_32(rte_be32_t x);
 
 /**
  * Convert a 64-bit value from big endian to CPU order.
  */
-static uint64_t rte_be_to_cpu_64(uint64_t x);
+static uint64_t rte_be_to_cpu_64(rte_be64_t x);
 
 #endif /* __DOXYGEN__ */
 
-- 
2.1.4

Re: [dpdk-dev] [PATCH v2 1/3] eal: introduce big and little endian types

[Thomas Monjalon]

Series applied, thanks

[dpdk-dev] [PATCH v2 2/3] eal: add static endianness conversion macros

[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>
---
 .../common/include/generic/rte_byteorder.h      | 70 ++++++++++++++++----
 1 file changed, 56 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/common/include/generic/rte_byteorder.h b/lib/librte_eal/common/include/generic/rte_byteorder.h
index df8cc84..e5e820d 100644
--- a/lib/librte_eal/common/include/generic/rte_byteorder.h
+++ b/lib/librte_eal/common/include/generic/rte_byteorder.h
@@ -74,6 +74,59 @@
 #elif defined __LITTLE_ENDIAN__
 #define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
 #endif
+#if !defined(RTE_BYTE_ORDER)
+#error Unknown endianness.
+#endif
+
+#define RTE_STATIC_BSWAP16(v) \
+	((((uint16_t)(v) & UINT16_C(0x00ff)) << 8) | \
+	 (((uint16_t)(v) & UINT16_C(0xff00)) >> 8))
+
+#define RTE_STATIC_BSWAP32(v) \
+	((((uint32_t)(v) & UINT32_C(0x000000ff)) << 24) | \
+	 (((uint32_t)(v) & UINT32_C(0x0000ff00)) <<  8) | \
+	 (((uint32_t)(v) & UINT32_C(0x00ff0000)) >>  8) | \
+	 (((uint32_t)(v) & UINT32_C(0xff000000)) >> 24))
+
+#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))
+
+/*
+ * These macros are functionally similar to rte_cpu_to_(be|le)(16|32|64)(),
+ * they take values in host CPU order and return them converted to the
+ * intended endianness.
+ *
+ * They resolve at compilation time to integer constants which can safely be
+ * used with static initializers, since those cannot involve function calls.
+ *
+ * On the other hand, they are not as optimized as their rte_cpu_to_*()
+ * counterparts, therefore applications should refrain from using them on
+ * variable values, particularly inside performance-sensitive code.
+ */
+#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+#define RTE_BE16(v) (rte_be16_t)(v)
+#define RTE_BE32(v) (rte_be32_t)(v)
+#define RTE_BE64(v) (rte_be64_t)(v)
+#define RTE_LE16(v) (rte_le16_t)(RTE_STATIC_BSWAP16(v))
+#define RTE_LE32(v) (rte_le32_t)(RTE_STATIC_BSWAP32(v))
+#define RTE_LE64(v) (rte_le64_t)(RTE_STATIC_BSWAP64(v))
+#elif RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+#define RTE_BE16(v) (rte_be16_t)(RTE_STATIC_BSWAP16(v))
+#define RTE_BE32(v) (rte_be32_t)(RTE_STATIC_BSWAP32(v))
+#define RTE_BE64(v) (rte_be64_t)(RTE_STATIC_BSWAP64(v))
+#define RTE_LE16(v) (rte_be16_t)(v)
+#define RTE_LE32(v) (rte_be32_t)(v)
+#define RTE_LE64(v) (rte_be64_t)(v)
+#else
+#error Unsupported endianness.
+#endif
 
 /*
  * The following types should be used when handling values according to a
@@ -98,8 +151,7 @@ typedef uint64_t rte_le64_t; /**< 64-bit little-endian value. */
 static inline uint16_t
 rte_constant_bswap16(uint16_t x)
 {
-	return (uint16_t)(((x & 0x00ffU) << 8) |
-		((x & 0xff00U) >> 8));
+	return RTE_STATIC_BSWAP16(x);
 }
 
 /*
@@ -111,10 +163,7 @@ rte_constant_bswap16(uint16_t x)
 static inline uint32_t
 rte_constant_bswap32(uint32_t x)
 {
-	return  ((x & 0x000000ffUL) << 24) |
-		((x & 0x0000ff00UL) << 8) |
-		((x & 0x00ff0000UL) >> 8) |
-		((x & 0xff000000UL) >> 24);
+	return RTE_STATIC_BSWAP32(x);
 }
 
 /*
@@ -126,14 +175,7 @@ rte_constant_bswap32(uint32_t x)
 static inline uint64_t
 rte_constant_bswap64(uint64_t x)
 {
-	return  ((x & 0x00000000000000ffULL) << 56) |
-		((x & 0x000000000000ff00ULL) << 40) |
-		((x & 0x0000000000ff0000ULL) << 24) |
-		((x & 0x00000000ff000000ULL) <<  8) |
-		((x & 0x000000ff00000000ULL) >>  8) |
-		((x & 0x0000ff0000000000ULL) >> 24) |
-		((x & 0x00ff000000000000ULL) >> 40) |
-		((x & 0xff00000000000000ULL) >> 56);
+	return RTE_STATIC_BSWAP64(x);
 }
 
 
-- 
2.1.4

[dpdk-dev] [PATCH v2 3/3] ethdev: tidy up endianness handling in flow API

[Adrien Mazarguil]

The flow API defines several structures whose fields must be specified in
network order. This commit documents them using explicit type names and
related endianness conversion macros.

No ABI change.

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
 lib/librte_ether/rte_flow.h | 52 ++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
index 34a5876..057f7e7 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -429,7 +429,7 @@ static const struct rte_flow_item_raw rte_flow_item_raw_mask = {
 struct rte_flow_item_eth {
 	struct ether_addr dst; /**< Destination MAC. */
 	struct ether_addr src; /**< Source MAC. */
-	uint16_t type; /**< EtherType. */
+	rte_be16_t type; /**< EtherType. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */
@@ -437,7 +437,7 @@ struct rte_flow_item_eth {
 static const struct rte_flow_item_eth rte_flow_item_eth_mask = {
 	.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
 	.src.addr_bytes = "\xff\xff\xff\xff\xff\xff",
-	.type = 0x0000,
+	.type = RTE_BE16(0x0000),
 };
 #endif
 
@@ -450,15 +450,15 @@ static const struct rte_flow_item_eth rte_flow_item_eth_mask = {
  * RTE_FLOW_ITEM_TYPE_VLAN.
  */
 struct rte_flow_item_vlan {
-	uint16_t tpid; /**< Tag protocol identifier. */
-	uint16_t tci; /**< Tag control information. */
+	rte_be16_t tpid; /**< Tag protocol identifier. */
+	rte_be16_t tci; /**< Tag control information. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
 #ifndef __cplusplus
 static const struct rte_flow_item_vlan rte_flow_item_vlan_mask = {
-	.tpid = 0x0000,
-	.tci = 0xffff,
+	.tpid = RTE_BE16(0x0000),
+	.tci = RTE_BE16(0xffff),
 };
 #endif
 
@@ -477,8 +477,8 @@ struct rte_flow_item_ipv4 {
 #ifndef __cplusplus
 static const struct rte_flow_item_ipv4 rte_flow_item_ipv4_mask = {
 	.hdr = {
-		.src_addr = 0xffffffff,
-		.dst_addr = 0xffffffff,
+		.src_addr = RTE_BE32(0xffffffff),
+		.dst_addr = RTE_BE32(0xffffffff),
 	},
 };
 #endif
@@ -540,8 +540,8 @@ struct rte_flow_item_udp {
 #ifndef __cplusplus
 static const struct rte_flow_item_udp rte_flow_item_udp_mask = {
 	.hdr = {
-		.src_port = 0xffff,
-		.dst_port = 0xffff,
+		.src_port = RTE_BE16(0xffff),
+		.dst_port = RTE_BE16(0xffff),
 	},
 };
 #endif
@@ -559,8 +559,8 @@ struct rte_flow_item_tcp {
 #ifndef __cplusplus
 static const struct rte_flow_item_tcp rte_flow_item_tcp_mask = {
 	.hdr = {
-		.src_port = 0xffff,
-		.dst_port = 0xffff,
+		.src_port = RTE_BE16(0xffff),
+		.dst_port = RTE_BE16(0xffff),
 	},
 };
 #endif
@@ -578,8 +578,8 @@ struct rte_flow_item_sctp {
 #ifndef __cplusplus
 static const struct rte_flow_item_sctp rte_flow_item_sctp_mask = {
 	.hdr = {
-		.src_port = 0xffff,
-		.dst_port = 0xffff,
+		.src_port = RTE_BE16(0xffff),
+		.dst_port = RTE_BE16(0xffff),
 	},
 };
 #endif
@@ -609,14 +609,14 @@ static const struct rte_flow_item_vxlan rte_flow_item_vxlan_mask = {
  * Matches a E-tag header.
  */
 struct rte_flow_item_e_tag {
-	uint16_t tpid; /**< Tag protocol identifier (0x893F). */
+	rte_be16_t tpid; /**< Tag protocol identifier (0x893F). */
 	/**
 	 * E-Tag control information (E-TCI).
 	 * E-PCP (3b), E-DEI (1b), ingress E-CID base (12b).
 	 */
-	uint16_t epcp_edei_in_ecid_b;
+	rte_be16_t epcp_edei_in_ecid_b;
 	/** Reserved (2b), GRP (2b), E-CID base (12b). */
-	uint16_t rsvd_grp_ecid_b;
+	rte_be16_t rsvd_grp_ecid_b;
 	uint8_t in_ecid_e; /**< Ingress E-CID ext. */
 	uint8_t ecid_e; /**< E-CID ext. */
 };
@@ -624,13 +624,7 @@ struct rte_flow_item_e_tag {
 /** Default mask for RTE_FLOW_ITEM_TYPE_E_TAG. */
 #ifndef __cplusplus
 static const struct rte_flow_item_e_tag rte_flow_item_e_tag_mask = {
-#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
-	.rsvd_grp_ecid_b = 0x3fff,
-#elif RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
-	.rsvd_grp_ecid_b = 0xff3f,
-#else
-#error Unsupported endianness.
-#endif
+	.rsvd_grp_ecid_b = RTE_BE16(0x3fff),
 };
 #endif
 
@@ -646,8 +640,8 @@ struct rte_flow_item_nvgre {
 	 *
 	 * c_k_s_rsvd0_ver must have value 0x2000 according to RFC 7637.
 	 */
-	uint16_t c_k_s_rsvd0_ver;
-	uint16_t protocol; /**< Protocol type (0x6558). */
+	rte_be16_t c_k_s_rsvd0_ver;
+	rte_be16_t protocol; /**< Protocol type (0x6558). */
 	uint8_t tni[3]; /**< Virtual subnet ID. */
 	uint8_t flow_id; /**< Flow ID. */
 };
@@ -689,14 +683,14 @@ struct rte_flow_item_gre {
 	 * Checksum (1b), reserved 0 (12b), version (3b).
 	 * Refer to RFC 2784.
 	 */
-	uint16_t c_rsvd0_ver;
-	uint16_t protocol; /**< Protocol type. */
+	rte_be16_t c_rsvd0_ver;
+	rte_be16_t protocol; /**< Protocol type. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_GRE. */
 #ifndef __cplusplus
 static const struct rte_flow_item_gre rte_flow_item_gre_mask = {
-	.protocol = 0xffff,
+	.protocol = RTE_BE16(0xffff),
 };
 #endif
 
-- 
2.1.4