On 5/19/2020 1:18 PM, Thomas Monjalon wrote:
19/05/2020 21:57, Dmitry Kozlyuk:
On Tue, 19 May 2020 20:49:50 +0200
Thomas Monjalon <tho...@monjalon.net> wrote:
+Cc more maintainers
19/05/2020 20:41, tal...@mellanox.com:
From: Tal Shnaiderman <tal...@mellanox.com>
Using uint32_t type bit-fields in Windows will pads the
'L2/L3/L4 and tunnel information' union with additional bits.
This padding causes rte_mbuf size misalignment and the total size
increases to 3 cache-lines.
Changed packet_type bit-fields types from uint32_t to uint8_t
to allow unified 2 cache-line structure size.
Added the __extension__ attribute over the modified struct to avoid
the warning:
type of bit-field ... is a GCC extension [-pedantic]
Signed-off-by: Tal Shnaiderman <tal...@mellanox.com>
---
lib/librte_mbuf/rte_mbuf_core.h | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/lib/librte_mbuf/rte_mbuf_core.h
b/lib/librte_mbuf/rte_mbuf_core.h index b9a59c879..82441555e 100644
--- a/lib/librte_mbuf/rte_mbuf_core.h
+++ b/lib/librte_mbuf/rte_mbuf_core.h
@@ -521,11 +521,12 @@ struct rte_mbuf {
RTE_STD_C11
union {
uint32_t packet_type; /**< L2/L3/L4 and tunnel
information. */
+ __extension__
struct {
- uint32_t l2_type:4; /**< (Outer) L2 type.
*/
- uint32_t l3_type:4; /**< (Outer) L3 type.
*/
- uint32_t l4_type:4; /**< (Outer) L4 type.
*/
- uint32_t tun_type:4; /**< Tunnel type. */
+ uint8_t l2_type:4; /**< (Outer) L2 type. */
+ uint8_t l3_type:4; /**< (Outer) L3 type. */
+ uint8_t l4_type:4; /**< (Outer) L4 type. */
+ uint8_t tun_type:4; /**< Tunnel type. */
RTE_STD_C11
union {
uint8_t inner_esp_next_proto;
@@ -541,7 +542,7 @@ struct rte_mbuf {
/**< Inner L3 type. */
};
};
- uint32_t inner_l4_type:4; /**< Inner L4
type. */
+ uint8_t inner_l4_type:4; /**< Inner L4
type. */ };
};
Such a clean and simple solution to what seemed to require compiler
workaround or fix! All offsets are equal on Windows and Linux for the
following toolchains, x86_64:
* cross-compilation with MinGW-w64 6.0.0 GCC 9.3.0
* Windows native MinGW-w64 6.0.0 GCC 8.1.0 and Clang 9.0.1
Tested-by: Dmitry Kozlyuk <dmitry.kozl...@gmail.com>
Would be interesting to see an offset comparison in little and big endian.
BTW, this is the exact fix we used for the alignment issue in the
Windows draft repo.
It completely skipped my mind, when we were discussing this during the
community call.
See this code section in lib/librte_mbuf/rte_mbuf.h in the draft repo:
uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */
struct {
#ifndef _WIN64
uint32_t l2_type:4; /**< (Outer) L2 type. */
uint32_t l3_type:4; /**< (Outer) L3 type. */
uint32_t l4_type:4; /**< (Outer) L4 type. */
uint32_t tun_type:4; /**< Tunnel type. */
#else
uint8_t l2_type:4; /**< (Outer) L2 type. */
uint8_t l3_type:4; /**< (Outer) L3 type. */
uint8_t l4_type:4; /**< (Outer) L4 type. */
uint8_t tun_type:4; /**< Tunnel type. */
#endif
RTE_STD_C11
union {
uint8_t inner_esp_next_proto;
/**< ESP next protocol type, valid if
* RTE_PTYPE_TUNNEL_ESP tunnel type is set
* on both Tx and Rx.
*/
__extension__
struct {
uint8_t inner_l2_type:4;
/**< Inner L2 type. */
uint8_t inner_l3_type:4;
/**< Inner L3 type. */
};
};
#ifndef _WIN64
uint32_t inner_l4_type:4; /**< Inner L4 type. */
#else
uint8_t inner_l4_type:4; /**< Inner L4 type. */
#endif
};
We didn't have the bandwidth to test this on other OS, so we used
#ifdef, but we know this allowed us to be as performant as Linux (using
L3Fwd).
Therefore:
Acked-by: Ranjit Menon <ranjit.me...@intel.com>
