* [dpdk-dev] [PATCH] mbuf: align rte_mbuf for Windows @ 2020-05-19 18:41 talshn 2020-05-19 18:49 ` Thomas Monjalon 0 siblings, 1 reply; 7+ messages in thread From: talshn @ 2020-05-19 18:41 UTC (permalink / raw) To: dev Cc: thomas, pallavi.kadam, dmitry.kozliuk, ranjit.menon, navasile, olivier.matz, harini.ramakrishnan, ocardona, Tal Shnaiderman 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. */ }; }; -- 2.16.1.windows.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] mbuf: align rte_mbuf for Windows 2020-05-19 18:41 [dpdk-dev] [PATCH] mbuf: align rte_mbuf for Windows talshn @ 2020-05-19 18:49 ` Thomas Monjalon 2020-05-19 19:57 ` Dmitry Kozlyuk 0 siblings, 1 reply; 7+ messages in thread From: Thomas Monjalon @ 2020-05-19 18:49 UTC (permalink / raw) To: talshn Cc: dev, pallavi.kadam, dmitry.kozliuk, ranjit.menon, navasile, olivier.matz, harini.ramakrishnan, ocardona, Tal Shnaiderman, olivier.matz, david.marchand, arybchenko, bruce.richardson, jerinj, drc, ruifeng.wang, konstantin.ananyev +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. */ > }; > }; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] mbuf: align rte_mbuf for Windows 2020-05-19 18:49 ` Thomas Monjalon @ 2020-05-19 19:57 ` Dmitry Kozlyuk 2020-05-19 20:18 ` Thomas Monjalon 0 siblings, 1 reply; 7+ messages in thread From: Dmitry Kozlyuk @ 2020-05-19 19:57 UTC (permalink / raw) To: Thomas Monjalon Cc: talshn, dev, pallavi.kadam, ranjit.menon, navasile, olivier.matz, harini.ramakrishnan, ocardona, david.marchand, arybchenko, bruce.richardson, jerinj, drc, ruifeng.wang, konstantin.ananyev 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> -- Dmitry Kozlyuk ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] mbuf: align rte_mbuf for Windows 2020-05-19 19:57 ` Dmitry Kozlyuk @ 2020-05-19 20:18 ` Thomas Monjalon 2020-05-19 22:15 ` Ranjit Menon 0 siblings, 1 reply; 7+ messages in thread From: Thomas Monjalon @ 2020-05-19 20:18 UTC (permalink / raw) To: Dmitry Kozlyuk, talshn Cc: dev, pallavi.kadam, ranjit.menon, navasile, olivier.matz, harini.ramakrishnan, ocardona, david.marchand, arybchenko, bruce.richardson, jerinj, drc, ruifeng.wang, konstantin.ananyev 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] mbuf: align rte_mbuf for Windows 2020-05-19 20:18 ` Thomas Monjalon @ 2020-05-19 22:15 ` Ranjit Menon 2020-06-11 11:43 ` Olivier Matz 0 siblings, 1 reply; 7+ messages in thread From: Ranjit Menon @ 2020-05-19 22:15 UTC (permalink / raw) To: Thomas Monjalon, Dmitry Kozlyuk, talshn Cc: dev, pallavi.kadam, navasile, olivier.matz, harini.ramakrishnan, ocardona, david.marchand, arybchenko, bruce.richardson, jerinj, drc, ruifeng.wang, konstantin.ananyev 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 <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. > > 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.menon@intel.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] mbuf: align rte_mbuf for Windows 2020-05-19 22:15 ` Ranjit Menon @ 2020-06-11 11:43 ` Olivier Matz 2020-06-11 14:29 ` Thomas Monjalon 0 siblings, 1 reply; 7+ messages in thread From: Olivier Matz @ 2020-06-11 11:43 UTC (permalink / raw) To: Ranjit Menon Cc: Thomas Monjalon, Dmitry Kozlyuk, talshn, dev, pallavi.kadam, navasile, harini.ramakrishnan, ocardona, david.marchand, arybchenko, bruce.richardson, jerinj, drc, ruifeng.wang, konstantin.ananyev On Tue, May 19, 2020 at 03:15:19PM -0700, Ranjit Menon wrote: > 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 <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> Acked-by: Olivier Matz <olivier.matz@6wind.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] mbuf: align rte_mbuf for Windows 2020-06-11 11:43 ` Olivier Matz @ 2020-06-11 14:29 ` Thomas Monjalon 0 siblings, 0 replies; 7+ messages in thread From: Thomas Monjalon @ 2020-06-11 14:29 UTC (permalink / raw) To: talshn Cc: Ranjit Menon, dev, Dmitry Kozlyuk, dev, pallavi.kadam, navasile, harini.ramakrishnan, ocardona, david.marchand, arybchenko, bruce.richardson, jerinj, drc, ruifeng.wang, konstantin.ananyev, Olivier Matz 11/06/2020 13:43, Olivier Matz: > On Tue, May 19, 2020 at 03:15:19PM -0700, Ranjit Menon wrote: > > 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 <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> > > Acked-by: Olivier Matz <olivier.matz@6wind.com> Applied, thanks ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-11 14:29 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-19 18:41 [dpdk-dev] [PATCH] mbuf: align rte_mbuf for Windows talshn 2020-05-19 18:49 ` Thomas Monjalon 2020-05-19 19:57 ` Dmitry Kozlyuk 2020-05-19 20:18 ` Thomas Monjalon 2020-05-19 22:15 ` Ranjit Menon 2020-06-11 11:43 ` Olivier Matz 2020-06-11 14:29 ` Thomas Monjalon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).