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@xxxxxxxxxx>
Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
---
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 {
