On Tue, Jul 23, 2024 at 3:35 PM YiFei Zhu <[email protected]> wrote: > > On Mon, Jul 22, 2024 at 10:56 PM Tony Ambardar <[email protected]> > wrote: > > > > Remove a redundant include of '<asm/types.h>', whose needed definitions are > > already included (via '<linux/types.h>') in cg_storage_multi_egress_only.c, > > cg_storage_multi_isolated.c, and cg_storage_multi_shared.c. This avoids > > redefinition errors seen compiling for mips64el/musl-libc like: > > > > In file included from progs/cg_storage_multi_egress_only.c:13: > > In file included from progs/cg_storage_multi.h:6: > > In file included from /usr/mips64el-linux-gnuabi64/include/asm/types.h:23: > > /usr/include/asm-generic/int-l64.h:29:25: error: typedef redefinition > > with different types ('long' vs 'long long') > > 29 | typedef __signed__ long __s64; > > | ^ > > /usr/include/asm-generic/int-ll64.h:30:44: note: previous definition is > > here > > 30 | __extension__ typedef __signed__ long long __s64; > > | ^ > > > > Fixes: 9e5bd1f7633b ("selftests/bpf: Test CGROUP_STORAGE map can't be used > > by multiple progs") > > Signed-off-by: Tony Ambardar <[email protected]> > > --- > > tools/testing/selftests/bpf/progs/cg_storage_multi.h | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/progs/cg_storage_multi.h > > b/tools/testing/selftests/bpf/progs/cg_storage_multi.h > > index a0778fe7857a..41d59f0ee606 100644 > > --- a/tools/testing/selftests/bpf/progs/cg_storage_multi.h > > +++ b/tools/testing/selftests/bpf/progs/cg_storage_multi.h > > @@ -3,8 +3,6 @@ > > #ifndef __PROGS_CG_STORAGE_MULTI_H > > #define __PROGS_CG_STORAGE_MULTI_H > > > > -#include <asm/types.h> > > - > > struct cgroup_value { > > __u32 egress_pkts; > > __u32 ingress_pkts; > > -- > > 2.34.1 > > > > Hmm, some linter checks prefer headers themselves include everything > they use. This header uses __u32 and after this patch it would include > no headers. Would it be okay to include <linux/types.h> or we don't > care? >
This header seems to be included both from user space code and BPF code, so it probably is for the better to not include specific headers and rely on respective code to provide "basic" type definitions like __u32. (For BPF side they might be coming from vmlinux.h, for example, which won't be compatible with anything included in user space code). > YiFei Zhu
