There is a potential for us to hit a type conflict when including
netinet/tcp.h with sys/socket.h, we can remove these as they are not
actually needed.
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]>
---
Changes v1 -> v2:
- just remove netinet/tcp.h and sys/socket.h
---
tools/testing/selftests/bpf/progs/bind4_prog.c | 2 --
tools/testing/selftests/bpf/progs/bind6_prog.c | 2 --
2 files changed, 4 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c
index 474c6a62078a..a487f60b73ac 100644
--- a/tools/testing/selftests/bpf/progs/bind4_prog.c
+++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
@@ -6,8 +6,6 @@
#include <linux/bpf.h>
#include <linux/in.h>
#include <linux/in6.h>
-#include <sys/socket.h>
-#include <netinet/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..d62cd9e9cf0e 100644
--- a/tools/testing/selftests/bpf/progs/bind6_prog.c
+++ b/tools/testing/selftests/bpf/progs/bind6_prog.c
@@ -6,8 +6,6 @@
#include <linux/bpf.h>
#include <linux/in.h>
#include <linux/in6.h>
-#include <sys/socket.h>
-#include <netinet/tcp.h>
#include <linux/if.h>
#include <errno.h>
--
2.34.1
On Thu, Aug 25, 2022 at 11:29:22PM -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 remove these as they are not
> actually needed.
>
> 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]>
> ---
> Changes v1 -> v2:
> - just remove netinet/tcp.h and sys/socket.h
> ---
> tools/testing/selftests/bpf/progs/bind4_prog.c | 2 --
> tools/testing/selftests/bpf/progs/bind6_prog.c | 2 --
> 2 files changed, 4 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c
> index 474c6a62078a..a487f60b73ac 100644
> --- a/tools/testing/selftests/bpf/progs/bind4_prog.c
> +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
> @@ -6,8 +6,6 @@
> #include <linux/bpf.h>
> #include <linux/in.h>
> #include <linux/in6.h>
> -#include <sys/socket.h>
> -#include <netinet/tcp.h>
> #include <linux/if.h>
Are the AF_INET and SOCK_STREAM coming from linux/if.h somehow
and they are not from indirectly including sys/socket.h ?
If the program does not need if.h, what should it use ?
There are other progs in selftest/bpf that include sys/socket.h
and they have no issue ?
On Fri, Aug 26, 2022 at 5:05 PM Martin KaFai Lau <[email protected]> wrote:
>
> On Thu, Aug 25, 2022 at 11:29:22PM -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 remove these as they are not
> > actually needed.
> >
> > 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]>
> > ---
> > Changes v1 -> v2:
> > - just remove netinet/tcp.h and sys/socket.h
> > ---
> > tools/testing/selftests/bpf/progs/bind4_prog.c | 2 --
> > tools/testing/selftests/bpf/progs/bind6_prog.c | 2 --
> > 2 files changed, 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c
> > index 474c6a62078a..a487f60b73ac 100644
> > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c
> > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
> > @@ -6,8 +6,6 @@
> > #include <linux/bpf.h>
> > #include <linux/in.h>
> > #include <linux/in6.h>
> > -#include <sys/socket.h>
> > -#include <netinet/tcp.h>
> > #include <linux/if.h>
> Are the AF_INET and SOCK_STREAM coming from linux/if.h somehow
> and they are not from indirectly including sys/socket.h ?
Hmm, seems they are both coming from sys/socket.h:
Tests with my v2 patch applied:
progs/bind4_prog.c:15: error: "AF_INET" redefined [-Werror]
15 | #define AF_INET nonsense
|
In file included from /usr/include/x86_64-linux-gnu/sys/socket.h:33,
from /usr/include/linux/if.h:28,
from progs/bind4_prog.c:9:
/usr/include/x86_64-linux-gnu/bits/socket.h:97: note: this is the
location of the previous definition
97 | #define AF_INET PF_INET
|
progs/bind4_prog.c:15: error: "SOCK_STREAM" redefined [-Werror]
15 | #define SOCK_STREAM nonsense
|
In file included from /usr/include/x86_64-linux-gnu/bits/socket.h:38,
from /usr/include/x86_64-linux-gnu/sys/socket.h:33,
from /usr/include/linux/if.h:28,
from progs/bind4_prog.c:9:
/usr/include/x86_64-linux-gnu/bits/socket_type.h:28: note: this is the
location of the previous definition
28 | #define SOCK_STREAM SOCK_STREAM
|
So I guess the problematic header is netinet/tcp.h and sys/socket.h is
just a redundant include?
Removing just netinet/tcp.h does appear sufficient to fix the issue.
>
> If the program does not need if.h, what should it use ?
> There are other progs in selftest/bpf that include sys/socket.h
> and they have no issue ?
I'm still working through gcc issues with the test suite so there's
probably some cases I haven't identified yet but this is the only one
that seemed to need any code changes when removing those 2
headers that I've found so far:
https://lore.kernel.org/bpf/[email protected]/
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <[email protected]>:
On Thu, 25 Aug 2022 23:29:22 -0600 you wrote:
> There is a potential for us to hit a type conflict when including
> netinet/tcp.h with sys/socket.h, we can remove these as they are not
> actually needed.
>
> 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
>
> [...]
Here is the summary with links:
- [v2] selftests/bpf: Fix bind{4,6} tcp/socket header type conflict
https://git.kernel.org/bpf/bpf-next/c/3721359d3907
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
On Sat, Aug 27, 2022 at 05:38:29AM -0600, James Hilliard wrote:
> On Fri, Aug 26, 2022 at 5:05 PM Martin KaFai Lau <[email protected]> wrote:
> >
> > On Thu, Aug 25, 2022 at 11:29:22PM -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 remove these as they are not
> > > actually needed.
> > >
> > > 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]>
> > > ---
> > > Changes v1 -> v2:
> > > - just remove netinet/tcp.h and sys/socket.h
> > > ---
> > > tools/testing/selftests/bpf/progs/bind4_prog.c | 2 --
> > > tools/testing/selftests/bpf/progs/bind6_prog.c | 2 --
> > > 2 files changed, 4 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c
> > > index 474c6a62078a..a487f60b73ac 100644
> > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c
> > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
> > > @@ -6,8 +6,6 @@
> > > #include <linux/bpf.h>
> > > #include <linux/in.h>
> > > #include <linux/in6.h>
> > > -#include <sys/socket.h>
> > > -#include <netinet/tcp.h>
> > > #include <linux/if.h>
> > Are the AF_INET and SOCK_STREAM coming from linux/if.h somehow
> > and they are not from indirectly including sys/socket.h ?
>
> Hmm, seems they are both coming from sys/socket.h:
>
> Tests with my v2 patch applied:
> progs/bind4_prog.c:15: error: "AF_INET" redefined [-Werror]
> 15 | #define AF_INET nonsense
> |
> In file included from /usr/include/x86_64-linux-gnu/sys/socket.h:33,
> from /usr/include/linux/if.h:28,
> from progs/bind4_prog.c:9:
> /usr/include/x86_64-linux-gnu/bits/socket.h:97: note: this is the
> location of the previous definition
> 97 | #define AF_INET PF_INET
> |
>
> progs/bind4_prog.c:15: error: "SOCK_STREAM" redefined [-Werror]
> 15 | #define SOCK_STREAM nonsense
> |
> In file included from /usr/include/x86_64-linux-gnu/bits/socket.h:38,
> from /usr/include/x86_64-linux-gnu/sys/socket.h:33,
> from /usr/include/linux/if.h:28,
> from progs/bind4_prog.c:9:
> /usr/include/x86_64-linux-gnu/bits/socket_type.h:28: note: this is the
> location of the previous definition
> 28 | #define SOCK_STREAM SOCK_STREAM
> |
>
> So I guess the problematic header is netinet/tcp.h and sys/socket.h is
> just a redundant include?
>
> Removing just netinet/tcp.h does appear sufficient to fix the issue.
Yeah, it is what I am puzzled and getting at.
<sys/socket.h> is fine and <netinet/tcp.h> is not ok.
They are both from glibc ? This kind of header changes
is hard to reason without doing the kind of experiment
that you just did.
>
> >
> > If the program does not need if.h, what should it use ?
> > There are other progs in selftest/bpf that include sys/socket.h
> > and they have no issue ?
>
> I'm still working through gcc issues with the test suite so there's
> probably some cases I haven't identified yet but this is the only one
> that seemed to need any code changes when removing those 2
> headers that I've found so far:
> https://lore.kernel.org/bpf/[email protected]/
On Mon, Aug 29, 2022 at 12:03 PM Martin KaFai Lau <[email protected]> wrote:
>
> On Sat, Aug 27, 2022 at 05:38:29AM -0600, James Hilliard wrote:
> > On Fri, Aug 26, 2022 at 5:05 PM Martin KaFai Lau <[email protected]> wrote:
> > >
> > > On Thu, Aug 25, 2022 at 11:29:22PM -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 remove these as they are not
> > > > actually needed.
> > > >
> > > > 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]>
> > > > ---
> > > > Changes v1 -> v2:
> > > > - just remove netinet/tcp.h and sys/socket.h
> > > > ---
> > > > tools/testing/selftests/bpf/progs/bind4_prog.c | 2 --
> > > > tools/testing/selftests/bpf/progs/bind6_prog.c | 2 --
> > > > 2 files changed, 4 deletions(-)
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c
> > > > index 474c6a62078a..a487f60b73ac 100644
> > > > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c
> > > > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
> > > > @@ -6,8 +6,6 @@
> > > > #include <linux/bpf.h>
> > > > #include <linux/in.h>
> > > > #include <linux/in6.h>
> > > > -#include <sys/socket.h>
> > > > -#include <netinet/tcp.h>
> > > > #include <linux/if.h>
> > > Are the AF_INET and SOCK_STREAM coming from linux/if.h somehow
> > > and they are not from indirectly including sys/socket.h ?
> >
> > Hmm, seems they are both coming from sys/socket.h:
> >
> > Tests with my v2 patch applied:
> > progs/bind4_prog.c:15: error: "AF_INET" redefined [-Werror]
> > 15 | #define AF_INET nonsense
> > |
> > In file included from /usr/include/x86_64-linux-gnu/sys/socket.h:33,
> > from /usr/include/linux/if.h:28,
> > from progs/bind4_prog.c:9:
> > /usr/include/x86_64-linux-gnu/bits/socket.h:97: note: this is the
> > location of the previous definition
> > 97 | #define AF_INET PF_INET
> > |
> >
> > progs/bind4_prog.c:15: error: "SOCK_STREAM" redefined [-Werror]
> > 15 | #define SOCK_STREAM nonsense
> > |
> > In file included from /usr/include/x86_64-linux-gnu/bits/socket.h:38,
> > from /usr/include/x86_64-linux-gnu/sys/socket.h:33,
> > from /usr/include/linux/if.h:28,
> > from progs/bind4_prog.c:9:
> > /usr/include/x86_64-linux-gnu/bits/socket_type.h:28: note: this is the
> > location of the previous definition
> > 28 | #define SOCK_STREAM SOCK_STREAM
> > |
> >
> > So I guess the problematic header is netinet/tcp.h and sys/socket.h is
> > just a redundant include?
> >
> > Removing just netinet/tcp.h does appear sufficient to fix the issue.
> Yeah, it is what I am puzzled and getting at.
> <sys/socket.h> is fine and <netinet/tcp.h> is not ok.
> They are both from glibc ? This kind of header changes
> is hard to reason without doing the kind of experiment
> that you just did.
I think so, it kind of looks to me like most of the tests were avoiding this
issue in various ways already, the percentage with this issue seems to
be fairly low.
>
> >
> > >
> > > If the program does not need if.h, what should it use ?
> > > There are other progs in selftest/bpf that include sys/socket.h
> > > and they have no issue ?
> >
> > I'm still working through gcc issues with the test suite so there's
> > probably some cases I haven't identified yet but this is the only one
> > that seemed to need any code changes when removing those 2
> > headers that I've found so far:
> > https://lore.kernel.org/bpf/[email protected]/