This patch simplified pm_nl_change_endpoint(), using id-based address
lookups only. And dropped the fragile way of parsing 'addr' and 'id'
from the output of pm_nl_show_endpoints().

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 37 ++++---------------
 1 file changed, 8 insertions(+), 29 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 2ddb373b7c1d..90a6adc36490 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -251,16 +251,6 @@ is_v6()
 	[ -z "${1##*:*}" ]
 }
 
-is_addr()
-{
-	[ -z "${1##*[.:]*}" ]
-}
-
-is_number()
-{
-	[[ $1 == ?(-)+([0-9]) ]]
-}
-
 # $1: ns, $2: port
 wait_local_port_listen()
 {
@@ -390,16 +380,13 @@ pm_nl_show_endpoints()
 pm_nl_change_endpoint()
 {
 	local ns=$1
-	local flags=$2
-	local id=$3
-	local addr=$4
-	local port=""
+	local id=$2
+	local flags=$3
 
 	if [ $ip_mptcp -eq 1 ]; then
 		ip -n $ns mptcp endpoint change id $id ${flags//","/" "}
 	else
-		if [ $5 -ne 0 ]; then port="port $5"; fi
-		ip netns exec $ns ./pm_nl_ctl set $addr flags $flags $port
+		ip netns exec $ns ./pm_nl_ctl set id $id flags $flags
 	fi
 }
 
@@ -602,24 +589,16 @@ do_transfer()
 		for netns in "$ns1" "$ns2"; do
 			pm_nl_show_endpoints $netns | while read line; do
 				local arr=($line)
-				local addr
-				local port=0
+				local nr=0
 				local id
 
 				for i in ${arr[@]}; do
-					if is_addr $i; then
-						addr=$i
-					elif is_number $i; then
-						# The minimum expected port number is 10000
-						if [ $i -gt 10000 ]; then
-							port=$i
-						# The maximum id number is 255
-						elif [ $i -lt 255 ]; then
-							id=$i
-						fi
+					if [ $i = "id" ]; then
+						id=${arr[$nr+1]}
 					fi
+					let nr+=1
 				done
-				pm_nl_change_endpoint $netns $sflags $id $addr $port
+				pm_nl_change_endpoint $netns $id $sflags
 			done
 		done
 	fi
-- 
2.31.1

On Mon, 7 Feb 2022, Geliang Tang wrote:

> This patch simplified pm_nl_change_endpoint(), using id-based address
> lookups only. And dropped the fragile way of parsing 'addr' and 'id'
> from the output of pm_nl_show_endpoints().
>

I agree, this should be less fragile. Thank you Geliang!

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>


> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> .../testing/selftests/net/mptcp/mptcp_join.sh | 37 ++++---------------
> 1 file changed, 8 insertions(+), 29 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 2ddb373b7c1d..90a6adc36490 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -251,16 +251,6 @@ is_v6()
> 	[ -z "${1##*:*}" ]
> }
>
> -is_addr()
> -{
> -	[ -z "${1##*[.:]*}" ]
> -}
> -
> -is_number()
> -{
> -	[[ $1 == ?(-)+([0-9]) ]]
> -}
> -
> # $1: ns, $2: port
> wait_local_port_listen()
> {
> @@ -390,16 +380,13 @@ pm_nl_show_endpoints()
> pm_nl_change_endpoint()
> {
> 	local ns=$1
> -	local flags=$2
> -	local id=$3
> -	local addr=$4
> -	local port=""
> +	local id=$2
> +	local flags=$3
>
> 	if [ $ip_mptcp -eq 1 ]; then
> 		ip -n $ns mptcp endpoint change id $id ${flags//","/" "}
> 	else
> -		if [ $5 -ne 0 ]; then port="port $5"; fi
> -		ip netns exec $ns ./pm_nl_ctl set $addr flags $flags $port
> +		ip netns exec $ns ./pm_nl_ctl set id $id flags $flags
> 	fi
> }
>
> @@ -602,24 +589,16 @@ do_transfer()
> 		for netns in "$ns1" "$ns2"; do
> 			pm_nl_show_endpoints $netns | while read line; do
> 				local arr=($line)
> -				local addr
> -				local port=0
> +				local nr=0
> 				local id
>
> 				for i in ${arr[@]}; do
> -					if is_addr $i; then
> -						addr=$i
> -					elif is_number $i; then
> -						# The minimum expected port number is 10000
> -						if [ $i -gt 10000 ]; then
> -							port=$i
> -						# The maximum id number is 255
> -						elif [ $i -lt 255 ]; then
> -							id=$i
> -						fi
> +					if [ $i = "id" ]; then
> +						id=${arr[$nr+1]}
> 					fi
> +					let nr+=1
> 				done
> -				pm_nl_change_endpoint $netns $sflags $id $addr $port
> +				pm_nl_change_endpoint $netns $id $sflags
> 			done
> 		done
> 	fi
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel

Hi Geliang, Mat,

On 07/02/2022 07:38, Geliang Tang wrote:
> This patch simplified pm_nl_change_endpoint(), using id-based address
> lookups only. And dropped the fragile way of parsing 'addr' and 'id'
> from the output of pm_nl_show_endpoints().

Thank you for the patch and the review!

Now in our tree (feat. for net-next) with Mat's RvB tag:

- dbe081a3e5c2: selftests: mptcp: simplify pm_nl_change_endpoint
- Results: f092cd7abb9f..e98f6976dd01

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220208T094154
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export

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