[PATCH mptcp-net v2] selftests: mptcp: join: fix subflow_send_ack lookup

[PATCH mptcp-net v2] selftests: mptcp: join: fix subflow_send_ack lookup

[Geliang Tang]

MPC backups tests will skip unexpected sometimes, since static functions
like mptcp_subflow_send_ack may also be listed in /proc/kallsyms, with a
't' in front of it, not 'T' ('T' is for a global function):

 > grep "mptcp_subflow_send_ack" /proc/kallsyms

 0000000000000000 T __pfx___mptcp_subflow_send_ack
 0000000000000000 T __mptcp_subflow_send_ack
 0000000000000000 t __pfx_mptcp_subflow_send_ack
 0000000000000000 t mptcp_subflow_send_ack

In this case, mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"
will be false, MPC backups tests will skip. This is not what we expected.

The correct logic here should be: if mptcp_subflow_send_ack is not a
global function in /proc/kallsyms, do these MPC backups tests. So a 'T'
must be added in front of mptcp_subflow_send_ack.

Fixes: 632978f0a961 ("selftests: mptcp: join: skip MPC backups tests if not supported")
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
v2:
 - Update commit log, fix typos in it.
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 1606474232f6..6fbc70332caa 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -2755,7 +2755,7 @@ backup_tests()
 	fi
 
 	if reset "mpc backup" &&
-	   continue_if mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"; then
+	   continue_if mptcp_lib_kallsyms_doesnt_have "T mptcp_subflow_send_ack$"; then
 		pm_nl_add_endpoint $ns2 10.0.1.2 flags subflow,backup
 		speed=slow \
 			run_tests $ns1 $ns2 10.0.1.1
@@ -2764,7 +2764,7 @@ backup_tests()
 	fi
 
 	if reset "mpc backup both sides" &&
-	   continue_if mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"; then
+	   continue_if mptcp_lib_kallsyms_doesnt_have "T mptcp_subflow_send_ack$"; then
 		pm_nl_add_endpoint $ns1 10.0.1.1 flags subflow,backup
 		pm_nl_add_endpoint $ns2 10.0.1.2 flags subflow,backup
 		speed=slow \
@@ -2774,7 +2774,7 @@ backup_tests()
 	fi
 
 	if reset "mpc switch to backup" &&
-	   continue_if mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"; then
+	   continue_if mptcp_lib_kallsyms_doesnt_have "T mptcp_subflow_send_ack$"; then
 		pm_nl_add_endpoint $ns2 10.0.1.2 flags subflow
 		sflags=backup speed=slow \
 			run_tests $ns1 $ns2 10.0.1.1
@@ -2783,7 +2783,7 @@ backup_tests()
 	fi
 
 	if reset "mpc switch to backup both sides" &&
-	   continue_if mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"; then
+	   continue_if mptcp_lib_kallsyms_doesnt_have "T mptcp_subflow_send_ack$"; then
 		pm_nl_add_endpoint $ns1 10.0.1.1 flags subflow
 		pm_nl_add_endpoint $ns2 10.0.1.2 flags subflow
 		sflags=backup speed=slow \
-- 
2.35.3

Re: selftests: mptcp: join: fix subflow_send_ack lookup: Tests Results

[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 (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/4901440322273280
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4901440322273280/summary/summary.txt

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

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

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

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


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 (Tessares)

Re: [PATCH mptcp-net v2] selftests: mptcp: join: fix subflow_send_ack lookup

[Geliang Tang]

On Tue, Nov 14, 2023 at 10:57:08PM +0800, Geliang Tang wrote:
> MPC backups tests will skip unexpected sometimes, since static functions
> like mptcp_subflow_send_ack may also be listed in /proc/kallsyms, with a
> 't' in front of it, not 'T' ('T' is for a global function):
> 
>  > grep "mptcp_subflow_send_ack" /proc/kallsyms
> 
>  0000000000000000 T __pfx___mptcp_subflow_send_ack
>  0000000000000000 T __mptcp_subflow_send_ack
>  0000000000000000 t __pfx_mptcp_subflow_send_ack
>  0000000000000000 t mptcp_subflow_send_ack

I did some tests again, and found that whether to list the static
functions in /proc/kallsyms depends on the version of gcc:

Use gcc-8, "t mptcp_subflow_send_ack" is in /proc/kallsyms.

> sudo dmesg | head
[    0.000000] Linux version 6.6.0-mptcp+ (tgl@localhost.localdomain) (gcc (SUSE Linux) 8.5.0, GNU ld (GNU Binutils; SUSE Linux Enterprise 15) 2.39.0.20220810-150100.7.40) #5 SMP PREEMPT_DYNAMIC Thu Nov 16 10:27:33 CST 2023
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-6.6.0-mptcp+ root=/dev/mapper/vg0-lvol0 splash=silent mitigations=auto quiet
[    0.000000] BIOS-provided physical RAM map:
[    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009efff] usable
[    0.000000] BIOS-e820: [mem 0x000000000009f000-0x00000000000fffff] reserved
[    0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000006a5b6fff] usable
[    0.000000] BIOS-e820: [mem 0x000000006a5b7000-0x000000006e794fff] reserved
[    0.000000] BIOS-e820: [mem 0x000000006e795000-0x000000006fb1efff] ACPI NVS
[    0.000000] BIOS-e820: [mem 0x000000006fb1f000-0x000000006fc4efff] ACPI data
[    0.000000] BIOS-e820: [mem 0x000000006fc4f000-0x000000006fc4ffff] usable

> sudo cat /proc/kallsyms | grep subflow_send_ack
ffffffffa1c0df80 T __pfx___mptcp_subflow_send_ack
ffffffffa1c0df90 T __mptcp_subflow_send_ack
ffffffffa1c0dfd0 t __pfx_mptcp_subflow_send_ack
ffffffffa1c0dfe0 t mptcp_subflow_send_ack

Use gcc-11, no mptcp_subflow_send_ack is in /proc/kallsyms.

> sudo dmesg | head
[sudo] password for root:
[    0.000000] Linux version 6.6.0-mptcp+ (tgl@bogon) (gcc (SUSE Linux) 11.4.1 20230630 [revision f03b182f7f23e14d587cfa1daec13ef7c3044ac0], GNU ld (GNU Binutils; SUSE Linux Enterprise 15) 2.39.0.20220810-150100.7.40) #25 SMP PREEMPT_DYNAMIC Thu Nov 16 22:26:41 CST 2023
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-6.6.0-mptcp+ root=/dev/mapper/vg0-lvol0 splash=silent mitigations=auto quiet
[    0.000000] BIOS-provided physical RAM map:
[    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009efff] usable
[    0.000000] BIOS-e820: [mem 0x000000000009f000-0x00000000000fffff] reserved
[    0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000006a5b6fff] usable
[    0.000000] BIOS-e820: [mem 0x000000006a5b7000-0x000000006e794fff] reserved
[    0.000000] BIOS-e820: [mem 0x000000006e795000-0x000000006fb1efff] ACPI NVS
[    0.000000] BIOS-e820: [mem 0x000000006fb1f000-0x000000006fc4efff] ACPI data
[    0.000000] BIOS-e820: [mem 0x000000006fc4f000-0x000000006fc4ffff] usable
> sudo cat /proc/kallsyms | grep subflow_send_ack
ffffffffaca54e80 T __pfx___mptcp_subflow_send_ack
ffffffffaca54e90 T __mptcp_subflow_send_ack

So MPC backups tests will skip unexpected when using gcc-8. This patch
does fix this issue. Only commit log needs to be updated:

'''
MPC backups tests will skip unexpected sometimes (For example, when
compiling kernel with an older version of gcc, such as gcc-8), since
static functions like mptcp_subflow_send_ack also be listed in
/proc/kallsyms, with a 't' in front of it, not 'T' ('T' is for a global
function):

 > grep "mptcp_subflow_send_ack" /proc/kallsyms

 0000000000000000 T __pfx___mptcp_subflow_send_ack
 0000000000000000 T __mptcp_subflow_send_ack
 0000000000000000 t __pfx_mptcp_subflow_send_ack
 0000000000000000 t mptcp_subflow_send_ack

In this case, mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"
will be false, MPC backups tests will skip. This is not what we expected.

The correct logic here should be: if mptcp_subflow_send_ack is not a
global function in /proc/kallsyms, do these MPC backups tests. So a 'T'
must be added in front of mptcp_subflow_send_ack.
'''

Thanks,
-Geliang

> 
> In this case, mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"
> will be false, MPC backups tests will skip. This is not what we expected.
> 
> The correct logic here should be: if mptcp_subflow_send_ack is not a
> global function in /proc/kallsyms, do these MPC backups tests. So a 'T'
> must be added in front of mptcp_subflow_send_ack.
> 
> Fixes: 632978f0a961 ("selftests: mptcp: join: skip MPC backups tests if not supported")
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> v2:
>  - Update commit log, fix typos in it.
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 1606474232f6..6fbc70332caa 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -2755,7 +2755,7 @@ backup_tests()
>  	fi
>  
>  	if reset "mpc backup" &&
> -	   continue_if mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"; then
> +	   continue_if mptcp_lib_kallsyms_doesnt_have "T mptcp_subflow_send_ack$"; then
>  		pm_nl_add_endpoint $ns2 10.0.1.2 flags subflow,backup
>  		speed=slow \
>  			run_tests $ns1 $ns2 10.0.1.1
> @@ -2764,7 +2764,7 @@ backup_tests()
>  	fi
>  
>  	if reset "mpc backup both sides" &&
> -	   continue_if mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"; then
> +	   continue_if mptcp_lib_kallsyms_doesnt_have "T mptcp_subflow_send_ack$"; then
>  		pm_nl_add_endpoint $ns1 10.0.1.1 flags subflow,backup
>  		pm_nl_add_endpoint $ns2 10.0.1.2 flags subflow,backup
>  		speed=slow \
> @@ -2774,7 +2774,7 @@ backup_tests()
>  	fi
>  
>  	if reset "mpc switch to backup" &&
> -	   continue_if mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"; then
> +	   continue_if mptcp_lib_kallsyms_doesnt_have "T mptcp_subflow_send_ack$"; then
>  		pm_nl_add_endpoint $ns2 10.0.1.2 flags subflow
>  		sflags=backup speed=slow \
>  			run_tests $ns1 $ns2 10.0.1.1
> @@ -2783,7 +2783,7 @@ backup_tests()
>  	fi
>  
>  	if reset "mpc switch to backup both sides" &&
> -	   continue_if mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"; then
> +	   continue_if mptcp_lib_kallsyms_doesnt_have "T mptcp_subflow_send_ack$"; then
>  		pm_nl_add_endpoint $ns1 10.0.1.1 flags subflow
>  		pm_nl_add_endpoint $ns2 10.0.1.2 flags subflow
>  		sflags=backup speed=slow \
> -- 
> 2.35.3
> 

Re: [PATCH mptcp-net v2] selftests: mptcp: join: fix subflow_send_ack lookup

[Mat Martineau]

On Tue, 14 Nov 2023, Geliang Tang wrote:

> MPC backups tests will skip unexpected sometimes, since static functions
> like mptcp_subflow_send_ack may also be listed in /proc/kallsyms, with a
> 't' in front of it, not 'T' ('T' is for a global function):
>
> > grep "mptcp_subflow_send_ack" /proc/kallsyms
>
> 0000000000000000 T __pfx___mptcp_subflow_send_ack
> 0000000000000000 T __mptcp_subflow_send_ack
> 0000000000000000 t __pfx_mptcp_subflow_send_ack
> 0000000000000000 t mptcp_subflow_send_ack
>
> In this case, mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"
> will be false, MPC backups tests will skip. This is not what we expected.
>
> The correct logic here should be: if mptcp_subflow_send_ack is not a
> global function in /proc/kallsyms, do these MPC backups tests. So a 'T'
> must be added in front of mptcp_subflow_send_ack.
>
> Fixes: 632978f0a961 ("selftests: mptcp: join: skip MPC backups tests if not supported")
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> v2:
> - Update commit log, fix typos in it.

v2 looks good, with Geliang's commit message change from the follow-up 
message 
(https://lore.kernel.org/mptcp/20231117053305.GA26319@localhost.localdomain/)

Reviewed-by: Mat Martineau <martineau@kernel.org>

> ---
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>

Re: selftests: mptcp: join: fix subflow_send_ack lookup: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- {"code":404,"message":
  - "Can't find artifacts containing file conclusion.txt"}:
  - Task: https://cirrus-ci.com/task/6254656741965824
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6254656741965824/summary/summary.txt

- {"code":404,"message":
  - "Can't find artifacts containing file conclusion.txt"}:
  - Task: https://cirrus-ci.com/task/4847281858412544
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4847281858412544/summary/summary.txt

- {"code":404,"message":
  - "Can't find artifacts containing file conclusion.txt"}:
  - Task: https://cirrus-ci.com/task/5973181765255168
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5973181765255168/summary/summary.txt

- {"code":404,"message":
  - "Can't find artifacts containing file conclusion.txt"}:
  - Task: https://cirrus-ci.com/task/5410231811833856
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5410231811833856/summary/summary.txt

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


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: [PATCH mptcp-net v2] selftests: mptcp: join: fix subflow_send_ack lookup

[Matthieu Baerts]

Hi Geliang,

On 14/11/2023 15:57, Geliang Tang wrote:
> MPC backups tests will skip unexpected sometimes, since static functions
> like mptcp_subflow_send_ack may also be listed in /proc/kallsyms, with a
> 't' in front of it, not 'T' ('T' is for a global function):
> 
>  > grep "mptcp_subflow_send_ack" /proc/kallsyms
> 
>  0000000000000000 T __pfx___mptcp_subflow_send_ack
>  0000000000000000 T __mptcp_subflow_send_ack
>  0000000000000000 t __pfx_mptcp_subflow_send_ack
>  0000000000000000 t mptcp_subflow_send_ack
> 
> In this case, mptcp_lib_kallsyms_doesnt_have "mptcp_subflow_send_ack$"
> will be false, MPC backups tests will skip. This is not what we expected.
> 
> The correct logic here should be: if mptcp_subflow_send_ack is not a
> global function in /proc/kallsyms, do these MPC backups tests. So a 'T'
> must be added in front of mptcp_subflow_send_ack.

With a bit of delay -- sorry for that -- I just applied your patch in
our tree (fixes for -net):

New patches for t/upstream-net and t/upstream:
- 6fb578086535: selftests: mptcp: join: fix subflow_send_ack lookup
- Results: 77d946064d59..8ec51043b53f (export-net)
- Results: 5a37296ad9fd..8fa5c227980d (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20231204T170318
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20231204T170318

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