| Message ID | 20250108-slft-mptcp-conn-misc-impr-v1-2-bdfbdba48a1f@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1cb853be929c4321f40bf96d1e054b24d7051e2f |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | selftests: mptcp: more info in case of errors | expand |
| Context | Check | Description |
|---|---|---|
| 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_ | success | Success! ✅ |
| matttbe/build | success | Build and static analysis OK |
| matttbe/checkpatch | warning | total: 0 errors, 6 warnings, 0 checks, 82 lines checked |
| matttbe/shellcheck | success | No ShellCheck issues |
Hi Matt, Thanks for this set. On Wed, 2025-01-08 at 19:40 +0100, Matthieu Baerts (NGI0) wrote: > A few MPTCP selftests are using the same code to print stats in case > of > error. This code can then be moved to mptcp_lib.sh. > > No behaviour changes intended, except to print the error in red and > to > stderr, like most error messages. > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > --- > tools/testing/selftests/net/mptcp/mptcp_connect.sh | 8 ++------ > tools/testing/selftests/net/mptcp/mptcp_join.sh | 9 ++------- > tools/testing/selftests/net/mptcp/mptcp_lib.sh | 21 > +++++++++++++++++++++ > tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 6 +----- > tools/testing/selftests/net/mptcp/simult_flows.sh | 8 ++------ > 5 files changed, 28 insertions(+), 24 deletions(-) > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh > b/tools/testing/selftests/net/mptcp/mptcp_connect.sh > index > b48b4e56826a9cfdb3501242b707ae2ebe29b220..bfdaecd0a6a0564020530345daf > 91bed296bc15c 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh > @@ -445,12 +445,8 @@ do_transfer() > printf "(duration %05sms) " "${duration}" > if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then > mptcp_lib_pr_fail "client exit code $retc, server > $rets" > - echo -e "\nnetns ${listener_ns} socket stat for > ${port}:" 1>&2 > - ip netns exec ${listener_ns} ss -Menita 1>&2 -o > "sport = :$port" > - cat /tmp/${listener_ns}.out > - echo -e "\nnetns ${connector_ns} socket stat for > ${port}:" 1>&2 > - ip netns exec ${connector_ns} ss -Menita 1>&2 -o > "dport = :$port" > - [ ${listener_ns} != ${connector_ns} ] && cat > /tmp/${connector_ns}.out > + mptcp_lib_pr_err_stats "${listener_ns}" > "${connector_ns}" "${port}" \ > + "/tmp/${listener_ns}.out" > "/tmp/${connector_ns}.out" > > echo > cat "$capout" > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh > b/tools/testing/selftests/net/mptcp/mptcp_join.sh > index > c07e2bd3a315aac9c422fed85c3196ec46e060f7..13a3b68181ee14eb628a858e573 > 8094c3c936b74 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > @@ -1039,13 +1039,8 @@ do_transfer() > > if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then > fail_test "client exit code $retc, server $rets" > - echo -e "\nnetns ${listener_ns} socket stat for > ${port}:" 1>&2 > - ip netns exec ${listener_ns} ss -Menita 1>&2 -o > "sport = :$port" > - cat /tmp/${listener_ns}.out > - echo -e "\nnetns ${connector_ns} socket stat for > ${port}:" 1>&2 > - ip netns exec ${connector_ns} ss -Menita 1>&2 -o > "dport = :$port" > - cat /tmp/${connector_ns}.out > - > + mptcp_lib_pr_err_stats "${listener_ns}" > "${connector_ns}" "${port}" \ > + "/tmp/${listener_ns}.out" > "/tmp/${connector_ns}.out" > return 1 > fi > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh > b/tools/testing/selftests/net/mptcp/mptcp_lib.sh > index > 975d4d4c862afff2e685e86dc08a892dbd09d783..50ca64357053b14cd4989e6647d > 825337978fdba 100644 > --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh > @@ -107,6 +107,27 @@ mptcp_lib_pr_info() { > mptcp_lib_print_info "INFO: ${*}" > } > > +# $1-2: listener/connector ns ; $3 port ; [ $4-5 listener/connector > stat file ] > +mptcp_lib_pr_err_stats() { > + local lns="${1}" > + local cns="${2}" > + local port="${3}" > + local lstat="${4:-}" > + local cstat="${5:-}" I think it's better to add a separate patch to add cat /tmp/${listener_ns}.out and cat /tmp/${connector_ns}.out in mptcp_sockopt.sh, then we can drop the last two arguments "lstat" and "cstat" of this helper. WDYT? Thanks, -Geliang > + > + echo -en "${MPTCP_LIB_COLOR_RED}" > + { > + printf "\nnetns %s (listener) socket stat for %d:\n" > "${lns}" "${port}" > + ip netns exec "${lns}" ss -Menita -o "sport = > :${port}" > + [ -s "${lstat}" ] && cat "${lstat}" > + > + printf "\nnetns %s (connector) socket stat for > %d:\n" "${cns}" "${port}" > + ip netns exec "${cns}" ss -Menita -o "dport = > :${port}" > + [ "${lstat}" != "${cstat}" ] && [ -s "${cstat}" ] && > cat "${cstat}" > + } 1>&2 > + echo -en "${MPTCP_LIB_COLOR_RESET}" > +} > + > # SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES env var can be set when > validating all > # features using the last version of the kernel and the selftests to > make sure > # a test is not being skipped by mistake. > diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh > b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh > index > 5e8d5b83e2d092879efc179f1a450542be4e575e..6b366c604a9eeccdb759f260be4 > feafc380ccb0b 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh > @@ -192,11 +192,7 @@ do_transfer() > print_title "Transfer ${ip:2}" > if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then > mptcp_lib_pr_fail "client exit code $retc, server > $rets" > - echo -e "\nnetns ${listener_ns} socket stat for > ${port}:" 1>&2 > - ip netns exec ${listener_ns} ss -Menita 1>&2 -o > "sport = :$port" > - > - echo -e "\nnetns ${connector_ns} socket stat for > ${port}:" 1>&2 > - ip netns exec ${connector_ns} ss -Menita 1>&2 -o > "dport = :$port" > + mptcp_lib_pr_err_stats "${listener_ns}" > "${connector_ns}" "${port}" > > mptcp_lib_result_fail "transfer ${ip}" > > diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh > b/tools/testing/selftests/net/mptcp/simult_flows.sh > index > e98e5907d52c2d0e9c0152efda82176861905cf1..9c2a415976cbf7a0b56cd4b2fbd > d36c9e1ef3c8c 100755 > --- a/tools/testing/selftests/net/mptcp/simult_flows.sh > +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh > @@ -204,12 +204,8 @@ do_transfer() > fi > > mptcp_lib_pr_fail "client exit code $retc, server $rets" > - echo -e "\nnetns ${ns3} socket stat for $port:" 1>&2 > - ip netns exec ${ns3} ss -Menita 1>&2 -o "sport = :$port" > - cat /tmp/${ns3}.out > - echo -e "\nnetns ${ns1} socket stat for $port:" 1>&2 > - ip netns exec ${ns1} ss -Menita 1>&2 -o "dport = :$port" > - cat /tmp/${ns1}.out > + mptcp_lib_pr_err_stats "${ns3}" "${ns1}" "${port}" \ > + "/tmp/${ns3}.out" "/tmp/${ns1}.out" > ls -l $sin $cout > ls -l $cin $sout > >
Hi Geliang, Thank you for your review! On 09/01/2025 10:24, Geliang Tang wrote: > Hi Matt, > > Thanks for this set. > > On Wed, 2025-01-08 at 19:40 +0100, Matthieu Baerts (NGI0) wrote: >> A few MPTCP selftests are using the same code to print stats in case >> of >> error. This code can then be moved to mptcp_lib.sh. >> >> No behaviour changes intended, except to print the error in red and >> to >> stderr, like most error messages. (...) >> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh >> b/tools/testing/selftests/net/mptcp/mptcp_lib.sh >> index >> 975d4d4c862afff2e685e86dc08a892dbd09d783..50ca64357053b14cd4989e6647d >> 825337978fdba 100644 >> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh >> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh >> @@ -107,6 +107,27 @@ mptcp_lib_pr_info() { >> mptcp_lib_print_info "INFO: ${*}" >> } >> >> +# $1-2: listener/connector ns ; $3 port ; [ $4-5 listener/connector >> stat file ] >> +mptcp_lib_pr_err_stats() { >> + local lns="${1}" >> + local cns="${2}" >> + local port="${3}" >> + local lstat="${4:-}" >> + local cstat="${5:-}" > > I think it's better to add a separate patch to add > > cat /tmp/${listener_ns}.out > > and > > cat /tmp/${connector_ns}.out > > in mptcp_sockopt.sh, mptcp_sockopt.sh currently doesn't have such file. I hesitated to add them using the content of nstat like in the other ones, but I didn't really see how it would help to fix issues with socket options. Or do you have something in mind? I guess we could add them "just in case", but... > then we can drop the last two arguments "lstat" > and "cstat" of this helper. ... sorry, I'm not sure to understand what you meant here. We still want to print the content of these files if available. Cheers, Matt
Hi Matt, On Thu, 2025-01-09 at 16:30 +0100, Matthieu Baerts wrote: > Hi Geliang, > > Thank you for your review! > > On 09/01/2025 10:24, Geliang Tang wrote: > > Hi Matt, > > > > Thanks for this set. > > > > On Wed, 2025-01-08 at 19:40 +0100, Matthieu Baerts (NGI0) wrote: > > > A few MPTCP selftests are using the same code to print stats in > > > case > > > of > > > error. This code can then be moved to mptcp_lib.sh. > > > > > > No behaviour changes intended, except to print the error in red > > > and > > > to > > > stderr, like most error messages. > > (...) > > > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh > > > b/tools/testing/selftests/net/mptcp/mptcp_lib.sh > > > index > > > 975d4d4c862afff2e685e86dc08a892dbd09d783..50ca64357053b14cd4989e6 > > > 647d > > > 825337978fdba 100644 > > > --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh > > > +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh > > > @@ -107,6 +107,27 @@ mptcp_lib_pr_info() { > > > mptcp_lib_print_info "INFO: ${*}" > > > } > > > > > > +# $1-2: listener/connector ns ; $3 port ; [ $4-5 > > > listener/connector > > > stat file ] > > > +mptcp_lib_pr_err_stats() { > > > + local lns="${1}" > > > + local cns="${2}" > > > + local port="${3}" > > > + local lstat="${4:-}" > > > + local cstat="${5:-}" > > > > I think it's better to add a separate patch to add > > > > cat /tmp/${listener_ns}.out > > > > and > > > > cat /tmp/${connector_ns}.out > > > > in mptcp_sockopt.sh, > > mptcp_sockopt.sh currently doesn't have such file. I hesitated to add > them using the content of nstat like in the other ones, but I didn't > really see how it would help to fix issues with socket options. Or do > you have something in mind? > > I guess we could add them "just in case", but... I just sent a patch named "selftests: mptcp: sockopt: save nstat infos" to do this. > > > then we can drop the last two arguments "lstat" > > and "cstat" of this helper. > > ... sorry, I'm not sure to understand what you meant here. We still > want > to print the content of these files if available. And a squash-to patch for this. Thanks, -Geliang > > > Cheers, > Matt
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh index b48b4e56826a9cfdb3501242b707ae2ebe29b220..bfdaecd0a6a0564020530345daf91bed296bc15c 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh @@ -445,12 +445,8 @@ do_transfer() printf "(duration %05sms) " "${duration}" if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then mptcp_lib_pr_fail "client exit code $retc, server $rets" - echo -e "\nnetns ${listener_ns} socket stat for ${port}:" 1>&2 - ip netns exec ${listener_ns} ss -Menita 1>&2 -o "sport = :$port" - cat /tmp/${listener_ns}.out - echo -e "\nnetns ${connector_ns} socket stat for ${port}:" 1>&2 - ip netns exec ${connector_ns} ss -Menita 1>&2 -o "dport = :$port" - [ ${listener_ns} != ${connector_ns} ] && cat /tmp/${connector_ns}.out + mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}" \ + "/tmp/${listener_ns}.out" "/tmp/${connector_ns}.out" echo cat "$capout" diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index c07e2bd3a315aac9c422fed85c3196ec46e060f7..13a3b68181ee14eb628a858e5738094c3c936b74 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -1039,13 +1039,8 @@ do_transfer() if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then fail_test "client exit code $retc, server $rets" - echo -e "\nnetns ${listener_ns} socket stat for ${port}:" 1>&2 - ip netns exec ${listener_ns} ss -Menita 1>&2 -o "sport = :$port" - cat /tmp/${listener_ns}.out - echo -e "\nnetns ${connector_ns} socket stat for ${port}:" 1>&2 - ip netns exec ${connector_ns} ss -Menita 1>&2 -o "dport = :$port" - cat /tmp/${connector_ns}.out - + mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}" \ + "/tmp/${listener_ns}.out" "/tmp/${connector_ns}.out" return 1 fi diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh index 975d4d4c862afff2e685e86dc08a892dbd09d783..50ca64357053b14cd4989e6647d825337978fdba 100644 --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh @@ -107,6 +107,27 @@ mptcp_lib_pr_info() { mptcp_lib_print_info "INFO: ${*}" } +# $1-2: listener/connector ns ; $3 port ; [ $4-5 listener/connector stat file ] +mptcp_lib_pr_err_stats() { + local lns="${1}" + local cns="${2}" + local port="${3}" + local lstat="${4:-}" + local cstat="${5:-}" + + echo -en "${MPTCP_LIB_COLOR_RED}" + { + printf "\nnetns %s (listener) socket stat for %d:\n" "${lns}" "${port}" + ip netns exec "${lns}" ss -Menita -o "sport = :${port}" + [ -s "${lstat}" ] && cat "${lstat}" + + printf "\nnetns %s (connector) socket stat for %d:\n" "${cns}" "${port}" + ip netns exec "${cns}" ss -Menita -o "dport = :${port}" + [ "${lstat}" != "${cstat}" ] && [ -s "${cstat}" ] && cat "${cstat}" + } 1>&2 + echo -en "${MPTCP_LIB_COLOR_RESET}" +} + # SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES env var can be set when validating all # features using the last version of the kernel and the selftests to make sure # a test is not being skipped by mistake. diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh index 5e8d5b83e2d092879efc179f1a450542be4e575e..6b366c604a9eeccdb759f260be4feafc380ccb0b 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh @@ -192,11 +192,7 @@ do_transfer() print_title "Transfer ${ip:2}" if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then mptcp_lib_pr_fail "client exit code $retc, server $rets" - echo -e "\nnetns ${listener_ns} socket stat for ${port}:" 1>&2 - ip netns exec ${listener_ns} ss -Menita 1>&2 -o "sport = :$port" - - echo -e "\nnetns ${connector_ns} socket stat for ${port}:" 1>&2 - ip netns exec ${connector_ns} ss -Menita 1>&2 -o "dport = :$port" + mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}" mptcp_lib_result_fail "transfer ${ip}" diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh index e98e5907d52c2d0e9c0152efda82176861905cf1..9c2a415976cbf7a0b56cd4b2fbdd36c9e1ef3c8c 100755 --- a/tools/testing/selftests/net/mptcp/simult_flows.sh +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh @@ -204,12 +204,8 @@ do_transfer() fi mptcp_lib_pr_fail "client exit code $retc, server $rets" - echo -e "\nnetns ${ns3} socket stat for $port:" 1>&2 - ip netns exec ${ns3} ss -Menita 1>&2 -o "sport = :$port" - cat /tmp/${ns3}.out - echo -e "\nnetns ${ns1} socket stat for $port:" 1>&2 - ip netns exec ${ns1} ss -Menita 1>&2 -o "dport = :$port" - cat /tmp/${ns1}.out + mptcp_lib_pr_err_stats "${ns3}" "${ns1}" "${port}" \ + "/tmp/${ns3}.out" "/tmp/${ns1}.out" ls -l $sin $cout ls -l $cin $sout
A few MPTCP selftests are using the same code to print stats in case of error. This code can then be moved to mptcp_lib.sh. No behaviour changes intended, except to print the error in red and to stderr, like most error messages. Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> --- tools/testing/selftests/net/mptcp/mptcp_connect.sh | 8 ++------ tools/testing/selftests/net/mptcp/mptcp_join.sh | 9 ++------- tools/testing/selftests/net/mptcp/mptcp_lib.sh | 21 +++++++++++++++++++++ tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 6 +----- tools/testing/selftests/net/mptcp/simult_flows.sh | 8 ++------ 5 files changed, 28 insertions(+), 24 deletions(-)