From: Geliang Tang <tanggeliang@kylinos.cn>
Only total test results are printed out in mptcp_sockopt.sh:
PASS: all packets had packet mark set
PASS: SOL_MPTCP getsockopt has expected information
PASS: TCP_INQ cmsg/ioctl -t tcp
PASS: TCP_INQ cmsg/ioctl -6 -t tcp
PASS: TCP_INQ cmsg/ioctl -r tcp
PASS: TCP_INQ cmsg/ioctl -6 -r tcp
This patch prints more info for every test result in each test
group:
transfer ipv4 [ OK ]
mark ipv4 [ OK ]
transfer ipv6 [ OK ]
mark ipv6 [ OK ]
PASS: all packets had packet mark set
sockopt v4 [ OK ]
sockopt v6 [ OK ]
PASS: SOL_MPTCP getsockopt has expected information
TCP_INQ: -t tcp [ OK ]
PASS: TCP_INQ cmsg/ioctl -t tcp
TCP_INQ: -6 -t tcp [ OK ]
PASS: TCP_INQ cmsg/ioctl -6 -t tcp
TCP_INQ: -r tcp [ OK ]
PASS: TCP_INQ cmsg/ioctl -r tcp
TCP_INQ: -6 -r tcp [ OK ]
PASS: TCP_INQ cmsg/ioctl -6 -r tcp
TCP_INQ: -r tcp -t tcp [ OK ]
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 6ed4aa32222f..f84185b5dc9f 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -161,6 +161,7 @@ do_transfer()
wait $spid
local rets=$?
+ printf "%-50s" "transfer ${ip}"
if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
echo " client exit code $retc, server $rets" 1>&2
echo -e "\nnetns ${listener_ns} socket stat for ${port}:" 1>&2
@@ -169,12 +170,15 @@ do_transfer()
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_print_err "[FAIL]"
mptcp_lib_result_fail "transfer ${ip}"
ret=1
return 1
fi
+ mptcp_lib_print_ok "[ OK ]"
+ printf "%-50s" "mark ${ip}"
if [ $local_addr = "::" ];then
check_mark $listener_ns 6 || retc=1
check_mark $connector_ns 6 || retc=1
@@ -190,8 +194,10 @@ do_transfer()
mptcp_lib_result_code "${rets}" "transfer ${ip}"
if [ $retc -eq 0 ] && [ $rets -eq 0 ];then
+ mptcp_lib_print_ok "[ OK ]"
return 0
fi
+ mptcp_lib_print_err "[FAIL]"
return 1
}
@@ -220,23 +226,27 @@ do_mptcp_sockopt_tests()
ip netns exec "$ns_sbox" ./mptcp_sockopt
lret=$?
+ printf "%-50s" "sockopt v4"
if [ $lret -ne 0 ]; then
echo "FAIL: SOL_MPTCP getsockopt" 1>&2
mptcp_lib_result_fail "sockopt v4"
ret=$lret
return
fi
+ mptcp_lib_print_ok "[ OK ]"
mptcp_lib_result_pass "sockopt v4"
ip netns exec "$ns_sbox" ./mptcp_sockopt -6
lret=$?
+ printf "%-50s" "sockopt v6"
if [ $lret -ne 0 ]; then
echo "FAIL: SOL_MPTCP getsockopt (ipv6)" 1>&2
mptcp_lib_result_fail "sockopt v6"
ret=$lret
return
fi
+ mptcp_lib_print_ok "[ OK ]"
mptcp_lib_result_pass "sockopt v6"
}
@@ -259,6 +269,7 @@ run_tests()
do_tcpinq_test()
{
+ printf "%-50s" "TCP_INQ: $*"
ip netns exec "$ns_sbox" ./mptcp_inq "$@"
local lret=$?
if [ $lret -ne 0 ];then
@@ -267,6 +278,7 @@ do_tcpinq_test()
mptcp_lib_result_fail "TCP_INQ: $*"
return $lret
fi
+ mptcp_lib_print_ok "[ OK ]"
echo "PASS: TCP_INQ cmsg/ioctl $*"
mptcp_lib_result_pass "TCP_INQ: $*"
--
2.40.1Hi Geliang,
On 26/02/2024 10:43, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Only total test results are printed out in mptcp_sockopt.sh:
>
> PASS: all packets had packet mark set
> PASS: SOL_MPTCP getsockopt has expected information
> PASS: TCP_INQ cmsg/ioctl -t tcp
> PASS: TCP_INQ cmsg/ioctl -6 -t tcp
> PASS: TCP_INQ cmsg/ioctl -r tcp
> PASS: TCP_INQ cmsg/ioctl -6 -r tcp
>
> This patch prints more info for every test result in each test
> group:
>
> transfer ipv4 [ OK ]
> mark ipv4 [ OK ]
> transfer ipv6 [ OK ]
> mark ipv6 [ OK ]
> PASS: all packets had packet mark set
> sockopt v4 [ OK ]
> sockopt v6 [ OK ]
> PASS: SOL_MPTCP getsockopt has expected information
> TCP_INQ: -t tcp [ OK ]
> PASS: TCP_INQ cmsg/ioctl -t tcp
> TCP_INQ: -6 -t tcp [ OK ]
> PASS: TCP_INQ cmsg/ioctl -6 -t tcp
> TCP_INQ: -r tcp [ OK ]
> PASS: TCP_INQ cmsg/ioctl -r tcp
> TCP_INQ: -6 -r tcp [ OK ]
> PASS: TCP_INQ cmsg/ioctl -6 -r tcp
> TCP_INQ: -r tcp -t tcp [ OK ]
Please clearly explain why this is interesting, even if it looks obvious.
Here, I don't know if this patch makes sense or not: to me, the output
is now confusing because there is a mix of '[ OK ]' and 'PASS'. Maybe:
- remove all the "PASS: xxx"? → I don't see what it brings more
- (or convert existing "PASS: xxx" and "FAIL: xxx" to use "[ OK ]"? But
there are fewer details)
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index 6ed4aa32222f..f84185b5dc9f 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> @@ -161,6 +161,7 @@ do_transfer()
> wait $spid
> local rets=$?
>
> + printf "%-50s" "transfer ${ip}"
Best to use a helper to avoid repeating '"%-50s"', and to be able to
change how the titles are displayed from a single place. (same below of
course)
Also, probably looking better if you use 'Transfer' instead of 'transfer'.
> if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
> echo " client exit code $retc, server $rets" 1>&2
> echo -e "\nnetns ${listener_ns} socket stat for ${port}:" 1>&2
> @@ -169,12 +170,15 @@ do_transfer()
> 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_print_err "[FAIL]"
> mptcp_lib_result_fail "transfer ${ip}"
>
> ret=1
> return 1
> fi
> + mptcp_lib_print_ok "[ OK ]"
(here as well, we could use 'mptcp_lib_print_success/fail/skip'
suggested in patch 1/12)
>
> + printf "%-50s" "mark ${ip}"
Same here: 'Mark' vs 'mark'.
> if [ $local_addr = "::" ];then
> check_mark $listener_ns 6 || retc=1
> check_mark $connector_ns 6 || retc=1
> @@ -190,8 +194,10 @@ do_transfer()
> mptcp_lib_result_code "${rets}" "transfer ${ip}"
>
> if [ $retc -eq 0 ] && [ $rets -eq 0 ];then
> + mptcp_lib_print_ok "[ OK ]"
> return 0
> fi
> + mptcp_lib_print_err "[FAIL]"
>
> return 1
> }
> @@ -220,23 +226,27 @@ do_mptcp_sockopt_tests()
> ip netns exec "$ns_sbox" ./mptcp_sockopt
> lret=$?
>
> + printf "%-50s" "sockopt v4"
Maybe 'SOL_MPTCP getsockopt v4'?
> if [ $lret -ne 0 ]; then
> echo "FAIL: SOL_MPTCP getsockopt" 1>&2
> mptcp_lib_result_fail "sockopt v4"
> ret=$lret
> return
> fi
> + mptcp_lib_print_ok "[ OK ]"
> mptcp_lib_result_pass "sockopt v4"
>
> ip netns exec "$ns_sbox" ./mptcp_sockopt -6
> lret=$?
>
> + printf "%-50s" "sockopt v6"
> if [ $lret -ne 0 ]; then
> echo "FAIL: SOL_MPTCP getsockopt (ipv6)" 1>&2
> mptcp_lib_result_fail "sockopt v6"
> ret=$lret
> return
> fi
> + mptcp_lib_print_ok "[ OK ]"
> mptcp_lib_result_pass "sockopt v6"
> }
>
> @@ -259,6 +269,7 @@ run_tests()
>
> do_tcpinq_test()
> {
> + printf "%-50s" "TCP_INQ: $*"
> ip netns exec "$ns_sbox" ./mptcp_inq "$@"
> local lret=$?
> if [ $lret -ne 0 ];then
> @@ -267,6 +278,7 @@ do_tcpinq_test()
> mptcp_lib_result_fail "TCP_INQ: $*"
> return $lret
> fi
> + mptcp_lib_print_ok "[ OK ]"
>
> echo "PASS: TCP_INQ cmsg/ioctl $*"
> mptcp_lib_result_pass "TCP_INQ: $*"
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
On Mon, 2024-02-26 at 13:40 +0100, Matthieu Baerts wrote:
> Hi Geliang,
>
> On 26/02/2024 10:43, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> >
> > Only total test results are printed out in mptcp_sockopt.sh:
> >
> > PASS: all packets had packet mark set
> > PASS: SOL_MPTCP getsockopt has expected information
> > PASS: TCP_INQ cmsg/ioctl -t tcp
> > PASS: TCP_INQ cmsg/ioctl -6 -t tcp
> > PASS: TCP_INQ cmsg/ioctl -r tcp
> > PASS: TCP_INQ cmsg/ioctl -6 -r tcp
> >
> > This patch prints more info for every test result in each test
> > group:
> >
> > transfer ipv4 [ OK ]
> > mark ipv4 [ OK ]
> > transfer ipv6 [ OK ]
> > mark ipv6 [ OK ]
> > PASS: all packets had packet mark set
> > sockopt v4 [ OK ]
> > sockopt v6 [ OK ]
> > PASS: SOL_MPTCP getsockopt has expected information
> > TCP_INQ: -t tcp [ OK ]
> > PASS: TCP_INQ cmsg/ioctl -t tcp
> > TCP_INQ: -6 -t tcp [ OK ]
> > PASS: TCP_INQ cmsg/ioctl -6 -t tcp
> > TCP_INQ: -r tcp [ OK ]
> > PASS: TCP_INQ cmsg/ioctl -r tcp
> > TCP_INQ: -6 -r tcp [ OK ]
> > PASS: TCP_INQ cmsg/ioctl -6 -r tcp
> > TCP_INQ: -r tcp -t tcp [ OK ]
>
> Please clearly explain why this is interesting, even if it looks
> obvious.
>
> Here, I don't know if this patch makes sense or not: to me, the
> output
> is now confusing because there is a mix of '[ OK ]' and 'PASS'.
> Maybe:
>
> - remove all the "PASS: xxx"? → I don't see what it brings more
> - (or convert existing "PASS: xxx" and "FAIL: xxx" to use "[ OK ]"?
> But
> there are fewer details)
This patch uses to match the output of this script:
INFO: PASS: all packets had packet mark set
INFO: PASS: SOL_MPTCP getsockopt has expected information
INFO: PASS: TCP_INQ cmsg/ioctl -t tcp
INFO: PASS: TCP_INQ cmsg/ioctl -6 -t tcp
INFO: PASS: TCP_INQ cmsg/ioctl -r tcp
INFO: PASS: TCP_INQ cmsg/ioctl -6 -r tcp
INFO: PASS: TCP_INQ cmsg/ioctl -r tcp -t tcp
with the test results of it:
ok 1 - mptcp_sockopt: mark ipv4
ok 2 - mptcp_sockopt: transfer ipv4
ok 3 - mptcp_sockopt: mark ipv6
ok 4 - mptcp_sockopt: transfer ipv6
ok 5 - mptcp_sockopt: sockopt v4
ok 6 - mptcp_sockopt: sockopt v6
ok 7 - mptcp_sockopt: TCP_INQ: -t tcp
ok 8 - mptcp_sockopt: TCP_INQ: -6 -t tcp
ok 9 - mptcp_sockopt: TCP_INQ: -r tcp
ok 10 - mptcp_sockopt: TCP_INQ: -6 -r tcp
ok 11 - mptcp_sockopt: TCP_INQ: -r tcp -t tcp
This patch is prepared for the coming printing counter patch, otherwise
the two output results may have different serial numbers, which can be
confusing.
>
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> > tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 12
> > ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> > b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> > index 6ed4aa32222f..f84185b5dc9f 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> > @@ -161,6 +161,7 @@ do_transfer()
> > wait $spid
> > local rets=$?
> >
> > + printf "%-50s" "transfer ${ip}"
>
> Best to use a helper to avoid repeating '"%-50s"', and to be able to
> change how the titles are displayed from a single place. (same below
> of
> course)
Add a new helper print_title() to do this.
>
> Also, probably looking better if you use 'Transfer' instead of
> 'transfer'.
Updated in v6.
>
> > if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
> > echo " client exit code $retc, server $rets" 1>&2
> > echo -e "\nnetns ${listener_ns} socket stat for
> > ${port}:" 1>&2
> > @@ -169,12 +170,15 @@ do_transfer()
> > 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_print_err "[FAIL]"
> > mptcp_lib_result_fail "transfer ${ip}"
> >
> > ret=1
> > return 1
> > fi
> > + mptcp_lib_print_ok "[ OK ]"
>
> (here as well, we could use 'mptcp_lib_print_success/fail/skip'
> suggested in patch 1/12)
Replaced by mptcp_lib_pr_ok.
>
> >
> > + printf "%-50s" "mark ${ip}"
>
> Same here: 'Mark' vs 'mark'.
Yes, updated.
>
> > if [ $local_addr = "::" ];then
> > check_mark $listener_ns 6 || retc=1
> > check_mark $connector_ns 6 || retc=1
> > @@ -190,8 +194,10 @@ do_transfer()
> > mptcp_lib_result_code "${rets}" "transfer ${ip}"
> >
> > if [ $retc -eq 0 ] && [ $rets -eq 0 ];then
> > + mptcp_lib_print_ok "[ OK ]"
> > return 0
> > fi
> > + mptcp_lib_print_err "[FAIL]"
> >
> > return 1
> > }
> > @@ -220,23 +226,27 @@ do_mptcp_sockopt_tests()
> > ip netns exec "$ns_sbox" ./mptcp_sockopt
> > lret=$?
> >
> > + printf "%-50s" "sockopt v4"
>
> Maybe 'SOL_MPTCP getsockopt v4'?
Updated.
>
> > if [ $lret -ne 0 ]; then
> > echo "FAIL: SOL_MPTCP getsockopt" 1>&2
> > mptcp_lib_result_fail "sockopt v4"
> > ret=$lret
> > return
> > fi
> > + mptcp_lib_print_ok "[ OK ]"
> > mptcp_lib_result_pass "sockopt v4"
> >
> > ip netns exec "$ns_sbox" ./mptcp_sockopt -6
> > lret=$?
> >
> > + printf "%-50s" "sockopt v6"
> > if [ $lret -ne 0 ]; then
> > echo "FAIL: SOL_MPTCP getsockopt (ipv6)" 1>&2
> > mptcp_lib_result_fail "sockopt v6"
> > ret=$lret
> > return
> > fi
> > + mptcp_lib_print_ok "[ OK ]"
> > mptcp_lib_result_pass "sockopt v6"
> > }
> >
> > @@ -259,6 +269,7 @@ run_tests()
> >
> > do_tcpinq_test()
> > {
> > + printf "%-50s" "TCP_INQ: $*"
> > ip netns exec "$ns_sbox" ./mptcp_inq "$@"
> > local lret=$?
> > if [ $lret -ne 0 ];then
> > @@ -267,6 +278,7 @@ do_tcpinq_test()
> > mptcp_lib_result_fail "TCP_INQ: $*"
> > return $lret
> > fi
> > + mptcp_lib_print_ok "[ OK ]"
> >
> > echo "PASS: TCP_INQ cmsg/ioctl $*"
> > mptcp_lib_result_pass "TCP_INQ: $*"
>
> Cheers,
> Matt
Hi Geliang, On 28/02/2024 08:57, Geliang Tang wrote: > On Mon, 2024-02-26 at 13:40 +0100, Matthieu Baerts wrote: >> Hi Geliang, >> >> On 26/02/2024 10:43, Geliang Tang wrote: >>> From: Geliang Tang <tanggeliang@kylinos.cn> >>> >>> Only total test results are printed out in mptcp_sockopt.sh: >>> >>> PASS: all packets had packet mark set >>> PASS: SOL_MPTCP getsockopt has expected information >>> PASS: TCP_INQ cmsg/ioctl -t tcp >>> PASS: TCP_INQ cmsg/ioctl -6 -t tcp >>> PASS: TCP_INQ cmsg/ioctl -r tcp >>> PASS: TCP_INQ cmsg/ioctl -6 -r tcp >>> >>> This patch prints more info for every test result in each test >>> group: >>> >>> transfer ipv4 [ OK ] >>> mark ipv4 [ OK ] >>> transfer ipv6 [ OK ] >>> mark ipv6 [ OK ] >>> PASS: all packets had packet mark set >>> sockopt v4 [ OK ] >>> sockopt v6 [ OK ] >>> PASS: SOL_MPTCP getsockopt has expected information >>> TCP_INQ: -t tcp [ OK ] >>> PASS: TCP_INQ cmsg/ioctl -t tcp >>> TCP_INQ: -6 -t tcp [ OK ] >>> PASS: TCP_INQ cmsg/ioctl -6 -t tcp >>> TCP_INQ: -r tcp [ OK ] >>> PASS: TCP_INQ cmsg/ioctl -r tcp >>> TCP_INQ: -6 -r tcp [ OK ] >>> PASS: TCP_INQ cmsg/ioctl -6 -r tcp >>> TCP_INQ: -r tcp -t tcp [ OK ] >> >> Please clearly explain why this is interesting, even if it looks >> obvious. >> >> Here, I don't know if this patch makes sense or not: to me, the >> output >> is now confusing because there is a mix of '[ OK ]' and 'PASS'. >> Maybe: >> >> - remove all the "PASS: xxx"? → I don't see what it brings more >> - (or convert existing "PASS: xxx" and "FAIL: xxx" to use "[ OK ]"? >> But >> there are fewer details) > > This patch uses to match the output of this script: > > INFO: PASS: all packets had packet mark set > INFO: PASS: SOL_MPTCP getsockopt has expected information > INFO: PASS: TCP_INQ cmsg/ioctl -t tcp > INFO: PASS: TCP_INQ cmsg/ioctl -6 -t tcp > INFO: PASS: TCP_INQ cmsg/ioctl -r tcp > INFO: PASS: TCP_INQ cmsg/ioctl -6 -r tcp > INFO: PASS: TCP_INQ cmsg/ioctl -r tcp -t tcp > > with the test results of it: > > ok 1 - mptcp_sockopt: mark ipv4 > ok 2 - mptcp_sockopt: transfer ipv4 > ok 3 - mptcp_sockopt: mark ipv6 > ok 4 - mptcp_sockopt: transfer ipv6 > ok 5 - mptcp_sockopt: sockopt v4 > ok 6 - mptcp_sockopt: sockopt v6 > ok 7 - mptcp_sockopt: TCP_INQ: -t tcp > ok 8 - mptcp_sockopt: TCP_INQ: -6 -t tcp > ok 9 - mptcp_sockopt: TCP_INQ: -r tcp > ok 10 - mptcp_sockopt: TCP_INQ: -6 -r tcp > ok 11 - mptcp_sockopt: TCP_INQ: -r tcp -t tcp > > This patch is prepared for the coming printing counter patch, otherwise > the two output results may have different serial numbers, which can be > confusing. Yes, I agree that it makes sense to print the individual results. But what I wanted to say is that maybe we no longer need these "INFO: PASS:" lines in the output? e.g. only printing this: # 01 Transfer ipv4 [ OK ] # 02 Mark ipv4 [ OK ] # 03 Transfer ipv6 [ OK ] # 04 Mark ipv6 [ OK ] # 05 SOL_MPTCP sockopt v4 [ OK ] # 06 SOL_MPTCP sockopt v6 [ OK ] # 07 TCP_INQ: -t tcp [ OK ] # 08 TCP_INQ: -6 -t tcp [ OK ] # 09 TCP_INQ: -r tcp [ OK ] # 10 TCP_INQ: -6 -r tcp [ OK ] # 11 TCP_INQ: -r tcp -t tcp [ OK ] I think these 'INFO: PASS:' lines don't bring more info, no? If they do, we can always modify what is printed above. (While at it, it might look nicer to do: s/ipv4/IPv4/ (or s/ipv4/v4) if we easily control that, I didn't check) BTW, thank you for the replies on the different patches, it helps here because there were a lot of comments on this v5. Cheers, Matt -- Sponsored by the NGI0 Core fund.
On Wed, 2024-02-28 at 10:13 +0100, Matthieu Baerts wrote: > Hi Geliang, > > On 28/02/2024 08:57, Geliang Tang wrote: > > On Mon, 2024-02-26 at 13:40 +0100, Matthieu Baerts wrote: > > > Hi Geliang, > > > > > > On 26/02/2024 10:43, Geliang Tang wrote: > > > > From: Geliang Tang <tanggeliang@kylinos.cn> > > > > > > > > Only total test results are printed out in mptcp_sockopt.sh: > > > > > > > > PASS: all packets had packet mark set > > > > PASS: SOL_MPTCP getsockopt has expected information > > > > PASS: TCP_INQ cmsg/ioctl -t tcp > > > > PASS: TCP_INQ cmsg/ioctl -6 -t tcp > > > > PASS: TCP_INQ cmsg/ioctl -r tcp > > > > PASS: TCP_INQ cmsg/ioctl -6 -r tcp > > > > > > > > This patch prints more info for every test result in each test > > > > group: > > > > > > > > transfer ipv4 [ > > > > OK ] > > > > mark ipv4 [ > > > > OK ] > > > > transfer ipv6 [ > > > > OK ] > > > > mark ipv6 [ > > > > OK ] > > > > PASS: all packets had packet mark set > > > > sockopt v4 [ > > > > OK ] > > > > sockopt v6 [ > > > > OK ] > > > > PASS: SOL_MPTCP getsockopt has expected information > > > > TCP_INQ: -t tcp [ > > > > OK ] > > > > PASS: TCP_INQ cmsg/ioctl -t tcp > > > > TCP_INQ: -6 -t tcp [ > > > > OK ] > > > > PASS: TCP_INQ cmsg/ioctl -6 -t tcp > > > > TCP_INQ: -r tcp [ > > > > OK ] > > > > PASS: TCP_INQ cmsg/ioctl -r tcp > > > > TCP_INQ: -6 -r tcp [ > > > > OK ] > > > > PASS: TCP_INQ cmsg/ioctl -6 -r tcp > > > > TCP_INQ: -r tcp -t tcp [ > > > > OK ] > > > > > > Please clearly explain why this is interesting, even if it looks > > > obvious. > > > > > > Here, I don't know if this patch makes sense or not: to me, the > > > output > > > is now confusing because there is a mix of '[ OK ]' and 'PASS'. > > > Maybe: > > > > > > - remove all the "PASS: xxx"? → I don't see what it brings more > > > - (or convert existing "PASS: xxx" and "FAIL: xxx" to use "[ OK > > > ]"? > > > But > > > there are fewer details) > > > > This patch uses to match the output of this script: > > > > INFO: PASS: all packets had packet mark set > > INFO: PASS: SOL_MPTCP getsockopt has expected information > > INFO: PASS: TCP_INQ cmsg/ioctl -t tcp > > INFO: PASS: TCP_INQ cmsg/ioctl -6 -t tcp > > INFO: PASS: TCP_INQ cmsg/ioctl -r tcp > > INFO: PASS: TCP_INQ cmsg/ioctl -6 -r tcp > > INFO: PASS: TCP_INQ cmsg/ioctl -r tcp -t tcp > > > > with the test results of it: > > > > ok 1 - mptcp_sockopt: mark ipv4 > > ok 2 - mptcp_sockopt: transfer ipv4 > > ok 3 - mptcp_sockopt: mark ipv6 > > ok 4 - mptcp_sockopt: transfer ipv6 > > ok 5 - mptcp_sockopt: sockopt v4 > > ok 6 - mptcp_sockopt: sockopt v6 > > ok 7 - mptcp_sockopt: TCP_INQ: -t tcp > > ok 8 - mptcp_sockopt: TCP_INQ: -6 -t tcp > > ok 9 - mptcp_sockopt: TCP_INQ: -r tcp > > ok 10 - mptcp_sockopt: TCP_INQ: -6 -r tcp > > ok 11 - mptcp_sockopt: TCP_INQ: -r tcp -t tcp > > > > This patch is prepared for the coming printing counter patch, > > otherwise > > the two output results may have different serial numbers, which can > > be > > confusing. > > Yes, I agree that it makes sense to print the individual results. But > what I wanted to say is that maybe we no longer need these "INFO: > PASS:" > lines in the output? > > e.g. only printing this: > > # 01 Transfer ipv4 [ OK ] > # 02 Mark ipv4 [ OK ] > # 03 Transfer ipv6 [ OK ] > # 04 Mark ipv6 [ OK ] > # 05 SOL_MPTCP sockopt v4 [ OK ] > # 06 SOL_MPTCP sockopt v6 [ OK ] > # 07 TCP_INQ: -t tcp [ OK ] > # 08 TCP_INQ: -6 -t tcp [ OK ] > # 09 TCP_INQ: -r tcp [ OK ] > # 10 TCP_INQ: -6 -r tcp [ OK ] > # 11 TCP_INQ: -r tcp -t tcp [ OK ] Yes, this is much better. Will update it in next version. > > I think these 'INFO: PASS:' lines don't bring more info, no? If they > do, > we can always modify what is printed above. > > (While at it, it might look nicer to do: s/ipv4/IPv4/ (or s/ipv4/v4) > if > we easily control that, I didn't check) 's/ipv4/v4' is better, will update them. Thanks, -Geliang > > > BTW, thank you for the replies on the different patches, it helps > here > because there were a lot of comments on this v5. > > Cheers, > Matt
© 2016 - 2026 Red Hat, Inc.