Re: [dpdk-dev] [PATCH] mbuf: align rte_mbuf for Windows

[Thomas Monjalon <thomas@monjalon.net>]

19/05/2020 21:57, Dmitry Kozlyuk:
> On Tue, 19 May 2020 20:49:50 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > +Cc more maintainers
> > 
> > 19/05/2020 20:41, talshn@mellanox.com:
> > > From: Tal Shnaiderman <talshn@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 <talshn@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.kozliuk@gmail.com>

Would be interesting to see an offset comparison in little and big endian.