On Tue, Jul 23, 2024 at 3:35 PM YiFei Zhu <zhuyifei@xxxxxxxxxx> wrote:
>
> On Mon, Jul 22, 2024 at 10:56 PM Tony Ambardar <tony.ambardar@xxxxxxxxx> 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 <tony.ambardar@xxxxxxxxx>
> > ---
> > 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
