Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [PATCH bpf-next v1 0/2] selftests: fix two small compilation errors
@ 2025-02-04  2:39 Jason Xing
  2025-02-04  2:39 ` [PATCH bpf-next v1 1/2] bpf: changes_pkt_data: correct the 'main' error Jason Xing
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Jason Xing @ 2025-02-04  2:39 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, mykolal, martin.lau, song,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah
  Cc: bpf, linux-kselftest, Jason Xing

Fix them separately in each patch.

Jason Xing (2):
  bpf: changes_pkt_data: correct the 'main' error
  bpf: sockopt_sk: fix 'undeclared' definition error

 .../selftests/bpf/prog_tests/changes_pkt_data.c      | 12 ++++++------
 tools/testing/selftests/bpf/prog_tests/sockopt_sk.c  |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

-- 
2.43.5


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH bpf-next v1 1/2] bpf: changes_pkt_data: correct the 'main' error
  2025-02-04  2:39 [PATCH bpf-next v1 0/2] selftests: fix two small compilation errors Jason Xing
@ 2025-02-04  2:39 ` Jason Xing
  2025-02-07  6:04   ` Martin KaFai Lau
  2025-02-04  2:39 ` [PATCH bpf-next v1 2/2] bpf: sockopt_sk: fix 'undeclared' definition error Jason Xing
  2025-02-11  7:52 ` [PATCH bpf-next v1 0/2] selftests: fix two small compilation errors Jason Xing
  2 siblings, 1 reply; 12+ messages in thread
From: Jason Xing @ 2025-02-04  2:39 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, mykolal, martin.lau, song,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah
  Cc: bpf, linux-kselftest, Jason Xing

When compiling the selftests, the following error is printed out:
selftests/bpf/prog_tests/changes_pkt_data.c: In function ‘test_aux’:
selftests/bpf/prog_tests/changes_pkt_data.c:22:27: error: ‘main’ is usually a function [-Werror=main]
  struct changes_pkt_data *main = NULL;
                           ^~~~

Fix it by replacing with 'main_data'.

Signed-off-by: Jason Xing <kerneljasonxing@gmail.com>
---
 .../selftests/bpf/prog_tests/changes_pkt_data.c      | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/changes_pkt_data.c b/tools/testing/selftests/bpf/prog_tests/changes_pkt_data.c
index 7526de379081..b5c9031ec470 100644
--- a/tools/testing/selftests/bpf/prog_tests/changes_pkt_data.c
+++ b/tools/testing/selftests/bpf/prog_tests/changes_pkt_data.c
@@ -19,7 +19,7 @@ static void test_aux(const char *main_prog_name,
 	struct bpf_program *freplace_prog = NULL;
 	struct bpf_program *main_prog = NULL;
 	LIBBPF_OPTS(bpf_object_open_opts, opts);
-	struct changes_pkt_data *main = NULL;
+	struct changes_pkt_data *main_data = NULL;
 	char log[16*1024];
 	int err;
 
@@ -27,14 +27,14 @@ static void test_aux(const char *main_prog_name,
 	opts.kernel_log_size = sizeof(log);
 	if (env.verbosity >= VERBOSE_SUPER)
 		opts.kernel_log_level = 1 | 2 | 4;
-	main = changes_pkt_data__open_opts(&opts);
-	if (!ASSERT_OK_PTR(main, "changes_pkt_data__open"))
+	main_data = changes_pkt_data__open_opts(&opts);
+	if (!ASSERT_OK_PTR(main_data, "changes_pkt_data__open"))
 		goto out;
-	main_prog = bpf_object__find_program_by_name(main->obj, main_prog_name);
+	main_prog = bpf_object__find_program_by_name(main_data->obj, main_prog_name);
 	if (!ASSERT_OK_PTR(main_prog, "main_prog"))
 		goto out;
 	bpf_program__set_autoload(main_prog, true);
-	err = changes_pkt_data__load(main);
+	err = changes_pkt_data__load(main_data);
 	print_verifier_log(log);
 	if (!ASSERT_OK(err, "changes_pkt_data__load"))
 		goto out;
@@ -60,7 +60,7 @@ static void test_aux(const char *main_prog_name,
 
 out:
 	changes_pkt_data_freplace__destroy(freplace);
-	changes_pkt_data__destroy(main);
+	changes_pkt_data__destroy(main_data);
 }
 
 /* There are two global subprograms in both changes_pkt_data.skel.h:
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH bpf-next v1 2/2] bpf: sockopt_sk: fix 'undeclared' definition error
  2025-02-04  2:39 [PATCH bpf-next v1 0/2] selftests: fix two small compilation errors Jason Xing
  2025-02-04  2:39 ` [PATCH bpf-next v1 1/2] bpf: changes_pkt_data: correct the 'main' error Jason Xing
@ 2025-02-04  2:39 ` Jason Xing
  2025-02-05  2:57   ` Hou Tao
  2025-02-11  7:52 ` [PATCH bpf-next v1 0/2] selftests: fix two small compilation errors Jason Xing
  2 siblings, 1 reply; 12+ messages in thread
From: Jason Xing @ 2025-02-04  2:39 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, mykolal, martin.lau, song,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah
  Cc: bpf, linux-kselftest, Jason Xing

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>
 #include <linux/netlink.h>
 #include "sockopt_sk.skel.h"
 
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH bpf-next v1 2/2] bpf: sockopt_sk: fix 'undeclared' definition error
  2025-02-04  2:39 ` [PATCH bpf-next v1 2/2] bpf: sockopt_sk: fix 'undeclared' definition error Jason Xing
@ 2025-02-05  2:57   ` Hou Tao
  2025-02-05  3:27     ` Jason Xing
  0 siblings, 1 reply; 12+ messages in thread
From: Hou Tao @ 2025-02-05  2:57 UTC (permalink / raw)
  To: Jason Xing, bpf, linux-kselftest
  Cc: ast, daniel, andrii, eddyz87, mykolal, martin.lau, song,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah

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"
>  


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH bpf-next v1 2/2] bpf: sockopt_sk: fix 'undeclared' definition error
  2025-02-05  2:57   ` Hou Tao
@ 2025-02-05  3:27     ` Jason Xing
  2025-02-05  9:30       ` Hou Tao
  2025-02-11 19:40       ` Martin KaFai Lau
  0 siblings, 2 replies; 12+ messages in thread
From: Jason Xing @ 2025-02-05  3:27 UTC (permalink / raw)
  To: Hou Tao
  Cc: bpf, linux-kselftest, ast, daniel, andrii, eddyz87, mykolal,
	martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa, shuah

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH bpf-next v1 2/2] bpf: sockopt_sk: fix 'undeclared' definition error
  2025-02-05  3:27     ` Jason Xing
@ 2025-02-05  9:30       ` Hou Tao
  2025-02-05  9:38         ` Jason Xing
  2025-02-11 19:40       ` Martin KaFai Lau
  1 sibling, 1 reply; 12+ messages in thread
From: Hou Tao @ 2025-02-05  9:30 UTC (permalink / raw)
  To: Jason Xing
  Cc: bpf, linux-kselftest, ast, daniel, andrii, eddyz87, mykolal,
	martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa, shuah

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
>
> .


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH bpf-next v1 2/2] bpf: sockopt_sk: fix 'undeclared' definition error
  2025-02-05  9:30       ` Hou Tao
@ 2025-02-05  9:38         ` Jason Xing
  0 siblings, 0 replies; 12+ messages in thread
From: Jason Xing @ 2025-02-05  9:38 UTC (permalink / raw)
  To: Hou Tao
  Cc: bpf, linux-kselftest, ast, daniel, andrii, eddyz87, mykolal,
	martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa, shuah

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH bpf-next v1 1/2] bpf: changes_pkt_data: correct the 'main' error
  2025-02-04  2:39 ` [PATCH bpf-next v1 1/2] bpf: changes_pkt_data: correct the 'main' error Jason Xing
@ 2025-02-07  6:04   ` Martin KaFai Lau
  2025-02-07  6:48     ` Jason Xing
  0 siblings, 1 reply; 12+ messages in thread
From: Martin KaFai Lau @ 2025-02-07  6:04 UTC (permalink / raw)
  To: Jason Xing
  Cc: ast, daniel, andrii, eddyz87, mykolal, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah, bpf,
	linux-kselftest

On 2/3/25 6:39 PM, Jason Xing wrote:
> When compiling the selftests, the following error is printed out:
> selftests/bpf/prog_tests/changes_pkt_data.c: In function ‘test_aux’:
> selftests/bpf/prog_tests/changes_pkt_data.c:22:27: error: ‘main’ is usually a function [-Werror=main]
>    struct changes_pkt_data *main = NULL;

The bpf CI has been testing this piece with different compilers. I also don't 
see it in my environment. How to reproduce it and which compiler?

> Fix it by replacing with 'main_data'.
> 
> Signed-off-by: Jason Xing <kerneljasonxing@gmail.com>
> ---
>   .../selftests/bpf/prog_tests/changes_pkt_data.c      | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/changes_pkt_data.c b/tools/testing/selftests/bpf/prog_tests/changes_pkt_data.c
> index 7526de379081..b5c9031ec470 100644
> --- a/tools/testing/selftests/bpf/prog_tests/changes_pkt_data.c
> +++ b/tools/testing/selftests/bpf/prog_tests/changes_pkt_data.c
> @@ -19,7 +19,7 @@ static void test_aux(const char *main_prog_name,
>   	struct bpf_program *freplace_prog = NULL;
>   	struct bpf_program *main_prog = NULL;
>   	LIBBPF_OPTS(bpf_object_open_opts, opts);
> -	struct changes_pkt_data *main = NULL;
> +	struct changes_pkt_data *main_data = NULL;
>   	char log[16*1024];
>   	int err;
>   
> @@ -27,14 +27,14 @@ static void test_aux(const char *main_prog_name,
>   	opts.kernel_log_size = sizeof(log);
>   	if (env.verbosity >= VERBOSE_SUPER)
>   		opts.kernel_log_level = 1 | 2 | 4;
> -	main = changes_pkt_data__open_opts(&opts);
> -	if (!ASSERT_OK_PTR(main, "changes_pkt_data__open"))
> +	main_data = changes_pkt_data__open_opts(&opts);
> +	if (!ASSERT_OK_PTR(main_data, "changes_pkt_data__open"))
>   		goto out;
> -	main_prog = bpf_object__find_program_by_name(main->obj, main_prog_name);
> +	main_prog = bpf_object__find_program_by_name(main_data->obj, main_prog_name);
>   	if (!ASSERT_OK_PTR(main_prog, "main_prog"))
>   		goto out;
>   	bpf_program__set_autoload(main_prog, true);
> -	err = changes_pkt_data__load(main);
> +	err = changes_pkt_data__load(main_data);
>   	print_verifier_log(log);
>   	if (!ASSERT_OK(err, "changes_pkt_data__load"))
>   		goto out;
> @@ -60,7 +60,7 @@ static void test_aux(const char *main_prog_name,
>   
>   out:
>   	changes_pkt_data_freplace__destroy(freplace);
> -	changes_pkt_data__destroy(main);
> +	changes_pkt_data__destroy(main_data);
>   }
>   
>   /* There are two global subprograms in both changes_pkt_data.skel.h:


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH bpf-next v1 1/2] bpf: changes_pkt_data: correct the 'main' error
  2025-02-07  6:04   ` Martin KaFai Lau
@ 2025-02-07  6:48     ` Jason Xing
  2025-02-11 20:06       ` Martin KaFai Lau
  0 siblings, 1 reply; 12+ messages in thread
From: Jason Xing @ 2025-02-07  6:48 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: ast, daniel, andrii, eddyz87, mykolal, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah, bpf,
	linux-kselftest

On Fri, Feb 7, 2025 at 2:04 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 2/3/25 6:39 PM, Jason Xing wrote:
> > When compiling the selftests, the following error is printed out:
> > selftests/bpf/prog_tests/changes_pkt_data.c: In function ‘test_aux’:
> > selftests/bpf/prog_tests/changes_pkt_data.c:22:27: error: ‘main’ is usually a function [-Werror=main]
> >    struct changes_pkt_data *main = NULL;
>
> The bpf CI has been testing this piece with different compilers. I also don't
> see it in my environment. How to reproduce it and which compiler?

The gcc version is "gcc version 8.5.0" which is an old one. Yep, there
are some cases sometimes where bpf CI doesn't report because the
compilation relies on the system environment? Sorry that I have no
clue about this happening. One thing I know is that if using some old
distros and cloud services, it's not hard to see this.

In this series, rename it to avoid error messages which could stop the
local compilation.

Thanks,
Jason

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH bpf-next v1 0/2] selftests: fix two small compilation errors
  2025-02-04  2:39 [PATCH bpf-next v1 0/2] selftests: fix two small compilation errors Jason Xing
  2025-02-04  2:39 ` [PATCH bpf-next v1 1/2] bpf: changes_pkt_data: correct the 'main' error Jason Xing
  2025-02-04  2:39 ` [PATCH bpf-next v1 2/2] bpf: sockopt_sk: fix 'undeclared' definition error Jason Xing
@ 2025-02-11  7:52 ` Jason Xing
  2 siblings, 0 replies; 12+ messages in thread
From: Jason Xing @ 2025-02-11  7:52 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, mykolal, martin.lau, song,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah
  Cc: bpf, linux-kselftest

Hi Maintainers from bpf,

On Tue, Feb 4, 2025 at 10:39 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> Fix them separately in each patch.

I'd like to know if it's possible to merge the series because it does
harm to some old distro with only updating the kernel?

Thanks,
Jason

>
> Jason Xing (2):
>   bpf: changes_pkt_data: correct the 'main' error
>   bpf: sockopt_sk: fix 'undeclared' definition error
>
>  .../selftests/bpf/prog_tests/changes_pkt_data.c      | 12 ++++++------
>  tools/testing/selftests/bpf/prog_tests/sockopt_sk.c  |  2 +-
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> --
> 2.43.5
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH bpf-next v1 2/2] bpf: sockopt_sk: fix 'undeclared' definition error
  2025-02-05  3:27     ` Jason Xing
  2025-02-05  9:30       ` Hou Tao
@ 2025-02-11 19:40       ` Martin KaFai Lau
  1 sibling, 0 replies; 12+ messages in thread
From: Martin KaFai Lau @ 2025-02-11 19:40 UTC (permalink / raw)
  To: Jason Xing
  Cc: bpf, Hou Tao, linux-kselftest, ast, daniel, andrii, eddyz87,
	mykolal, song, yonghong.song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa, shuah

On 2/4/25 7:27 PM, 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?

afaik, uapi/ or not does not make a difference.

> 
>> 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
>            |        ^~~~~~

e.g. this will happen to the tcphdr also when sockopt_sk.c starts including 
network_helpers.h.

> 
>      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

This patch just undo exactly what this commit a2f482c34a52 tries to solve for 
tcp.h also, no?

pw-bot: cr

>      program part as well, because of some headers sharing between both
>      sides.
> 
> Thanks,
> Jason


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH bpf-next v1 1/2] bpf: changes_pkt_data: correct the 'main' error
  2025-02-07  6:48     ` Jason Xing
@ 2025-02-11 20:06       ` Martin KaFai Lau
  0 siblings, 0 replies; 12+ messages in thread
From: Martin KaFai Lau @ 2025-02-11 20:06 UTC (permalink / raw)
  To: Jason Xing
  Cc: ast, daniel, andrii, eddyz87, mykolal, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah, bpf,
	linux-kselftest

On 2/6/25 10:48 PM, Jason Xing wrote:
> On Fri, Feb 7, 2025 at 2:04 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>>
>> On 2/3/25 6:39 PM, Jason Xing wrote:
>>> When compiling the selftests, the following error is printed out:
>>> selftests/bpf/prog_tests/changes_pkt_data.c: In function ‘test_aux’:
>>> selftests/bpf/prog_tests/changes_pkt_data.c:22:27: error: ‘main’ is usually a function [-Werror=main]
>>>     struct changes_pkt_data *main = NULL;
>>
>> The bpf CI has been testing this piece with different compilers. I also don't
>> see it in my environment. How to reproduce it and which compiler?
> 
> The gcc version is "gcc version 8.5.0" which is an old one. Yep, there

gcc 8.5 is old. I don't even know if it supports compiling the bpf prog in the 
bpf selftests.  Please update the compiler.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-02-11 20:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-04  2:39 [PATCH bpf-next v1 0/2] selftests: fix two small compilation errors Jason Xing
2025-02-04  2:39 ` [PATCH bpf-next v1 1/2] bpf: changes_pkt_data: correct the 'main' error Jason Xing
2025-02-07  6:04   ` Martin KaFai Lau
2025-02-07  6:48     ` Jason Xing
2025-02-11 20:06       ` Martin KaFai Lau
2025-02-04  2:39 ` [PATCH bpf-next v1 2/2] bpf: sockopt_sk: fix 'undeclared' definition error Jason Xing
2025-02-05  2:57   ` Hou Tao
2025-02-05  3:27     ` Jason Xing
2025-02-05  9:30       ` Hou Tao
2025-02-05  9:38         ` Jason Xing
2025-02-11 19:40       ` Martin KaFai Lau
2025-02-11  7:52 ` [PATCH bpf-next v1 0/2] selftests: fix two small compilation errors Jason Xing

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox