MPTCP Linux Development
 help / color / mirror / Atom feed
From: Matthieu Baerts <matthieu.baerts@tessares.net>
To: Geliang Tang <geliang.tang@suse.com>, mptcp@lists.linux.dev
Subject: Re: [PATCH mptcp-next v15 7/7] selftests: mptcp: set endpoint out of transfer
Date: Wed, 31 May 2023 17:23:28 +0200	[thread overview]
Message-ID: <14fae2d2-35fa-daf9-e269-4131fffe0ae6@tessares.net> (raw)
In-Reply-To: <46e0ef22b358efde64e3783c4cce79dbcc45f417.1685523463.git.geliang.tang@suse.com>

Hi Geliang,

On 31/05/2023 10:58, Geliang Tang wrote:
> This patch moves endpoint settings out of do_transfer() into a new
> function pm_nl_set_endpoint(), then addr_nr_ns1 and addr_nr_ns2
> arguments can be removed for do_transfer() and run_tests().

(...)

> @@ -2276,7 +2302,7 @@ remove_tests()
>  		pm_nl_set_limits $ns2 0 2
>  		pm_nl_add_endpoint $ns2 10.0.2.2 flags subflow
>  		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
> -		run_tests $ns1 $ns2 10.0.1.1 0 0 -2 slow
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 0 -2 speed_10 "" 1

Why did you switch from slow to speed_10 here? (see below for more
questions)

Same below ↓

>  		chk_join_nr 2 2 2
>  		chk_rm_nr 2 2
>  	fi

(...)

> @@ -2350,7 +2376,7 @@ remove_tests()
>  		pm_nl_set_limits $ns2 1 3
>  		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
>  		pm_nl_add_endpoint $ns2 10.0.4.2 flags subflow
> -		run_tests $ns1 $ns2 10.0.1.1 0 -8 -8 slow
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 -8 -8 speed_10 "" 1
>  		chk_join_nr 3 3 3
>  		chk_add_nr 1 1
>  		chk_rm_nr 1 3 invert simult
> @@ -2363,7 +2389,7 @@ remove_tests()
>  		pm_nl_add_endpoint $ns2 10.0.2.2 flags subflow id 150
>  		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
>  		pm_nl_add_endpoint $ns2 10.0.4.2 flags subflow
> -		run_tests $ns1 $ns2 10.0.1.1 0 -8 -8 slow
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 -8 -8 speed_10 "" 1
>  		chk_join_nr 3 3 3
>  
>  		if mptcp_lib_kversion_ge 5.18; then
> @@ -2381,7 +2407,7 @@ remove_tests()
>  		pm_nl_add_endpoint $ns1 10.0.3.1 flags signal
>  		pm_nl_add_endpoint $ns1 10.0.4.1 flags signal
>  		pm_nl_set_limits $ns2 3 3
> -		run_tests $ns1 $ns2 10.0.1.1 0 -8 -8 slow
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 -8 -8 speed_10 "" 1
>  		chk_join_nr 3 3 3
>  		chk_add_nr 3 3
>  		chk_rm_nr 3 3 invert simult
> @@ -2394,7 +2420,7 @@ remove_tests()
>  		pm_nl_add_endpoint $ns1 10.0.3.1 flags signal
>  		pm_nl_add_endpoint $ns1 10.0.14.1 flags signal
>  		pm_nl_set_limits $ns2 3 3
> -		run_tests $ns1 $ns2 10.0.1.1 0 -8 0 slow
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 -8 0 speed_10 "" 1
>  		chk_join_nr 1 1 1
>  		chk_add_nr 3 3
>  		chk_rm_nr 3 1 invert
> @@ -2405,7 +2431,7 @@ remove_tests()
>  		pm_nl_set_limits $ns1 0 1
>  		pm_nl_set_limits $ns2 0 1
>  		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
> -		run_tests $ns1 $ns2 10.0.1.1 0 0 -9 slow
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 0 -9 speed_10 "" 1
>  		chk_join_nr 1 1 1
>  		chk_rm_nr 1 1
>  	fi
> @@ -2415,7 +2441,7 @@ remove_tests()
>  		pm_nl_set_limits $ns1 0 1
>  		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
>  		pm_nl_set_limits $ns2 1 1
> -		run_tests $ns1 $ns2 10.0.1.1 0 -9 0 slow
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 -9 0 speed_10 "" 1
>  		chk_join_nr 1 1 1
>  		chk_add_nr 1 1
>  		chk_rm_nr 1 1 invert

(...)

> @@ -2501,7 +2527,7 @@ ipv6_tests()
>  		pm_nl_set_limits $ns1 0 1
>  		pm_nl_add_endpoint $ns1 dead:beef:2::1 flags signal
>  		pm_nl_set_limits $ns2 1 1
> -		run_tests $ns1 $ns2 dead:beef:1::1 0 -1 0 slow
> +		run_tests_bg $ns1 $ns2 dead:beef:1::1 0 -1 0 speed_10 "" 1
>  		chk_join_nr 1 1 1
>  		chk_add_nr 1 1
>  		chk_rm_nr 1 1 invert
> @@ -2513,7 +2539,7 @@ ipv6_tests()
>  		pm_nl_add_endpoint $ns1 dead:beef:2::1 flags signal
>  		pm_nl_set_limits $ns2 1 2
>  		pm_nl_add_endpoint $ns2 dead:beef:3::2 dev ns2eth3 flags subflow
> -		run_tests $ns1 $ns2 dead:beef:1::1 0 -1 -1 slow
> +		run_tests_bg $ns1 $ns2 dead:beef:1::1 0 -1 -1 speed_10 "" 1
>  		chk_join_nr 2 2 2
>  		chk_add_nr 1 1
>  		chk_rm_nr 1 1

(...)

> @@ -2647,7 +2673,7 @@ mixed_tests()
>  		pm_nl_set_limits $ns2 2 4
>  		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
>  		pm_nl_add_endpoint $ns1 dead:beef:2::1 flags signal
> -		run_tests $ns1 $ns2 dead:beef:1::1 0 0 fullmesh_1 slow
> +		run_tests_bg $ns1 $ns2 dead:beef:1::1 0 0 fullmesh_1 speed_10 "" 1
>  		chk_join_nr 4 4 4
>  	fi
>  }
> @@ -2660,7 +2686,7 @@ backup_tests()
>  		pm_nl_set_limits $ns1 0 1
>  		pm_nl_set_limits $ns2 0 1
>  		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow,backup
> -		run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow nobackup
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 0 0 speed_10 nobackup 1
>  		chk_join_nr 1 1 1
>  		chk_prio_nr 0 1
>  	fi
> @@ -2671,7 +2697,7 @@ backup_tests()
>  		pm_nl_set_limits $ns1 0 1
>  		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
>  		pm_nl_set_limits $ns2 1 1
> -		run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow backup
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 0 0 speed_10 backup 1
>  		chk_join_nr 1 1 1
>  		chk_add_nr 1 1
>  		chk_prio_nr 1 1
> @@ -2683,7 +2709,7 @@ backup_tests()
>  		pm_nl_set_limits $ns1 0 1
>  		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal port 10100
>  		pm_nl_set_limits $ns2 1 1
> -		run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow backup
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 0 0 speed_10 backup 1
>  		chk_join_nr 1 1 1
>  		chk_add_nr 1 1
>  		chk_prio_nr 1 1

(...)

> @@ -2825,7 +2851,7 @@ add_addr_ports_tests()
>  		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal port 10100
>  		pm_nl_set_limits $ns2 1 2
>  		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
> -		run_tests $ns1 $ns2 10.0.1.1 0 -1 -1 slow
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 -1 -1 speed_10
>  		chk_join_nr 2 2 2
>  		chk_add_nr 1 1 1
>  		chk_rm_nr 1 1
> @@ -2838,7 +2864,7 @@ add_addr_ports_tests()
>  		pm_nl_set_limits $ns2 1 3
>  		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
>  		pm_nl_add_endpoint $ns2 10.0.4.2 flags subflow
> -		run_tests $ns1 $ns2 10.0.1.1 0 -8 -2 slow
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 -8 -2 speed_10 "" 1
>  		chk_join_nr 3 3 3
>  		chk_add_nr 1 1
>  		chk_rm_nr 1 3 invert simult

(...)

> @@ -3122,7 +3148,7 @@ fullmesh_tests()
>  		pm_nl_set_limits $ns1 4 4
>  		pm_nl_set_limits $ns2 4 4
>  		pm_nl_add_endpoint $ns2 10.0.2.2 flags subflow,backup,fullmesh
> -		run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow nobackup,nofullmesh
> +		run_tests_bg $ns1 $ns2 10.0.1.1 0 0 0 speed_10 nobackup,nofullmesh 1
>  		chk_join_nr 2 2 2
>  		chk_prio_nr 0 1
>  		chk_rm_nr 0 1
Does it mean that this selftest will be even slower than before?

(also, that's quite a lot of modifications, it might be a bit annoying
for the backports but I think it will help for the maintenance)

(just an idea: if we want to reduce the number of args, we could also
pass env vars to the different functions)

  speed=speed_10 sfflags=nobackup,nofullmesh wait_join=1 \
      run_tests_bg $ns1 $ns2 10.0.1.1

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

  parent reply	other threads:[~2023-05-31 15:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-31  8:58 [PATCH mptcp-next v15 0/7] update userspace pm mptcp_info fields part 2 Geliang Tang
2023-05-31  8:58 ` [PATCH mptcp-next v15 1/7] selftests: mptcp: skip tests when features are not supported Geliang Tang
2023-05-31  8:58 ` [PATCH mptcp-next v15 2/7] mptcp: pass addr to mptcp_pm_alloc_anno_list Geliang Tang
2023-05-31  8:58 ` [PATCH mptcp-next v15 3/7] selftests: mptcp: test userspace pm out of transfer Geliang Tang
2023-05-31 15:22   ` Matthieu Baerts
2023-05-31  8:58 ` [PATCH mptcp-next v15 4/7] selftests: mptcp: check subflows infos Geliang Tang
2023-05-31  8:58 ` [PATCH mptcp-next v15 5/7] selftests: mptcp: check add_addr infos Geliang Tang
2023-05-31 15:23   ` Matthieu Baerts
2023-05-31  8:58 ` [PATCH mptcp-next v15 6/7] selftests: mptcp: pass fastclose to sflags Geliang Tang
2023-05-31  8:58 ` [PATCH mptcp-next v15 7/7] selftests: mptcp: set endpoint out of transfer Geliang Tang
2023-05-31 10:28   ` selftests: mptcp: set endpoint out of transfer: Tests Results MPTCP CI
2023-05-31 15:23   ` Matthieu Baerts [this message]
2023-05-31 15:21 ` [PATCH mptcp-next v15 0/7] update userspace pm mptcp_info fields part 2 Matthieu Baerts

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=14fae2d2-35fa-daf9-e269-4131fffe0ae6@tessares.net \
    --to=matthieu.baerts@tessares.net \
    --cc=geliang.tang@suse.com \
    --cc=mptcp@lists.linux.dev \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox