DPDK patches and discussions
 help / color / mirror / Atom feed
From: Raslan Darawsheh <rasland@mellanox.com>
To: Olivier Matz <olivier.matz@6wind.com>,
	Ferruh Yigit <ferruh.yigit@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] net: fix compilation with pedantic enabled
Date: Tue, 21 Jul 2020 07:37:51 +0000	[thread overview]
Message-ID: <AM0PR05MB6707A3491F43D1DF53B15F04C2780@AM0PR05MB6707.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <20200721070925.GF5869@platinum>

Hi,
> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Tuesday, July 21, 2020 10:09 AM
> To: Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: Raslan Darawsheh <rasland@mellanox.com>; dev@dpdk.org;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net: fix compilation with pedantic enabled
> 
> Hi,
> 
> On Tue, Jul 21, 2020 at 01:05:57AM +0100, Ferruh Yigit wrote:
> > On 7/16/2020 1:12 PM, Raslan Darawsheh wrote:
> > > when trying to compile rte_mpls with pedantic enabled,
> > > it will complain about bit field defintion.
> > > error: type of bit-field 'bs' is a GCC extension [-Werror=pedantic]
> > > error: type of bit-field 'tc' is a GCC extension [-Werror=pedantic]
> > > error: type of bit-field 'tag_lsb' is a GCC extension [-Werror=pedantic]
> > '
> > I tried to reproduce by adding to '-pedantic' to 'rte_net.c' (which uses
> > 'rte_mpls.h') but not able to get the warning. Is this happen with specific
> > version of the compiler?

Yes It happens only with old compilers, maybe I should have mentioned that in the commit log (my mistake).
gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28)

> >
> > >
> > > This fixes the compilation error.
> > >
> > > Fixes: e480cf487a0d ("net: add MPLS header structure")
> > > Cc: olivier.matz@6wind.com
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> > > ---
> > >  lib/librte_net/rte_mpls.h | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/lib/librte_net/rte_mpls.h b/lib/librte_net/rte_mpls.h
> > > index db91707..ecd1f64 100644
> > > --- a/lib/librte_net/rte_mpls.h
> > > +++ b/lib/librte_net/rte_mpls.h
> > > @@ -24,13 +24,13 @@ extern "C" {
> > >  struct rte_mpls_hdr {
> > >  	uint16_t tag_msb;   /**< Label(msb). */
> > >  #if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > > -	uint8_t tag_lsb:4;  /**< Label(lsb). */
> > > -	uint8_t tc:3;       /**< Traffic class. */
> > > -	uint8_t bs:1;       /**< Bottom of stack. */
> > > +	uint32_t tag_lsb:4;  /**< Label(lsb). */
> > > +	uint32_t tc:3;       /**< Traffic class. */
> > > +	uint32_t bs:1;       /**< Bottom of stack. */
> > >  #else
> > > -	uint8_t bs:1;       /**< Bottom of stack. */
> > > -	uint8_t tc:3;       /**< Traffic class. */
> > > -	uint8_t tag_lsb:4;  /**< label(lsb) */
> > > +	uint32_t bs:1;       /**< Bottom of stack. */
> > > +	uint32_t tc:3;       /**< Traffic class. */
> > > +	uint32_t tag_lsb:4;  /**< label(lsb) */
> > >  #endif
> > >  	uint8_t  ttl;       /**< Time to live. */
> > >  } __rte_packed;
> >
> > The struct size keeps same after change, do you know if this behavior is
> part of
> > standard and guaranteed?
> 
> I have the same fear.
To my understanding and please correct me if I'm wrong, the type of the bit fields shouldn't change the size of the structure,
As long as the bit order is kept the same, and I made a small test for it and checked the size of the struct it gave 4 bytes (sizeof()) with both definitions.

> 
> Would it make sense to add __extension__ instead? We already do that
> for gre, for instance.
Yes I guess this can work as well,

Kindest regards
Raslan Darawsheh

  reply	other threads:[~2020-07-21  7:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-16 12:12 Raslan Darawsheh
2020-07-21  0:05 ` Ferruh Yigit
2020-07-21  7:09   ` Olivier Matz
2020-07-21  7:37     ` Raslan Darawsheh [this message]
2020-07-21  7:50       ` Olivier Matz
2020-07-21  8:11         ` Raslan Darawsheh
2020-07-21  8:31 ` [dpdk-dev] [PATCH v2] " Raslan Darawsheh
2020-07-21 15:43   ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM0PR05MB6707A3491F43D1DF53B15F04C2780@AM0PR05MB6707.eurprd05.prod.outlook.com \
    --to=rasland@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).