| Message ID | 20230801102942.2629385-1-alan.maguire@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf] selftests/bpf: fix static assert compilation issue for test_cls_*.c | expand |
On 8/1/23 3:29 AM, Alan Maguire wrote: > commit bdeeed3498c7 ("libbpf: fix offsetof() and container_of() to work with CO-RE") > > ...was backported to stable trees such as 5.15. The problem is that with older > LLVM/clang (14/15) - which is often used for older kernels - we see compilation > failures in BPF selftests now: > > In file included from progs/test_cls_redirect_subprogs.c:2: > progs/test_cls_redirect.c:90:2: error: static assertion expression is not an integral constant expression > sizeof(flow_ports_t) != > ^~~~~~~~~~~~~~~~~~~~~~~ > progs/test_cls_redirect.c:91:3: note: cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression > offsetofend(struct bpf_sock_tuple, ipv4.dport) - > ^ > progs/test_cls_redirect.c:32:3: note: expanded from macro 'offsetofend' > (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) > ^ > tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:86:33: note: expanded from macro 'offsetof' > ^ > In file included from progs/test_cls_redirect_subprogs.c:2: > progs/test_cls_redirect.c:95:2: error: static assertion expression is not an integral constant expression > sizeof(flow_ports_t) != > ^~~~~~~~~~~~~~~~~~~~~~~ > progs/test_cls_redirect.c:96:3: note: cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression > offsetofend(struct bpf_sock_tuple, ipv6.dport) - > ^ > progs/test_cls_redirect.c:32:3: note: expanded from macro 'offsetofend' > (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) > ^ > tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:86:33: note: expanded from macro 'offsetof' > ^ > 2 errors generated. > make: *** [Makefile:594: tools/testing/selftests/bpf/test_cls_redirect_subprogs.bpf.o] Error 1 > > The problem is the new offsetof() does not play nice with static asserts. > Given that the context is a static assert (and CO-RE relocation is not > needed at compile time), offsetof() usage can be replaced by > __builtin_offsetof(), and all is well. Define __builtin_offsetofend() > to be used in static asserts also, since offsetofend() is also defined in > bpf_util.h and is used in userspace progs, so redefining offsetofend() > in test_cls_redirect.h won't work. > > Fixes: bdeeed3498c7 ("libbpf: fix offsetof() and container_of() to work with CO-RE") > Reported-by: Colm Harrington <colm.harrington@oracle.com> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com> > --- > tools/testing/selftests/bpf/progs/test_cls_redirect.c | 11 ++++------- > tools/testing/selftests/bpf/progs/test_cls_redirect.h | 3 +++ > .../selftests/bpf/progs/test_cls_redirect_dynptr.c | 11 ++++------- > 3 files changed, 11 insertions(+), 14 deletions(-) > > diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect.c b/tools/testing/selftests/bpf/progs/test_cls_redirect.c > index 66b304982245..e68e0544827c 100644 > --- a/tools/testing/selftests/bpf/progs/test_cls_redirect.c > +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect.c > @@ -28,9 +28,6 @@ > #define INLINING __always_inline > #endif > > -#define offsetofend(TYPE, MEMBER) \ > - (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) > - > #define IP_OFFSET_MASK (0x1FFF) > #define IP_MF (0x2000) > > @@ -88,13 +85,13 @@ typedef struct { > > _Static_assert( > sizeof(flow_ports_t) != > - offsetofend(struct bpf_sock_tuple, ipv4.dport) - > - offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, > + __builtin_offsetofend(struct bpf_sock_tuple, ipv4.dport) - > + __builtin_offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, > "flow_ports_t must match sport and dport in struct bpf_sock_tuple"); > _Static_assert( > sizeof(flow_ports_t) != > - offsetofend(struct bpf_sock_tuple, ipv6.dport) - > - offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, > + __builtin_offsetofend(struct bpf_sock_tuple, ipv6.dport) - > + __builtin_offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, > "flow_ports_t must match sport and dport in struct bpf_sock_tuple"); > > typedef int ret_t; > diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect.h b/tools/testing/selftests/bpf/progs/test_cls_redirect.h > index 76eab0aacba0..1de0b727a3f6 100644 > --- a/tools/testing/selftests/bpf/progs/test_cls_redirect.h > +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect.h > @@ -12,6 +12,9 @@ > #include <linux/ipv6.h> > #include <linux/udp.h> > > +#define __builtin_offsetofend(TYPE, MEMBER) \ > + (__builtin_offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) I think this can be simplified to undef and re-define offsetof like below: #ifdef offsetof #undef offsetof #define offsetof(type, member) __builtin_offsetof(type, member) #endif Then other changes in this patch become unnecessary. You can add comments for the above code to explain why you want to redefine 'offsetof'. > + > struct gre_base_hdr { > uint16_t flags; > uint16_t protocol; > diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c b/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c > index f41c81212ee9..463b0513f871 100644 > --- a/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c > +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c > @@ -23,9 +23,6 @@ > #include "test_cls_redirect.h" > #include "bpf_kfuncs.h" > > -#define offsetofend(TYPE, MEMBER) \ > - (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) > - > #define IP_OFFSET_MASK (0x1FFF) > #define IP_MF (0x2000) > > @@ -83,13 +80,13 @@ typedef struct { > > _Static_assert( > sizeof(flow_ports_t) != > - offsetofend(struct bpf_sock_tuple, ipv4.dport) - > - offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, > + __builtin_offsetofend(struct bpf_sock_tuple, ipv4.dport) - > + __builtin_offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, > "flow_ports_t must match sport and dport in struct bpf_sock_tuple"); > _Static_assert( > sizeof(flow_ports_t) != > - offsetofend(struct bpf_sock_tuple, ipv6.dport) - > - offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, > + __builtin_offsetofend(struct bpf_sock_tuple, ipv6.dport) - > + __builtin_offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, > "flow_ports_t must match sport and dport in struct bpf_sock_tuple"); > > struct iphdr_info {
On 01/08/2023 18:09, Yonghong Song wrote: > > > On 8/1/23 3:29 AM, Alan Maguire wrote: >> commit bdeeed3498c7 ("libbpf: fix offsetof() and container_of() to >> work with CO-RE") >> >> ...was backported to stable trees such as 5.15. The problem is that >> with older >> LLVM/clang (14/15) - which is often used for older kernels - we see >> compilation >> failures in BPF selftests now: >> >> In file included from progs/test_cls_redirect_subprogs.c:2: >> progs/test_cls_redirect.c:90:2: error: static assertion expression is >> not an integral constant expression >> sizeof(flow_ports_t) != >> ^~~~~~~~~~~~~~~~~~~~~~~ >> progs/test_cls_redirect.c:91:3: note: cast that performs the >> conversions of a reinterpret_cast is not allowed in a constant expression >> offsetofend(struct bpf_sock_tuple, ipv4.dport) - >> ^ >> progs/test_cls_redirect.c:32:3: note: expanded from macro 'offsetofend' >> (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) >> ^ >> tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:86:33: >> note: expanded from macro 'offsetof' >> ^ >> In file included from progs/test_cls_redirect_subprogs.c:2: >> progs/test_cls_redirect.c:95:2: error: static assertion expression is >> not an integral constant expression >> sizeof(flow_ports_t) != >> ^~~~~~~~~~~~~~~~~~~~~~~ >> progs/test_cls_redirect.c:96:3: note: cast that performs the >> conversions of a reinterpret_cast is not allowed in a constant expression >> offsetofend(struct bpf_sock_tuple, ipv6.dport) - >> ^ >> progs/test_cls_redirect.c:32:3: note: expanded from macro 'offsetofend' >> (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) >> ^ >> tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:86:33: >> note: expanded from macro 'offsetof' >> ^ >> 2 errors generated. >> make: *** [Makefile:594: >> tools/testing/selftests/bpf/test_cls_redirect_subprogs.bpf.o] Error 1 >> >> The problem is the new offsetof() does not play nice with static asserts. >> Given that the context is a static assert (and CO-RE relocation is not >> needed at compile time), offsetof() usage can be replaced by >> __builtin_offsetof(), and all is well. Define __builtin_offsetofend() >> to be used in static asserts also, since offsetofend() is also defined in >> bpf_util.h and is used in userspace progs, so redefining offsetofend() >> in test_cls_redirect.h won't work. >> >> Fixes: bdeeed3498c7 ("libbpf: fix offsetof() and container_of() to >> work with CO-RE") >> Reported-by: Colm Harrington <colm.harrington@oracle.com> >> Signed-off-by: Alan Maguire <alan.maguire@oracle.com> >> --- >> tools/testing/selftests/bpf/progs/test_cls_redirect.c | 11 ++++------- >> tools/testing/selftests/bpf/progs/test_cls_redirect.h | 3 +++ >> .../selftests/bpf/progs/test_cls_redirect_dynptr.c | 11 ++++------- >> 3 files changed, 11 insertions(+), 14 deletions(-) >> >> diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect.c >> b/tools/testing/selftests/bpf/progs/test_cls_redirect.c >> index 66b304982245..e68e0544827c 100644 >> --- a/tools/testing/selftests/bpf/progs/test_cls_redirect.c >> +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect.c >> @@ -28,9 +28,6 @@ >> #define INLINING __always_inline >> #endif >> -#define offsetofend(TYPE, MEMBER) \ >> - (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) >> - >> #define IP_OFFSET_MASK (0x1FFF) >> #define IP_MF (0x2000) >> @@ -88,13 +85,13 @@ typedef struct { >> _Static_assert( >> sizeof(flow_ports_t) != >> - offsetofend(struct bpf_sock_tuple, ipv4.dport) - >> - offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, >> + __builtin_offsetofend(struct bpf_sock_tuple, ipv4.dport) - >> + __builtin_offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, >> "flow_ports_t must match sport and dport in struct >> bpf_sock_tuple"); >> _Static_assert( >> sizeof(flow_ports_t) != >> - offsetofend(struct bpf_sock_tuple, ipv6.dport) - >> - offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, >> + __builtin_offsetofend(struct bpf_sock_tuple, ipv6.dport) - >> + __builtin_offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, >> "flow_ports_t must match sport and dport in struct >> bpf_sock_tuple"); >> typedef int ret_t; >> diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect.h >> b/tools/testing/selftests/bpf/progs/test_cls_redirect.h >> index 76eab0aacba0..1de0b727a3f6 100644 >> --- a/tools/testing/selftests/bpf/progs/test_cls_redirect.h >> +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect.h >> @@ -12,6 +12,9 @@ >> #include <linux/ipv6.h> >> #include <linux/udp.h> >> +#define __builtin_offsetofend(TYPE, MEMBER) \ >> + (__builtin_offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) > > I think this can be simplified to undef and re-define offsetof like below: > > #ifdef offsetof > #undef offsetof > #define offsetof(type, member) __builtin_offsetof(type, member) > #endif > > Then other changes in this patch become unnecessary. > > You can add comments for the above code to explain > why you want to redefine 'offsetof'. > That's one way to solve it alright, but then other instances of offsetof() in the BPF code (that are not part of static asserts) aren't CO-RE-safe. Probably not a big concern for a test case that is usually run against the same kernel, but it's perhaps worth retaining the distinction between compile-time and run-time needs? Alan > >> + >> struct gre_base_hdr { >> uint16_t flags; >> uint16_t protocol; >> diff --git >> a/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c >> b/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c >> index f41c81212ee9..463b0513f871 100644 >> --- a/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c >> +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c >> @@ -23,9 +23,6 @@ >> #include "test_cls_redirect.h" >> #include "bpf_kfuncs.h" >> -#define offsetofend(TYPE, MEMBER) \ >> - (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) >> - >> #define IP_OFFSET_MASK (0x1FFF) >> #define IP_MF (0x2000) >> @@ -83,13 +80,13 @@ typedef struct { >> _Static_assert( >> sizeof(flow_ports_t) != >> - offsetofend(struct bpf_sock_tuple, ipv4.dport) - >> - offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, >> + __builtin_offsetofend(struct bpf_sock_tuple, ipv4.dport) - >> + __builtin_offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, >> "flow_ports_t must match sport and dport in struct >> bpf_sock_tuple"); >> _Static_assert( >> sizeof(flow_ports_t) != >> - offsetofend(struct bpf_sock_tuple, ipv6.dport) - >> - offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, >> + __builtin_offsetofend(struct bpf_sock_tuple, ipv6.dport) - >> + __builtin_offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, >> "flow_ports_t must match sport and dport in struct >> bpf_sock_tuple"); >> struct iphdr_info {
On 8/1/23 10:57 AM, Alan Maguire wrote: > On 01/08/2023 18:09, Yonghong Song wrote: >> >> >> On 8/1/23 3:29 AM, Alan Maguire wrote: >>> commit bdeeed3498c7 ("libbpf: fix offsetof() and container_of() to >>> work with CO-RE") >>> >>> ...was backported to stable trees such as 5.15. The problem is that >>> with older >>> LLVM/clang (14/15) - which is often used for older kernels - we see >>> compilation >>> failures in BPF selftests now: >>> >>> In file included from progs/test_cls_redirect_subprogs.c:2: >>> progs/test_cls_redirect.c:90:2: error: static assertion expression is >>> not an integral constant expression >>> sizeof(flow_ports_t) != >>> ^~~~~~~~~~~~~~~~~~~~~~~ >>> progs/test_cls_redirect.c:91:3: note: cast that performs the >>> conversions of a reinterpret_cast is not allowed in a constant expression >>> offsetofend(struct bpf_sock_tuple, ipv4.dport) - >>> ^ >>> progs/test_cls_redirect.c:32:3: note: expanded from macro 'offsetofend' >>> (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) >>> ^ >>> tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:86:33: >>> note: expanded from macro 'offsetof' >>> ^ >>> In file included from progs/test_cls_redirect_subprogs.c:2: >>> progs/test_cls_redirect.c:95:2: error: static assertion expression is >>> not an integral constant expression >>> sizeof(flow_ports_t) != >>> ^~~~~~~~~~~~~~~~~~~~~~~ >>> progs/test_cls_redirect.c:96:3: note: cast that performs the >>> conversions of a reinterpret_cast is not allowed in a constant expression >>> offsetofend(struct bpf_sock_tuple, ipv6.dport) - >>> ^ >>> progs/test_cls_redirect.c:32:3: note: expanded from macro 'offsetofend' >>> (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) >>> ^ >>> tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:86:33: >>> note: expanded from macro 'offsetof' >>> ^ >>> 2 errors generated. >>> make: *** [Makefile:594: >>> tools/testing/selftests/bpf/test_cls_redirect_subprogs.bpf.o] Error 1 >>> >>> The problem is the new offsetof() does not play nice with static asserts. >>> Given that the context is a static assert (and CO-RE relocation is not >>> needed at compile time), offsetof() usage can be replaced by >>> __builtin_offsetof(), and all is well. Define __builtin_offsetofend() >>> to be used in static asserts also, since offsetofend() is also defined in >>> bpf_util.h and is used in userspace progs, so redefining offsetofend() >>> in test_cls_redirect.h won't work. >>> >>> Fixes: bdeeed3498c7 ("libbpf: fix offsetof() and container_of() to >>> work with CO-RE") >>> Reported-by: Colm Harrington <colm.harrington@oracle.com> >>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com> >>> --- >>> tools/testing/selftests/bpf/progs/test_cls_redirect.c | 11 ++++------- >>> tools/testing/selftests/bpf/progs/test_cls_redirect.h | 3 +++ >>> .../selftests/bpf/progs/test_cls_redirect_dynptr.c | 11 ++++------- >>> 3 files changed, 11 insertions(+), 14 deletions(-) >>> >>> diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect.c >>> b/tools/testing/selftests/bpf/progs/test_cls_redirect.c >>> index 66b304982245..e68e0544827c 100644 >>> --- a/tools/testing/selftests/bpf/progs/test_cls_redirect.c >>> +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect.c >>> @@ -28,9 +28,6 @@ >>> #define INLINING __always_inline >>> #endif >>> -#define offsetofend(TYPE, MEMBER) \ >>> - (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) >>> - >>> #define IP_OFFSET_MASK (0x1FFF) >>> #define IP_MF (0x2000) >>> @@ -88,13 +85,13 @@ typedef struct { >>> _Static_assert( >>> sizeof(flow_ports_t) != >>> - offsetofend(struct bpf_sock_tuple, ipv4.dport) - >>> - offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, >>> + __builtin_offsetofend(struct bpf_sock_tuple, ipv4.dport) - >>> + __builtin_offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, >>> "flow_ports_t must match sport and dport in struct >>> bpf_sock_tuple"); >>> _Static_assert( >>> sizeof(flow_ports_t) != >>> - offsetofend(struct bpf_sock_tuple, ipv6.dport) - >>> - offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, >>> + __builtin_offsetofend(struct bpf_sock_tuple, ipv6.dport) - >>> + __builtin_offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, >>> "flow_ports_t must match sport and dport in struct >>> bpf_sock_tuple"); >>> typedef int ret_t; >>> diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect.h >>> b/tools/testing/selftests/bpf/progs/test_cls_redirect.h >>> index 76eab0aacba0..1de0b727a3f6 100644 >>> --- a/tools/testing/selftests/bpf/progs/test_cls_redirect.h >>> +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect.h >>> @@ -12,6 +12,9 @@ >>> #include <linux/ipv6.h> >>> #include <linux/udp.h> >>> +#define __builtin_offsetofend(TYPE, MEMBER) \ >>> + (__builtin_offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) >> >> I think this can be simplified to undef and re-define offsetof like below: >> >> #ifdef offsetof >> #undef offsetof >> #define offsetof(type, member) __builtin_offsetof(type, member) >> #endif >> >> Then other changes in this patch become unnecessary. >> >> You can add comments for the above code to explain >> why you want to redefine 'offsetof'. >> > > That's one way to solve it alright, but then other instances of > offsetof() in the BPF code (that are not part of static asserts) aren't > CO-RE-safe. Probably not a big concern for a test case that is usually There are no CORE relocation here. vmlinux.h is not involved and no explicit CORE relocation requests in the related C code. Also I checked all offsetof usage in related C files (test_cls_redirect.c, test_cls_redirect_dynptr.c) and the offsetof only operates on uapi headers so CORE relocation for them are not needed. > run against the same kernel, but it's perhaps worth retaining the > distinction between compile-time and run-time needs? > > Alan > >> >>> + >>> struct gre_base_hdr { >>> uint16_t flags; >>> uint16_t protocol; >>> diff --git >>> a/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c >>> b/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c >>> index f41c81212ee9..463b0513f871 100644 >>> --- a/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c >>> +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c >>> @@ -23,9 +23,6 @@ >>> #include "test_cls_redirect.h" >>> #include "bpf_kfuncs.h" >>> -#define offsetofend(TYPE, MEMBER) \ >>> - (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) >>> - >>> #define IP_OFFSET_MASK (0x1FFF) >>> #define IP_MF (0x2000) >>> @@ -83,13 +80,13 @@ typedef struct { >>> _Static_assert( >>> sizeof(flow_ports_t) != >>> - offsetofend(struct bpf_sock_tuple, ipv4.dport) - >>> - offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, >>> + __builtin_offsetofend(struct bpf_sock_tuple, ipv4.dport) - >>> + __builtin_offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, >>> "flow_ports_t must match sport and dport in struct >>> bpf_sock_tuple"); >>> _Static_assert( >>> sizeof(flow_ports_t) != >>> - offsetofend(struct bpf_sock_tuple, ipv6.dport) - >>> - offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, >>> + __builtin_offsetofend(struct bpf_sock_tuple, ipv6.dport) - >>> + __builtin_offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, >>> "flow_ports_t must match sport and dport in struct >>> bpf_sock_tuple"); >>> struct iphdr_info {
diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect.c b/tools/testing/selftests/bpf/progs/test_cls_redirect.c index 66b304982245..e68e0544827c 100644 --- a/tools/testing/selftests/bpf/progs/test_cls_redirect.c +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect.c @@ -28,9 +28,6 @@ #define INLINING __always_inline #endif -#define offsetofend(TYPE, MEMBER) \ - (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) - #define IP_OFFSET_MASK (0x1FFF) #define IP_MF (0x2000) @@ -88,13 +85,13 @@ typedef struct { _Static_assert( sizeof(flow_ports_t) != - offsetofend(struct bpf_sock_tuple, ipv4.dport) - - offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, + __builtin_offsetofend(struct bpf_sock_tuple, ipv4.dport) - + __builtin_offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, "flow_ports_t must match sport and dport in struct bpf_sock_tuple"); _Static_assert( sizeof(flow_ports_t) != - offsetofend(struct bpf_sock_tuple, ipv6.dport) - - offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, + __builtin_offsetofend(struct bpf_sock_tuple, ipv6.dport) - + __builtin_offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, "flow_ports_t must match sport and dport in struct bpf_sock_tuple"); typedef int ret_t; diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect.h b/tools/testing/selftests/bpf/progs/test_cls_redirect.h index 76eab0aacba0..1de0b727a3f6 100644 --- a/tools/testing/selftests/bpf/progs/test_cls_redirect.h +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect.h @@ -12,6 +12,9 @@ #include <linux/ipv6.h> #include <linux/udp.h> +#define __builtin_offsetofend(TYPE, MEMBER) \ + (__builtin_offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) + struct gre_base_hdr { uint16_t flags; uint16_t protocol; diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c b/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c index f41c81212ee9..463b0513f871 100644 --- a/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c @@ -23,9 +23,6 @@ #include "test_cls_redirect.h" #include "bpf_kfuncs.h" -#define offsetofend(TYPE, MEMBER) \ - (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) - #define IP_OFFSET_MASK (0x1FFF) #define IP_MF (0x2000) @@ -83,13 +80,13 @@ typedef struct { _Static_assert( sizeof(flow_ports_t) != - offsetofend(struct bpf_sock_tuple, ipv4.dport) - - offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, + __builtin_offsetofend(struct bpf_sock_tuple, ipv4.dport) - + __builtin_offsetof(struct bpf_sock_tuple, ipv4.sport) - 1, "flow_ports_t must match sport and dport in struct bpf_sock_tuple"); _Static_assert( sizeof(flow_ports_t) != - offsetofend(struct bpf_sock_tuple, ipv6.dport) - - offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, + __builtin_offsetofend(struct bpf_sock_tuple, ipv6.dport) - + __builtin_offsetof(struct bpf_sock_tuple, ipv6.sport) - 1, "flow_ports_t must match sport and dport in struct bpf_sock_tuple"); struct iphdr_info {
commit bdeeed3498c7 ("libbpf: fix offsetof() and container_of() to work with CO-RE") ...was backported to stable trees such as 5.15. The problem is that with older LLVM/clang (14/15) - which is often used for older kernels - we see compilation failures in BPF selftests now: In file included from progs/test_cls_redirect_subprogs.c:2: progs/test_cls_redirect.c:90:2: error: static assertion expression is not an integral constant expression sizeof(flow_ports_t) != ^~~~~~~~~~~~~~~~~~~~~~~ progs/test_cls_redirect.c:91:3: note: cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression offsetofend(struct bpf_sock_tuple, ipv4.dport) - ^ progs/test_cls_redirect.c:32:3: note: expanded from macro 'offsetofend' (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) ^ tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:86:33: note: expanded from macro 'offsetof' ^ In file included from progs/test_cls_redirect_subprogs.c:2: progs/test_cls_redirect.c:95:2: error: static assertion expression is not an integral constant expression sizeof(flow_ports_t) != ^~~~~~~~~~~~~~~~~~~~~~~ progs/test_cls_redirect.c:96:3: note: cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression offsetofend(struct bpf_sock_tuple, ipv6.dport) - ^ progs/test_cls_redirect.c:32:3: note: expanded from macro 'offsetofend' (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER))) ^ tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:86:33: note: expanded from macro 'offsetof' ^ 2 errors generated. make: *** [Makefile:594: tools/testing/selftests/bpf/test_cls_redirect_subprogs.bpf.o] Error 1 The problem is the new offsetof() does not play nice with static asserts. Given that the context is a static assert (and CO-RE relocation is not needed at compile time), offsetof() usage can be replaced by __builtin_offsetof(), and all is well. Define __builtin_offsetofend() to be used in static asserts also, since offsetofend() is also defined in bpf_util.h and is used in userspace progs, so redefining offsetofend() in test_cls_redirect.h won't work. Fixes: bdeeed3498c7 ("libbpf: fix offsetof() and container_of() to work with CO-RE") Reported-by: Colm Harrington <colm.harrington@oracle.com> Signed-off-by: Alan Maguire <alan.maguire@oracle.com> --- tools/testing/selftests/bpf/progs/test_cls_redirect.c | 11 ++++------- tools/testing/selftests/bpf/progs/test_cls_redirect.h | 3 +++ .../selftests/bpf/progs/test_cls_redirect_dynptr.c | 11 ++++------- 3 files changed, 11 insertions(+), 14 deletions(-)