1. Patchew
  2. MPTCP
  3. [PATCH mptcp-next v3 0/2] fixes for "map v4 address to v6"
  4. View patch

[PATCH mptcp-next v3 2/2] selftests: mptcp: rm subflow with v4/v4mapped addr

Geliang Tang posted 2 patches 11 months, 1 week ago
[PATCH mptcp-next v3 2/2] selftests: mptcp: rm subflow with v4/v4mapped addr
Posted by Geliang Tang 11 months, 1 week ago 
From: Geliang Tang <tanggeliang@kylinos.cn>

Now both a v4 address and a v4-mapped address are supported when
destroying a userspace pm subflow, this patch adds a second subflow
to "userspace pm add & remove address" test, and two subflows could
be removed two different ways, one with the v4mapped and one with v4.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/387
Fixes: 48d73f609dcc ("selftests: mptcp: update userspace pm addr tests")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 28 +++++++++++--------
 .../testing/selftests/net/mptcp/mptcp_lib.sh  |  7 +++--
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index c07386e21e0a..e68b1bc2c2e4 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3333,16 +3333,17 @@ userspace_pm_rm_sf()
 {
 	local evts=$evts_ns1
 	local t=${3:-1}
-	local ip=4
+	local ip
 	local tk da dp sp
 	local cnt
 
 	[ "$1" == "$ns2" ] && evts=$evts_ns2
-	if mptcp_lib_is_v6 $2; then ip=6; fi
+	[ -n "$(mptcp_lib_evts_get_info "saddr4" "$evts" $t)" ] && ip=4
+	[ -n "$(mptcp_lib_evts_get_info "saddr6" "$evts" $t)" ] && ip=6
 	tk=$(mptcp_lib_evts_get_info token "$evts")
-	da=$(mptcp_lib_evts_get_info "daddr$ip" "$evts" $t)
-	dp=$(mptcp_lib_evts_get_info dport "$evts" $t)
-	sp=$(mptcp_lib_evts_get_info sport "$evts" $t)
+	da=$(mptcp_lib_evts_get_info "daddr$ip" "$evts" $t $2)
+	dp=$(mptcp_lib_evts_get_info dport "$evts" $t $2)
+	sp=$(mptcp_lib_evts_get_info sport "$evts" $t $2)
 
 	cnt=$(rm_sf_count ${1})
 	ip netns exec $1 ./pm_nl_ctl dsf lip $2 lport $sp \
@@ -3429,20 +3430,23 @@ userspace_tests()
 	if reset_with_events "userspace pm add & remove address" &&
 	   continue_if mptcp_lib_has_file '/proc/sys/net/mptcp/pm_type'; then
 		set_userspace_pm $ns1
-		pm_nl_set_limits $ns2 1 1
+		pm_nl_set_limits $ns2 2 2
 		speed=5 \
 			run_tests $ns1 $ns2 10.0.1.1 &
 		local tests_pid=$!
 		wait_mpj $ns1
 		userspace_pm_add_addr $ns1 10.0.2.1 10
-		chk_join_nr 1 1 1
-		chk_add_nr 1 1
-		chk_mptcp_info subflows 1 subflows 1
-		chk_subflows_total 2 2
-		chk_mptcp_info add_addr_signal 1 add_addr_accepted 1
+		userspace_pm_add_addr $ns1 10.0.3.1 20
+		chk_join_nr 2 2 2
+		chk_add_nr 2 2
+		chk_mptcp_info subflows 2 subflows 2
+		chk_subflows_total 3 3
+		chk_mptcp_info add_addr_signal 2 add_addr_accepted 2
 		userspace_pm_rm_addr $ns1 10
 		userspace_pm_rm_sf $ns1 "::ffff:10.0.2.1" $SUB_ESTABLISHED
-		chk_rm_nr 1 1 invert
+		userspace_pm_rm_addr $ns1 20
+		userspace_pm_rm_sf $ns1 10.0.3.1 $SUB_ESTABLISHED
+		chk_rm_nr 2 2 invert
 		chk_mptcp_info subflows 0 subflows 0
 		chk_subflows_total 1 1
 		kill_events_pids
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 3a2abae5993e..69001d2a8dab 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -213,9 +213,12 @@ mptcp_lib_get_info_value() {
 	grep "${2}" | sed -n 's/.*\('"${1}"':\)\([0-9a-f:.]*\).*$/\2/p;q'
 }
 
-# $1: info name ; $2: evts_ns ; $3: event type
+# $1: info name ; $2: evts_ns ; $3: event type; $4: addr
 mptcp_lib_evts_get_info() {
-	mptcp_lib_get_info_value "${1}" "^type:${3:-1}," < "${2}"
+	local addr=${4:-""}
+
+	cat "${2}" | grep "${addr}" |
+	mptcp_lib_get_info_value "${1}" "^type:${3:-1},"
 }
 
 # $1: PID
-- 
2.40.1
Re: [PATCH mptcp-next v3 2/2] selftests: mptcp: rm subflow with v4/v4mapped addr
Posted by Matthieu Baerts 11 months ago 
Hi Geliang,

On 07/02/2024 02:55, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Now both a v4 address and a v4-mapped address are supported when
> destroying a userspace pm subflow, this patch adds a second subflow
> to "userspace pm add & remove address" test, and two subflows could
> be removed two different ways, one with the v4mapped and one with v4.

Thank you for validating this.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 3a2abae5993e..69001d2a8dab 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -213,9 +213,12 @@ mptcp_lib_get_info_value() {
>  	grep "${2}" | sed -n 's/.*\('"${1}"':\)\([0-9a-f:.]*\).*$/\2/p;q'
>  }
>  
> -# $1: info name ; $2: evts_ns ; $3: event type
> +# $1: info name ; $2: evts_ns ; $3: event type; $4: addr
>  mptcp_lib_evts_get_info() {
> -	mptcp_lib_get_info_value "${1}" "^type:${3:-1}," < "${2}"
> +	local addr=${4:-""}
> +
> +	cat "${2}" | grep "${addr}" |
> +	mptcp_lib_get_info_value "${1}" "^type:${3:-1},"

When modifying shell scripts, please make sure 'shellcheck' is happy
with the modifications you did.

> $ shellcheck -x mptcp_lib.sh
> 
> In mptcp_lib.sh line 220:
>         cat "${2}" | grep "${addr}" |
>             ^----^ SC2002 (style): Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead.
> 
> For more information:
>   https://www.shellcheck.net/wiki/SC2002 -- Useless cat. Consider 'cmd < file...

I then did this modification when applying the patch:

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 69001d2a8dab..3777d66fc56d 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -213,12 +213,9 @@ mptcp_lib_get_info_value() {
>         grep "${2}" | sed -n 's/.*\('"${1}"':\)\([0-9a-f:.]*\).*$/\2/p;q'
>  }
>  
> -# $1: info name ; $2: evts_ns ; $3: event type; $4: addr
> +# $1: info name ; $2: evts_ns ; [$3: event type; [$4: addr]]
>  mptcp_lib_evts_get_info() {
> -       local addr=${4:-""}
> -
> -       cat "${2}" | grep "${addr}" |
> -       mptcp_lib_get_info_value "${1}" "^type:${3:-1},"
> +       grep "${4:-}" "${2}" | mptcp_lib_get_info_value "${1}" "^type:${3:-1},"
>  }
>  
>  # $1: PID

Note that it is not strictly needed to add ':-' in '${4:-}' (we don't
use 'set -u'), but it clearly shows the argument is optional.

I also edited the comment above the declaration of the function to
reflect that.

About 'shellcheck', please note that for the moment, not all scripts are
shellcheck compliant, only 'mptcp_join.sh', 'mptcp_lib.sh' and
'userspace_pm.sh'. For the others, it would be great if at the least the
new code is shellcheck compliant. I don't think it is needed to modify
the existing ones, they are not often modified and they work. (Except
maybe if there are some other big modifications needed.)

Also, some checks can be ignored with '# shellcheck disable=XXXX'
directive if needed:

  https://www.shellcheck.net/wiki/Ignore

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH mptcp-next v3 2/2] selftests: mptcp: rm subflow with v4/v4mapped addr
Posted by Geliang Tang 11 months ago 
Hi Matt,

On Wed, Feb 14, 2024 at 11:18:04AM +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 07/02/2024 02:55, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > Now both a v4 address and a v4-mapped address are supported when
> > destroying a userspace pm subflow, this patch adds a second subflow
> > to "userspace pm add & remove address" test, and two subflows could
> > be removed two different ways, one with the v4mapped and one with v4.
> 
> Thank you for validating this.
> 
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > index 3a2abae5993e..69001d2a8dab 100644
> > --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > @@ -213,9 +213,12 @@ mptcp_lib_get_info_value() {
> >  	grep "${2}" | sed -n 's/.*\('"${1}"':\)\([0-9a-f:.]*\).*$/\2/p;q'
> >  }
> >  
> > -# $1: info name ; $2: evts_ns ; $3: event type
> > +# $1: info name ; $2: evts_ns ; $3: event type; $4: addr
> >  mptcp_lib_evts_get_info() {
> > -	mptcp_lib_get_info_value "${1}" "^type:${3:-1}," < "${2}"
> > +	local addr=${4:-""}
> > +
> > +	cat "${2}" | grep "${addr}" |
> > +	mptcp_lib_get_info_value "${1}" "^type:${3:-1},"
> 
> When modifying shell scripts, please make sure 'shellcheck' is happy
> with the modifications you did.

Sure, I'll use shellcheck to check my script modifications next time.

> 
> > $ shellcheck -x mptcp_lib.sh
> > 
> > In mptcp_lib.sh line 220:
> >         cat "${2}" | grep "${addr}" |
> >             ^----^ SC2002 (style): Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead.
> > 
> > For more information:
> >   https://www.shellcheck.net/wiki/SC2002 -- Useless cat. Consider 'cmd < file...
> 
> I then did this modification when applying the patch:

Thanks a lot.

-Geliang

> 
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > index 69001d2a8dab..3777d66fc56d 100644
> > --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > @@ -213,12 +213,9 @@ mptcp_lib_get_info_value() {
> >         grep "${2}" | sed -n 's/.*\('"${1}"':\)\([0-9a-f:.]*\).*$/\2/p;q'
> >  }
> >  
> > -# $1: info name ; $2: evts_ns ; $3: event type; $4: addr
> > +# $1: info name ; $2: evts_ns ; [$3: event type; [$4: addr]]
> >  mptcp_lib_evts_get_info() {
> > -       local addr=${4:-""}
> > -
> > -       cat "${2}" | grep "${addr}" |
> > -       mptcp_lib_get_info_value "${1}" "^type:${3:-1},"
> > +       grep "${4:-}" "${2}" | mptcp_lib_get_info_value "${1}" "^type:${3:-1},"
> >  }
> >  
> >  # $1: PID
> 
> Note that it is not strictly needed to add ':-' in '${4:-}' (we don't
> use 'set -u'), but it clearly shows the argument is optional.
> 
> I also edited the comment above the declaration of the function to
> reflect that.
> 
> About 'shellcheck', please note that for the moment, not all scripts are
> shellcheck compliant, only 'mptcp_join.sh', 'mptcp_lib.sh' and
> 'userspace_pm.sh'. For the others, it would be great if at the least the
> new code is shellcheck compliant. I don't think it is needed to modify
> the existing ones, they are not often modified and they work. (Except
> maybe if there are some other big modifications needed.)
> 
> Also, some checks can be ignored with '# shellcheck disable=XXXX'
> directive if needed:
> 
>   https://www.shellcheck.net/wiki/Ignore
> 
> Cheers,
> Matt
> -- 
> Sponsored by the NGI0 Core fund.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.