examples/bpf: fix compilation issue
Checks
Commit Message
Example BPF programs t1.c, t2.c, t3.c in folder examples/bpf are
failing to compile with latest dpdk.org master.
The reason is changes in some core DPDK header files, that causes
now inclusion of x86 specific headers.
To overcome the issue, minimize inclusion of DPDK header files
into BPF source code.
Bugzilla ID: 321
Fixes: 9dfc06c26a8b ("test/bpf: add samples")
Cc: stable@dpdk.org
Reported-by: Michel Machado <michel@digirati.com.br>
Suggested-by: Michel Machado <michel@digirati.com.br>
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
examples/bpf/mbuf.h | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
Comments
30/07/2019 12:19, Konstantin Ananyev:
> Example BPF programs t1.c, t2.c, t3.c in folder examples/bpf are
> failing to compile with latest dpdk.org master.
> The reason is changes in some core DPDK header files, that causes
> now inclusion of x86 specific headers.
> To overcome the issue, minimize inclusion of DPDK header files
> into BPF source code.
>
> Bugzilla ID: 321
>
> Fixes: 9dfc06c26a8b ("test/bpf: add samples")
> Cc: stable@dpdk.org
>
> Reported-by: Michel Machado <michel@digirati.com.br>
> Suggested-by: Michel Machado <michel@digirati.com.br>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
> examples/bpf/mbuf.h | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
I think that's really a bad idea to have this file.
The BPF applications are supposed to update their own copy of mbuf?
Please could you try to include rte_mbuf.h
instead of duplicating the mbuf layout?
snipped
> Example BPF programs t1.c, t2.c, t3.c in folder examples/bpf are failing to
> compile with latest dpdk.org master.
As a note, the file t3.c is one which fails to get compiled.
> The reason is changes in some core DPDK header files, that causes now inclusion
> of x86 specific headers.
snipped
>
> #include <stdint.h>
> #include <rte_common.h>
> -#include <rte_memory.h>
>
> #ifdef __cplusplus
> extern "C" {
> @@ -364,6 +363,23 @@ typedef struct {
> volatile int16_t cnt; /**< An internal counter value. */ } rte_atomic16_t;
>
> +#define RTE_CACHE_LINE_MIN_SIZE 64 /**< Minimum Cache line size. */
The definition for RTE_CACHE_LINE_MIN_SIZE is present in ` rte_config.h`. Using the same did not cause compilation issues. Is there specific reason not to use the same?
Snipped
The code is tested and verified with clang6.0 and clang8.0 on Ubuntu 18.04.2 LTS.
Tested-by: Vipin Varghese <vipin.varghese@intel.com>
>
> > Example BPF programs t1.c, t2.c, t3.c in folder examples/bpf are failing to
> > compile with latest dpdk.org master.
>
> As a note, the file t3.c is one which fails to get compiled.
t2.c also uses rte_mbuf, so same story for both.
>
> > The reason is changes in some core DPDK header files, that causes now inclusion
> > of x86 specific headers.
>
> snipped
>
>
> >
> > #include <stdint.h>
> > #include <rte_common.h>
> > -#include <rte_memory.h>
> >
> > #ifdef __cplusplus
> > extern "C" {
> > @@ -364,6 +363,23 @@ typedef struct {
> > volatile int16_t cnt; /**< An internal counter value. */ } rte_atomic16_t;
> >
> > +#define RTE_CACHE_LINE_MIN_SIZE 64 /**< Minimum Cache line size. */
>
> The definition for RTE_CACHE_LINE_MIN_SIZE is present in ` rte_config.h`.
I believe it is not:
$ find lib config -type f | xargs grep CACHE_LINE_MIN_SIZE
lib/librte_eal/linux/eal/include/rte_kni_common.h:#define RTE_CACHE_LINE_MIN_SIZE 64
lib/librte_eal/linux/eal/include/rte_kni_common.h: char pad3[8] __attribute__((__aligned__(RTE_CACHE_LINE_MIN_SIZE)));
lib/librte_eal/common/include/rte_memory.h:#define RTE_CACHE_LINE_MIN_SIZE 64 /**< Minimum Cache line size. */
lib/librte_eal/common/include/rte_memory.h:#define __rte_cache_min_aligned __rte_aligned(RTE_CACHE_LINE_MIN_SIZE)
Konstantin
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Tuesday, July 30, 2019 6:53 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; olivier.matz@6wind.com
> Subject: Re: [dpdk-stable] [PATCH] examples/bpf: fix compilation issue
>
> 30/07/2019 12:19, Konstantin Ananyev:
> > Example BPF programs t1.c, t2.c, t3.c in folder examples/bpf are
> > failing to compile with latest dpdk.org master.
> > The reason is changes in some core DPDK header files, that causes
> > now inclusion of x86 specific headers.
> > To overcome the issue, minimize inclusion of DPDK header files
> > into BPF source code.
> >
> > Bugzilla ID: 321
> >
> > Fixes: 9dfc06c26a8b ("test/bpf: add samples")
> > Cc: stable@dpdk.org
> >
> > Reported-by: Michel Machado <michel@digirati.com.br>
> > Suggested-by: Michel Machado <michel@digirati.com.br>
> > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > ---
> > examples/bpf/mbuf.h | 24 ++++++++++++++++++++++--
> > 1 file changed, 22 insertions(+), 2 deletions(-)
>
> I think that's really a bad idea to have this file.
> The BPF applications are supposed to update their own copy of mbuf?
Right now, yes (same as KNI).
> Please could you try to include rte_mbuf.h
> instead of duplicating the mbuf layout?
I don't think it is possible without some rework on rte_mbuf.h itself.
I thought about that, but for that we'll probably need to put just
struct rte_mbuf definition into a separate file (rte_mbuf_core.h or so)
and might be some related definitions into rte_common.h or so.
Then re_mbuf.h will include rte_mbuf_core.h while bpf (and might be KNI?)
can include just rte_mbuf_core.h without any extra arch specific headers.
Another alternative probably to define bpf as a separate arch
(though don't know how big effort it will be).
I planned to try something like that, but then totally forgot.
And now it is too late, we are at RC4 already .
Konstantin
Snipped
> > > Example BPF programs t1.c, t2.c, t3.c in folder examples/bpf are
> > > failing to compile with latest dpdk.org master.
> >
> > As a note, the file t3.c is one which fails to get compiled.
>
> t2.c also uses rte_mbuf, so same story for both.
Thank you. So, the rewrite will be ' Example BPF programs t2.c, and t3.c in folder examples/bpf are failing to compile with latest dpdk.org master.'
>
> >
> > > The reason is changes in some core DPDK header files, that causes
> > > now inclusion of x86 specific headers.
> >
> > snipped
> >
> >
> > >
> > > #include <stdint.h>
> > > #include <rte_common.h>
> > > -#include <rte_memory.h>
> > >
> > > #ifdef __cplusplus
> > > extern "C" {
> > > @@ -364,6 +363,23 @@ typedef struct {
> > > volatile int16_t cnt; /**< An internal counter value. */ }
> > > rte_atomic16_t;
> > >
> > > +#define RTE_CACHE_LINE_MIN_SIZE 64 /**< Minimum Cache line size.
> */
> >
> > The definition for RTE_CACHE_LINE_MIN_SIZE is present in ` rte_config.h`.
>
> I believe it is not:
> $ find lib config -type f | xargs grep CACHE_LINE_MIN_SIZE
> lib/librte_eal/linux/eal/include/rte_kni_common.h:#define
> RTE_CACHE_LINE_MIN_SIZE 64
> lib/librte_eal/linux/eal/include/rte_kni_common.h: char pad3[8]
> __attribute__((__aligned__(RTE_CACHE_LINE_MIN_SIZE)));
> lib/librte_eal/common/include/rte_memory.h:#define
> RTE_CACHE_LINE_MIN_SIZE 64 /**< Minimum Cache line size. */
> lib/librte_eal/common/include/rte_memory.h:#define
> __rte_cache_min_aligned __rte_aligned(RTE_CACHE_LINE_MIN_SIZE)
>
> Konstantin
Thanks. So, is not the best approach to place RTE_CACHE_LINE_MIN_SIZE in rte_config.h to prevent multiple definition (rte_kni_common.h, rte_memory.h and examples/bpf/mbuf.h)?
> -----Original Message-----
> From: Varghese, Vipin
> Sent: Wednesday, July 31, 2019 9:20 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] examples/bpf: fix compilation issue
>
> Snipped
>
> > > > Example BPF programs t1.c, t2.c, t3.c in folder examples/bpf are
> > > > failing to compile with latest dpdk.org master.
> > >
> > > As a note, the file t3.c is one which fails to get compiled.
> >
> > t2.c also uses rte_mbuf, so same story for both.
>
> Thank you. So, the rewrite will be ' Example BPF programs t2.c, and t3.c in folder examples/bpf are failing to compile with latest dpdk.org
> master.'
>
> >
> > >
> > > > The reason is changes in some core DPDK header files, that causes
> > > > now inclusion of x86 specific headers.
> > >
> > > snipped
> > >
> > >
> > > >
> > > > #include <stdint.h>
> > > > #include <rte_common.h>
> > > > -#include <rte_memory.h>
> > > >
> > > > #ifdef __cplusplus
> > > > extern "C" {
> > > > @@ -364,6 +363,23 @@ typedef struct {
> > > > volatile int16_t cnt; /**< An internal counter value. */ }
> > > > rte_atomic16_t;
> > > >
> > > > +#define RTE_CACHE_LINE_MIN_SIZE 64 /**< Minimum Cache line size.
> > */
> > >
> > > The definition for RTE_CACHE_LINE_MIN_SIZE is present in ` rte_config.h`.
> >
> > I believe it is not:
> > $ find lib config -type f | xargs grep CACHE_LINE_MIN_SIZE
> > lib/librte_eal/linux/eal/include/rte_kni_common.h:#define
> > RTE_CACHE_LINE_MIN_SIZE 64
> > lib/librte_eal/linux/eal/include/rte_kni_common.h: char pad3[8]
> > __attribute__((__aligned__(RTE_CACHE_LINE_MIN_SIZE)));
> > lib/librte_eal/common/include/rte_memory.h:#define
> > RTE_CACHE_LINE_MIN_SIZE 64 /**< Minimum Cache line size. */
> > lib/librte_eal/common/include/rte_memory.h:#define
> > __rte_cache_min_aligned __rte_aligned(RTE_CACHE_LINE_MIN_SIZE)
> >
> > Konstantin
>
> Thanks. So, is not the best approach to place RTE_CACHE_LINE_MIN_SIZE in rte_config.h to prevent multiple definition
> (rte_kni_common.h, rte_memory.h and examples/bpf/mbuf.h)?
Probably, but I don't want to touch rte_common.h in RC4.
Konstantin
31/07/2019 09:26, Ananyev, Konstantin:
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 30/07/2019 12:19, Konstantin Ananyev:
> > > Example BPF programs t1.c, t2.c, t3.c in folder examples/bpf are
> > > failing to compile with latest dpdk.org master.
> > > The reason is changes in some core DPDK header files, that causes
> > > now inclusion of x86 specific headers.
> > > To overcome the issue, minimize inclusion of DPDK header files
> > > into BPF source code.
> > >
> > > Bugzilla ID: 321
> > >
> > > Fixes: 9dfc06c26a8b ("test/bpf: add samples")
> > > Cc: stable@dpdk.org
> > >
> > > Reported-by: Michel Machado <michel@digirati.com.br>
> > > Suggested-by: Michel Machado <michel@digirati.com.br>
> > > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > > ---
> > > examples/bpf/mbuf.h | 24 ++++++++++++++++++++++--
> > > 1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > I think that's really a bad idea to have this file.
> > The BPF applications are supposed to update their own copy of mbuf?
>
> Right now, yes (same as KNI).
>
> > Please could you try to include rte_mbuf.h
> > instead of duplicating the mbuf layout?
>
> I don't think it is possible without some rework on rte_mbuf.h itself.
> I thought about that, but for that we'll probably need to put just
> struct rte_mbuf definition into a separate file (rte_mbuf_core.h or so)
> and might be some related definitions into rte_common.h or so.
> Then re_mbuf.h will include rte_mbuf_core.h while bpf (and might be KNI?)
> can include just rte_mbuf_core.h without any extra arch specific headers.
> Another alternative probably to define bpf as a separate arch
> (though don't know how big effort it will be).
> I planned to try something like that, but then totally forgot.
> And now it is too late, we are at RC4 already .
Applied as workaround for 19.08.
Please try to remove this file for 19.11.
@@ -13,7 +13,6 @@
#include <stdint.h>
#include <rte_common.h>
-#include <rte_memory.h>
#ifdef __cplusplus
extern "C" {
@@ -364,6 +363,23 @@ typedef struct {
volatile int16_t cnt; /**< An internal counter value. */
} rte_atomic16_t;
+#define RTE_CACHE_LINE_MIN_SIZE 64 /**< Minimum Cache line size. */
+
+/**
+ * Force minimum cache line alignment.
+ */
+#define __rte_cache_min_aligned __rte_aligned(RTE_CACHE_LINE_MIN_SIZE)
+
+/**
+ * IO virtual address type.
+ * When the physical addressing mode (IOVA as PA) is in use,
+ * the translation from an IO virtual address (IOVA) to a physical address
+ * is a direct mapping, i.e. the same value.
+ * Otherwise, in virtual mode (IOVA as VA), an IOMMU may do the translation.
+ */
+typedef uint64_t rte_iova_t;
+#define RTE_BAD_IOVA ((rte_iova_t)-1)
+
/**
* The generic rte_mbuf, containing a packet mbuf.
*/
@@ -377,7 +393,11 @@ struct rte_mbuf {
* same mbuf cacheline0 layout for 32-bit and 64-bit. This makes
* working on vector drivers easier.
*/
- phys_addr_t buf_physaddr __rte_aligned(sizeof(phys_addr_t));
+ RTE_STD_C11
+ union {
+ rte_iova_t buf_iova;
+ rte_iova_t buf_physaddr; /**< deprecated */
+ } __rte_aligned(sizeof(rte_iova_t));
/* next 8 bytes are initialised on RX descriptor rearm */
MARKER64 rearm_data;