Hi,
On 2/5/2025 11:27 AM, Jason Xing wrote:
> On Wed, Feb 5, 2025 at 10:57 AM Hou Tao <houtao@xxxxxxxxxxxxxxx> 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@xxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxx>
> 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
>
> .
