[PATCH mptcp-next 0/3] selftests: mptcp: fixes for issues seen on v5.15.x

[PATCH mptcp-next 0/3] selftests: mptcp: fixes for issues seen on v5.15.x

[Matthieu Baerts (NGI0)]

When testing the last version of the MPTCP kselftests on the v5.15.148
kernel, I noticed a few small issues. Nothing important, just some error
messages printed in the console and some tests marked as "OK" instead of
"SKIP".

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (3):
      selftests: mptcp: pm nl: also list skipped tests
      selftests: mptcp: pm nl: avoid error msg on older kernels
      selftests: mptcp: diag: fix bash warnings on older kernels

 tools/testing/selftests/net/mptcp/diag.sh       | 4 ++--
 tools/testing/selftests/net/mptcp/pm_netlink.sh | 8 +++++++-
 2 files changed, 9 insertions(+), 3 deletions(-)
---
base-commit: c2469b38e369c3f2b9577beeb9470cc757abc1b9
change-id: 20240207-selftests-mptcp-fix-5-15-8ca479089520

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

[PATCH mptcp-next 1/3] selftests: mptcp: pm nl: also list skipped tests

[Matthieu Baerts (NGI0)]

If the feature is not supported by older kernels, and instead of just
ignoring some tests, we should mark them as skipped, so we can still
track them.

Fixes: d85555ac11f9 ("selftests: mptcp: pm_netlink: format subtests results in TAP")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/pm_netlink.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index 8f4ff123a7eb..79e83a2c95de 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -194,6 +194,12 @@ subflow 10.0.1.1" "          (nofullmesh)"
 	ip netns exec $ns1 ./pm_nl_ctl set id 1 flags backup,fullmesh
 	check "ip netns exec $ns1 ./pm_nl_ctl dump" "id 1 flags \
 subflow,backup,fullmesh 10.0.1.1" "          (backup,fullmesh)"
+else
+	for st in fullmesh nofullmesh backup,fullmesh; do
+		st="          (${st})"
+		printf "%-50s%s\n" "${st}" "[SKIP]"
+		mptcp_lib_result_skip "${st}"
+	done
 fi
 
 mptcp_lib_result_print_all_tap

-- 
2.43.0

Re: [PATCH mptcp-next 1/3] selftests: mptcp: pm nl: also list skipped tests

[Geliang Tang]

Hi Matt,

Matthieu Baerts (NGI0) <matttbe@kernel.org> 于2024年2月8日周四 02:18写道：
>
> If the feature is not supported by older kernels, and instead of just
> ignoring some tests, we should mark them as skipped, so we can still
> track them.
>
> Fixes: d85555ac11f9 ("selftests: mptcp: pm_netlink: format subtests results in TAP")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/pm_netlink.sh | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
> index 8f4ff123a7eb..79e83a2c95de 100755
> --- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
> +++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
> @@ -194,6 +194,12 @@ subflow 10.0.1.1" "          (nofullmesh)"
>         ip netns exec $ns1 ./pm_nl_ctl set id 1 flags backup,fullmesh
>         check "ip netns exec $ns1 ./pm_nl_ctl dump" "id 1 flags \
>  subflow,backup,fullmesh 10.0.1.1" "          (backup,fullmesh)"
> +else
> +       for st in fullmesh nofullmesh backup,fullmesh; do
> +               st="          (${st})"
> +               printf "%-50s%s\n" "${st}" "[SKIP]"

I prefer to use "[ SKIP ]" here. Otherwise, this series looks good!

Reviewed-by: Geliang Tang <geliang@kernel.org>

Thanks,
-Geliang

> +               mptcp_lib_result_skip "${st}"
> +       done
>  fi
>
>  mptcp_lib_result_print_all_tap
>
> --
> 2.43.0
>
>

Re: [PATCH mptcp-next 1/3] selftests: mptcp: pm nl: also list skipped tests

[Matthieu Baerts]

Hi Geliang,

Thank you for the review!

On 13/02/2024 14:35, Geliang Tang wrote:
> Hi Matt,
> 
> Matthieu Baerts (NGI0) <matttbe@kernel.org> 于2024年2月8日周四 02:18写道：
>>
>> If the feature is not supported by older kernels, and instead of just
>> ignoring some tests, we should mark them as skipped, so we can still
>> track them.
>>
>> Fixes: d85555ac11f9 ("selftests: mptcp: pm_netlink: format subtests results in TAP")
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  tools/testing/selftests/net/mptcp/pm_netlink.sh | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
>> index 8f4ff123a7eb..79e83a2c95de 100755
>> --- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
>> +++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
>> @@ -194,6 +194,12 @@ subflow 10.0.1.1" "          (nofullmesh)"
>>         ip netns exec $ns1 ./pm_nl_ctl set id 1 flags backup,fullmesh
>>         check "ip netns exec $ns1 ./pm_nl_ctl dump" "id 1 flags \
>>  subflow,backup,fullmesh 10.0.1.1" "          (backup,fullmesh)"
>> +else
>> +       for st in fullmesh nofullmesh backup,fullmesh; do
>> +               st="          (${st})"
>> +               printf "%-50s%s\n" "${st}" "[SKIP]"
> 
> I prefer to use "[ SKIP ]" here.

I would prefer too, but in this file, we are currently using:

  [ OK ]
  [FAIL]

I think it is then better to continue using 4 letters between the [], no?

  [ OK ]
  [FAIL]
  [SKIP]

If we want to change the format, and even add colours, we can do that in
net-next, not as part of a fix for -net.

> Otherwise, this series looks good!
>
> Reviewed-by: Geliang Tang <geliang@kernel.org>

If you don't mind, when a whole series is OK for you, can you share your
"Reviewed-By" tag on the cover-letter please? By doing that, your tag
will be "propagated" to each patch. While here, 'b4' will see it as only
valid for patch 1/3, but not the rest of the series.

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

Re: [PATCH mptcp-next 1/3] selftests: mptcp: pm nl: also list skipped tests

[Matthieu Baerts]

Hi Geliang,

On 13/02/2024 15:59, Matthieu Baerts wrote:
> Hi Geliang,
> 
> Thank you for the review!
> 
> On 13/02/2024 14:35, Geliang Tang wrote:
>> Hi Matt,
>>
>> Matthieu Baerts (NGI0) <matttbe@kernel.org> 于2024年2月8日周四 02:18写道：
>>>
>>> If the feature is not supported by older kernels, and instead of just
>>> ignoring some tests, we should mark them as skipped, so we can still
>>> track them.
>>>
>>> Fixes: d85555ac11f9 ("selftests: mptcp: pm_netlink: format subtests results in TAP")
>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>> ---
>>>  tools/testing/selftests/net/mptcp/pm_netlink.sh | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
>>> index 8f4ff123a7eb..79e83a2c95de 100755
>>> --- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
>>> +++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
>>> @@ -194,6 +194,12 @@ subflow 10.0.1.1" "          (nofullmesh)"
>>>         ip netns exec $ns1 ./pm_nl_ctl set id 1 flags backup,fullmesh
>>>         check "ip netns exec $ns1 ./pm_nl_ctl dump" "id 1 flags \
>>>  subflow,backup,fullmesh 10.0.1.1" "          (backup,fullmesh)"
>>> +else
>>> +       for st in fullmesh nofullmesh backup,fullmesh; do
>>> +               st="          (${st})"
>>> +               printf "%-50s%s\n" "${st}" "[SKIP]"
>>
>> I prefer to use "[ SKIP ]" here.
> 
> I would prefer too, but in this file, we are currently using:
> 
>   [ OK ]
>   [FAIL]
> 
> I think it is then better to continue using 4 letters between the [], no?
> 
>   [ OK ]
>   [FAIL]
>   [SKIP]
> 
> If we want to change the format, and even add colours, we can do that in
> net-next, not as part of a fix for -net.

I hope that's OK: I just applied this series, with your RvB tag, in our
tree for -net. As always, I can always apply squash-to patches if
needed, if that was not OK for you (or anyone else).

New patches for t/upstream-net and t/upstream:
- 0d8688d3e27b: selftests: mptcp: pm nl: also list skipped tests
- 559d098b600a: selftests: mptcp: pm nl: avoid error msg on older kernels
- dc7dc31c08fa: selftests: mptcp: diag: fix bash warnings on older kernels
- Results: 65bddb3d48dc..4a5e7bd9b1b6 (export-net)
- Results: e1e18543311d..5051f12eb83c (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20240213T150358
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20240213T150358

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

[PATCH mptcp-next 2/3] selftests: mptcp: pm nl: avoid error msg on older kernels

[Matthieu Baerts (NGI0)]

Since the 'Fixes' commit mentioned below, and if the kernel being tested
doesn't support the 'fullmesh' flag, this error will be printed:

  netlink error -22 (Invalid argument)
  ./pm_nl_ctl: bailing out due to netlink error[s]

But that can be normal if the kernel doesn't support the feature, no
need to print this worrying error message while everything else looks
OK. So we can mute stderr. Failures will still be detected if any.

Fixes: 1dc88d241f92 ("selftests: mptcp: pm_nl_ctl: always look for errors")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/pm_netlink.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index 79e83a2c95de..71899a3ffa7a 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -183,7 +183,7 @@ check "ip netns exec $ns1 ./pm_nl_ctl dump" "id 1 flags \
 subflow 10.0.1.1" "          (nobackup)"
 
 # fullmesh support has been added later
-ip netns exec $ns1 ./pm_nl_ctl set id 1 flags fullmesh
+ip netns exec $ns1 ./pm_nl_ctl set id 1 flags fullmesh 2>/dev/null
 if ip netns exec $ns1 ./pm_nl_ctl dump | grep -q "fullmesh" ||
    mptcp_lib_expect_all_features; then
 	check "ip netns exec $ns1 ./pm_nl_ctl dump" "id 1 flags \

-- 
2.43.0

[PATCH mptcp-next 3/3] selftests: mptcp: diag: fix bash warnings on older kernels

[Matthieu Baerts (NGI0)]

Since the 'Fixes' commit mentioned below, the command that is executed
in __chk_nr() helper can return nothing if the feature is not supported.
This is the case when the MPTCP CURRESTAB counter is not supported.

To avoid this warning ...

  ./diag.sh: line 65: [: !=: unary operator expected

... we just need to surround '$nr' with double quotes, to support an
empty string when the feature is not supported.

Fixes: 81ab772819da ("selftests: mptcp: diag: check CURRESTAB counters")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/diag.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 04fcb8a077c9..e0615c6ffb8d 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -62,8 +62,8 @@ __chk_nr()
 	nr=$(eval $command)
 
 	printf "%-50s" "$msg"
-	if [ $nr != $expected ]; then
-		if [ $nr = "$skip" ] && ! mptcp_lib_expect_all_features; then
+	if [ "$nr" != "$expected" ]; then
+		if [ "$nr" = "$skip" ] && ! mptcp_lib_expect_all_features; then
 			echo "[ skip ] Feature probably not supported"
 			mptcp_lib_result_skip "${msg}"
 		else

-- 
2.43.0

Re: selftests: mptcp: diag: fix bash warnings on older kernels: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7819509269

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/bb075ba934d0


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: selftests: mptcp: diag: fix bash warnings on older kernels: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Unstable: 1 failed test(s): packetdrill_regressions 🔴:
  - Task: https://cirrus-ci.com/task/5232589070401536
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5232589070401536/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6358488977244160
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6358488977244160/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/bb075ba934d0


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

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: selftests: mptcp: diag: fix bash warnings on older kernels: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7887523975

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/a0e162ea2e56


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: selftests: mptcp: diag: fix bash warnings on older kernels: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Script error! ❓:
  - Task: https://cirrus-ci.com/task/5220473605193728
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5220473605193728/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6152222736646144
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6152222736646144/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/a0e162ea2e56


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

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)