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.