* [PATCH mptcp-next] selftests: mptcp: tweak simult_flows for debug kernels.
@ 2022-06-16 13:55 Paolo Abeni
2022-06-16 15:27 ` selftests: mptcp: tweak simult_flows for debug kernels.: Tests Results MPTCP CI
2022-06-20 16:38 ` [PATCH mptcp-next] selftests: mptcp: tweak simult_flows for debug kernels Matthieu Baerts
0 siblings, 2 replies; 5+ messages in thread
From: Paolo Abeni @ 2022-06-16 13:55 UTC (permalink / raw)
To: mptcp
The mentioned test measures the transfer run-time to verify
that the user-space program is able to use the full aggregate B/W.
Even on (virtual) link-speed-bound tests, debug kernel can slow
down the transfer enough to cause sporadic test failures.
Instead of unconditionally raising the maximum allowed run-time,
tweak when the running kernel is a debug one, and use some simple/
rough heuristic to guess such scenarios.
Note: this intentionally avoids looking for /boot/config-<version> as
the latter file is not always available in our reference CI
environments.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
tools/testing/selftests/net/mptcp/simult_flows.sh | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index f441ff7904fc..141fcf0d40d1 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -12,6 +12,7 @@ timeout_test=$((timeout_poll * 2 + 1))
test_cnt=1
ret=0
bail=0
+slack=50
usage() {
echo "Usage: $0 [ -b ] [ -c ] [ -d ]"
@@ -52,6 +53,7 @@ setup()
cout=$(mktemp)
capout=$(mktemp)
size=$((2 * 2048 * 4096))
+
dd if=/dev/zero of=$small bs=4096 count=20 >/dev/null 2>&1
dd if=/dev/zero of=$large bs=4096 count=$((size / 4096)) >/dev/null 2>&1
@@ -104,6 +106,13 @@ setup()
ip -net "$ns3" route add default via dead:beef:3::2
ip netns exec "$ns3" ./pm_nl_ctl limits 1 1
+
+ # debug build can slow down measurably the test program
+ # we use quite tight time limit on the run-time, to ensure
+ # maximum B/W usage.
+ # Use the kmemleak file presence as a rough estimate for this being
+ # a debug kernel and increase the maximum run-time accordingly
+ [ -f /sys/kernel/debug/kmemleak ] && slack=$((slack+200))
}
# $1: ns, $2: port
@@ -241,7 +250,7 @@ run_test()
# mptcp_connect will do some sleeps to allow the mp_join handshake
# completion (see mptcp_connect): 200ms on each side, add some slack
- time=$((time + 450))
+ time=$((time + 400 + $slack))
printf "%-60s" "$msg"
do_transfer $small $large $time
--
2.35.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: selftests: mptcp: tweak simult_flows for debug kernels.: Tests Results
2022-06-16 13:55 [PATCH mptcp-next] selftests: mptcp: tweak simult_flows for debug kernels Paolo Abeni
@ 2022-06-16 15:27 ` MPTCP CI
2022-06-17 22:13 ` several messages Mat Martineau
2022-06-20 16:38 ` [PATCH mptcp-next] selftests: mptcp: tweak simult_flows for debug kernels Matthieu Baerts
1 sibling, 1 reply; 5+ messages in thread
From: MPTCP CI @ 2022-06-16 15:27 UTC (permalink / raw)
To: Paolo Abeni; +Cc: mptcp
Hi Paolo,
Thank you for your modifications, that's great!
Our CI did some validations and here is its report:
- KVM Validation: normal:
- Success! ✅:
- Task: https://cirrus-ci.com/task/5708948505362432
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5708948505362432/summary/summary.txt
- KVM Validation: debug:
- Unstable: 3 failed test(s): packetdrill_add_addr selftest_diag selftest_mptcp_join 🔴:
- Task: https://cirrus-ci.com/task/5145998551941120
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5145998551941120/summary/summary.txt
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/727243b29682
If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:
$ cd [kernel source code]
$ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
--pull always mptcp/mptcp-upstream-virtme-docker:latest \
auto-debug
For more details:
https://github.com/multipath-tcp/mptcp-upstream-virtme-docker
Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)
Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: several messages
2022-06-16 15:27 ` selftests: mptcp: tweak simult_flows for debug kernels.: Tests Results MPTCP CI
@ 2022-06-17 22:13 ` Mat Martineau
0 siblings, 0 replies; 5+ messages in thread
From: Mat Martineau @ 2022-06-17 22:13 UTC (permalink / raw)
To: Paolo Abeni, mptcp
[-- Attachment #1: Type: text/plain, Size: 3986 bytes --]
On Thu, 16 Jun 2022, Paolo Abeni wrote:
> The mentioned test measures the transfer run-time to verify
> that the user-space program is able to use the full aggregate B/W.
>
> Even on (virtual) link-speed-bound tests, debug kernel can slow
> down the transfer enough to cause sporadic test failures.
>
> Instead of unconditionally raising the maximum allowed run-time,
> tweak when the running kernel is a debug one, and use some simple/
> rough heuristic to guess such scenarios.
>
> Note: this intentionally avoids looking for /boot/config-<version> as
> the latter file is not always available in our reference CI
> environments.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Looks good, runs fine in my vm with debug kernel config:
Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---
> tools/testing/selftests/net/mptcp/simult_flows.sh | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
> index f441ff7904fc..141fcf0d40d1 100755
> --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
> +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
> @@ -12,6 +12,7 @@ timeout_test=$((timeout_poll * 2 + 1))
> test_cnt=1
> ret=0
> bail=0
> +slack=50
>
> usage() {
> echo "Usage: $0 [ -b ] [ -c ] [ -d ]"
> @@ -52,6 +53,7 @@ setup()
> cout=$(mktemp)
> capout=$(mktemp)
> size=$((2 * 2048 * 4096))
> +
> dd if=/dev/zero of=$small bs=4096 count=20 >/dev/null 2>&1
> dd if=/dev/zero of=$large bs=4096 count=$((size / 4096)) >/dev/null 2>&1
>
> @@ -104,6 +106,13 @@ setup()
> ip -net "$ns3" route add default via dead:beef:3::2
>
> ip netns exec "$ns3" ./pm_nl_ctl limits 1 1
> +
> + # debug build can slow down measurably the test program
> + # we use quite tight time limit on the run-time, to ensure
> + # maximum B/W usage.
> + # Use the kmemleak file presence as a rough estimate for this being
> + # a debug kernel and increase the maximum run-time accordingly
> + [ -f /sys/kernel/debug/kmemleak ] && slack=$((slack+200))
> }
>
> # $1: ns, $2: port
> @@ -241,7 +250,7 @@ run_test()
>
> # mptcp_connect will do some sleeps to allow the mp_join handshake
> # completion (see mptcp_connect): 200ms on each side, add some slack
> - time=$((time + 450))
> + time=$((time + 400 + $slack))
>
> printf "%-60s" "$msg"
> do_transfer $small $large $time
> --
> 2.35.3
>
>
>
On Thu, 16 Jun 2022, MPTCP CI wrote:
> Hi Paolo,
>
> Thank you for your modifications, that's great!
>
> Our CI did some validations and here is its report:
>
> - KVM Validation: normal:
> - Success! ✅:
> - Task: https://cirrus-ci.com/task/5708948505362432
> - Summary: https://api.cirrus-ci.com/v1/artifact/task/5708948505362432/summary/summary.txt
>
> - KVM Validation: debug:
> - Unstable: 3 failed test(s): packetdrill_add_addr selftest_diag selftest_mptcp_join 🔴:
> - Task: https://cirrus-ci.com/task/5145998551941120
> - Summary: https://api.cirrus-ci.com/v1/artifact/task/5145998551941120/summary/summary.txt
>
> Initiator: Patchew Applier
> Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/727243b29682
>
>
> If there are some issues, you can reproduce them using the same environment as
> the one used by the CI thanks to a docker image, e.g.:
>
> $ cd [kernel source code]
> $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
> --pull always mptcp/mptcp-upstream-virtme-docker:latest \
> auto-debug
>
> For more details:
>
> https://github.com/multipath-tcp/mptcp-upstream-virtme-docker
>
>
> Please note that despite all the efforts that have been already done to have a
> stable tests suite when executed on a public CI like here, it is possible some
> reported issues are not due to your modifications. Still, do not hesitate to
> help us improve that ;-)
>
> Cheers,
> MPTCP GH Action bot
> Bot operated by Matthieu Baerts (Tessares)
>
>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH mptcp-next] selftests: mptcp: tweak simult_flows for debug kernels.
2022-06-16 13:55 [PATCH mptcp-next] selftests: mptcp: tweak simult_flows for debug kernels Paolo Abeni
2022-06-16 15:27 ` selftests: mptcp: tweak simult_flows for debug kernels.: Tests Results MPTCP CI
@ 2022-06-20 16:38 ` Matthieu Baerts
1 sibling, 0 replies; 5+ messages in thread
From: Matthieu Baerts @ 2022-06-20 16:38 UTC (permalink / raw)
To: Paolo Abeni, mptcp
Hi Paolo, Mat,
On 16/06/2022 15:55, Paolo Abeni wrote:
> The mentioned test measures the transfer run-time to verify
> that the user-space program is able to use the full aggregate B/W.
>
> Even on (virtual) link-speed-bound tests, debug kernel can slow
> down the transfer enough to cause sporadic test failures.
>
> Instead of unconditionally raising the maximum allowed run-time,
> tweak when the running kernel is a debug one, and use some simple/
> rough heuristic to guess such scenarios.
>
> Note: this intentionally avoids looking for /boot/config-<version> as
> the latter file is not always available in our reference CI
> environments.
Thank you for the patch and review!
Now in our tree (feat. for net-next but it could also go in 'fixes for
-net', no?) with Mat's RvB tag and a small diff, see below:
New patches for t/upstream:
- 87eefff9e9e8: selftests: mptcp: tweak simult_flows for debug kernels
- Results: 1445c5fa4907..065936dc9f58 (export)
Builds and tests are now in progress:
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220620T163537
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export
> diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
> index f441ff7904fc..141fcf0d40d1 100755
> --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
> +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
(...)
> @@ -241,7 +250,7 @@ run_test()
>
> # mptcp_connect will do some sleeps to allow the mp_join handshake
> # completion (see mptcp_connect): 200ms on each side, add some slack
> - time=$((time + 450))
> + time=$((time + 400 + $slack))
(detail: I removed the extra '$' before 'slack' just for the uniformity :) )
Cheers,
Matt
>
> printf "%-60s" "$msg"
> do_transfer $small $large $time
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: several messages
2023-06-13 17:37 ` mptcp: drop legacy code.: Tests Results MPTCP CI
@ 2023-06-16 22:54 ` Mat Martineau
0 siblings, 0 replies; 5+ messages in thread
From: Mat Martineau @ 2023-06-16 22:54 UTC (permalink / raw)
To: Paolo Abeni, mptcp
[-- Attachment #1: Type: text/plain, Size: 6401 bytes --]
On Mon, 12 Jun 2023, Paolo Abeni wrote:
> Thanks to the previous patch we can finally drop the "temporary hack"
> used to detect rx eof.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> "previous patch" should be replace with a proper commit reference
> once such patch is merged on -net, unless we target both patches on
> the same tree (but this one is really net-next material, while the fix
> is for -net)
Hi Paolo,
To be clear, the "previous patch" is "mptcp: consolidate fallback and non
fallback state machine"?
> ---
> net/mptcp/protocol.c | 49 --------------------------------------------
> net/mptcp/protocol.h | 5 +----
> 2 files changed, 1 insertion(+), 53 deletions(-)
Hooray for deleting code!
Reviewed-by: Mat Martineau <martineau@kernel.org>
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 8f3e50065c13..feaedfd2b3eb 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -898,49 +898,6 @@ bool mptcp_schedule_work(struct sock *sk)
> return false;
> }
>
> -void mptcp_subflow_eof(struct sock *sk)
> -{
> - if (!test_and_set_bit(MPTCP_WORK_EOF, &mptcp_sk(sk)->flags))
> - mptcp_schedule_work(sk);
> -}
> -
> -static void mptcp_check_for_eof(struct mptcp_sock *msk)
> -{
> - struct mptcp_subflow_context *subflow;
> - struct sock *sk = (struct sock *)msk;
> - int receivers = 0;
> -
> - mptcp_for_each_subflow(msk, subflow)
> - receivers += !subflow->rx_eof;
> - if (receivers)
> - return;
> -
> - if (!(sk->sk_shutdown & RCV_SHUTDOWN)) {
> - /* hopefully temporary hack: propagate shutdown status
> - * to msk, when all subflows agree on it
> - */
> - WRITE_ONCE(sk->sk_shutdown, sk->sk_shutdown | RCV_SHUTDOWN);
> -
> - smp_mb__before_atomic(); /* SHUTDOWN must be visible first */
> - sk->sk_data_ready(sk);
> - }
> -
> - switch (sk->sk_state) {
> - case TCP_ESTABLISHED:
> - inet_sk_state_store(sk, TCP_CLOSE_WAIT);
> - break;
> - case TCP_FIN_WAIT1:
> - inet_sk_state_store(sk, TCP_CLOSING);
> - break;
> - case TCP_FIN_WAIT2:
> - inet_sk_state_store(sk, TCP_CLOSE);
> - break;
> - default:
> - return;
> - }
> - mptcp_close_wake_up(sk);
> -}
> -
> static struct sock *mptcp_subflow_recv_lookup(const struct mptcp_sock *msk)
> {
> struct mptcp_subflow_context *subflow;
> @@ -2193,9 +2150,6 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> break;
> }
>
> - if (test_and_clear_bit(MPTCP_WORK_EOF, &msk->flags))
> - mptcp_check_for_eof(msk);
> -
> if (sk->sk_shutdown & RCV_SHUTDOWN) {
> /* race breaker: the shutdown could be after the
> * previous receive queue check
> @@ -2726,9 +2680,6 @@ static void mptcp_worker(struct work_struct *work)
>
> mptcp_pm_nl_work(msk);
>
> - if (test_and_clear_bit(MPTCP_WORK_EOF, &msk->flags))
> - mptcp_check_for_eof(msk);
> -
> mptcp_check_send_data_fin(sk);
> mptcp_check_data_fin_ack(sk);
> mptcp_check_data_fin(sk);
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index d2e59cf33f57..528586e2ed73 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -113,7 +113,6 @@
> /* MPTCP socket atomic flags */
> #define MPTCP_NOSPACE 1
> #define MPTCP_WORK_RTX 2
> -#define MPTCP_WORK_EOF 3
> #define MPTCP_FALLBACK_DONE 4
> #define MPTCP_WORK_CLOSE_SUBFLOW 5
>
> @@ -481,14 +480,13 @@ struct mptcp_subflow_context {
> send_mp_fail : 1,
> send_fastclose : 1,
> send_infinite_map : 1,
> - rx_eof : 1,
> remote_key_valid : 1, /* received the peer key from */
> disposable : 1, /* ctx can be free at ulp release time */
> stale : 1, /* unable to snd/rcv data, do not use for xmit */
> local_id_valid : 1, /* local_id is correctly initialized */
> valid_csum_seen : 1, /* at least one csum validated */
> is_mptfo : 1, /* subflow is doing TFO */
> - __unused : 8;
> + __unused : 9;
> enum mptcp_data_avail data_avail;
> bool scheduled;
> u32 remote_nonce;
> @@ -744,7 +742,6 @@ static inline u64 mptcp_expand_seq(u64 old_seq, u64 cur_seq, bool use_64bit)
> void __mptcp_check_push(struct sock *sk, struct sock *ssk);
> void __mptcp_data_acked(struct sock *sk);
> void __mptcp_error_report(struct sock *sk);
> -void mptcp_subflow_eof(struct sock *sk);
> bool mptcp_update_rcv_data_fin(struct mptcp_sock *msk, u64 data_fin_seq, bool use_64bit);
> static inline bool mptcp_data_fin_enabled(const struct mptcp_sock *msk)
> {
> --
> 2.40.1
>
>
>
On Tue, 13 Jun 2023, MPTCP CI wrote:
> Hi Paolo,
>
> Thank you for your modifications, that's great!
>
> Our CI did some validations and here is its report:
>
> - KVM Validation: normal (except selftest_mptcp_join):
> - Success! ✅:
> - Task: https://cirrus-ci.com/task/6108358801883136
> - Summary: https://api.cirrus-ci.com/v1/artifact/task/6108358801883136/summary/summary.txt
>
> - KVM Validation: debug (only selftest_mptcp_join):
> - Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
> - Task: https://cirrus-ci.com/task/4630615174152192
> - Summary: https://api.cirrus-ci.com/v1/artifact/task/4630615174152192/summary/summary.txt
>
> - KVM Validation: normal (only selftest_mptcp_join):
> - Success! ✅:
> - Task: https://cirrus-ci.com/task/5545408848461824
> - Summary: https://api.cirrus-ci.com/v1/artifact/task/5545408848461824/summary/summary.txt
>
> - KVM Validation: debug (except selftest_mptcp_join):
> - Success! ✅:
> - Task: https://cirrus-ci.com/task/6671308755304448
> - Summary: https://api.cirrus-ci.com/v1/artifact/task/6671308755304448/summary/summary.txt
>
> Initiator: Matthieu Baerts
> Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/bdbf7858d22c
>
>
> If there are some issues, you can reproduce them using the same environment as
> the one used by the CI thanks to a docker image, e.g.:
>
> $ cd [kernel source code]
> $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
> --pull always mptcp/mptcp-upstream-virtme-docker:latest \
> auto-debug
>
> For more details:
>
> https://github.com/multipath-tcp/mptcp-upstream-virtme-docker
>
>
> Please note that despite all the efforts that have been already done to have a
> stable tests suite when executed on a public CI like here, it is possible some
> reported issues are not due to your modifications. Still, do not hesitate to
> help us improve that ;-)
>
> Cheers,
> MPTCP GH Action bot
> Bot operated by Matthieu Baerts (Tessares)
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-16 22:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-16 13:55 [PATCH mptcp-next] selftests: mptcp: tweak simult_flows for debug kernels Paolo Abeni
2022-06-16 15:27 ` selftests: mptcp: tweak simult_flows for debug kernels.: Tests Results MPTCP CI
2022-06-17 22:13 ` several messages Mat Martineau
2022-06-20 16:38 ` [PATCH mptcp-next] selftests: mptcp: tweak simult_flows for debug kernels Matthieu Baerts
2023-06-12 16:02 [PATCH mptcp-next] mptcp: drop legacy code Paolo Abeni
2023-06-13 17:37 ` mptcp: drop legacy code.: Tests Results MPTCP CI
2023-06-16 22:54 ` several messages Mat Martineau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).