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'.
+
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 {