| Message ID | 20250204023946.16031-3-kerneljasonxing@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests: fix two small compilation errors | expand |
Hi, On 2/4/2025 10:39 AM, Jason Xing wrote: > Error messages: > selftests/bpf/prog_tests/sockopt_sk.c: In function ‘getsetsockopt’: > selftests/bpf/prog_tests/sockopt_sk.c:22:31: error: field ‘zc’ has incomplete type > struct tcp_zerocopy_receive zc; > ^~ > selftests/bpf/prog_tests/sockopt_sk.c:169:32: error: ‘TCP_ZEROCOPY_RECEIVE’ undeclared (first use in this function) > err = getsockopt(fd, SOL_TCP, TCP_ZEROCOPY_RECEIVE, &buf, &optlen); > ^~~~~~~~~~~~~~~~~~~~ > > Fix it by introducing the right header. > > Signed-off-by: Jason Xing <kerneljasonxing@gmail.com> > --- > tools/testing/selftests/bpf/prog_tests/sockopt_sk.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c > index ba6b3ec1156a..e0a9785ffcdc 100644 > --- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c > +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c > @@ -2,7 +2,7 @@ > #include <test_progs.h> > #include "cgroup_helpers.h" > > -#include <netinet/tcp.h> > +#include <uapi/linux/tcp.h> Should it be <linux/tcp.h> instead ? Directly including uapi header file in application seems weird. > #include <linux/netlink.h> > #include "sockopt_sk.skel.h" >
On Wed, Feb 5, 2025 at 10:57 AM Hou Tao <houtao@huaweicloud.com> wrote: > > Hi, > > On 2/4/2025 10:39 AM, Jason Xing wrote: > > Error messages: > > selftests/bpf/prog_tests/sockopt_sk.c: In function ‘getsetsockopt’: > > selftests/bpf/prog_tests/sockopt_sk.c:22:31: error: field ‘zc’ has incomplete type > > struct tcp_zerocopy_receive zc; > > ^~ > > selftests/bpf/prog_tests/sockopt_sk.c:169:32: error: ‘TCP_ZEROCOPY_RECEIVE’ undeclared (first use in this function) > > err = getsockopt(fd, SOL_TCP, TCP_ZEROCOPY_RECEIVE, &buf, &optlen); > > ^~~~~~~~~~~~~~~~~~~~ > > > > Fix it by introducing the right header. > > > > Signed-off-by: Jason Xing <kerneljasonxing@gmail.com> > > --- > > tools/testing/selftests/bpf/prog_tests/sockopt_sk.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c > > index ba6b3ec1156a..e0a9785ffcdc 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c > > +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c > > @@ -2,7 +2,7 @@ > > #include <test_progs.h> > > #include "cgroup_helpers.h" > > > > -#include <netinet/tcp.h> > > +#include <uapi/linux/tcp.h> > > Should it be <linux/tcp.h> instead ? I thought that too, but I altered my thoughts after reading this commit[1], totally without knowing why the tcp part should be changed. Should I change it back? > Directly including uapi header file > in application seems weird. After greping the tools/testing/selftests/bpf, we see some similar usage like including a uapi header file. [1] commit a2f482c34a52176ae89d143979bbc9e7a72857c8 Author: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com> Date: Wed Nov 20 08:43:21 2024 +0100 selftests/bpf: use the same udp and tcp headers in tests under test_progs Trying to add udp-dedicated helpers in network_helpers involves including some udp header, which makes multiple test_progs tests build fail: In file included from ./progs/test_cls_redirect.h:13, from [...]/prog_tests/cls_redirect.c:15: [...]/usr/include/linux/udp.h:23:8: error: redefinition of ‘struct udphdr’ 23 | struct udphdr { | ^~~~~~ In file included from ./network_helpers.h:17, from [...]/prog_tests/cls_redirect.c:13: [...]/usr/include/netinet/udp.h:55:8: note: originally defined here 55 | struct udphdr | ^~~~~~ This error is due to struct udphdr being defined in both <linux/udp.h> and <netinet/udp.h>. Use only <netinet/udp.h> in every test. While at it, perform the same for tcp.h. For some tests, the change needs to be done in the eBPF program part as well, because of some headers sharing between both sides. Thanks, Jason
Hi, On 2/5/2025 11:27 AM, Jason Xing wrote: > On Wed, Feb 5, 2025 at 10:57 AM Hou Tao <houtao@huaweicloud.com> wrote: >> Hi, >> >> On 2/4/2025 10:39 AM, Jason Xing wrote: >>> Error messages: >>> selftests/bpf/prog_tests/sockopt_sk.c: In function ‘getsetsockopt’: >>> selftests/bpf/prog_tests/sockopt_sk.c:22:31: error: field ‘zc’ has incomplete type >>> struct tcp_zerocopy_receive zc; >>> ^~ >>> selftests/bpf/prog_tests/sockopt_sk.c:169:32: error: ‘TCP_ZEROCOPY_RECEIVE’ undeclared (first use in this function) >>> err = getsockopt(fd, SOL_TCP, TCP_ZEROCOPY_RECEIVE, &buf, &optlen); >>> ^~~~~~~~~~~~~~~~~~~~ >>> >>> Fix it by introducing the right header. >>> >>> Signed-off-by: Jason Xing <kerneljasonxing@gmail.com> >>> --- >>> tools/testing/selftests/bpf/prog_tests/sockopt_sk.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c >>> index ba6b3ec1156a..e0a9785ffcdc 100644 >>> --- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c >>> +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c >>> @@ -2,7 +2,7 @@ >>> #include <test_progs.h> >>> #include "cgroup_helpers.h" >>> >>> -#include <netinet/tcp.h> >>> +#include <uapi/linux/tcp.h> >> Should it be <linux/tcp.h> instead ? > I thought that too, but I altered my thoughts after reading this > commit[1], totally without knowing why the tcp part should be changed. > Should I change it back? Thanks for pointing the commit to me. Under my local environment, it seems both netinet/tcp.h and linux/tcp define tcp_zerocopy_receive and tcphdr, and I think that is the reason why the commit changes tcp as well. For the following build error: selftests/bpf/prog_tests/sockopt_sk.c:22:31: error: field ‘zc’ has incomplete type struct tcp_zerocopy_receive zc; I think maybe your local environment is a bit out-of-date. I prefer to keep it as-is. > >> Directly including uapi header file >> in application seems weird. > After greping the tools/testing/selftests/bpf, we see some similar > usage like including a uapi header file. > > [1] > commit a2f482c34a52176ae89d143979bbc9e7a72857c8 > Author: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com> > Date: Wed Nov 20 08:43:21 2024 +0100 > > selftests/bpf: use the same udp and tcp headers in tests under test_progs > > Trying to add udp-dedicated helpers in network_helpers involves > including some udp header, which makes multiple test_progs tests build > fail: > > In file included from ./progs/test_cls_redirect.h:13, > from [...]/prog_tests/cls_redirect.c:15: > [...]/usr/include/linux/udp.h:23:8: error: redefinition of ‘struct udphdr’ > 23 | struct udphdr { > | ^~~~~~ > In file included from ./network_helpers.h:17, > from [...]/prog_tests/cls_redirect.c:13: > [...]/usr/include/netinet/udp.h:55:8: note: originally defined here > 55 | struct udphdr > | ^~~~~~ > > This error is due to struct udphdr being defined in both <linux/udp.h> > and <netinet/udp.h>. > > Use only <netinet/udp.h> in every test. While at it, perform the same > for tcp.h. For some tests, the change needs to be done in the eBPF > program part as well, because of some headers sharing between both > sides. > > Thanks, > Jason > > .
On Wed, Feb 5, 2025 at 5:30 PM Hou Tao <houtao@huaweicloud.com> wrote: > > Hi, > > On 2/5/2025 11:27 AM, Jason Xing wrote: > > On Wed, Feb 5, 2025 at 10:57 AM Hou Tao <houtao@huaweicloud.com> wrote: > >> Hi, > >> > >> On 2/4/2025 10:39 AM, Jason Xing wrote: > >>> Error messages: > >>> selftests/bpf/prog_tests/sockopt_sk.c: In function ‘getsetsockopt’: > >>> selftests/bpf/prog_tests/sockopt_sk.c:22:31: error: field ‘zc’ has incomplete type > >>> struct tcp_zerocopy_receive zc; > >>> ^~ > >>> selftests/bpf/prog_tests/sockopt_sk.c:169:32: error: ‘TCP_ZEROCOPY_RECEIVE’ undeclared (first use in this function) > >>> err = getsockopt(fd, SOL_TCP, TCP_ZEROCOPY_RECEIVE, &buf, &optlen); > >>> ^~~~~~~~~~~~~~~~~~~~ > >>> > >>> Fix it by introducing the right header. > >>> > >>> Signed-off-by: Jason Xing <kerneljasonxing@gmail.com> > >>> --- > >>> tools/testing/selftests/bpf/prog_tests/sockopt_sk.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c > >>> index ba6b3ec1156a..e0a9785ffcdc 100644 > >>> --- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c > >>> +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c > >>> @@ -2,7 +2,7 @@ > >>> #include <test_progs.h> > >>> #include "cgroup_helpers.h" > >>> > >>> -#include <netinet/tcp.h> > >>> +#include <uapi/linux/tcp.h> > >> Should it be <linux/tcp.h> instead ? > > I thought that too, but I altered my thoughts after reading this > > commit[1], totally without knowing why the tcp part should be changed. > > Should I change it back? > > Thanks for pointing the commit to me. Under my local environment, it > seems both netinet/tcp.h and linux/tcp define tcp_zerocopy_receive and > tcphdr, and I think that is the reason why the commit changes tcp as > well. For the following build error: > > selftests/bpf/prog_tests/sockopt_sk.c:22:31: error: field ‘zc’ has > incomplete type > struct tcp_zerocopy_receive zc; > > I think maybe your local environment is a bit out-of-date. I prefer to > keep it as-is. Thanks for your review. Right, but I believe many users can't manage to upgrade to the latest version for the whole system. The selftests are supposed to be compatible, I reckon. It's surely not bad to consider the compatibility after adjusting the header file. Thanks, Jason
diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c index ba6b3ec1156a..e0a9785ffcdc 100644 --- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c @@ -2,7 +2,7 @@ #include <test_progs.h> #include "cgroup_helpers.h" -#include <netinet/tcp.h> +#include <uapi/linux/tcp.h> #include <linux/netlink.h> #include "sockopt_sk.skel.h"
Error messages: selftests/bpf/prog_tests/sockopt_sk.c: In function ‘getsetsockopt’: selftests/bpf/prog_tests/sockopt_sk.c:22:31: error: field ‘zc’ has incomplete type struct tcp_zerocopy_receive zc; ^~ selftests/bpf/prog_tests/sockopt_sk.c:169:32: error: ‘TCP_ZEROCOPY_RECEIVE’ undeclared (first use in this function) err = getsockopt(fd, SOL_TCP, TCP_ZEROCOPY_RECEIVE, &buf, &optlen); ^~~~~~~~~~~~~~~~~~~~ Fix it by introducing the right header. Signed-off-by: Jason Xing <kerneljasonxing@gmail.com> --- tools/testing/selftests/bpf/prog_tests/sockopt_sk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)