From: Matthieu Baerts <matttbe@kernel.org>
To: Geliang Tang <geliang@kernel.org>, mptcp@lists.linux.dev
Cc: Geliang Tang <tanggeliang@kylinos.cn>
Subject: Re: [PATCH mptcp-next v2 3/3] selftests: mptcp: add last time actions tests
Date: Fri, 29 Mar 2024 17:15:43 +0100 [thread overview]
Message-ID: <d74027a8-fcb7-4dd5-b203-876858a75c49@kernel.org> (raw)
In-Reply-To: <5dd35f7df3332535dc938d1dfcc1c41ee7c717f0.1711702915.git.tanggeliang@kylinos.cn>
Hi Geliang,
On 29/03/2024 10:07, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> This patch adds a new helper chk_msk_info() to show the counters in
> mptcp_info of the given infos, and check that the timestamps move
> forward. Use it to show newly added last_data_sent, last_data_recv
> and last_ack_recv in mptcp_info in diag.sh.
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> tools/testing/selftests/net/mptcp/diag.sh | 24 +++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
> index bc97ab33a00e..6e865f95f85e 100755
> --- a/tools/testing/selftests/net/mptcp/diag.sh
> +++ b/tools/testing/selftests/net/mptcp/diag.sh
> @@ -200,6 +200,27 @@ chk_msk_cestab()
> "${expected}" "${msg}" ""
> }
>
> +chk_msk_info()
> +{
> + local info
> +
> + for info in "${@}"; do
> + local cnt1 cnt2 msg
> +
> + cnt1=$(ss -N ${ns} -inmHM | mptcp_lib_get_info_value "$info" "$info")
Mmh, when there are many connections in parallel or a fallback to TCP,
what are you going to look at? Would it not be better to restrict this
test only when there is one connection ("after MPC handshake"), using a
counter for the client side and one for the server side, using ss
'[sd]port ${port}' filter?
Also, do you need 'ss -m' here?
> + cnt2=$(ss -N ${ns} -inmHM | mptcp_lib_get_info_value "$info" "$info")
Should you maybe wait half a second between the two call of 'ss', just
to be sure data have been exchanged on very slow environments? By doing
that, you can additionally check that the timestamps are different by at
least 250ms, no?
> + msg="....chk ${info:0:15}=$cnt1:$cnt2"
${msg} is used in mptcp_lib_result_XXX(), which mean it is used as title
for the subtest in the TAP format. This title should always have the
same name in case of error or success for the same test. Here, the same
test will always have a different name, that's not OK.
(It is also important to have different names for different subtests, so
if the helper is used multiple time, you cannot just show '${info:0:15}'
here)
> + mptcp_lib_print_title "${msg}"
> + if [ "${cnt1}" -lt "${cnt2}" ]; then
> + mptcp_lib_pr_ok
> + mptcp_lib_result_pass "${msg}"
> + else
> + mptcp_lib_pr_skip
> + mptcp_lib_result_skip "${msg}"
(the fail part is missing, but I see you added it in a squash-to patch
later, I will comment there).
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
next prev parent reply other threads:[~2024-03-29 16:15 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-29 9:06 [PATCH mptcp-next v2 0/3] display "last time" actions info Geliang Tang
2024-03-29 9:06 ` [PATCH mptcp-next v2 1/3] mptcp: add last time fields in mptcp_sock Geliang Tang
2024-03-29 9:07 ` [PATCH mptcp-next v2 2/3] mptcp: add last time fields in mptcp_info Geliang Tang
2024-03-29 9:07 ` [PATCH mptcp-next v2 3/3] selftests: mptcp: add last time actions tests Geliang Tang
2024-03-29 16:15 ` Matthieu Baerts [this message]
2024-03-29 10:00 ` [PATCH mptcp-next v2 0/3] display "last time" actions info MPTCP CI
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d74027a8-fcb7-4dd5-b203-876858a75c49@kernel.org \
--to=matttbe@kernel.org \
--cc=geliang@kernel.org \
--cc=mptcp@lists.linux.dev \
--cc=tanggeliang@kylinos.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.