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 <[email protected]>
---
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 3:18 PM James Hilliard
<[email protected]> 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 <[email protected]>
Still compiles in my environment:
Reviewed-by: Stanislav Fomichev <[email protected]>
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 <[email protected]>
> ---
> 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 <[email protected]> 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 <[email protected]>
> > ---
> > 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/[email protected]/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 <[email protected]> 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 <[email protected]>
> > > ---
> > > 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/[email protected]/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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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/[email protected]/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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
>> > > > ---
>> > > > 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/[email protected]/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 <[email protected]>
Reviewed-by: Muhammad Usama Anjum <[email protected]>
> ---
> 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>
>
--
Muhammad Usama Anjum
On Fri, Aug 26, 2022 at 7:19 AM Jose E. Marchesi
<[email protected]> wrote:
>
>
> > On Thu, Aug 25, 2022 at 11:49 PM Martin KaFai Lau <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >> > > > ---
> >> > > > 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/[email protected]/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
> <[email protected]> wrote:
>>
>>
>> > On Thu, Aug 25, 2022 at 11:49 PM Martin KaFai Lau <[email protected]> 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 <[email protected]> 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 <[email protected]>
>> >> > > > ---
>> >> > > > 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/[email protected]/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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > > ---
> > > > > 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 10:33 AM Jose E. Marchesi
<[email protected]> wrote:
>
>
> > On Fri, Aug 26, 2022 at 7:19 AM Jose E. Marchesi
> > <[email protected]> wrote:
> >>
> >>
> >> > On Thu, Aug 25, 2022 at 11:49 PM Martin KaFai Lau <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >> >> > > > ---
> >> >> > > > 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/[email protected]/
>
> 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/[email protected]/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 11:17 AM Martin KaFai Lau <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > > > ---
> > > > > > 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > > > > ---
> > > > > > > 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
>> > > > > > > ---
>> > > > > > > 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.