[PATCH] cpp: Fix missing extern "C" closing bracket

[PATCH] cpp: Fix missing extern "C" closing bracket

[Edvard Fagerholm]

RTE_FORCE_INTRINSICS with a C++ compiler causes rte_atomic.h and
rte_byteorder.h to emit on x86 an extern "C" without a closing
brace.
---
 lib/eal/x86/include/rte_atomic.h    | 4 ++--
 lib/eal/x86/include/rte_byteorder.h | 9 +++++----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/eal/x86/include/rte_atomic.h b/lib/eal/x86/include/rte_atomic.h
index c72c47c83e..68629016b4 100644
--- a/lib/eal/x86/include/rte_atomic.h
+++ b/lib/eal/x86/include/rte_atomic.h
@@ -282,10 +282,10 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t *v)
 	return ret != 0;
 }
 
-#ifdef __cplusplus
-}
 #endif
 
+#ifdef __cplusplus
+}
 #endif
 
 #ifdef RTE_ARCH_I686
diff --git a/lib/eal/x86/include/rte_byteorder.h b/lib/eal/x86/include/rte_byteorder.h
index 5a49ffcd50..62a87c1aa6 100644
--- a/lib/eal/x86/include/rte_byteorder.h
+++ b/lib/eal/x86/include/rte_byteorder.h
@@ -48,10 +48,6 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
 	return x;
 }
 
-#ifdef __cplusplus
-}
-#endif
-
 #define rte_bswap16(x) ((uint16_t)(__builtin_constant_p(x) ?		\
 				   rte_constant_bswap16(x) :		\
 				   rte_arch_bswap16(x)))
@@ -69,6 +65,11 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
 #else
 #include "rte_byteorder_64.h"
 #endif
+
+#endif
+
+#ifdef __cplusplus
+}
 #endif
 
 #define rte_cpu_to_le_16(x) (x)
-- 
2.34.1

Re: [PATCH] cpp: Fix missing extern "C" closing bracket

[Bruce Richardson]

On Tue, Jun 24, 2025 at 02:12:53PM +0300, Edvard Fagerholm wrote:
> RTE_FORCE_INTRINSICS with a C++ compiler causes rte_atomic.h and
> rte_byteorder.h to emit on x86 an extern "C" without a closing
> brace.
> ---

Unfortunately, patch is missing a signoff, so isn't eligible for merge. Can
you please submit a v2 with the appropriate signoff included. See [1] for
more details.

For the patch itself, it looks ok to me, but really the various #endif and
#else tags need comments on them to indicate which #ifdef they belong to
(unless it's a very short ifdef). If you do a v2 would you consider adding
those comments to help make the code clearer?

With or without that suggestion, you can include my ack on v2.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Regards,
/Bruce

[1] https://doc.dpdk.org/guides/contributing/patches.html

>  lib/eal/x86/include/rte_atomic.h    | 4 ++--
>  lib/eal/x86/include/rte_byteorder.h | 9 +++++----
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/eal/x86/include/rte_atomic.h b/lib/eal/x86/include/rte_atomic.h
> index c72c47c83e..68629016b4 100644
> --- a/lib/eal/x86/include/rte_atomic.h
> +++ b/lib/eal/x86/include/rte_atomic.h
> @@ -282,10 +282,10 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t *v)
>  	return ret != 0;
>  }
>  
> -#ifdef __cplusplus
> -}
>  #endif
>  
> +#ifdef __cplusplus
> +}
>  #endif
>  
>  #ifdef RTE_ARCH_I686
> diff --git a/lib/eal/x86/include/rte_byteorder.h b/lib/eal/x86/include/rte_byteorder.h
> index 5a49ffcd50..62a87c1aa6 100644
> --- a/lib/eal/x86/include/rte_byteorder.h
> +++ b/lib/eal/x86/include/rte_byteorder.h
> @@ -48,10 +48,6 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
>  	return x;
>  }
>  
> -#ifdef __cplusplus
> -}
> -#endif
> -
>  #define rte_bswap16(x) ((uint16_t)(__builtin_constant_p(x) ?		\
>  				   rte_constant_bswap16(x) :		\
>  				   rte_arch_bswap16(x)))
> @@ -69,6 +65,11 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
>  #else
>  #include "rte_byteorder_64.h"
>  #endif
> +
> +#endif
> +
> +#ifdef __cplusplus
> +}
>  #endif
>  
>  #define rte_cpu_to_le_16(x) (x)
> -- 
> 2.34.1
> 

Re: [PATCH] cpp: Fix missing extern "C" closing bracket

[Edvard Fagerholm]

On Tue, Jun 24, 2025 at 4:53 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Tue, Jun 24, 2025 at 02:12:53PM +0300, Edvard Fagerholm wrote:
> > RTE_FORCE_INTRINSICS with a C++ compiler causes rte_atomic.h and
> > rte_byteorder.h to emit on x86 an extern "C" without a closing
> > brace.
> > ---
>
> Unfortunately, patch is missing a signoff, so isn't eligible for merge. Can
> you please submit a v2 with the appropriate signoff included. See [1] for
> more details.
>
> For the patch itself, it looks ok to me, but really the various #endif and
> #else tags need comments on them to indicate which #ifdef they belong to
> (unless it's a very short ifdef). If you do a v2 would you consider adding
> those comments to help make the code clearer?
>
> With or without that suggestion, you can include my ack on v2.
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
> Regards,
> /Bruce
>
> [1] https://doc.dpdk.org/guides/contributing/patches.html

Thanks. I'll make another patch with the signoff and add the comments.

Best,
Edvard

>
> >  lib/eal/x86/include/rte_atomic.h    | 4 ++--
> >  lib/eal/x86/include/rte_byteorder.h | 9 +++++----
> >  2 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/eal/x86/include/rte_atomic.h b/lib/eal/x86/include/rte_atomic.h
> > index c72c47c83e..68629016b4 100644
> > --- a/lib/eal/x86/include/rte_atomic.h
> > +++ b/lib/eal/x86/include/rte_atomic.h
> > @@ -282,10 +282,10 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t *v)
> >       return ret != 0;
> >  }
> >
> > -#ifdef __cplusplus
> > -}
> >  #endif
> >
> > +#ifdef __cplusplus
> > +}
> >  #endif
> >
> >  #ifdef RTE_ARCH_I686
> > diff --git a/lib/eal/x86/include/rte_byteorder.h b/lib/eal/x86/include/rte_byteorder.h
> > index 5a49ffcd50..62a87c1aa6 100644
> > --- a/lib/eal/x86/include/rte_byteorder.h
> > +++ b/lib/eal/x86/include/rte_byteorder.h
> > @@ -48,10 +48,6 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
> >       return x;
> >  }
> >
> > -#ifdef __cplusplus
> > -}
> > -#endif
> > -
> >  #define rte_bswap16(x) ((uint16_t)(__builtin_constant_p(x) ?         \
> >                                  rte_constant_bswap16(x) :            \
> >                                  rte_arch_bswap16(x)))
> > @@ -69,6 +65,11 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
> >  #else
> >  #include "rte_byteorder_64.h"
> >  #endif
> > +
> > +#endif
> > +
> > +#ifdef __cplusplus
> > +}
> >  #endif
> >
> >  #define rte_cpu_to_le_16(x) (x)
> > --
> > 2.34.1
> >

[PATCH v2] cpp: Fix missing extern "C" closing bracket

[Edvard Fagerholm]

RTE_FORCE_INTRINSICS with a C++ compiler causes rte_atomic.h and
rte_byteorder.h to emit on x86 an extern "C" without a closing
brace.

Signed-off-by: Edvard Fagerholm <edvard.fagerholm@gmail.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/x86/include/rte_atomic.h    | 6 +++---
 lib/eal/x86/include/rte_byteorder.h | 9 +++++----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/eal/x86/include/rte_atomic.h b/lib/eal/x86/include/rte_atomic.h
index c72c47c83e..7ba1722683 100644
--- a/lib/eal/x86/include/rte_atomic.h
+++ b/lib/eal/x86/include/rte_atomic.h
@@ -282,18 +282,18 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t *v)
 	return ret != 0;
 }
 
+#endif /* RTE_FORCE_INTRINSICS */
+
 #ifdef __cplusplus
 }
 #endif
 
-#endif
-
 #ifdef RTE_ARCH_I686
 #include "rte_atomic_32.h"
 #else
 #include "rte_atomic_64.h"
 #endif
 
-#endif
+#endif /* RTE_TOOLCHAIN_MSVC */
 
 #endif /* _RTE_ATOMIC_X86_H_ */
diff --git a/lib/eal/x86/include/rte_byteorder.h b/lib/eal/x86/include/rte_byteorder.h
index 5a49ffcd50..f54bb07cd0 100644
--- a/lib/eal/x86/include/rte_byteorder.h
+++ b/lib/eal/x86/include/rte_byteorder.h
@@ -48,10 +48,6 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
 	return x;
 }
 
-#ifdef __cplusplus
-}
-#endif
-
 #define rte_bswap16(x) ((uint16_t)(__builtin_constant_p(x) ?		\
 				   rte_constant_bswap16(x) :		\
 				   rte_arch_bswap16(x)))
@@ -69,6 +65,11 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
 #else
 #include "rte_byteorder_64.h"
 #endif
+
+#endif /* RTE_FORCE_INTRINSICS */
+
+#ifdef __cplusplus
+}
 #endif
 
 #define rte_cpu_to_le_16(x) (x)
-- 
2.34.1

Re: [PATCH v2] cpp: Fix missing extern "C" closing bracket

[Thomas Monjalon]

24/06/2025 21:58, Edvard Fagerholm:
> RTE_FORCE_INTRINSICS with a C++ compiler causes rte_atomic.h and
> rte_byteorder.h to emit on x86 an extern "C" without a closing
> brace.
> 
> Signed-off-by: Edvard Fagerholm <edvard.fagerholm@gmail.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Added root cause tracking:
Fixes: 719834a6849e ("use C linkage where appropriate in headers")
Cc: stable@dpdk.org

And changed the #endif comments to show the #ifndef condition is negative
with an exclamation mark:
#endif /* !RTE_FORCE_INTRINSICS */

Applied, thanks and welcome!

PS: I see you used AI to code your DPDK app, that's interesting
and we will probably look to improve such tools in a near future.

PS2: Thanks for the mobile games ;)

Re: [PATCH v2] cpp: Fix missing extern "C" closing bracket

[Edvard Fagerholm]

On Fri, Jun 27, 2025 at 6:16 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 24/06/2025 21:58, Edvard Fagerholm:
> > RTE_FORCE_INTRINSICS with a C++ compiler causes rte_atomic.h and
> > rte_byteorder.h to emit on x86 an extern "C" without a closing
> > brace.
> >
> > Signed-off-by: Edvard Fagerholm <edvard.fagerholm@gmail.com>
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
> Added root cause tracking:
> Fixes: 719834a6849e ("use C linkage where appropriate in headers")
> Cc: stable@dpdk.org
>
> And changed the #endif comments to show the #ifndef condition is negative
> with an exclamation mark:
> #endif /* !RTE_FORCE_INTRINSICS */
>
> Applied, thanks and welcome!
>
> PS: I see you used AI to code your DPDK app, that's interesting
> and we will probably look to improve such tools in a near future.
>
> PS2: Thanks for the mobile games ;)
>

LLMs do manage to create the scaffolding for a DPDK application quite
well. Basically, NIC setup with different hardware accelerations is
quite tedious to set up for the first time together with starting
worker threads on the right cores. Since I needed to create my
application for ARM and x86 the LLM produced some clearly incorrect
patterns like turning RTE_FORCE_INTRINSICS on for x86 in the generated
cmake file. Hence, why I ran into this bug in the first place...

However, they also generated very inefficient code like updating
atomic variables with stats on every packet as well as making a copy
of each packet (into a malloc'd buffer). In other words, you won't be
able to vibe code yourself into a very efficient application just yet.
I did get all the scaffolding I needed to port an existing XDP
application to DPDK in only a few days, since the XDP application's
entry point is just getting the raw buffer with the packet, modifying
it in-place and sending it back. This is basically what you get in
DPDK too.

Best,
Edvard