| Message ID | e34e86805f3fe69e0aec08c83c596c49799573c7.1743666387.git.tanggeliang@kylinos.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f2405c27bbf038d4d0aad4594c2e94798e2918da |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | [mptcp-next] selftests: mptcp: diag: use mptcp_lib_get_info_value | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/build | success | Build and static analysis OK |
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 11 lines checked |
| matttbe/shellcheck | success | No ShellCheck issues |
| matttbe/KVM_Validation__normal | success | Success! ✅ |
| matttbe/KVM_Validation__debug | success | Success! ✅ |
| matttbe/KVM_Validation__btf-normal__only_bpftest_all_ | success | Success! ✅ |
| matttbe/KVM_Validation__btf-debug__only_bpftest_all_ | warning | Unstable: 1 failed test(s): bpftest_test_progs-no_alu32_mptcp |
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
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
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
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
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}')"