[PATCH mptcp-next] selftests: mptcp: diag: use mptcp_lib_get_info_value

[PATCH mptcp-next] selftests: mptcp: diag: use mptcp_lib_get_info_value

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

When running diag.sh in a loop, chk_dump_one will report the following
"grep: write error":

 13 ....chk 2 cestab                                  [ OK ]
 grep: write error
 14 ....chk dump_one                                  [ OK ]
 15 ....chk 2->0 msk in use after flush               [ OK ]
 16 ....chk 2->0 cestab after flush                   [ OK ]

This error is caused by a broken pipe. When the output of 'ss' is processed
by grep, 'head -n 1' will exit immediately after getting the first line,
causing the subsequent pipe to close. At this time, if 'grep' is still
trying to write data to the closed pipe, it will trigger a SIGPIPE signal,
causing a write error.

One solution is not to use this problematic "head -n 1" command, but to use
mptcp_lib_get_info_value() helper defined in mptcp_lib.sh to get the value
of 'token'.

Fixes: ba2400166570 ("selftests: mptcp: add a test for mptcp_diag_dump_one")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/diag.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 4f55477ffe08..e7a75341f0f3 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -206,9 +206,8 @@ chk_dump_one()
 	local token
 	local msg
 
-	ss_token="$(ss -inmHMN $ns | grep 'token:' |\
-		    head -n 1 |\
-		    sed 's/.*token:\([0-9a-f]*\).*/\1/')"
+	ss_token="$(ss -inmHMN $ns |
+		    mptcp_lib_get_info_value "token" "token")"
 
 	token="$(ip netns exec $ns ./mptcp_diag -t $ss_token |\
 		 awk -F':[ \t]+' '/^token/ {print $2}')"
-- 
2.43.0

Re: [PATCH mptcp-next] selftests: mptcp: diag: use mptcp_lib_get_info_value

[Gang Yan]

On Thu, Apr 03, 2025 at 03:46:37PM +0800, Geliang Tang wrote:
Hi, Geliang:

Thanks for the patch, this patch also works well in my enviroment.
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> When running diag.sh in a loop, chk_dump_one will report the following
> "grep: write error":
> 
>  13 ....chk 2 cestab                                  [ OK ]
>  grep: write error
>  14 ....chk dump_one                                  [ OK ]
>  15 ....chk 2->0 msk in use after flush               [ OK ]
>  16 ....chk 2->0 cestab after flush                   [ OK ]
> 
> This error is caused by a broken pipe. When the output of 'ss' is processed
> by grep, 'head -n 1' will exit immediately after getting the first line,
> causing the subsequent pipe to close. At this time, if 'grep' is still
> trying to write data to the closed pipe, it will trigger a SIGPIPE signal,
> causing a write error.
> 
> One solution is not to use this problematic "head -n 1" command, but to use
> mptcp_lib_get_info_value() helper defined in mptcp_lib.sh to get the value
> of 'token'.
> 
> Fixes: ba2400166570 ("selftests: mptcp: add a test for mptcp_diag_dump_one")
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Tested-by: Gang Yan <yangang@kylinos.cn>
> ---
>  tools/testing/selftests/net/mptcp/diag.sh | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
> index 4f55477ffe08..e7a75341f0f3 100755
> --- a/tools/testing/selftests/net/mptcp/diag.sh
> +++ b/tools/testing/selftests/net/mptcp/diag.sh
> @@ -206,9 +206,8 @@ chk_dump_one()
>  	local token
>  	local msg
>  
> -	ss_token="$(ss -inmHMN $ns | grep 'token:' |\
> -		    head -n 1 |\
> -		    sed 's/.*token:\([0-9a-f]*\).*/\1/')"
> +	ss_token="$(ss -inmHMN $ns |
> +		    mptcp_lib_get_info_value "token" "token")"
>  
>  	token="$(ip netns exec $ns ./mptcp_diag -t $ss_token |\
>  		 awk -F':[ \t]+' '/^token/ {print $2}')"
> -- 
> 2.43.0
> 
> 
Best wishes!
Gang

Re: [PATCH mptcp-next] selftests: mptcp: diag: use mptcp_lib_get_info_value

[Matthieu Baerts]

Hi Geliang,

On 03/04/2025 09:46, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> When running diag.sh in a loop, chk_dump_one will report the following
> "grep: write error":
> 
>  13 ....chk 2 cestab                                  [ OK ]
>  grep: write error
>  14 ....chk dump_one                                  [ OK ]
>  15 ....chk 2->0 msk in use after flush               [ OK ]
>  16 ....chk 2->0 cestab after flush                   [ OK ]
> 
> This error is caused by a broken pipe. When the output of 'ss' is processed
> by grep, 'head -n 1' will exit immediately after getting the first line,
> causing the subsequent pipe to close. At this time, if 'grep' is still
> trying to write data to the closed pipe, it will trigger a SIGPIPE signal,
> causing a write error.
> 
> One solution is not to use this problematic "head -n 1" command, but to use
> mptcp_lib_get_info_value() helper defined in mptcp_lib.sh to get the value
> of 'token'.

I was wondering why this helper would help because there will be 2
connections, and it is still required to limit the output to one. Then I
saw this helper does limit to the first entry because of 'sed (...);q'.

I will modify the message here to explain that. And maybe add a comment
in mptcp_lib.sh as well for later.

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

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

Re: [PATCH mptcp-next] selftests: mptcp: diag: use mptcp_lib_get_info_value

[Matthieu Baerts]

On 10/04/2025 18:58, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 03/04/2025 09:46, Geliang Tang wrote:
>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> When running diag.sh in a loop, chk_dump_one will report the following
>> "grep: write error":
>>
>>  13 ....chk 2 cestab                                  [ OK ]
>>  grep: write error
>>  14 ....chk dump_one                                  [ OK ]
>>  15 ....chk 2->0 msk in use after flush               [ OK ]
>>  16 ....chk 2->0 cestab after flush                   [ OK ]
>>
>> This error is caused by a broken pipe. When the output of 'ss' is processed
>> by grep, 'head -n 1' will exit immediately after getting the first line,
>> causing the subsequent pipe to close. At this time, if 'grep' is still
>> trying to write data to the closed pipe, it will trigger a SIGPIPE signal,
>> causing a write error.
>>
>> One solution is not to use this problematic "head -n 1" command, but to use
>> mptcp_lib_get_info_value() helper defined in mptcp_lib.sh to get the value
>> of 'token'.
> 
> I was wondering why this helper would help because there will be 2
> connections, and it is still required to limit the output to one. Then I
> saw this helper does limit to the first entry because of 'sed (...);q'.
> 
> I will modify the message here to explain that. And maybe add a comment
> in mptcp_lib.sh as well for later.

Note: technically, we can still have the issue with
mptcp_lib_get_info_value because we also have 'grep | sed' where 'sed'
can finish while 'grep' is still writing. I guess you no longer see the
warning because 'sed' will be slower than 'head -n1'. But OK, this can
be improved later.

> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> Cheers,
> Matt

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

Re: [PATCH mptcp-next] selftests: mptcp: diag: use mptcp_lib_get_info_value

[Matthieu Baerts]

Hi Geliang,

On 03/04/2025 09:46, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> When running diag.sh in a loop, chk_dump_one will report the following
> "grep: write error":
> 
>  13 ....chk 2 cestab                                  [ OK ]
>  grep: write error
>  14 ....chk dump_one                                  [ OK ]
>  15 ....chk 2->0 msk in use after flush               [ OK ]
>  16 ....chk 2->0 cestab after flush                   [ OK ]
> 
> This error is caused by a broken pipe. When the output of 'ss' is processed
> by grep, 'head -n 1' will exit immediately after getting the first line,
> causing the subsequent pipe to close. At this time, if 'grep' is still
> trying to write data to the closed pipe, it will trigger a SIGPIPE signal,
> causing a write error.
> 
> One solution is not to use this problematic "head -n 1" command, but to use
> mptcp_lib_get_info_value() helper defined in mptcp_lib.sh to get the value
> of 'token'.
> 
> Fixes: ba2400166570 ("selftests: mptcp: add a test for mptcp_diag_dump_one")

Applied in our tree (fixes for -net):

New patches for t/upstream-net and t/upstream:
- f2405c27bbf0: selftests: mptcp: diag: use mptcp_lib_get_info_value
- Results: c7abceef96b5..60b01f3eddf6 (export-net)
- Results: c9648c9d9f4c..244c301d3de6 (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/6d6923e5c2c0de07b702491eab14aa7288cded9c/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/e236c593b8bd23ed60fc7c001dab703df232b089/checks

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