[PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Add pm_nl_ctl.
Use SYS_NOFAIL in _ss_search().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/Makefile           |  2 +-
 tools/testing/selftests/bpf/pm_nl_ctl.c        |  1 +
 tools/testing/selftests/bpf/prog_tests/mptcp.c | 15 ++++-----------
 3 files changed, 6 insertions(+), 12 deletions(-)
 create mode 120000 tools/testing/selftests/bpf/pm_nl_ctl.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index b90c718218ae..bd2e3b138a5c 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -131,7 +131,7 @@ TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
 	flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
 	test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
 	xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \
-	xdp_features bpf_test_no_cfi.ko
+	xdp_features bpf_test_no_cfi.ko pm_nl_ctl
 
 TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
 
diff --git a/tools/testing/selftests/bpf/pm_nl_ctl.c b/tools/testing/selftests/bpf/pm_nl_ctl.c
new file mode 120000
index 000000000000..5a08c255b278
--- /dev/null
+++ b/tools/testing/selftests/bpf/pm_nl_ctl.c
@@ -0,0 +1 @@
+../net/mptcp/pm_nl_ctl.c
\ No newline at end of file
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 793b4b9c2bd2..9c6d1e4f6f35 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -362,7 +362,8 @@ static int endpoint_init(char *flags)
 	SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
 	SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST, ADDR_2);
 	SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
-	SYS(fail, "ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags);
+	if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags))
+		SYS(fail, "ip netns exec %s ./pm_nl_ctl add %s flags %s", NS_TEST, ADDR_2, flags);
 
 	return 0;
 fail:
@@ -371,16 +372,8 @@ static int endpoint_init(char *flags)
 
 static int _ss_search(char *src, char *dst, char *port, char *keyword)
 {
-	char cmd[128];
-	int n;
-
-	n = snprintf(cmd, sizeof(cmd),
-		     "ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
-		     NS_TEST, src, dst, port, PORT_1, keyword);
-	if (n < 0 || n >= sizeof(cmd))
-		return -1;
-
-	return system(cmd);
+	return SYS_NOFAIL("ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
+			  NS_TEST, src, dst, port, PORT_1, keyword);
 }
 
 static int ss_search(char *src, char *keyword)
-- 
2.43.0

Re: [PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/9043165180

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/925c86d2d498
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=852516


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Matthieu Baerts]

Hi Geliang,

Thank you for this fix!

11 May 2024 12:42:08 Geliang Tang <geliang@kernel.org>:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Add pm_nl_ctl.
> Use SYS_NOFAIL in _ss_search().
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> tools/testing/selftests/bpf/Makefile           |  2 +-
> tools/testing/selftests/bpf/pm_nl_ctl.c        |  1 +
> tools/testing/selftests/bpf/prog_tests/mptcp.c | 15 ++++-----------
> 3 files changed, 6 insertions(+), 12 deletions(-)
> create mode 120000 tools/testing/selftests/bpf/pm_nl_ctl.c
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index b90c718218ae..bd2e3b138a5c 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -131,7 +131,7 @@ TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
>     flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
>     test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
>     xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \
> -   xdp_features bpf_test_no_cfi.ko
> +   xdp_features bpf_test_no_cfi.ko pm_nl_ctl
>
> TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
>
> diff --git a/tools/testing/selftests/bpf/pm_nl_ctl.c b/tools/testing/selftests/bpf/pm_nl_ctl.c
> new file mode 120000
> index 000000000000..5a08c255b278
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/pm_nl_ctl.c

Best to prefix it with "mptcp_": clearer and it will be linked to MPTCP in the maintainers file.

> @@ -0,0 +1 @@
> +../net/mptcp/pm_nl_ctl.c
> \ No newline at end of file
> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> index 793b4b9c2bd2..9c6d1e4f6f35 100644
> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> @@ -362,7 +362,8 @@ static int endpoint_init(char *flags)
>     SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
>     SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST, ADDR_2);
>     SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
> -   SYS(fail, "ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags);
> +   if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags))

It is maybe better to only use mptcp_pm_nl_ctl here: to maintain one way here, our CI will do the same as what the BPF one will do, and we avoid errors printed in stderr if "ip mptcp" is not supported.

> +       SYS(fail, "ip netns exec %s ./pm_nl_ctl add %s flags %s", NS_TEST, ADDR_2, flags);
>
>     return 0;
> fail:
> @@ -371,16 +372,8 @@ static int endpoint_init(char *flags)
>
> static int _ss_search(char *src, char *dst, char *port, char *keyword)
> {
> -   char cmd[128];
> -   int n;
> -
> -   n = snprintf(cmd, sizeof(cmd),
> -            "ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
> -            NS_TEST, src, dst, port, PORT_1, keyword);
> -   if (n < 0 || n >= sizeof(cmd))
> -       return -1;
> -
> -   return system(cmd);
> +   return SYS_NOFAIL("ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
> +             NS_TEST, src, dst, port, PORT_1, keyword);

If "ip mptcp" is not supported, I guess "ss -M" will not be supported as well, no?

Do we need -M here for these tests?

Cheers,
Matt

> }
>
> static int ss_search(char *src, char *keyword)
> --
> 2.43.0

Re: [PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Geliang Tang]

On Sat, May 11, 2024 at 03:50:29PM +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> Thank you for this fix!
> 
> 11 May 2024 12:42:08 Geliang Tang <geliang@kernel.org>:
> 
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> >
> > Add pm_nl_ctl.
> > Use SYS_NOFAIL in _ss_search().
> >
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> > tools/testing/selftests/bpf/Makefile           |  2 +-
> > tools/testing/selftests/bpf/pm_nl_ctl.c        |  1 +
> > tools/testing/selftests/bpf/prog_tests/mptcp.c | 15 ++++-----------
> > 3 files changed, 6 insertions(+), 12 deletions(-)
> > create mode 120000 tools/testing/selftests/bpf/pm_nl_ctl.c
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index b90c718218ae..bd2e3b138a5c 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -131,7 +131,7 @@ TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
> >     flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
> >     test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
> >     xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \
> > -   xdp_features bpf_test_no_cfi.ko
> > +   xdp_features bpf_test_no_cfi.ko pm_nl_ctl
> >
> > TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
> >
> > diff --git a/tools/testing/selftests/bpf/pm_nl_ctl.c b/tools/testing/selftests/bpf/pm_nl_ctl.c
> > new file mode 120000
> > index 000000000000..5a08c255b278
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/pm_nl_ctl.c
> 
> Best to prefix it with "mptcp_": clearer and it will be linked to MPTCP in the maintainers file.

mptcp_ prefix has already added in v2. Please update the maintainers
file when merging this.

> 
> > @@ -0,0 +1 @@
> > +../net/mptcp/pm_nl_ctl.c
> > \ No newline at end of file
> > diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > index 793b4b9c2bd2..9c6d1e4f6f35 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > @@ -362,7 +362,8 @@ static int endpoint_init(char *flags)
> >     SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
> >     SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST, ADDR_2);
> >     SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
> > -   SYS(fail, "ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags);
> > +   if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags))
> 
> It is maybe better to only use mptcp_pm_nl_ctl here: to maintain one way here, our CI will do the same as what the BPF one will do, and we avoid errors printed in stderr if "ip mptcp" is not supported.
>

No, I prefer this one. "ip mptcp" should be used here since it's
the normal way to set mptcp. pm_nl_ctl is just a work-around for it.

No error printed in stderr if "ip mptcp" is not supported since
SYS_NOFAIL is used here, not SYS. I have tested this case already.

> > +       SYS(fail, "ip netns exec %s ./pm_nl_ctl add %s flags %s", NS_TEST, ADDR_2, flags);
> >
> >     return 0;
> > fail:
> > @@ -371,16 +372,8 @@ static int endpoint_init(char *flags)
> >
> > static int _ss_search(char *src, char *dst, char *port, char *keyword)
> > {
> > -   char cmd[128];
> > -   int n;
> > -
> > -   n = snprintf(cmd, sizeof(cmd),
> > -            "ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
> > -            NS_TEST, src, dst, port, PORT_1, keyword);
> > -   if (n < 0 || n >= sizeof(cmd))
> > -       return -1;
> > -
> > -   return system(cmd);
> > +   return SYS_NOFAIL("ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
> > +             NS_TEST, src, dst, port, PORT_1, keyword);
> 
> If "ip mptcp" is not supported, I guess "ss -M" will not be supported as well, no?
> 
> Do we need -M here for these tests?

No need to do this yet until we actually encounter it.

Regards,
Geliang

> 
> Cheers,
> Matt
> 
> > }
> >
> > static int ss_search(char *src, char *keyword)
> > --
> > 2.43.0

Re: [PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Matthieu Baerts]

Hi Geliang,

12 May 2024 01:17:13 Geliang Tang <geliang@kernel.org>:
> On Sat, May 11, 2024 at 03:50:29PM +0200, Matthieu Baerts wrote:
>> 11 May 2024 12:42:08 Geliang Tang <geliang@kernel.org>:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>

(...)

>>> @@ -0,0 +1 @@
>>> +../net/mptcp/pm_nl_ctl.c
>>> \ No newline at end of file
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>> index 793b4b9c2bd2..9c6d1e4f6f35 100644
>>> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>> @@ -362,7 +362,8 @@ static int endpoint_init(char *flags)
>>>     SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
>>>     SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST, ADDR_2);
>>>     SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
>>> -   SYS(fail, "ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags);
>>> +   if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags))
>>
>> It is maybe better to only use mptcp_pm_nl_ctl here: to maintain one way here, our CI will do the same as what the BPF one will do, and we avoid errors printed in stderr if "ip mptcp" is not supported.
>>
>
> No, I prefer this one. "ip mptcp" should be used here since it's
> the normal way to set mptcp. pm_nl_ctl is just a work-around for it.
>
> No error printed in stderr if "ip mptcp" is not supported since
> SYS_NOFAIL is used here, not SYS. I have tested this case already.

SYS_NOFAIL doesn't redirect stderr to /dev/null, does it?
If "ip mptcp" is not supported and a fatal error happens, will you not see
in the logs "ip mptcp is not supported" as well, creating confusions?

I would also prefer to use "ip mptcp" if available, but my main reason to
use only the workaround, is: if we change the syntax of pm_nl_ctl, not
realising we have to modify the code here as well, our CI will not
complain, but BPF CI will later, likely when validating something else.
We want our CI to catch such issues before upstreaming patches.

Why not adding a comment instead?

  /* Equivalent of: "ip -net %s mptcp endpoint add %s %s" */

So people interested in knowing how to do that the "proper" way will see
what to do, no?

>
>>> +       SYS(fail, "ip netns exec %s ./pm_nl_ctl add %s flags %s", NS_TEST, ADDR_2, flags);
>>>
>>>     return 0;
>>> fail:
>>> @@ -371,16 +372,8 @@ static int endpoint_init(char *flags)
>>>
>>> static int _ss_search(char *src, char *dst, char *port, char *keyword)
>>> {
>>> -   char cmd[128];
>>> -   int n;
>>> -
>>> -   n = snprintf(cmd, sizeof(cmd),
>>> -            "ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
>>> -            NS_TEST, src, dst, port, PORT_1, keyword);
>>> -   if (n < 0 || n >= sizeof(cmd))
>>> -       return -1;
>>> -
>>> -   return system(cmd);
>>> +   return SYS_NOFAIL("ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
>>> +             NS_TEST, src, dst, port, PORT_1, keyword);
>>
>> If "ip mptcp" is not supported, I guess "ss -M" will not be supported as well, no?
>>
>> Do we need -M here for these tests?
>
> No need to do this yet until we actually encounter it.

"-e" is not needed as well I think. Don't hesitate to remove it as well.

Re: [PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Geliang Tang]

On Sun, May 12, 2024 at 11:14:38AM +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> 12 May 2024 01:17:13 Geliang Tang <geliang@kernel.org>:
> > On Sat, May 11, 2024 at 03:50:29PM +0200, Matthieu Baerts wrote:
> >> 11 May 2024 12:42:08 Geliang Tang <geliang@kernel.org>:
> >>> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> (...)
> 
> >>> @@ -0,0 +1 @@
> >>> +../net/mptcp/pm_nl_ctl.c
> >>> \ No newline at end of file
> >>> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> >>> index 793b4b9c2bd2..9c6d1e4f6f35 100644
> >>> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> >>> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> >>> @@ -362,7 +362,8 @@ static int endpoint_init(char *flags)
> >>>     SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
> >>>     SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST, ADDR_2);
> >>>     SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
> >>> -   SYS(fail, "ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags);
> >>> +   if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags))
> >>
> >> It is maybe better to only use mptcp_pm_nl_ctl here: to maintain one way here, our CI will do the same as what the BPF one will do, and we avoid errors printed in stderr if "ip mptcp" is not supported.
> >>
> >
> > No, I prefer this one. "ip mptcp" should be used here since it's
> > the normal way to set mptcp. pm_nl_ctl is just a work-around for it.
> >
> > No error printed in stderr if "ip mptcp" is not supported since
> > SYS_NOFAIL is used here, not SYS. I have tested this case already.
> 
> SYS_NOFAIL doesn't redirect stderr to /dev/null, does it?
> If "ip mptcp" is not supported and a fatal error happens, will you not see
> in the logs "ip mptcp is not supported" as well, creating confusions?
> 
> I would also prefer to use "ip mptcp" if available, but my main reason to
> use only the workaround, is: if we change the syntax of pm_nl_ctl, not
> realising we have to modify the code here as well, our CI will not
> complain, but BPF CI will later, likely when validating something else.
> We want our CI to catch such issues before upstreaming patches.
> 
> Why not adding a comment instead?
> 
>   /* Equivalent of: "ip -net %s mptcp endpoint add %s %s" */
> 
> So people interested in knowing how to do that the "proper" way will see
> what to do, no?

SYS_NOFAIL is defined in test_progs.h:

388 #define ALL_TO_DEV_NULL " >/dev/null 2>&1"
389
390 #define SYS_NOFAIL(fmt, ...)                                            \
391         ({                                                              \
392                 char cmd[1024];                                         \
393                 int n;                                                  \
394                 n = snprintf(cmd, sizeof(cmd), fmt, ##__VA_ARGS__);     \
395                 if (n < sizeof(cmd) && sizeof(cmd) - n >= sizeof(ALL_TO_DEV_NULL)) \
396                         strcat(cmd, ALL_TO_DEV_NULL);                   \
397                 system(cmd);                                            \
398         })

See ALL_TO_DEV_NULL here.

> 
> >
> >>> +       SYS(fail, "ip netns exec %s ./pm_nl_ctl add %s flags %s", NS_TEST, ADDR_2, flags);
> >>>
> >>>     return 0;
> >>> fail:
> >>> @@ -371,16 +372,8 @@ static int endpoint_init(char *flags)
> >>>
> >>> static int _ss_search(char *src, char *dst, char *port, char *keyword)
> >>> {
> >>> -   char cmd[128];
> >>> -   int n;
> >>> -
> >>> -   n = snprintf(cmd, sizeof(cmd),
> >>> -            "ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
> >>> -            NS_TEST, src, dst, port, PORT_1, keyword);
> >>> -   if (n < 0 || n >= sizeof(cmd))
> >>> -       return -1;
> >>> -
> >>> -   return system(cmd);
> >>> +   return SYS_NOFAIL("ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
> >>> +             NS_TEST, src, dst, port, PORT_1, keyword);
> >>
> >> If "ip mptcp" is not supported, I guess "ss -M" will not be supported as well, no?
> >>
> >> Do we need -M here for these tests?
> >
> > No need to do this yet until we actually encounter it.
> 
> "-e" is not needed as well I think. Don't hesitate to remove it as well.

"-e" is needed by has_bytes_sent:

458 static int has_bytes_sent(char *dst)
459 {
460         return _ss_search(ADDR_1, dst, "sport", "bytes_sent:");
461 }

BPF sched tests fail if remove it.

Regards,
Geliang

Re: [PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Matthieu Baerts]

12 May 2024 13:45:15 Geliang Tang <geliang@kernel.org>:

> On Sun, May 12, 2024 at 11:14:38AM +0200, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> 12 May 2024 01:17:13 Geliang Tang <geliang@kernel.org>:
>>> On Sat, May 11, 2024 at 03:50:29PM +0200, Matthieu Baerts wrote:
>>>> 11 May 2024 12:42:08 Geliang Tang <geliang@kernel.org>:
>>>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> (...)
>>
>>>>> @@ -0,0 +1 @@
>>>>> +../net/mptcp/pm_nl_ctl.c
>>>>> \ No newline at end of file
>>>>> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>>>> index 793b4b9c2bd2..9c6d1e4f6f35 100644
>>>>> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>>>> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>>>> @@ -362,7 +362,8 @@ static int endpoint_init(char *flags)
>>>>>     SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
>>>>>     SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST, ADDR_2);
>>>>>     SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
>>>>> -   SYS(fail, "ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags);
>>>>> +   if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags))
>>>>
>>>> It is maybe better to only use mptcp_pm_nl_ctl here: to maintain one way here, our CI will do the same as what the BPF one will do, and we avoid errors printed in stderr if "ip mptcp" is not supported.
>>>>
>>>
>>> No, I prefer this one. "ip mptcp" should be used here since it's
>>> the normal way to set mptcp. pm_nl_ctl is just a work-around for it.
>>>
>>> No error printed in stderr if "ip mptcp" is not supported since
>>> SYS_NOFAIL is used here, not SYS. I have tested this case already.
>>
>> SYS_NOFAIL doesn't redirect stderr to /dev/null, does it?
>> If "ip mptcp" is not supported and a fatal error happens, will you not see
>> in the logs "ip mptcp is not supported" as well, creating confusions?
>>
>> I would also prefer to use "ip mptcp" if available, but my main reason to
>> use only the workaround, is: if we change the syntax of pm_nl_ctl, not
>> realising we have to modify the code here as well, our CI will not
>> complain, but BPF CI will later, likely when validating something else.
>> We want our CI to catch such issues before upstreaming patches.
>>
>> Why not adding a comment instead?
>>
>>   /* Equivalent of: "ip -net %s mptcp endpoint add %s %s" */
>>
>> So people interested in knowing how to do that the "proper" way will see
>> what to do, no?
>
> SYS_NOFAIL is defined in test_progs.h:
>
> 388 #define ALL_TO_DEV_NULL " >/dev/null 2>&1"
> 389
> 390 #define SYS_NOFAIL(fmt, ...)                                            \
> 391         ({                                                              \
> 392                 char cmd[1024];                                         \
> 393                 int n;                                                  \
> 394                 n = snprintf(cmd, sizeof(cmd), fmt, ##__VA_ARGS__);     \
> 395                 if (n < sizeof(cmd) && sizeof(cmd) - n >= sizeof(ALL_TO_DEV_NULL)) \
> 396                         strcat(cmd, ALL_TO_DEV_NULL);                   \
> 397                 system(cmd);                                            \
> 398         })
>
> See ALL_TO_DEV_NULL here.

My bad, I was looking at the code from v6.8.

But still, I think it would be better to avoid sending patches upstream
and potentially breaking stuff on BPF side because we didn't test
the same way as what is done on their side, no?

>
>>
>>>
>>>>> +       SYS(fail, "ip netns exec %s ./pm_nl_ctl add %s flags %s", NS_TEST, ADDR_2, flags);
>>>>>
>>>>>     return 0;
>>>>> fail:
>>>>> @@ -371,16 +372,8 @@ static int endpoint_init(char *flags)
>>>>>
>>>>> static int _ss_search(char *src, char *dst, char *port, char *keyword)
>>>>> {
>>>>> -   char cmd[128];
>>>>> -   int n;
>>>>> -
>>>>> -   n = snprintf(cmd, sizeof(cmd),
>>>>> -            "ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
>>>>> -            NS_TEST, src, dst, port, PORT_1, keyword);
>>>>> -   if (n < 0 || n >= sizeof(cmd))
>>>>> -       return -1;
>>>>> -
>>>>> -   return system(cmd);
>>>>> +   return SYS_NOFAIL("ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
>>>>> +             NS_TEST, src, dst, port, PORT_1, keyword);
>>>>
>>>> If "ip mptcp" is not supported, I guess "ss -M" will not be supported as well, no?
>>>>
>>>> Do we need -M here for these tests?
>>>
>>> No need to do this yet until we actually encounter it.
>>
>> "-e" is not needed as well I think. Don't hesitate to remove it as well.
>
> "-e" is needed by has_bytes_sent:
>
> 458 static int has_bytes_sent(char *dst)
> 459 {
> 460         return _ss_search(ADDR_1, dst, "sport", "bytes_sent:");
> 461 }
>
> BPF sched tests fail if remove it.

Thank you for having checked! It looks like ss' man page is missing
this case.

Cheers,
Matt

[PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Skip the test with test__skip() for systems that do not
support "ip mptcp", so that CI can also pass.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/prog_tests/mptcp.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 00f63f3f19f4..ddef8c61360f 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -357,10 +357,11 @@ static int endpoint_init(char *flags)
 	SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
 	SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST, ADDR_2);
 	SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
-	/* It would be better to use  "ip -net %s mptcp endpoint add %s %s",
-	 * but the BPF CI is using an old version of IPRoute (5.5.0).
-	 */
-	SYS(fail, "ip netns exec %s ./mptcp_pm_nl_ctl add %s flags %s", NS_TEST, ADDR_2, flags);
+	if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags)) {
+		/* "ip mptcp" not support, skip this test. */
+		test__skip();
+		goto fail;
+	}
 
 	return 0;
 fail:
@@ -435,7 +436,7 @@ static void test_subflow(void)
 	if (!ASSERT_OK_PTR(nstoken, "create_netns: mptcp_subflow"))
 		goto skel_destroy;
 
-	if (!ASSERT_OK(endpoint_init("subflow"), "endpoint_init"))
+	if (endpoint_init("subflow"))
 		goto close_netns;
 
 	run_subflow(skel->data->cc);
-- 
2.43.0

Re: [PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/10086967692

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/4340d56e5f7c
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=873699


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Matthieu Baerts]

Hi Geliang,

Thank you for the patch!

On 25/07/2024 03:58, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Skip the test with test__skip() for systems that do not
> support "ip mptcp", so that CI can also pass.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  tools/testing/selftests/bpf/prog_tests/mptcp.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> index 00f63f3f19f4..ddef8c61360f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> @@ -357,10 +357,11 @@ static int endpoint_init(char *flags)
>  	SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
>  	SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST, ADDR_2);
>  	SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
> -	/* It would be better to use  "ip -net %s mptcp endpoint add %s %s",
> -	 * but the BPF CI is using an old version of IPRoute (5.5.0).
> -	 */
> -	SYS(fail, "ip netns exec %s ./mptcp_pm_nl_ctl add %s flags %s", NS_TEST, ADDR_2, flags);
> +	if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags)) {

This will silently mark the test as skipped, because stderr will be muted.

> +		/* "ip mptcp" not support, skip this test. */

Could it be possible to at least print this comment? I can add:

  fprintf(stderr, "'ip mptcp' not supported, skip this test.\n");

when applying the patch. WDYT?

> +		test__skip();
> +		goto fail;
> +	}
>  
>  	return 0;
>  fail:
> @@ -435,7 +436,7 @@ static void test_subflow(void)
>  	if (!ASSERT_OK_PTR(nstoken, "create_netns: mptcp_subflow"))
>  		goto skel_destroy;
>  
> -	if (!ASSERT_OK(endpoint_init("subflow"), "endpoint_init"))
> +	if (endpoint_init("subflow"))
>  		goto close_netns;
>  
>  	run_subflow(skel->data->cc);

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Geliang Tang]

Hi Matt,

On Fri, 2024-07-26 at 11:18 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> Thank you for the patch!
> 
> On 25/07/2024 03:58, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > Skip the test with test__skip() for systems that do not
> > support "ip mptcp", so that CI can also pass.
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >  tools/testing/selftests/bpf/prog_tests/mptcp.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > index 00f63f3f19f4..ddef8c61360f 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > @@ -357,10 +357,11 @@ static int endpoint_init(char *flags)
> >  	SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
> >  	SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST,
> > ADDR_2);
> >  	SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
> > -	/* It would be better to use  "ip -net %s mptcp endpoint
> > add %s %s",
> > -	 * but the BPF CI is using an old version of IPRoute
> > (5.5.0).
> > -	 */
> > -	SYS(fail, "ip netns exec %s ./mptcp_pm_nl_ctl add %s flags
> > %s", NS_TEST, ADDR_2, flags);
> > +	if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s",
> > NS_TEST, ADDR_2, flags)) {
> 
> This will silently mark the test as skipped, because stderr will be
> muted.
> 
> > +		/* "ip mptcp" not support, skip this test. */
> 
> Could it be possible to at least print this comment? I can add:
> 
>   fprintf(stderr, "'ip mptcp' not supported, skip this test.\n");
> 
> when applying the patch. WDYT?

Martin, BPF maintainer, replied on "[v3 2/3] selftests/bpf: Add mptcp
pm_nl_ctl link" recently to suggest us to send a v4 of it, if then, no
need to apply this squash-to patch at this moment. Please send a v4 to
continue using mptcp pm_nl_ctl.

Sorry for the confusion caused.

Thanks,
-Geliang

> 
> > +		test__skip();
> > +		goto fail;
> > +	}
> >  
> >  	return 0;
> >  fail:
> > @@ -435,7 +436,7 @@ static void test_subflow(void)
> >  	if (!ASSERT_OK_PTR(nstoken, "create_netns:
> > mptcp_subflow"))
> >  		goto skel_destroy;
> >  
> > -	if (!ASSERT_OK(endpoint_init("subflow"), "endpoint_init"))
> > +	if (endpoint_init("subflow"))
> >  		goto close_netns;
> >  
> >  	run_subflow(skel->data->cc);
> 
> Cheers,
> Matt

Re: [PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Matthieu Baerts]

Hi Geliang,

On 27/07/2024 03:12, Geliang Tang wrote:
> Hi Matt,
> 
> On Fri, 2024-07-26 at 11:18 +0200, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> Thank you for the patch!
>>
>> On 25/07/2024 03:58, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> Skip the test with test__skip() for systems that do not
>>> support "ip mptcp", so that CI can also pass.
>>>
>>> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
>>> ---
>>>  tools/testing/selftests/bpf/prog_tests/mptcp.c | 11 ++++++-----
>>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>> b/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>> index 00f63f3f19f4..ddef8c61360f 100644
>>> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>> @@ -357,10 +357,11 @@ static int endpoint_init(char *flags)
>>>  	SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
>>>  	SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST,
>>> ADDR_2);
>>>  	SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
>>> -	/* It would be better to use  "ip -net %s mptcp endpoint
>>> add %s %s",
>>> -	 * but the BPF CI is using an old version of IPRoute
>>> (5.5.0).
>>> -	 */
>>> -	SYS(fail, "ip netns exec %s ./mptcp_pm_nl_ctl add %s flags
>>> %s", NS_TEST, ADDR_2, flags);
>>> +	if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s",
>>> NS_TEST, ADDR_2, flags)) {
>>
>> This will silently mark the test as skipped, because stderr will be
>> muted.
>>
>>> +		/* "ip mptcp" not support, skip this test. */
>>
>> Could it be possible to at least print this comment? I can add:
>>
>>   fprintf(stderr, "'ip mptcp' not supported, skip this test.\n");
>>
>> when applying the patch. WDYT?
> 
> Martin, BPF maintainer, replied on "[v3 2/3] selftests/bpf: Add mptcp
> pm_nl_ctl link" recently to suggest us to send a v4 of it, if then, no
> need to apply this squash-to patch at this moment. Please send a v4 to
> continue using mptcp pm_nl_ctl.

Thank you for this note. The modifications I suggested might require
some changes of their CI. I think it should not, but I need to check. In
other words, it might take some time.

Would it be OK to apply your patch (with the fprintf()) to remove the
dependency on mptcp_pm_nl_ctl, and send this first? After or in
parallel, I can check what I suggested before. If this modification is
accepted, and their CI is still using an old IPRoute version, then we
can (re-)add mptcp_pm_nl_ctl. WDYT?

> 
> Sorry for the confusion caused.
> 
> Thanks,
> -Geliang
> 
>>
>>> +		test__skip();
>>> +		goto fail;
>>> +	}
>>>  
>>>  	return 0;
>>>  fail:
>>> @@ -435,7 +436,7 @@ static void test_subflow(void)
>>>  	if (!ASSERT_OK_PTR(nstoken, "create_netns:
>>> mptcp_subflow"))
>>>  		goto skel_destroy;
>>>  
>>> -	if (!ASSERT_OK(endpoint_init("subflow"), "endpoint_init"))
>>> +	if (endpoint_init("subflow"))
>>>  		goto close_netns;
>>>  
>>>  	run_subflow(skel->data->cc);
>>
>> Cheers,
>> Matt
> 

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

[PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Should use dedicated ASSERT_OK_FD() to check the fds.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/prog_tests/mptcp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 93869c873cad..987692863fde 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -394,11 +394,11 @@ static void run_subflow(void)
 	socklen_t len;
 
 	server_fd = start_mptcp_server(AF_INET, ADDR_1, PORT_1, 0);
-	if (!ASSERT_GE(server_fd, 0, "start_mptcp_server"))
+	if (!ASSERT_OK_FD(server_fd, "start_mptcp_server"))
 		return;
 
 	client_fd = connect_to_fd(server_fd, 0);
-	if (!ASSERT_GE(client_fd, 0, "connect to fd"))
+	if (!ASSERT_OK_FD(client_fd, "connect_to_fd"))
 		goto close_server;
 
 	send_byte(client_fd);
@@ -429,7 +429,7 @@ static void test_subflow(void)
 	struct bpf_link *link;
 
 	cgroup_fd = test__join_cgroup("/mptcp_subflow");
-	if (!ASSERT_GE(cgroup_fd, 0, "join_cgroup: mptcp_subflow"))
+	if (!ASSERT_OK_FD(cgroup_fd, "join_cgroup: mptcp_subflow"))
 		return;
 
 	skel = mptcp_subflow__open_and_load();
-- 
2.43.0

Re: [PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/10748445039

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/0145299d2c69
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=887967


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Geliang Tang]

Superseded. Move into "cleanups for bpf schedulers" set v2.

Thanks,
-Geliang

On Sat, 2024-09-07 at 11:45 +0800, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Should use dedicated ASSERT_OK_FD() to check the fds.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  tools/testing/selftests/bpf/prog_tests/mptcp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> index 93869c873cad..987692863fde 100644
> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> @@ -394,11 +394,11 @@ static void run_subflow(void)
>  	socklen_t len;
>  
>  	server_fd = start_mptcp_server(AF_INET, ADDR_1, PORT_1, 0);
> -	if (!ASSERT_GE(server_fd, 0, "start_mptcp_server"))
> +	if (!ASSERT_OK_FD(server_fd, "start_mptcp_server"))
>  		return;
>  
>  	client_fd = connect_to_fd(server_fd, 0);
> -	if (!ASSERT_GE(client_fd, 0, "connect to fd"))
> +	if (!ASSERT_OK_FD(client_fd, "connect_to_fd"))
>  		goto close_server;
>  
>  	send_byte(client_fd);
> @@ -429,7 +429,7 @@ static void test_subflow(void)
>  	struct bpf_link *link;
>  
>  	cgroup_fd = test__join_cgroup("/mptcp_subflow");
> -	if (!ASSERT_GE(cgroup_fd, 0, "join_cgroup: mptcp_subflow"))
> +	if (!ASSERT_OK_FD(cgroup_fd, "join_cgroup: mptcp_subflow"))
>  		return;
>  
>  	skel = mptcp_subflow__open_and_load();

[PATCH mptcp-next] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Matthieu Baerts (NGI0)]

Fix sleep() usage: sleep() takes an integer, not a float/double.

  tools/testing/selftests/bpf/prog_tests/mptcp.c:373:9: error: implicit conversion from 'double' to 'unsigned int' changes value from 0.1 to 0 [-Werror,-Wliteral-conversion]
    373 |                 sleep(0.1);
        |                 ~~~~~ ^~~

Use usleep() instead.

Link: https://github.com/kernel-patches/bpf/actions/runs/10794529238/job/29939101033
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/bpf/prog_tests/mptcp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index c30f032edaca..c76a0d8c8f93 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -5,6 +5,7 @@
 #include <linux/const.h>
 #include <netinet/in.h>
 #include <test_progs.h>
+#include <unistd.h>
 #include "cgroup_helpers.h"
 #include "network_helpers.h"
 #include "mptcp_sock.skel.h"
@@ -370,7 +371,7 @@ static void wait_for_new_subflows(int fd)
 		if (!err && subflows > 0)
 			break;
 
-		sleep(0.1);
+		usleep(100000); /* 0.1s */
 	}
 }
 
-- 
2.45.2