| Message ID | 20220825221751.258958-1-james.hilliard1@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/bpf: Fix bind{4,6} tcp/socket header type conflict | expand |
On Thu, Aug 25, 2022 at 3:18 PM James Hilliard <james.hilliard1@gmail.com> wrote: > > There is a potential for us to hit a type conflict when including > netinet/tcp.h with sys/socket.h, we can replace both of these includes > with linux/tcp.h to avoid this conflict. > > Fixes errors like: > In file included from /usr/include/netinet/tcp.h:91, > from progs/bind4_prog.c:10: > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' > 34 | typedef __INT8_TYPE__ int8_t; > | ^~~~~~ > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, > from progs/bind4_prog.c:9: > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} > 24 | typedef __int8_t int8_t; > | ^~~~~~ > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: error: conflicting types for 'int64_t'; have 'long int' > 43 | typedef __INT64_TYPE__ int64_t; > | ^~~~~~~ > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: previous declaration of 'int64_t' with type 'int64_t' {aka 'long long int'} > 27 | typedef __int64_t int64_t; > | ^~~~~~~ > make: *** [Makefile:537: /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] Error 1 > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> Still compiles in my environment: Reviewed-by: Stanislav Fomichev <sdf@google.com> Not sure we want it for the tests, but just in case: Fixes: a999696c547f ("selftests/bpf: Rewrite test_sock_addr bind bpf into C") > --- > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c > index 474c6a62078a..6bd20042fd53 100644 > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c > @@ -6,8 +6,7 @@ > #include <linux/bpf.h> > #include <linux/in.h> > #include <linux/in6.h> > -#include <sys/socket.h> > -#include <netinet/tcp.h> > +#include <linux/tcp.h> > #include <linux/if.h> > #include <errno.h> > > diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c > index c19cfa869f30..f37617b35a55 100644 > --- a/tools/testing/selftests/bpf/progs/bind6_prog.c > +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c > @@ -6,8 +6,7 @@ > #include <linux/bpf.h> > #include <linux/in.h> > #include <linux/in6.h> > -#include <sys/socket.h> > -#include <netinet/tcp.h> > +#include <linux/tcp.h> > #include <linux/if.h> > #include <errno.h> > > -- > 2.34.1 >
On Thu, Aug 25, 2022 at 04:17:49PM -0600, James Hilliard wrote: > There is a potential for us to hit a type conflict when including > netinet/tcp.h with sys/socket.h, we can replace both of these includes > with linux/tcp.h to avoid this conflict. > > Fixes errors like: > In file included from /usr/include/netinet/tcp.h:91, > from progs/bind4_prog.c:10: > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' > 34 | typedef __INT8_TYPE__ int8_t; > | ^~~~~~ > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, > from progs/bind4_prog.c:9: > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} > 24 | typedef __int8_t int8_t; > | ^~~~~~ > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: error: conflicting types for 'int64_t'; have 'long int' > 43 | typedef __INT64_TYPE__ int64_t; > | ^~~~~~~ > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: previous declaration of 'int64_t' with type 'int64_t' {aka 'long long int'} > 27 | typedef __int64_t int64_t; > | ^~~~~~~ > make: *** [Makefile:537: /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] Error 1 > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c > index 474c6a62078a..6bd20042fd53 100644 > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c > @@ -6,8 +6,7 @@ > #include <linux/bpf.h> > #include <linux/in.h> > #include <linux/in6.h> > -#include <sys/socket.h> > -#include <netinet/tcp.h> These includes look normal to me. What environment is hitting this. I don't prefer the selftest writers need to remember this rule. Beside, afaict, tcp.h should be removed because I don't see this test needs it. I tried removing it and it works fine. It should be removed instead of replacing it with another unnecessary tcp.h. > +#include <linux/tcp.h> > #include <linux/if.h> > #include <errno.h> > > diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c > index c19cfa869f30..f37617b35a55 100644 > --- a/tools/testing/selftests/bpf/progs/bind6_prog.c > +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c > @@ -6,8 +6,7 @@ > #include <linux/bpf.h> > #include <linux/in.h> > #include <linux/in6.h> > -#include <sys/socket.h> > -#include <netinet/tcp.h> > +#include <linux/tcp.h> > #include <linux/if.h> > #include <errno.h> > > -- > 2.34.1 >
On Thu, Aug 25, 2022 at 11:16 PM Martin KaFai Lau <kafai@fb.com> wrote: > > On Thu, Aug 25, 2022 at 04:17:49PM -0600, James Hilliard wrote: > > There is a potential for us to hit a type conflict when including > > netinet/tcp.h with sys/socket.h, we can replace both of these includes > > with linux/tcp.h to avoid this conflict. > > > > Fixes errors like: > > In file included from /usr/include/netinet/tcp.h:91, > > from progs/bind4_prog.c:10: > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' > > 34 | typedef __INT8_TYPE__ int8_t; > > | ^~~~~~ > > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, > > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, > > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, > > from progs/bind4_prog.c:9: > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} > > 24 | typedef __int8_t int8_t; > > | ^~~~~~ > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: error: conflicting types for 'int64_t'; have 'long int' > > 43 | typedef __INT64_TYPE__ int64_t; > > | ^~~~~~~ > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: previous declaration of 'int64_t' with type 'int64_t' {aka 'long long int'} > > 27 | typedef __int64_t int64_t; > > | ^~~~~~~ > > make: *** [Makefile:537: /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] Error 1 > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > --- > > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- > > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- > > 2 files changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c > > index 474c6a62078a..6bd20042fd53 100644 > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c > > @@ -6,8 +6,7 @@ > > #include <linux/bpf.h> > > #include <linux/in.h> > > #include <linux/in6.h> > > -#include <sys/socket.h> > > -#include <netinet/tcp.h> > These includes look normal to me. What environment is hitting this. I was hitting this error with GCC 13(GCC master branch). > I don't prefer the selftest writers need to remember this rule. > > Beside, afaict, tcp.h should be removed because > I don't see this test needs it. I tried removing it > and it works fine. It should be removed instead of replacing it > with another unnecessary tcp.h. Oh, that does also appear to work, thought I had tried that already but I guess I hadn't, sent a v2 with them removed: https://lore.kernel.org/bpf/20220826052925.980431-1-james.hilliard1@gmail.com/T/#u > > > +#include <linux/tcp.h> > > #include <linux/if.h> > > #include <errno.h> > > > > diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c > > index c19cfa869f30..f37617b35a55 100644 > > --- a/tools/testing/selftests/bpf/progs/bind6_prog.c > > +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c > > @@ -6,8 +6,7 @@ > > #include <linux/bpf.h> > > #include <linux/in.h> > > #include <linux/in6.h> > > -#include <sys/socket.h> > > -#include <netinet/tcp.h> > > +#include <linux/tcp.h> > > #include <linux/if.h> > > #include <errno.h> > > > > -- > > 2.34.1 > >
On Thu, Aug 25, 2022 at 11:31:15PM -0600, James Hilliard wrote: > On Thu, Aug 25, 2022 at 11:16 PM Martin KaFai Lau <kafai@fb.com> wrote: > > > > On Thu, Aug 25, 2022 at 04:17:49PM -0600, James Hilliard wrote: > > > There is a potential for us to hit a type conflict when including > > > netinet/tcp.h with sys/socket.h, we can replace both of these includes > > > with linux/tcp.h to avoid this conflict. > > > > > > Fixes errors like: > > > In file included from /usr/include/netinet/tcp.h:91, > > > from progs/bind4_prog.c:10: > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' > > > 34 | typedef __INT8_TYPE__ int8_t; > > > | ^~~~~~ > > > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, > > > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, > > > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, > > > from progs/bind4_prog.c:9: > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} > > > 24 | typedef __int8_t int8_t; > > > | ^~~~~~ > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: error: conflicting types for 'int64_t'; have 'long int' > > > 43 | typedef __INT64_TYPE__ int64_t; > > > | ^~~~~~~ > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: previous declaration of 'int64_t' with type 'int64_t' {aka 'long long int'} > > > 27 | typedef __int64_t int64_t; > > > | ^~~~~~~ > > > make: *** [Makefile:537: /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] Error 1 > > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > --- > > > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- > > > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- > > > 2 files changed, 2 insertions(+), 4 deletions(-) > > > > > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c > > > index 474c6a62078a..6bd20042fd53 100644 > > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c > > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c > > > @@ -6,8 +6,7 @@ > > > #include <linux/bpf.h> > > > #include <linux/in.h> > > > #include <linux/in6.h> > > > -#include <sys/socket.h> > > > -#include <netinet/tcp.h> > > These includes look normal to me. What environment is hitting this. > > I was hitting this error with GCC 13(GCC master branch). These two includes (<sys/socket.h> and <netinet/tcp.h>) are normal, so does it mean all existing programs need to change to use gcc 13 ? > > > I don't prefer the selftest writers need to remember this rule. > > > > Beside, afaict, tcp.h should be removed because > > I don't see this test needs it. I tried removing it > > and it works fine. It should be removed instead of replacing it > > with another unnecessary tcp.h. > > Oh, that does also appear to work, thought I had tried that already but I guess > I hadn't, sent a v2 with them removed: > https://lore.kernel.org/bpf/20220826052925.980431-1-james.hilliard1@gmail.com/T/#u > > > > > > +#include <linux/tcp.h> > > > #include <linux/if.h> > > > #include <errno.h> > > > > > > diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c > > > index c19cfa869f30..f37617b35a55 100644 > > > --- a/tools/testing/selftests/bpf/progs/bind6_prog.c > > > +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c > > > @@ -6,8 +6,7 @@ > > > #include <linux/bpf.h> > > > #include <linux/in.h> > > > #include <linux/in6.h> > > > -#include <sys/socket.h> > > > -#include <netinet/tcp.h> > > > +#include <linux/tcp.h> > > > #include <linux/if.h> > > > #include <errno.h> > > > > > > -- > > > 2.34.1 > > >
On Thu, Aug 25, 2022 at 11:49 PM Martin KaFai Lau <kafai@fb.com> wrote: > > On Thu, Aug 25, 2022 at 11:31:15PM -0600, James Hilliard wrote: > > On Thu, Aug 25, 2022 at 11:16 PM Martin KaFai Lau <kafai@fb.com> wrote: > > > > > > On Thu, Aug 25, 2022 at 04:17:49PM -0600, James Hilliard wrote: > > > > There is a potential for us to hit a type conflict when including > > > > netinet/tcp.h with sys/socket.h, we can replace both of these includes > > > > with linux/tcp.h to avoid this conflict. > > > > > > > > Fixes errors like: > > > > In file included from /usr/include/netinet/tcp.h:91, > > > > from progs/bind4_prog.c:10: > > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' > > > > 34 | typedef __INT8_TYPE__ int8_t; > > > > | ^~~~~~ > > > > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, > > > > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, > > > > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, > > > > from progs/bind4_prog.c:9: > > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} > > > > 24 | typedef __int8_t int8_t; > > > > | ^~~~~~ > > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: error: conflicting types for 'int64_t'; have 'long int' > > > > 43 | typedef __INT64_TYPE__ int64_t; > > > > | ^~~~~~~ > > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: previous declaration of 'int64_t' with type 'int64_t' {aka 'long long int'} > > > > 27 | typedef __int64_t int64_t; > > > > | ^~~~~~~ > > > > make: *** [Makefile:537: /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] Error 1 > > > > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > > --- > > > > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- > > > > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- > > > > 2 files changed, 2 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c > > > > index 474c6a62078a..6bd20042fd53 100644 > > > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c > > > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c > > > > @@ -6,8 +6,7 @@ > > > > #include <linux/bpf.h> > > > > #include <linux/in.h> > > > > #include <linux/in6.h> > > > > -#include <sys/socket.h> > > > > -#include <netinet/tcp.h> > > > These includes look normal to me. What environment is hitting this. > > > > I was hitting this error with GCC 13(GCC master branch). > These two includes (<sys/socket.h> and <netinet/tcp.h>) are normal, > so does it mean all existing programs need to change to use gcc 13 ? Well I think it's mostly just an issue getting hit with GCC-BPF as it looks to me like a cross compilation host/target header conflict. > > > > > > I don't prefer the selftest writers need to remember this rule. > > > > > > Beside, afaict, tcp.h should be removed because > > > I don't see this test needs it. I tried removing it > > > and it works fine. It should be removed instead of replacing it > > > with another unnecessary tcp.h. > > > > Oh, that does also appear to work, thought I had tried that already but I guess > > I hadn't, sent a v2 with them removed: > > https://lore.kernel.org/bpf/20220826052925.980431-1-james.hilliard1@gmail.com/T/#u > > > > > > > > > +#include <linux/tcp.h> > > > > #include <linux/if.h> > > > > #include <errno.h> > > > > > > > > diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c > > > > index c19cfa869f30..f37617b35a55 100644 > > > > --- a/tools/testing/selftests/bpf/progs/bind6_prog.c > > > > +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c > > > > @@ -6,8 +6,7 @@ > > > > #include <linux/bpf.h> > > > > #include <linux/in.h> > > > > #include <linux/in6.h> > > > > -#include <sys/socket.h> > > > > -#include <netinet/tcp.h> > > > > +#include <linux/tcp.h> > > > > #include <linux/if.h> > > > > #include <errno.h> > > > > > > > > -- > > > > 2.34.1 > > > >
> On Thu, Aug 25, 2022 at 11:49 PM Martin KaFai Lau <kafai@fb.com> wrote: >> >> On Thu, Aug 25, 2022 at 11:31:15PM -0600, James Hilliard wrote: >> > On Thu, Aug 25, 2022 at 11:16 PM Martin KaFai Lau <kafai@fb.com> wrote: >> > > >> > > On Thu, Aug 25, 2022 at 04:17:49PM -0600, James Hilliard wrote: >> > > > There is a potential for us to hit a type conflict when including >> > > > netinet/tcp.h with sys/socket.h, we can replace both of these includes >> > > > with linux/tcp.h to avoid this conflict. >> > > > >> > > > Fixes errors like: >> > > > In file included from /usr/include/netinet/tcp.h:91, >> > > > from progs/bind4_prog.c:10: >> > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' >> > > > 34 | typedef __INT8_TYPE__ int8_t; >> > > > | ^~~~~~ >> > > > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, >> > > > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, >> > > > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, >> > > > from progs/bind4_prog.c:9: >> > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} >> > > > 24 | typedef __int8_t int8_t; >> > > > | ^~~~~~ >> > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: >> > > > error: conflicting types for 'int64_t'; have 'long int' >> > > > 43 | typedef __INT64_TYPE__ int64_t; >> > > > | ^~~~~~~ >> > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: >> > > > previous declaration of 'int64_t' with type 'int64_t' {aka >> > > > 'long long int'} >> > > > 27 | typedef __int64_t int64_t; >> > > > | ^~~~~~~ >> > > > make: *** [Makefile:537: >> > > > /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] >> > > > Error 1 >> > > > >> > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> >> > > > --- >> > > > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- >> > > > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- >> > > > 2 files changed, 2 insertions(+), 4 deletions(-) >> > > > >> > > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c >> > > > b/tools/testing/selftests/bpf/progs/bind4_prog.c >> > > > index 474c6a62078a..6bd20042fd53 100644 >> > > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c >> > > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c >> > > > @@ -6,8 +6,7 @@ >> > > > #include <linux/bpf.h> >> > > > #include <linux/in.h> >> > > > #include <linux/in6.h> >> > > > -#include <sys/socket.h> >> > > > -#include <netinet/tcp.h> >> > > These includes look normal to me. What environment is hitting this. >> > >> > I was hitting this error with GCC 13(GCC master branch). >> These two includes (<sys/socket.h> and <netinet/tcp.h>) are normal, >> so does it mean all existing programs need to change to use gcc 13 ? > > Well I think it's mostly just an issue getting hit with GCC-BPF as it > looks to me like a cross compilation host/target header conflict. This is an interesting issue. Right now the BPF GCC target is a sort of a bare-metal target. As such, it provides a set of header files that implement ISO C types and other machinery (i.e. it doesn't rely on a C library to provide them): iso646.h stdalign.h stdarg.h stdatomic.h stdbool.h stddef.h stdfix.h stdint.h stdnoreturn.h tgmath.h unwind.h varargs.h This is because we were expecting this to be used like: <compiler-provided std C headers> | | v | <kernel headers> | | | v v <BPF C program> However, if it is expected/intended for C BPF programs to include libc headers, such as sys/socket.h, this can quickly go sour as you have found with that conflict. So this leads to the question: should we turn the BPF target into a target that assumes a libc? This basically means we will be assuming BPF programs are always compiled in an environment that provides a standard stdint.h, stdbool.h and friends. Thoughts? >> > > I don't prefer the selftest writers need to remember this rule. >> > > >> > > Beside, afaict, tcp.h should be removed because >> > > I don't see this test needs it. I tried removing it >> > > and it works fine. It should be removed instead of replacing it >> > > with another unnecessary tcp.h. >> > >> > Oh, that does also appear to work, thought I had tried that already but I guess >> > I hadn't, sent a v2 with them removed: >> > https://lore.kernel.org/bpf/20220826052925.980431-1-james.hilliard1@gmail.com/T/#u >> > >> > > >> > > > +#include <linux/tcp.h> >> > > > #include <linux/if.h> >> > > > #include <errno.h> >> > > > >> > > > diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c >> > > > b/tools/testing/selftests/bpf/progs/bind6_prog.c >> > > > index c19cfa869f30..f37617b35a55 100644 >> > > > --- a/tools/testing/selftests/bpf/progs/bind6_prog.c >> > > > +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c >> > > > @@ -6,8 +6,7 @@ >> > > > #include <linux/bpf.h> >> > > > #include <linux/in.h> >> > > > #include <linux/in6.h> >> > > > -#include <sys/socket.h> >> > > > -#include <netinet/tcp.h> >> > > > +#include <linux/tcp.h> >> > > > #include <linux/if.h> >> > > > #include <errno.h> >> > > > >> > > > -- >> > > > 2.34.1 >> > > >
On 8/26/22 3:17 AM, James Hilliard wrote: > There is a potential for us to hit a type conflict when including > netinet/tcp.h with sys/socket.h, we can replace both of these includes > with linux/tcp.h to avoid this conflict. > > Fixes errors like: > In file included from /usr/include/netinet/tcp.h:91, > from progs/bind4_prog.c:10: > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' > 34 | typedef __INT8_TYPE__ int8_t; > | ^~~~~~ > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, > from progs/bind4_prog.c:9: > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} > 24 | typedef __int8_t int8_t; > | ^~~~~~ > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: error: conflicting types for 'int64_t'; have 'long int' > 43 | typedef __INT64_TYPE__ int64_t; > | ^~~~~~~ > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: previous declaration of 'int64_t' with type 'int64_t' {aka 'long long int'} > 27 | typedef __int64_t int64_t; > | ^~~~~~~ > make: *** [Makefile:537: /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] Error 1 > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c > index 474c6a62078a..6bd20042fd53 100644 > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c > @@ -6,8 +6,7 @@ > #include <linux/bpf.h> > #include <linux/in.h> > #include <linux/in6.h> > -#include <sys/socket.h> > -#include <netinet/tcp.h> > +#include <linux/tcp.h> > #include <linux/if.h> > #include <errno.h> > > diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c > index c19cfa869f30..f37617b35a55 100644 > --- a/tools/testing/selftests/bpf/progs/bind6_prog.c > +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c > @@ -6,8 +6,7 @@ > #include <linux/bpf.h> > #include <linux/in.h> > #include <linux/in6.h> > -#include <sys/socket.h> > -#include <netinet/tcp.h> > +#include <linux/tcp.h> > #include <linux/if.h> > #include <errno.h> >
On Fri, Aug 26, 2022 at 7:19 AM Jose E. Marchesi <jose.marchesi@oracle.com> wrote: > > > > On Thu, Aug 25, 2022 at 11:49 PM Martin KaFai Lau <kafai@fb.com> wrote: > >> > >> On Thu, Aug 25, 2022 at 11:31:15PM -0600, James Hilliard wrote: > >> > On Thu, Aug 25, 2022 at 11:16 PM Martin KaFai Lau <kafai@fb.com> wrote: > >> > > > >> > > On Thu, Aug 25, 2022 at 04:17:49PM -0600, James Hilliard wrote: > >> > > > There is a potential for us to hit a type conflict when including > >> > > > netinet/tcp.h with sys/socket.h, we can replace both of these includes > >> > > > with linux/tcp.h to avoid this conflict. > >> > > > > >> > > > Fixes errors like: > >> > > > In file included from /usr/include/netinet/tcp.h:91, > >> > > > from progs/bind4_prog.c:10: > >> > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' > >> > > > 34 | typedef __INT8_TYPE__ int8_t; > >> > > > | ^~~~~~ > >> > > > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, > >> > > > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, > >> > > > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, > >> > > > from progs/bind4_prog.c:9: > >> > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} > >> > > > 24 | typedef __int8_t int8_t; > >> > > > | ^~~~~~ > >> > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: > >> > > > error: conflicting types for 'int64_t'; have 'long int' > >> > > > 43 | typedef __INT64_TYPE__ int64_t; > >> > > > | ^~~~~~~ > >> > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: > >> > > > previous declaration of 'int64_t' with type 'int64_t' {aka > >> > > > 'long long int'} > >> > > > 27 | typedef __int64_t int64_t; > >> > > > | ^~~~~~~ > >> > > > make: *** [Makefile:537: > >> > > > /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] > >> > > > Error 1 > >> > > > > >> > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > >> > > > --- > >> > > > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- > >> > > > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- > >> > > > 2 files changed, 2 insertions(+), 4 deletions(-) > >> > > > > >> > > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c > >> > > > b/tools/testing/selftests/bpf/progs/bind4_prog.c > >> > > > index 474c6a62078a..6bd20042fd53 100644 > >> > > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c > >> > > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c > >> > > > @@ -6,8 +6,7 @@ > >> > > > #include <linux/bpf.h> > >> > > > #include <linux/in.h> > >> > > > #include <linux/in6.h> > >> > > > -#include <sys/socket.h> > >> > > > -#include <netinet/tcp.h> > >> > > These includes look normal to me. What environment is hitting this. > >> > > >> > I was hitting this error with GCC 13(GCC master branch). > >> These two includes (<sys/socket.h> and <netinet/tcp.h>) are normal, > >> so does it mean all existing programs need to change to use gcc 13 ? > > > > Well I think it's mostly just an issue getting hit with GCC-BPF as it > > looks to me like a cross compilation host/target header conflict. > > This is an interesting issue. > > Right now the BPF GCC target is a sort of a bare-metal target. As such, > it provides a set of header files that implement ISO C types and other > machinery (i.e. it doesn't rely on a C library to provide them): > > iso646.h > stdalign.h > stdarg.h > stdatomic.h > stdbool.h > stddef.h > stdfix.h > stdint.h > stdnoreturn.h > tgmath.h > unwind.h > varargs.h > > This is because we were expecting this to be used like: > > <compiler-provided std C headers> > | | > v | > <kernel headers> | > | | > v v > <BPF C program> > > However, if it is expected/intended for C BPF programs to include libc > headers, such as sys/socket.h, this can quickly go sour as you have > found with that conflict. > > So this leads to the question: should we turn the BPF target into a > target that assumes a libc? This basically means we will be assuming > BPF programs are always compiled in an environment that provides a > standard stdint.h, stdbool.h and friends. Well for a normal GCC BPF setup we're basically cross compiling for the BPF bare metal target while sharing headers with the build host(for libbpf and any other libc headers that get included). On the other hand when using GCC BPF as part of a full cross toolchain we actually end up sharing headers with our real cross target architecture sysroot(which would provide a libc), essentially in that case BPF is a bare metal cross target which shares headers with the real cross target(which is not a bare metal target). For this libbpf is installed to the real cross target sysroot which is used by both GCC BPF(for bpf progs) and the real cross target GCC compiler(for userspace side). From my understanding with this setup GCC BPF will pick up the real cross target libc headers as a fallback which may sometimes have conflict/compatibility issues with the kernel headers. I think it's probably best to avoid depending on libc headers as things may otherwise get even more complex. You would essentially have 2 libc's in a normal GCC BPF setup and 3 libc's in a full cross toolchain setup(you'd have one for the build host, one for the real cross target arch and one for the BPF target arch). Cross build systems will typically allow a libc choice as well(glibc/musl/uclibc) and we don't really want the bpf programs to have to care about the specific libc being used as they are bare metal programs which shouldn't depend on a libc. > > Thoughts? > > >> > > I don't prefer the selftest writers need to remember this rule. > >> > > > >> > > Beside, afaict, tcp.h should be removed because > >> > > I don't see this test needs it. I tried removing it > >> > > and it works fine. It should be removed instead of replacing it > >> > > with another unnecessary tcp.h. > >> > > >> > Oh, that does also appear to work, thought I had tried that already but I guess > >> > I hadn't, sent a v2 with them removed: > >> > https://lore.kernel.org/bpf/20220826052925.980431-1-james.hilliard1@gmail.com/T/#u > >> > > >> > > > >> > > > +#include <linux/tcp.h> > >> > > > #include <linux/if.h> > >> > > > #include <errno.h> > >> > > > > >> > > > diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c > >> > > > b/tools/testing/selftests/bpf/progs/bind6_prog.c > >> > > > index c19cfa869f30..f37617b35a55 100644 > >> > > > --- a/tools/testing/selftests/bpf/progs/bind6_prog.c > >> > > > +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c > >> > > > @@ -6,8 +6,7 @@ > >> > > > #include <linux/bpf.h> > >> > > > #include <linux/in.h> > >> > > > #include <linux/in6.h> > >> > > > -#include <sys/socket.h> > >> > > > -#include <netinet/tcp.h> > >> > > > +#include <linux/tcp.h> > >> > > > #include <linux/if.h> > >> > > > #include <errno.h> > >> > > > > >> > > > -- > >> > > > 2.34.1 > >> > > >
> On Fri, Aug 26, 2022 at 7:19 AM Jose E. Marchesi > <jose.marchesi@oracle.com> wrote: >> >> >> > On Thu, Aug 25, 2022 at 11:49 PM Martin KaFai Lau <kafai@fb.com> wrote: >> >> >> >> On Thu, Aug 25, 2022 at 11:31:15PM -0600, James Hilliard wrote: >> >> > On Thu, Aug 25, 2022 at 11:16 PM Martin KaFai Lau <kafai@fb.com> wrote: >> >> > > >> >> > > On Thu, Aug 25, 2022 at 04:17:49PM -0600, James Hilliard wrote: >> >> > > > There is a potential for us to hit a type conflict when including >> >> > > > netinet/tcp.h with sys/socket.h, we can replace both of these includes >> >> > > > with linux/tcp.h to avoid this conflict. >> >> > > > >> >> > > > Fixes errors like: >> >> > > > In file included from /usr/include/netinet/tcp.h:91, >> >> > > > from progs/bind4_prog.c:10: >> >> > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' >> >> > > > 34 | typedef __INT8_TYPE__ int8_t; >> >> > > > | ^~~~~~ >> >> > > > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, >> >> > > > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, >> >> > > > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, >> >> > > > from progs/bind4_prog.c:9: >> >> > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} >> >> > > > 24 | typedef __int8_t int8_t; >> >> > > > | ^~~~~~ >> >> > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: >> >> > > > error: conflicting types for 'int64_t'; have 'long int' >> >> > > > 43 | typedef __INT64_TYPE__ int64_t; >> >> > > > | ^~~~~~~ >> >> > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: >> >> > > > previous declaration of 'int64_t' with type 'int64_t' {aka >> >> > > > 'long long int'} >> >> > > > 27 | typedef __int64_t int64_t; >> >> > > > | ^~~~~~~ >> >> > > > make: *** [Makefile:537: >> >> > > > /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] >> >> > > > Error 1 >> >> > > > >> >> > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> >> >> > > > --- >> >> > > > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- >> >> > > > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- >> >> > > > 2 files changed, 2 insertions(+), 4 deletions(-) >> >> > > > >> >> > > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c >> >> > > > b/tools/testing/selftests/bpf/progs/bind4_prog.c >> >> > > > index 474c6a62078a..6bd20042fd53 100644 >> >> > > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c >> >> > > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c >> >> > > > @@ -6,8 +6,7 @@ >> >> > > > #include <linux/bpf.h> >> >> > > > #include <linux/in.h> >> >> > > > #include <linux/in6.h> >> >> > > > -#include <sys/socket.h> >> >> > > > -#include <netinet/tcp.h> >> >> > > These includes look normal to me. What environment is hitting this. >> >> > >> >> > I was hitting this error with GCC 13(GCC master branch). >> >> These two includes (<sys/socket.h> and <netinet/tcp.h>) are normal, >> >> so does it mean all existing programs need to change to use gcc 13 ? >> > >> > Well I think it's mostly just an issue getting hit with GCC-BPF as it >> > looks to me like a cross compilation host/target header conflict. >> >> This is an interesting issue. >> >> Right now the BPF GCC target is a sort of a bare-metal target. As such, >> it provides a set of header files that implement ISO C types and other >> machinery (i.e. it doesn't rely on a C library to provide them): >> >> iso646.h >> stdalign.h >> stdarg.h >> stdatomic.h >> stdbool.h >> stddef.h >> stdfix.h >> stdint.h >> stdnoreturn.h >> tgmath.h >> unwind.h >> varargs.h >> >> This is because we were expecting this to be used like: >> >> <compiler-provided std C headers> >> | | >> v | >> <kernel headers> | >> | | >> v v >> <BPF C program> >> >> However, if it is expected/intended for C BPF programs to include libc >> headers, such as sys/socket.h, this can quickly go sour as you have >> found with that conflict. >> >> So this leads to the question: should we turn the BPF target into a >> target that assumes a libc? This basically means we will be assuming >> BPF programs are always compiled in an environment that provides a >> standard stdint.h, stdbool.h and friends. > > Well for a normal GCC BPF setup we're basically cross compiling for the > BPF bare metal target while sharing headers with the build host(for libbpf > and any other libc headers that get included). > > On the other hand when using GCC BPF as part of a full cross toolchain > we actually end up sharing headers with our real cross target architecture > sysroot(which would provide a libc), essentially in that case BPF is a bare > metal cross target which shares headers with the real cross target(which > is not a bare metal target). For this libbpf is installed to the real > cross target > sysroot which is used by both GCC BPF(for bpf progs) and the real cross > target GCC compiler(for userspace side). From my understanding with this > setup GCC BPF will pick up the real cross target libc headers as a fallback > which may sometimes have conflict/compatibility issues with the kernel > headers. > > I think it's probably best to avoid depending on libc headers as things may > otherwise get even more complex. You would essentially have 2 libc's > in a normal GCC BPF setup and 3 libc's in a full cross toolchain setup(you'd > have one for the build host, one for the real cross target arch and one for > the BPF target arch). > > Cross build systems will typically allow a libc choice as > well(glibc/musl/uclibc) > and we don't really want the bpf programs to have to care about the specific > libc being used as they are bare metal programs which shouldn't depend on > a libc. > I don't understand what do you mean with "real cross target". From the toolchain perspective, the compiler is targetted to just one platform: bpf-unknown-none. As is usual for bare-metal targets, the compiler provides headers to implement the C standard with things like floating-point types and standard integer types, `bool', etc. If you then -I directories in order to "share headers with the build host" or with that "real cross target", or to use any other header that may implement the same types (typically a libc) then well, thats when the problem arises. I don't know how much sense does it makes to include glibc headers like sys/socket.h in BPF C programs: I'm no BPF programmer. But if it is something to be supported, we will have to change the compiler to not provide the standard headers. >> Thoughts? >> >> >> > > I don't prefer the selftest writers need to remember this rule. >> >> > > >> >> > > Beside, afaict, tcp.h should be removed because >> >> > > I don't see this test needs it. I tried removing it >> >> > > and it works fine. It should be removed instead of replacing it >> >> > > with another unnecessary tcp.h. >> >> > >> >> > Oh, that does also appear to work, thought I had tried that already but I guess >> >> > I hadn't, sent a v2 with them removed: >> >> > https://lore.kernel.org/bpf/20220826052925.980431-1-james.hilliard1@gmail.com/T/#u >> >> > >> >> > > >> >> > > > +#include <linux/tcp.h> >> >> > > > #include <linux/if.h> >> >> > > > #include <errno.h> >> >> > > > >> >> > > > diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c >> >> > > > b/tools/testing/selftests/bpf/progs/bind6_prog.c >> >> > > > index c19cfa869f30..f37617b35a55 100644 >> >> > > > --- a/tools/testing/selftests/bpf/progs/bind6_prog.c >> >> > > > +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c >> >> > > > @@ -6,8 +6,7 @@ >> >> > > > #include <linux/bpf.h> >> >> > > > #include <linux/in.h> >> >> > > > #include <linux/in6.h> >> >> > > > -#include <sys/socket.h> >> >> > > > -#include <netinet/tcp.h> >> >> > > > +#include <linux/tcp.h> >> >> > > > #include <linux/if.h> >> >> > > > #include <errno.h> >> >> > > > >> >> > > > -- >> >> > > > 2.34.1 >> >> > > >
On Fri, Aug 26, 2022 at 10:33 AM Jose E. Marchesi <jose.marchesi@oracle.com> wrote: > > > > On Fri, Aug 26, 2022 at 7:19 AM Jose E. Marchesi > > <jose.marchesi@oracle.com> wrote: > >> > >> > >> > On Thu, Aug 25, 2022 at 11:49 PM Martin KaFai Lau <kafai@fb.com> wrote: > >> >> > >> >> On Thu, Aug 25, 2022 at 11:31:15PM -0600, James Hilliard wrote: > >> >> > On Thu, Aug 25, 2022 at 11:16 PM Martin KaFai Lau <kafai@fb.com> wrote: > >> >> > > > >> >> > > On Thu, Aug 25, 2022 at 04:17:49PM -0600, James Hilliard wrote: > >> >> > > > There is a potential for us to hit a type conflict when including > >> >> > > > netinet/tcp.h with sys/socket.h, we can replace both of these includes > >> >> > > > with linux/tcp.h to avoid this conflict. > >> >> > > > > >> >> > > > Fixes errors like: > >> >> > > > In file included from /usr/include/netinet/tcp.h:91, > >> >> > > > from progs/bind4_prog.c:10: > >> >> > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' > >> >> > > > 34 | typedef __INT8_TYPE__ int8_t; > >> >> > > > | ^~~~~~ > >> >> > > > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, > >> >> > > > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, > >> >> > > > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, > >> >> > > > from progs/bind4_prog.c:9: > >> >> > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} > >> >> > > > 24 | typedef __int8_t int8_t; > >> >> > > > | ^~~~~~ > >> >> > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: > >> >> > > > error: conflicting types for 'int64_t'; have 'long int' > >> >> > > > 43 | typedef __INT64_TYPE__ int64_t; > >> >> > > > | ^~~~~~~ > >> >> > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: > >> >> > > > previous declaration of 'int64_t' with type 'int64_t' {aka > >> >> > > > 'long long int'} > >> >> > > > 27 | typedef __int64_t int64_t; > >> >> > > > | ^~~~~~~ > >> >> > > > make: *** [Makefile:537: > >> >> > > > /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] > >> >> > > > Error 1 > >> >> > > > > >> >> > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > >> >> > > > --- > >> >> > > > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- > >> >> > > > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- > >> >> > > > 2 files changed, 2 insertions(+), 4 deletions(-) > >> >> > > > > >> >> > > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c > >> >> > > > b/tools/testing/selftests/bpf/progs/bind4_prog.c > >> >> > > > index 474c6a62078a..6bd20042fd53 100644 > >> >> > > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c > >> >> > > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c > >> >> > > > @@ -6,8 +6,7 @@ > >> >> > > > #include <linux/bpf.h> > >> >> > > > #include <linux/in.h> > >> >> > > > #include <linux/in6.h> > >> >> > > > -#include <sys/socket.h> > >> >> > > > -#include <netinet/tcp.h> > >> >> > > These includes look normal to me. What environment is hitting this. > >> >> > > >> >> > I was hitting this error with GCC 13(GCC master branch). > >> >> These two includes (<sys/socket.h> and <netinet/tcp.h>) are normal, > >> >> so does it mean all existing programs need to change to use gcc 13 ? > >> > > >> > Well I think it's mostly just an issue getting hit with GCC-BPF as it > >> > looks to me like a cross compilation host/target header conflict. > >> > >> This is an interesting issue. > >> > >> Right now the BPF GCC target is a sort of a bare-metal target. As such, > >> it provides a set of header files that implement ISO C types and other > >> machinery (i.e. it doesn't rely on a C library to provide them): > >> > >> iso646.h > >> stdalign.h > >> stdarg.h > >> stdatomic.h > >> stdbool.h > >> stddef.h > >> stdfix.h > >> stdint.h > >> stdnoreturn.h > >> tgmath.h > >> unwind.h > >> varargs.h > >> > >> This is because we were expecting this to be used like: > >> > >> <compiler-provided std C headers> > >> | | > >> v | > >> <kernel headers> | > >> | | > >> v v > >> <BPF C program> > >> > >> However, if it is expected/intended for C BPF programs to include libc > >> headers, such as sys/socket.h, this can quickly go sour as you have > >> found with that conflict. > >> > >> So this leads to the question: should we turn the BPF target into a > >> target that assumes a libc? This basically means we will be assuming > >> BPF programs are always compiled in an environment that provides a > >> standard stdint.h, stdbool.h and friends. > > > > Well for a normal GCC BPF setup we're basically cross compiling for the > > BPF bare metal target while sharing headers with the build host(for libbpf > > and any other libc headers that get included). > > > > On the other hand when using GCC BPF as part of a full cross toolchain > > we actually end up sharing headers with our real cross target architecture > > sysroot(which would provide a libc), essentially in that case BPF is a bare > > metal cross target which shares headers with the real cross target(which > > is not a bare metal target). For this libbpf is installed to the real > > cross target > > sysroot which is used by both GCC BPF(for bpf progs) and the real cross > > target GCC compiler(for userspace side). From my understanding with this > > setup GCC BPF will pick up the real cross target libc headers as a fallback > > which may sometimes have conflict/compatibility issues with the kernel > > headers. > > > > I think it's probably best to avoid depending on libc headers as things may > > otherwise get even more complex. You would essentially have 2 libc's > > in a normal GCC BPF setup and 3 libc's in a full cross toolchain setup(you'd > > have one for the build host, one for the real cross target arch and one for > > the BPF target arch). > > > > Cross build systems will typically allow a libc choice as > > well(glibc/musl/uclibc) > > and we don't really want the bpf programs to have to care about the specific > > libc being used as they are bare metal programs which shouldn't depend on > > a libc. > > > > I don't understand what do you mean with "real cross target". I mean the real cross target architecture as in the real hardware target architecture, for example aarch64 when cross compiling from a x86_64 build host. > > From the toolchain perspective, the compiler is targetted to just one > platform: bpf-unknown-none. As is usual for bare-metal targets, the > compiler provides headers to implement the C standard with things like > floating-point types and standard integer types, `bool', etc. Yeah, I mean gcc doesn't have proper multi-arch support like llvm does so a complete gcc cross toolchain(one which is sufficient to build kernel/userspace needed for say an aarch64 cross target along with bpf programs) is effectively two gcc toolchains bundled together. In some ways it gets used more like a separate language than a separate target. I have a pending series for buildroot adding gcc-bpf support if you're curious what this currently looks like: https://lore.kernel.org/buildroot/20220809094109.2279598-1-james.hilliard1@gmail.com/ > > If you then -I directories in order to "share headers with the build > host" or with that "real cross target", or to use any other header that > may implement the same types (typically a libc) then well, thats when > the problem arises. Well I'm using -idirafter for including those build host/real cross target header directories with lowest priority, since those directories have least priority the conflicts would otherwise be missing header errors AFAIU if I didn't include them. From my understanding we need to include these directories as they provide the kernel headers required by many bpf programs. > > I don't know how much sense does it makes to include glibc headers like > sys/socket.h in BPF C programs: I'm no BPF programmer. But if it is > something to be supported, we will have to change the compiler to not > provide the standard headers. I think it's best to just avoid libc headers in BPF programs. > > >> Thoughts? > >> > >> >> > > I don't prefer the selftest writers need to remember this rule. > >> >> > > > >> >> > > Beside, afaict, tcp.h should be removed because > >> >> > > I don't see this test needs it. I tried removing it > >> >> > > and it works fine. It should be removed instead of replacing it > >> >> > > with another unnecessary tcp.h. > >> >> > > >> >> > Oh, that does also appear to work, thought I had tried that already but I guess > >> >> > I hadn't, sent a v2 with them removed: > >> >> > https://lore.kernel.org/bpf/20220826052925.980431-1-james.hilliard1@gmail.com/T/#u > >> >> > > >> >> > > > >> >> > > > +#include <linux/tcp.h> > >> >> > > > #include <linux/if.h> > >> >> > > > #include <errno.h> > >> >> > > > > >> >> > > > diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c > >> >> > > > b/tools/testing/selftests/bpf/progs/bind6_prog.c > >> >> > > > index c19cfa869f30..f37617b35a55 100644 > >> >> > > > --- a/tools/testing/selftests/bpf/progs/bind6_prog.c > >> >> > > > +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c > >> >> > > > @@ -6,8 +6,7 @@ > >> >> > > > #include <linux/bpf.h> > >> >> > > > #include <linux/in.h> > >> >> > > > #include <linux/in6.h> > >> >> > > > -#include <sys/socket.h> > >> >> > > > -#include <netinet/tcp.h> > >> >> > > > +#include <linux/tcp.h> > >> >> > > > #include <linux/if.h> > >> >> > > > #include <errno.h> > >> >> > > > > >> >> > > > -- > >> >> > > > 2.34.1 > >> >> > > >
On Fri, Aug 26, 2022 at 12:13:54AM -0600, James Hilliard wrote: > On Thu, Aug 25, 2022 at 11:49 PM Martin KaFai Lau <kafai@fb.com> wrote: > > > > On Thu, Aug 25, 2022 at 11:31:15PM -0600, James Hilliard wrote: > > > On Thu, Aug 25, 2022 at 11:16 PM Martin KaFai Lau <kafai@fb.com> wrote: > > > > > > > > On Thu, Aug 25, 2022 at 04:17:49PM -0600, James Hilliard wrote: > > > > > There is a potential for us to hit a type conflict when including > > > > > netinet/tcp.h with sys/socket.h, we can replace both of these includes > > > > > with linux/tcp.h to avoid this conflict. > > > > > > > > > > Fixes errors like: > > > > > In file included from /usr/include/netinet/tcp.h:91, > > > > > from progs/bind4_prog.c:10: > > > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' > > > > > 34 | typedef __INT8_TYPE__ int8_t; > > > > > | ^~~~~~ > > > > > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, > > > > > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, > > > > > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, > > > > > from progs/bind4_prog.c:9: > > > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} > > > > > 24 | typedef __int8_t int8_t; > > > > > | ^~~~~~ > > > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: error: conflicting types for 'int64_t'; have 'long int' > > > > > 43 | typedef __INT64_TYPE__ int64_t; > > > > > | ^~~~~~~ > > > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: previous declaration of 'int64_t' with type 'int64_t' {aka 'long long int'} > > > > > 27 | typedef __int64_t int64_t; > > > > > | ^~~~~~~ > > > > > make: *** [Makefile:537: /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] Error 1 > > > > > > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > > > --- > > > > > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- > > > > > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- > > > > > 2 files changed, 2 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c > > > > > index 474c6a62078a..6bd20042fd53 100644 > > > > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c > > > > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c > > > > > @@ -6,8 +6,7 @@ > > > > > #include <linux/bpf.h> > > > > > #include <linux/in.h> > > > > > #include <linux/in6.h> > > > > > -#include <sys/socket.h> > > > > > -#include <netinet/tcp.h> > > > > These includes look normal to me. What environment is hitting this. > > > > > > I was hitting this error with GCC 13(GCC master branch). > > These two includes (<sys/socket.h> and <netinet/tcp.h>) are normal, > > so does it mean all existing programs need to change to use gcc 13 ? > > Well I think it's mostly just an issue getting hit with GCC-BPF as it > looks to me like a cross compilation host/target header conflict. The users have been using these headers in the bpf progs. The solution should be on the GCC-BPF side instead of changing all bpf progs.
On Fri, Aug 26, 2022 at 11:17 AM Martin KaFai Lau <kafai@fb.com> wrote: > > On Fri, Aug 26, 2022 at 12:13:54AM -0600, James Hilliard wrote: > > On Thu, Aug 25, 2022 at 11:49 PM Martin KaFai Lau <kafai@fb.com> wrote: > > > > > > On Thu, Aug 25, 2022 at 11:31:15PM -0600, James Hilliard wrote: > > > > On Thu, Aug 25, 2022 at 11:16 PM Martin KaFai Lau <kafai@fb.com> wrote: > > > > > > > > > > On Thu, Aug 25, 2022 at 04:17:49PM -0600, James Hilliard wrote: > > > > > > There is a potential for us to hit a type conflict when including > > > > > > netinet/tcp.h with sys/socket.h, we can replace both of these includes > > > > > > with linux/tcp.h to avoid this conflict. > > > > > > > > > > > > Fixes errors like: > > > > > > In file included from /usr/include/netinet/tcp.h:91, > > > > > > from progs/bind4_prog.c:10: > > > > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' > > > > > > 34 | typedef __INT8_TYPE__ int8_t; > > > > > > | ^~~~~~ > > > > > > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, > > > > > > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, > > > > > > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, > > > > > > from progs/bind4_prog.c:9: > > > > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} > > > > > > 24 | typedef __int8_t int8_t; > > > > > > | ^~~~~~ > > > > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: error: conflicting types for 'int64_t'; have 'long int' > > > > > > 43 | typedef __INT64_TYPE__ int64_t; > > > > > > | ^~~~~~~ > > > > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: previous declaration of 'int64_t' with type 'int64_t' {aka 'long long int'} > > > > > > 27 | typedef __int64_t int64_t; > > > > > > | ^~~~~~~ > > > > > > make: *** [Makefile:537: /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] Error 1 > > > > > > > > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > > > > --- > > > > > > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- > > > > > > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- > > > > > > 2 files changed, 2 insertions(+), 4 deletions(-) > > > > > > > > > > > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c > > > > > > index 474c6a62078a..6bd20042fd53 100644 > > > > > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c > > > > > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c > > > > > > @@ -6,8 +6,7 @@ > > > > > > #include <linux/bpf.h> > > > > > > #include <linux/in.h> > > > > > > #include <linux/in6.h> > > > > > > -#include <sys/socket.h> > > > > > > -#include <netinet/tcp.h> > > > > > These includes look normal to me. What environment is hitting this. > > > > > > > > I was hitting this error with GCC 13(GCC master branch). > > > These two includes (<sys/socket.h> and <netinet/tcp.h>) are normal, > > > so does it mean all existing programs need to change to use gcc 13 ? > > > > Well I think it's mostly just an issue getting hit with GCC-BPF as it > > looks to me like a cross compilation host/target header conflict. > The users have been using these headers in the bpf progs. Users can migrate away from libc headers over time, migrating away shouldn't cause regressions and should improve reliability. > The solution should be on the GCC-BPF side instead of changing > all bpf progs. I mean, GCC doesn't really control which libc is available, it seems to be a bad idea to use libc headers in general as they are developed separately from GCC and the kernel/libbpf. I'm not really sure how one would fix this on the GCC-BPF side without introducing more potential header conflicts.
On Fri, Aug 26, 2022 at 11:42:10AM -0600, James Hilliard wrote: > On Fri, Aug 26, 2022 at 11:17 AM Martin KaFai Lau <kafai@fb.com> wrote: > > > > On Fri, Aug 26, 2022 at 12:13:54AM -0600, James Hilliard wrote: > > > On Thu, Aug 25, 2022 at 11:49 PM Martin KaFai Lau <kafai@fb.com> wrote: > > > > > > > > On Thu, Aug 25, 2022 at 11:31:15PM -0600, James Hilliard wrote: > > > > > On Thu, Aug 25, 2022 at 11:16 PM Martin KaFai Lau <kafai@fb.com> wrote: > > > > > > > > > > > > On Thu, Aug 25, 2022 at 04:17:49PM -0600, James Hilliard wrote: > > > > > > > There is a potential for us to hit a type conflict when including > > > > > > > netinet/tcp.h with sys/socket.h, we can replace both of these includes > > > > > > > with linux/tcp.h to avoid this conflict. > > > > > > > > > > > > > > Fixes errors like: > > > > > > > In file included from /usr/include/netinet/tcp.h:91, > > > > > > > from progs/bind4_prog.c:10: > > > > > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' > > > > > > > 34 | typedef __INT8_TYPE__ int8_t; > > > > > > > | ^~~~~~ > > > > > > > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, > > > > > > > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, > > > > > > > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, > > > > > > > from progs/bind4_prog.c:9: > > > > > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} > > > > > > > 24 | typedef __int8_t int8_t; > > > > > > > | ^~~~~~ > > > > > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: error: conflicting types for 'int64_t'; have 'long int' > > > > > > > 43 | typedef __INT64_TYPE__ int64_t; > > > > > > > | ^~~~~~~ > > > > > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: previous declaration of 'int64_t' with type 'int64_t' {aka 'long long int'} > > > > > > > 27 | typedef __int64_t int64_t; > > > > > > > | ^~~~~~~ > > > > > > > make: *** [Makefile:537: /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] Error 1 > > > > > > > > > > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > > > > > --- > > > > > > > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- > > > > > > > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- > > > > > > > 2 files changed, 2 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c > > > > > > > index 474c6a62078a..6bd20042fd53 100644 > > > > > > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c > > > > > > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c > > > > > > > @@ -6,8 +6,7 @@ > > > > > > > #include <linux/bpf.h> > > > > > > > #include <linux/in.h> > > > > > > > #include <linux/in6.h> > > > > > > > -#include <sys/socket.h> > > > > > > > -#include <netinet/tcp.h> > > > > > > These includes look normal to me. What environment is hitting this. > > > > > > > > > > I was hitting this error with GCC 13(GCC master branch). > > > > These two includes (<sys/socket.h> and <netinet/tcp.h>) are normal, > > > > so does it mean all existing programs need to change to use gcc 13 ? > > > > > > Well I think it's mostly just an issue getting hit with GCC-BPF as it > > > looks to me like a cross compilation host/target header conflict. > > The users have been using these headers in the bpf progs. > > Users can migrate away from libc headers over time, migrating away imo, not without a good reason. > shouldn't cause regressions and should improve reliability. May be I am missing something. I also don't understand the reliability part. In this sys/socket.h as an example, what is wrong in using "'int8_t' {aka 'signed char'}" from libc and the one from gcc "'int8_t'; have 'char'" must be used instead. Why LLVM bpf does not have issue ? > > > The solution should be on the GCC-BPF side instead of changing > > all bpf progs. > > I mean, GCC doesn't really control which libc is available, it seems to > be a bad idea to use libc headers in general as they are developed > separately from GCC and the kernel/libbpf. > > I'm not really sure how one would fix this on the GCC-BPF side without > introducing more potential header conflicts.
> On Fri, Aug 26, 2022 at 11:42:10AM -0600, James Hilliard wrote: >> On Fri, Aug 26, 2022 at 11:17 AM Martin KaFai Lau <kafai@fb.com> wrote: >> > >> > On Fri, Aug 26, 2022 at 12:13:54AM -0600, James Hilliard wrote: >> > > On Thu, Aug 25, 2022 at 11:49 PM Martin KaFai Lau <kafai@fb.com> wrote: >> > > > >> > > > On Thu, Aug 25, 2022 at 11:31:15PM -0600, James Hilliard wrote: >> > > > > On Thu, Aug 25, 2022 at 11:16 PM Martin KaFai Lau <kafai@fb.com> wrote: >> > > > > > >> > > > > > On Thu, Aug 25, 2022 at 04:17:49PM -0600, James Hilliard wrote: >> > > > > > > There is a potential for us to hit a type conflict when including >> > > > > > > netinet/tcp.h with sys/socket.h, we can replace both of these includes >> > > > > > > with linux/tcp.h to avoid this conflict. >> > > > > > > >> > > > > > > Fixes errors like: >> > > > > > > In file included from /usr/include/netinet/tcp.h:91, >> > > > > > > from progs/bind4_prog.c:10: >> > > > > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' >> > > > > > > 34 | typedef __INT8_TYPE__ int8_t; >> > > > > > > | ^~~~~~ >> > > > > > > In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, >> > > > > > > from /usr/include/x86_64-linux-gnu/bits/socket.h:29, >> > > > > > > from /usr/include/x86_64-linux-gnu/sys/socket.h:33, >> > > > > > > from progs/bind4_prog.c:9: >> > > > > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} >> > > > > > > 24 | typedef __int8_t int8_t; >> > > > > > > | ^~~~~~ >> > > > > > > /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: error: conflicting types for 'int64_t'; have 'long int' >> > > > > > > 43 | typedef __INT64_TYPE__ int64_t; >> > > > > > > | ^~~~~~~ >> > > > > > > /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: previous declaration of 'int64_t' with type 'int64_t' {aka 'long long int'} >> > > > > > > 27 | typedef __int64_t int64_t; >> > > > > > > | ^~~~~~~ >> > > > > > > make: *** [Makefile:537: /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] Error 1 >> > > > > > > >> > > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> >> > > > > > > --- >> > > > > > > tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- >> > > > > > > tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- >> > > > > > > 2 files changed, 2 insertions(+), 4 deletions(-) >> > > > > > > >> > > > > > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c >> > > > > > > index 474c6a62078a..6bd20042fd53 100644 >> > > > > > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c >> > > > > > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c >> > > > > > > @@ -6,8 +6,7 @@ >> > > > > > > #include <linux/bpf.h> >> > > > > > > #include <linux/in.h> >> > > > > > > #include <linux/in6.h> >> > > > > > > -#include <sys/socket.h> >> > > > > > > -#include <netinet/tcp.h> >> > > > > > These includes look normal to me. What environment is hitting this. >> > > > > >> > > > > I was hitting this error with GCC 13(GCC master branch). >> > > > These two includes (<sys/socket.h> and <netinet/tcp.h>) are normal, >> > > > so does it mean all existing programs need to change to use gcc 13 ? >> > > >> > > Well I think it's mostly just an issue getting hit with GCC-BPF as it >> > > looks to me like a cross compilation host/target header conflict. >> > The users have been using these headers in the bpf progs. >> >> Users can migrate away from libc headers over time, migrating away > imo, not without a good reason. Something that may be a good reason is that there is no BPF port of glibc (nor of musl, nor of newlib.) And given BPF's restrictions as an architecture (no more than 5 arguments supported in function calls, etc) it is very unlikely that there will ever be one. Including certain libc headers may work well enough, but in general when including libc headers you risk dragging in x86, or aarch64, or whatever architecture-specific headers as well, directly or indirectly. C libraries (and system libraries in general) are targetted at particular architectures/ABIs/OSes. This means that the same BPF program may be using different data structures depending on the system where you build it. In the worst case, it may choke on assembler snippets. Thats why the GCC port offers certain headers to provide standard C, like stdint.h. That's the usual way to go for bare-metal targets where no libc is available. Again, we will be happy to change that if that's what you want. In that case, it will be up to the user to provide the standard definitions, be it by including glibc headers targetted at some other architecture, or by some other mean. Not that I would personally recommend that, but it is up to you. > >> shouldn't cause regressions and should improve reliability. > May be I am missing something. I also don't understand the reliability > part. > > In this sys/socket.h as an example, what is wrong in using > "'int8_t' {aka 'signed char'}" from libc and the one from > gcc "'int8_t'; have 'char'" must be used instead. > > Why LLVM bpf does not have issue ? > >> >> > The solution should be on the GCC-BPF side instead of changing >> > all bpf progs. >> >> I mean, GCC doesn't really control which libc is available, it seems to >> be a bad idea to use libc headers in general as they are developed >> separately from GCC and the kernel/libbpf. >> >> I'm not really sure how one would fix this on the GCC-BPF side without >> introducing more potential header conflicts.
On Sat, Aug 27, 2022 at 03:13:41AM +0200, Jose E. Marchesi wrote: > >> Users can migrate away from libc headers over time, migrating away > > imo, not without a good reason. > > Something that may be a good reason is that there is no BPF port of > glibc (nor of musl, nor of newlib.) And given BPF's restrictions as an > architecture (no more than 5 arguments supported in function calls, etc) > it is very unlikely that there will ever be one. > > Including certain libc headers may work well enough, but in general when > including libc headers you risk dragging in x86, or aarch64, or whatever > architecture-specific headers as well, directly or indirectly. C > libraries (and system libraries in general) are targetted at particular > architectures/ABIs/OSes. > > This means that the same BPF program may be using different data > structures depending on the system where you build it. Note that the data structure difference is not unique to different arch. A more common case can already happen across different kernel versions or different kconfig of the same arch. BPF CO-RE is there to handle this case. > In the worst > case, it may choke on assembler snippets. > > Thats why the GCC port offers certain headers to provide standard C, > like stdint.h. That's the usual way to go for bare-metal targets where > no libc is available. > > Again, we will be happy to change that if that's what you want. In that > case, it will be up to the user to provide the standard definitions, be > it by including glibc headers targetted at some other architecture, or > by some other mean. Not that I would personally recommend that, but it > is up to you. Not sure if the user can always stay with vmlinux.h + a few bpf_tracing_*.h and if that is enough to avoid knowing this difference between GCC and LLVM bpf on libc-vs-gcc stdint...etc. The header changes is hard to swim through to make it compile in GCC BPF. In this case, it is because netinet/tcp.h brought in a different int8_t from gcc than the sys/socket.h. My preference is not to have to dive into this kind of header details. I would like to hear how others think about it. > > > >> shouldn't cause regressions and should improve reliability. > > May be I am missing something. I also don't understand the reliability > > part. > > > > In this sys/socket.h as an example, what is wrong in using > > "'int8_t' {aka 'signed char'}" from libc and the one from > > gcc "'int8_t'; have 'char'" must be used instead. > > > > Why LLVM bpf does not have issue ? > > > >> > >> > The solution should be on the GCC-BPF side instead of changing > >> > all bpf progs. > >> > >> I mean, GCC doesn't really control which libc is available, it seems to > >> be a bad idea to use libc headers in general as they are developed > >> separately from GCC and the kernel/libbpf. > >> > >> I'm not really sure how one would fix this on the GCC-BPF side without > >> introducing more potential header conflicts.
diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c index 474c6a62078a..6bd20042fd53 100644 --- a/tools/testing/selftests/bpf/progs/bind4_prog.c +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c @@ -6,8 +6,7 @@ #include <linux/bpf.h> #include <linux/in.h> #include <linux/in6.h> -#include <sys/socket.h> -#include <netinet/tcp.h> +#include <linux/tcp.h> #include <linux/if.h> #include <errno.h> diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c index c19cfa869f30..f37617b35a55 100644 --- a/tools/testing/selftests/bpf/progs/bind6_prog.c +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c @@ -6,8 +6,7 @@ #include <linux/bpf.h> #include <linux/in.h> #include <linux/in6.h> -#include <sys/socket.h> -#include <netinet/tcp.h> +#include <linux/tcp.h> #include <linux/if.h> #include <errno.h>
There is a potential for us to hit a type conflict when including netinet/tcp.h with sys/socket.h, we can replace both of these includes with linux/tcp.h to avoid this conflict. Fixes errors like: In file included from /usr/include/netinet/tcp.h:91, from progs/bind4_prog.c:10: /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:34:23: error: conflicting types for 'int8_t'; have 'char' 34 | typedef __INT8_TYPE__ int8_t; | ^~~~~~ In file included from /usr/include/x86_64-linux-gnu/sys/types.h:155, from /usr/include/x86_64-linux-gnu/bits/socket.h:29, from /usr/include/x86_64-linux-gnu/sys/socket.h:33, from progs/bind4_prog.c:9: /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:24:18: note: previous declaration of 'int8_t' with type 'int8_t' {aka 'signed char'} 24 | typedef __int8_t int8_t; | ^~~~~~ /home/buildroot/opt/cross/lib/gcc/bpf/13.0.0/include/stdint.h:43:24: error: conflicting types for 'int64_t'; have 'long int' 43 | typedef __INT64_TYPE__ int64_t; | ^~~~~~~ /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: previous declaration of 'int64_t' with type 'int64_t' {aka 'long long int'} 27 | typedef __int64_t int64_t; | ^~~~~~~ make: *** [Makefile:537: /home/buildroot/bpf-next/tools/testing/selftests/bpf/bpf_gcc/bind4_prog.o] Error 1 Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- tools/testing/selftests/bpf/progs/bind4_prog.c | 3 +-- tools/testing/selftests/bpf/progs/bind6_prog.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)