[dpdk-dev] ixgbe: fix compile error with gcc4.4 (used RHEL 6)
Commit Message
On Thu, Sep 18, 2014 at 12:35:21PM +0000, Richardson, Bruce wrote:
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Thursday, September 18, 2014 1:25 PM
> > To: Thomas Monjalon
> > Cc: Richardson, Bruce; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL
> > 6)
> >
> > On Thu, Sep 18, 2014 at 01:09:16PM +0200, Thomas Monjalon wrote:
> > > > The refcnt field is contained within an anonymous union within the mbuf
> > > > data structure, and gcc 4.4 gives an error about an unknown field unless
> > > > the initialiser for the field is contained within extra braces.
> > > >
> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > >
> > > Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > >
> > > Thanks Bruce, it is now applied.
> >
> > Hang on here, we use anonymous unions all the time in RHEL6, and make
> > assignments to them frequently, and the compiler doesn't complain (see the
> > dropcount variable in sk_buff for an example). Not saying that this is a big
> > deal, but can you explain a little more about what you're seeing when this error
> > occurs, before we just paper over it?
> >
>
> Originally reported on RHEL6 as a build failure. When I use gcc4.4 on Fedora 20, I get the following without this change:
>
> CC ixgbe_rxtx_vec.o
> == Build lib/librte_table
> /home/bruce/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c: In function 'ixgbe_rxq_vec_setup':
> /home/bruce/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c:726: error: unknown field 'refcnt' specified in initializer
> compilation terminated due to -Wfatal-errors.
> make[5]: *** [ixgbe_rxtx_vec.o] Error 1
> make[4]: *** [librte_pmd_ixgbe] Error 2
> make[4]: *** Waiting for unfinished jobs....
> make[3]: *** [lib] Error 2
> make[2]: *** [all] Error 2
> make[1]: *** [x86_64-native-linuxapp-gcc_install] Error 2
> make: *** [install] Error 2
>
> Regards,
> /Bruce
>
Thank you, I've reproduced the problem here, and you're right, it is a compiler
bug, but I really hate the idea of just throwing braces around something to shut
the compiler up, especially when the compiler has since been fixed. Looking at
it a bit more closely it seems like the the thing to do is actually just
consistently use rte_mbuf_refcnt_set, since thats what the rte_mbuf header
documentation says to do, and protects you if the internal structure changes, as
well as prevents you from having to spread ifdef RTE_MBUF_REFCNT all over the
place.
This patch does a bit more than that too. With a little creative typedef-ing we
don't need the anonymous union at all, and lets us just use a single refcnt
variable, and I think eliminate that odd refcnt_reserved member of the union,
that, as far as I can see, does nothing.
Thoughts?
Comments
On Thu, Sep 18, 2014 at 11:03:12AM -0400, Neil Horman wrote:
> On Thu, Sep 18, 2014 at 12:35:21PM +0000, Richardson, Bruce wrote:
> > > -----Original Message-----
> > > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > > Sent: Thursday, September 18, 2014 1:25 PM
> > > To: Thomas Monjalon
> > > Cc: Richardson, Bruce; dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL
> > > 6)
> > >
> > > On Thu, Sep 18, 2014 at 01:09:16PM +0200, Thomas Monjalon wrote:
> > > > > The refcnt field is contained within an anonymous union within the mbuf
> > > > > data structure, and gcc 4.4 gives an error about an unknown field unless
> > > > > the initialiser for the field is contained within extra braces.
> > > > >
> > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > >
> > > > Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > > >
> > > > Thanks Bruce, it is now applied.
> > >
> > > Hang on here, we use anonymous unions all the time in RHEL6, and make
> > > assignments to them frequently, and the compiler doesn't complain (see the
> > > dropcount variable in sk_buff for an example). Not saying that this is a big
> > > deal, but can you explain a little more about what you're seeing when this error
> > > occurs, before we just paper over it?
> > >
> >
> > Originally reported on RHEL6 as a build failure. When I use gcc4.4 on Fedora 20, I get the following without this change:
> >
> > CC ixgbe_rxtx_vec.o
> > == Build lib/librte_table
> > /home/bruce/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c: In function 'ixgbe_rxq_vec_setup':
> > /home/bruce/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c:726: error: unknown field 'refcnt' specified in initializer
> > compilation terminated due to -Wfatal-errors.
> > make[5]: *** [ixgbe_rxtx_vec.o] Error 1
> > make[4]: *** [librte_pmd_ixgbe] Error 2
> > make[4]: *** Waiting for unfinished jobs....
> > make[3]: *** [lib] Error 2
> > make[2]: *** [all] Error 2
> > make[1]: *** [x86_64-native-linuxapp-gcc_install] Error 2
> > make: *** [install] Error 2
> >
> > Regards,
> > /Bruce
> >
>
>
> Thank you, I've reproduced the problem here, and you're right, it is a compiler
> bug, but I really hate the idea of just throwing braces around something to shut
> the compiler up, especially when the compiler has since been fixed. Looking at
> it a bit more closely it seems like the the thing to do is actually just
> consistently use rte_mbuf_refcnt_set, since thats what the rte_mbuf header
> documentation says to do, and protects you if the internal structure changes, as
> well as prevents you from having to spread ifdef RTE_MBUF_REFCNT all over the
> place.
>
> This patch does a bit more than that too. With a little creative typedef-ing we
> don't need the anonymous union at all, and lets us just use a single refcnt
> variable, and I think eliminate that odd refcnt_reserved member of the union,
> that, as far as I can see, does nothing.
>
> Thoughts?
>
I like the idea of using a typedef, though are we not adjusting the mbuf
layout if the refcount is disabled? A follow-on question would also be - do
we really need an option to disable the reference count, do we not always
just leave it on?
As for using refcnt_set - that would be a solution that would work too, but
that approach needs to be checked for performance impacts as we are going
from a constant initializer value to having an extra assignment instruction
in the fast-path.
The simplest fix is probably just to add the braces, which isn't really much
of a big deal.
/Bruce
Hi Neil,
2014-09-18 11:03, Neil Horman:
> Thank you, I've reproduced the problem here, and you're right, it is a
> compiler bug, but I really hate the idea of just throwing braces around
> something to shut the compiler up, especially when the compiler has since
> been fixed. Looking at it a bit more closely it seems like the the thing
> to do is actually just consistently use rte_mbuf_refcnt_set, since thats
> what the rte_mbuf header documentation says to do, and protects you if the
> internal structure changes, as well as prevents you from having to spread
> ifdef RTE_MBUF_REFCNT all over the place.
>
> This patch does a bit more than that too. With a little creative
> typedef-ing we don't need the anonymous union at all, and lets us just use
> a single refcnt variable, and I think eliminate that odd refcnt_reserved
> member of the union, that, as far as I can see, does nothing.
> --- a/config/common_linuxapp
> +++ b/config/common_linuxapp
> @@ -123,7 +123,7 @@ CONFIG_RTE_LOG_HISTORY=256
> CONFIG_RTE_LIBEAL_USE_HPET=n
> CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n
> CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n
> -CONFIG_RTE_EAL_IGB_UIO=y
> +CONFIG_RTE_EAL_IGB_UIO=n
> CONFIG_RTE_EAL_VFIO=y
Hum, indeed this patch does a bit more ;)
[...]
> -CONFIG_RTE_LIBRTE_KNI=y
> +CONFIG_RTE_LIBRTE_KNI=n
> CONFIG_RTE_KNI_KO_DEBUG=n
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -120,6 +120,15 @@ extern "C" {
> typedef void *MARKER[0]; /**< generic marker for a point in a
> structure */ typedef uint64_t MARKER64[0]; /**< marker that allows us to
> overwrite 8 bytes * with a single assignment */
> +
> +#ifdef RTE_MBUF_REFCNT
> +#ifdef RTE_MBUF_REFCNT_ATOMIC
> +typedef rte_atomic16_t rte_refcnt;
> +#else
> +typedef uint16_t rte_refcnt;
> +#endif
> +#endif
> +
> /**
> * The generic rte_mbuf, containing a packet mbuf.
> */
> @@ -142,13 +151,9 @@ struct rte_mbuf {
> * or non-atomic) is controlled by the CONFIG_RTE_MBUF_REFCNT_ATOMIC
> * config option.
> */
> - union {
> #ifdef RTE_MBUF_REFCNT
> - rte_atomic16_t refcnt_atomic; /**< Atomically accessed refcnt */
> - uint16_t refcnt; /**< Non-atomically accessed refcnt */
> + rte_refcnt refcnt;
> #endif
> - uint16_t refcnt_reserved; /**< Do not use this field */
> - };
You are changing the mbuf alignment if MBUF_REFCNT is disabled.
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> @@ -722,11 +722,9 @@ ixgbe_rxq_vec_setup(struct igb_rx_queue *rxq)
> static struct rte_mbuf mb_def = {
> .nb_segs = 1,
> .data_off = RTE_PKTMBUF_HEADROOM,
> -#ifdef RTE_MBUF_REFCNT
> - { .refcnt = 1, }
> -#endif
> };
>
> + rte_mbuf_refcnt_set(&mb_def, 1);
Performance impact must be checked here.
On Thu, Sep 18, 2014 at 04:16:00PM +0100, Bruce Richardson wrote:
> On Thu, Sep 18, 2014 at 11:03:12AM -0400, Neil Horman wrote:
> > On Thu, Sep 18, 2014 at 12:35:21PM +0000, Richardson, Bruce wrote:
> > > > -----Original Message-----
> > > > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > > > Sent: Thursday, September 18, 2014 1:25 PM
> > > > To: Thomas Monjalon
> > > > Cc: Richardson, Bruce; dev@dpdk.org
> > > > Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL
> > > > 6)
> > > >
> > > > On Thu, Sep 18, 2014 at 01:09:16PM +0200, Thomas Monjalon wrote:
> > > > > > The refcnt field is contained within an anonymous union within the mbuf
> > > > > > data structure, and gcc 4.4 gives an error about an unknown field unless
> > > > > > the initialiser for the field is contained within extra braces.
> > > > > >
> > > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > >
> > > > > Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > > > >
> > > > > Thanks Bruce, it is now applied.
> > > >
> > > > Hang on here, we use anonymous unions all the time in RHEL6, and make
> > > > assignments to them frequently, and the compiler doesn't complain (see the
> > > > dropcount variable in sk_buff for an example). Not saying that this is a big
> > > > deal, but can you explain a little more about what you're seeing when this error
> > > > occurs, before we just paper over it?
> > > >
> > >
> > > Originally reported on RHEL6 as a build failure. When I use gcc4.4 on Fedora 20, I get the following without this change:
> > >
> > > CC ixgbe_rxtx_vec.o
> > > == Build lib/librte_table
> > > /home/bruce/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c: In function 'ixgbe_rxq_vec_setup':
> > > /home/bruce/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c:726: error: unknown field 'refcnt' specified in initializer
> > > compilation terminated due to -Wfatal-errors.
> > > make[5]: *** [ixgbe_rxtx_vec.o] Error 1
> > > make[4]: *** [librte_pmd_ixgbe] Error 2
> > > make[4]: *** Waiting for unfinished jobs....
> > > make[3]: *** [lib] Error 2
> > > make[2]: *** [all] Error 2
> > > make[1]: *** [x86_64-native-linuxapp-gcc_install] Error 2
> > > make: *** [install] Error 2
> > >
> > > Regards,
> > > /Bruce
> > >
> >
> >
> > Thank you, I've reproduced the problem here, and you're right, it is a compiler
> > bug, but I really hate the idea of just throwing braces around something to shut
> > the compiler up, especially when the compiler has since been fixed. Looking at
> > it a bit more closely it seems like the the thing to do is actually just
> > consistently use rte_mbuf_refcnt_set, since thats what the rte_mbuf header
> > documentation says to do, and protects you if the internal structure changes, as
> > well as prevents you from having to spread ifdef RTE_MBUF_REFCNT all over the
> > place.
> >
> > This patch does a bit more than that too. With a little creative typedef-ing we
> > don't need the anonymous union at all, and lets us just use a single refcnt
> > variable, and I think eliminate that odd refcnt_reserved member of the union,
> > that, as far as I can see, does nothing.
> >
> > Thoughts?
> >
>
> I like the idea of using a typedef, though are we not adjusting the mbuf
> layout if the refcount is disabled?
We are, theres still an RTE_MBUF_REFCNT ifdef around the definition of the
refcnt variable. The only change is that its consistently the same type
(rte_refcnt), its just the typedef that changes based on the build configuration
(which is in keeping with how the api implementation is toggled).
A follow-on question would also be - do
> we really need an option to disable the reference count, do we not always
> just leave it on?
>
I would strongly agree with this, I find it hard to fathom a situation in which
you don't want or need to refcount buffers. I might be mistaken, but that seems
like folly. Though I left that issue alone in this patch.
> As for using refcnt_set - that would be a solution that would work too, but
> that approach needs to be checked for performance impacts as we are going
> from a constant initializer value to having an extra assignment instruction
> in the fast-path.
>
Its what you're documentation requires though (at least currently). Certainly
check, but I would say the impact is zero, as the set routine is static inline,
and the assignment value is a constant, which means the compiler should be able
to optimize it to occur in parallel with the stack adjustment in the preamble.
> The simplest fix is probably just to add the braces, which isn't really much
> of a big deal.
>
Its duct tape. Its a simple fix that seems like a great idea today,
specifically becuase its simple, but it will cause maintenence headaches in the
future, because you will constantly have to watch out in subsequent patch
submissions for someone out of hand just fixing what by all rights is a silly
extra set of braces that really shouldn't need to be there.
At the very least, consider this change because it means that you don't need to
sprinkle code if ifdef RTE_MBUF_REFCNT. If you really feel like you need a
static initalizer, lets make something akin to linux's ATOMIC_INIT, in which you
can package the brace magic, or define to nothing if you don't compile
RTE_MBUF_REFCNT on.
Neil
On Thu, Sep 18, 2014 at 05:38:58PM +0200, Thomas Monjalon wrote:
> Hi Neil,
>
> 2014-09-18 11:03, Neil Horman:
> > Thank you, I've reproduced the problem here, and you're right, it is a
> > compiler bug, but I really hate the idea of just throwing braces around
> > something to shut the compiler up, especially when the compiler has since
> > been fixed. Looking at it a bit more closely it seems like the the thing
> > to do is actually just consistently use rte_mbuf_refcnt_set, since thats
> > what the rte_mbuf header documentation says to do, and protects you if the
> > internal structure changes, as well as prevents you from having to spread
> > ifdef RTE_MBUF_REFCNT all over the place.
> >
> > This patch does a bit more than that too. With a little creative
> > typedef-ing we don't need the anonymous union at all, and lets us just use
> > a single refcnt variable, and I think eliminate that odd refcnt_reserved
> > member of the union, that, as far as I can see, does nothing.
>
> > --- a/config/common_linuxapp
> > +++ b/config/common_linuxapp
> > @@ -123,7 +123,7 @@ CONFIG_RTE_LOG_HISTORY=256
> > CONFIG_RTE_LIBEAL_USE_HPET=n
> > CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n
> > CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n
> > -CONFIG_RTE_EAL_IGB_UIO=y
> > +CONFIG_RTE_EAL_IGB_UIO=n
> > CONFIG_RTE_EAL_VFIO=y
>
> Hum, indeed this patch does a bit more ;)
>
Sorry, meant to clip that out, I was building without kernel headers, so I had
to disable igb_uio and kni. I'll propose this patch properly if theres
consensus on the valid bits.
Neil
> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Thursday, September 18, 2014 4:51 PM
> To: Thomas Monjalon
> Cc: Richardson, Bruce; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL
> 6)
>
> On Thu, Sep 18, 2014 at 05:38:58PM +0200, Thomas Monjalon wrote:
> > Hi Neil,
> >
> > 2014-09-18 11:03, Neil Horman:
> > > Thank you, I've reproduced the problem here, and you're right, it is a
> > > compiler bug, but I really hate the idea of just throwing braces around
> > > something to shut the compiler up, especially when the compiler has since
> > > been fixed. Looking at it a bit more closely it seems like the the thing
> > > to do is actually just consistently use rte_mbuf_refcnt_set, since thats
> > > what the rte_mbuf header documentation says to do, and protects you if the
> > > internal structure changes, as well as prevents you from having to spread
> > > ifdef RTE_MBUF_REFCNT all over the place.
> > >
> > > This patch does a bit more than that too. With a little creative
> > > typedef-ing we don't need the anonymous union at all, and lets us just use
> > > a single refcnt variable, and I think eliminate that odd refcnt_reserved
> > > member of the union, that, as far as I can see, does nothing.
> >
> > > --- a/config/common_linuxapp
> > > +++ b/config/common_linuxapp
> > > @@ -123,7 +123,7 @@ CONFIG_RTE_LOG_HISTORY=256
> > > CONFIG_RTE_LIBEAL_USE_HPET=n
> > > CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n
> > > CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n
> > > -CONFIG_RTE_EAL_IGB_UIO=y
> > > +CONFIG_RTE_EAL_IGB_UIO=n
> > > CONFIG_RTE_EAL_VFIO=y
> >
> > Hum, indeed this patch does a bit more ;)
> >
> Sorry, meant to clip that out, I was building without kernel headers, so I had
> to disable igb_uio and kni. I'll propose this patch properly if theres
> consensus on the valid bits.
> Neil
If we get rid of the REFCNT compile-time option (but not the REFCNT_ATOMIC one), then we should be able to get rid of the union, as we can do the atomic/non-atomic entirely inside the refcnt manipulation routines (since a regular uint16_t can be typecast directly to an atomic form of the same). Once we get rid of the union we can get rid of the extra braces that go along with the union, and we can just have the static initialiser cleanly put in the code.
Whaddaya think?
/Bruce
On Thu, Sep 18, 2014 at 04:01:32PM +0000, Richardson, Bruce wrote:
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Thursday, September 18, 2014 4:51 PM
> > To: Thomas Monjalon
> > Cc: Richardson, Bruce; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL
> > 6)
> >
> > On Thu, Sep 18, 2014 at 05:38:58PM +0200, Thomas Monjalon wrote:
> > > Hi Neil,
> > >
> > > 2014-09-18 11:03, Neil Horman:
> > > > Thank you, I've reproduced the problem here, and you're right, it is a
> > > > compiler bug, but I really hate the idea of just throwing braces around
> > > > something to shut the compiler up, especially when the compiler has since
> > > > been fixed. Looking at it a bit more closely it seems like the the thing
> > > > to do is actually just consistently use rte_mbuf_refcnt_set, since thats
> > > > what the rte_mbuf header documentation says to do, and protects you if the
> > > > internal structure changes, as well as prevents you from having to spread
> > > > ifdef RTE_MBUF_REFCNT all over the place.
> > > >
> > > > This patch does a bit more than that too. With a little creative
> > > > typedef-ing we don't need the anonymous union at all, and lets us just use
> > > > a single refcnt variable, and I think eliminate that odd refcnt_reserved
> > > > member of the union, that, as far as I can see, does nothing.
> > >
> > > > --- a/config/common_linuxapp
> > > > +++ b/config/common_linuxapp
> > > > @@ -123,7 +123,7 @@ CONFIG_RTE_LOG_HISTORY=256
> > > > CONFIG_RTE_LIBEAL_USE_HPET=n
> > > > CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n
> > > > CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n
> > > > -CONFIG_RTE_EAL_IGB_UIO=y
> > > > +CONFIG_RTE_EAL_IGB_UIO=n
> > > > CONFIG_RTE_EAL_VFIO=y
> > >
> > > Hum, indeed this patch does a bit more ;)
> > >
> > Sorry, meant to clip that out, I was building without kernel headers, so I had
> > to disable igb_uio and kni. I'll propose this patch properly if theres
> > consensus on the valid bits.
> > Neil
>
> If we get rid of the REFCNT compile-time option (but not the REFCNT_ATOMIC one), then we should be able to get rid of the union, as we can do the atomic/non-atomic entirely inside the refcnt manipulation routines (since a regular uint16_t can be typecast directly to an atomic form of the same). Once we get rid of the union we can get rid of the extra braces that go along with the union, and we can just have the static initialiser cleanly put in the code.
> Whaddaya think?
>
Yeah, that sounds good. Reduces the code complexity by a good amount, makes
configuration simpler and allows for easy static initalization.
> /Bruce
>
@@ -760,14 +760,14 @@ test_failing_mbuf_sanity_check(void)
#ifdef RTE_MBUF_REFCNT
badbuf = *buf;
- badbuf.refcnt = 0;
+ rte_mbuf_refcnt_set(&badbuf, 0);
if (verify_mbuf_check_panics(&badbuf)) {
printf("Error with bad-refcnt(0) mbuf test\n");
return -1;
}
badbuf = *buf;
- badbuf.refcnt = UINT16_MAX;
+ rte_mbuf_refcnt_set(&badbuf, UINT16_MAX);
if (verify_mbuf_check_panics(&badbuf)) {
printf("Error with bad-refcnt(MAX) mbuf test\n");
return -1;
@@ -123,7 +123,7 @@ CONFIG_RTE_LOG_HISTORY=256
CONFIG_RTE_LIBEAL_USE_HPET=n
CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n
CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n
-CONFIG_RTE_EAL_IGB_UIO=y
+CONFIG_RTE_EAL_IGB_UIO=n
CONFIG_RTE_EAL_VFIO=y
#
@@ -381,7 +381,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y
#
# Compile librte_kni
#
-CONFIG_RTE_LIBRTE_KNI=y
+CONFIG_RTE_LIBRTE_KNI=n
CONFIG_RTE_KNI_KO_DEBUG=n
CONFIG_RTE_KNI_VHOST=n
CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024
@@ -120,6 +120,15 @@ extern "C" {
typedef void *MARKER[0]; /**< generic marker for a point in a structure */
typedef uint64_t MARKER64[0]; /**< marker that allows us to overwrite 8 bytes
* with a single assignment */
+
+#ifdef RTE_MBUF_REFCNT
+#ifdef RTE_MBUF_REFCNT_ATOMIC
+typedef rte_atomic16_t rte_refcnt;
+#else
+typedef uint16_t rte_refcnt;
+#endif
+#endif
+
/**
* The generic rte_mbuf, containing a packet mbuf.
*/
@@ -142,13 +151,9 @@ struct rte_mbuf {
* or non-atomic) is controlled by the CONFIG_RTE_MBUF_REFCNT_ATOMIC
* config option.
*/
- union {
#ifdef RTE_MBUF_REFCNT
- rte_atomic16_t refcnt_atomic; /**< Atomically accessed refcnt */
- uint16_t refcnt; /**< Non-atomically accessed refcnt */
+ rte_refcnt refcnt;
#endif
- uint16_t refcnt_reserved; /**< Do not use this field */
- };
uint8_t nb_segs; /**< Number of segments. */
uint8_t port; /**< Input port. */
@@ -250,6 +255,7 @@ if (!(exp)) { \
#ifdef RTE_MBUF_REFCNT
#ifdef RTE_MBUF_REFCNT_ATOMIC
+
/**
* Adds given value to an mbuf's refcnt and returns its new value.
* @param m
@@ -262,7 +268,7 @@ if (!(exp)) { \
static inline uint16_t
rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)
{
- return (uint16_t)(rte_atomic16_add_return(&m->refcnt_atomic, value));
+ return (uint16_t)(rte_atomic16_add_return(&m->refcnt, value));
}
/**
@@ -275,7 +281,7 @@ rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)
static inline uint16_t
rte_mbuf_refcnt_read(const struct rte_mbuf *m)
{
- return (uint16_t)(rte_atomic16_read(&m->refcnt_atomic));
+ return (uint16_t)(rte_atomic16_read(&m->refcnt));
}
/**
@@ -288,7 +294,7 @@ rte_mbuf_refcnt_read(const struct rte_mbuf *m)
static inline void
rte_mbuf_refcnt_set(struct rte_mbuf *m, uint16_t new_value)
{
- rte_atomic16_set(&m->refcnt_atomic, new_value);
+ rte_atomic16_set(&m->refcnt, new_value);
}
#else /* ! RTE_MBUF_REFCNT_ATOMIC */
@@ -722,11 +722,9 @@ ixgbe_rxq_vec_setup(struct igb_rx_queue *rxq)
static struct rte_mbuf mb_def = {
.nb_segs = 1,
.data_off = RTE_PKTMBUF_HEADROOM,
-#ifdef RTE_MBUF_REFCNT
- { .refcnt = 1, }
-#endif
};
+ rte_mbuf_refcnt_set(&mb_def, 1);
mb_def.buf_len = rxq->mb_pool->elt_size - sizeof(struct rte_mbuf);
mb_def.port = rxq->port_id;
rxq->mbuf_initializer = *((uint64_t *)&mb_def.rearm_data);