[PATCH mptcp-next 10/11] selftests: mptcp: add wrapper for setting flags

Geliang Tang posted 11 patches 4 years ago
Maintainers: Jakub Kicinski <kuba@kernel.org>, "David S. Miller" <davem@davemloft.net>, Mat Martineau <mathew.j.martineau@linux.intel.com>, Matthieu Baerts <matthieu.baerts@tessares.net>, Shuah Khan <shuah@kernel.org>
There is a newer version of this series
[PATCH mptcp-next 10/11] selftests: mptcp: add wrapper for setting flags
Posted by Geliang Tang 4 years ago
This patch implemented a new function named pm_nl_set_endpoint(), wraped
the PM netlink commands 'ip mptcp' and 'pm_nl_ctl' in it, and used a new
argument 'ip_mptcp' to choose which one to use to set the flags of the PM
endpoint.

'ip mptcp' used the ID number argument to find out the address to change
flags, while 'pm_nl_ctl' used the address and port number arguments. So
we need to parse the address ID from the PM dump output as well as the
address and port number.

Used this wrapper in do_transfer() instead of using the pm_nl_ctl command
directly.

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 24cabed18f58..4df7417013d3 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -377,6 +377,22 @@ pm_nl_show_endpoints()
 	fi
 }
 
+pm_nl_change_endpoint()
+{
+	local ns=$1
+	local flags=$2
+	local id=$3
+	local addr=$4
+	local port=""
+
+	if [ $ip_mptcp -eq 1 ]; then
+		ip -n $ns mptcp endpoint change id $id ${flags//","/" "}
+	else
+		[ ! -z $5 ]; port="port $5"
+		ip netns exec $ns ./pm_nl_ctl set $addr flags $flags $port
+	fi
+}
+
 do_transfer()
 {
 	listener_ns="$1"
@@ -578,7 +594,7 @@ do_transfer()
 				local arr=($line)
 				local addr
 				local port=0
-				local _port=""
+				local id
 
 				for i in ${arr[@]}; do
 					if is_addr $i; then
@@ -587,11 +603,13 @@ do_transfer()
 						# 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
 					fi
 				done
-				[ ! -z $port ]; _port="port $port"
-				ip netns exec $netns ./pm_nl_ctl set $addr flags $sflags $_port
+				pm_nl_change_endpoint $netns $sflags $id $addr $port
 			done
 		done
 	fi
-- 
2.31.1


Re: [PATCH mptcp-next 10/11] selftests: mptcp: add wrapper for setting flags
Posted by Mat Martineau 4 years ago
On Fri, 14 Jan 2022, Geliang Tang wrote:

> This patch implemented a new function named pm_nl_set_endpoint(), wraped
> the PM netlink commands 'ip mptcp' and 'pm_nl_ctl' in it, and used a new
> argument 'ip_mptcp' to choose which one to use to set the flags of the PM
> endpoint.
>
> 'ip mptcp' used the ID number argument to find out the address to change
> flags, while 'pm_nl_ctl' used the address and port number arguments. So
> we need to parse the address ID from the PM dump output as well as the
> address and port number.
>
> Used this wrapper in do_transfer() instead of using the pm_nl_ctl command
> directly.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> .../testing/selftests/net/mptcp/mptcp_join.sh | 24 ++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 24cabed18f58..4df7417013d3 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -377,6 +377,22 @@ pm_nl_show_endpoints()
> 	fi
> }
>
> +pm_nl_change_endpoint()
> +{
> +	local ns=$1
> +	local flags=$2
> +	local id=$3
> +	local addr=$4
> +	local port=""
> +
> +	if [ $ip_mptcp -eq 1 ]; then
> +		ip -n $ns mptcp endpoint change id $id ${flags//","/" "}
> +	else
> +		[ ! -z $5 ]; port="port $5"
> +		ip netns exec $ns ./pm_nl_ctl set $addr flags $flags $port
> +	fi
> +}
> +
> do_transfer()
> {
> 	listener_ns="$1"
> @@ -578,7 +594,7 @@ do_transfer()
> 				local arr=($line)
> 				local addr
> 				local port=0
> -				local _port=""
> +				local id
>
> 				for i in ${arr[@]}; do
> 					if is_addr $i; then
> @@ -587,11 +603,13 @@ do_transfer()
> 						# 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

Hi Geliang -

Given that we control pm_nl_ctl (the default tool creating the output to 
parse here), this seems ok to me.

There's a risk that a future version of 'ip mptcp' could have a new number 
field in the output of 'ip mptcp endpoint show' that would confuse this 
script, but since the '-i' flag needs to be used it should be obvious that 
the output of 'ip mptcp' needs to be examined and the comments explain 
what's going on. (So, the 'ip mptcp' parsing is a little fragile here but 
in a way that's not too harmful)

-Mat

> 						fi
> 					fi
> 				done
> -				[ ! -z $port ]; _port="port $port"
> -				ip netns exec $netns ./pm_nl_ctl set $addr flags $sflags $_port
> +				pm_nl_change_endpoint $netns $sflags $id $addr $port
> 			done
> 		done
> 	fi
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next 10/11] selftests: mptcp: add wrapper for setting flags
Posted by Matthieu Baerts 4 years ago
Hi Mat, Geliang,

Thank you for working on that!

On 15/01/2022 01:55, Mat Martineau wrote:
> On Fri, 14 Jan 2022, Geliang Tang wrote:
> 
>> This patch implemented a new function named pm_nl_set_endpoint(), wraped
>> the PM netlink commands 'ip mptcp' and 'pm_nl_ctl' in it, and used a new
>> argument 'ip_mptcp' to choose which one to use to set the flags of the PM
>> endpoint.
>>
>> 'ip mptcp' used the ID number argument to find out the address to change
>> flags, while 'pm_nl_ctl' used the address and port number arguments. So
>> we need to parse the address ID from the PM dump output as well as the
>> address and port number.
>>
>> Used this wrapper in do_transfer() instead of using the pm_nl_ctl command
>> directly.
>>
>> Signed-off-by: Geliang Tang <geliang.tang@suse.com>

(...)

>> @@ -587,11 +603,13 @@ do_transfer()
>>                         # 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
> 
> Hi Geliang -
> 
> Given that we control pm_nl_ctl (the default tool creating the output to
> parse here), this seems ok to me.
> 
> There's a risk that a future version of 'ip mptcp' could have a new
> number field in the output of 'ip mptcp endpoint show' that would
> confuse this script, but since the '-i' flag needs to be used it should
> be obvious that the output of 'ip mptcp' needs to be examined and the
> comments explain what's going on. (So, the 'ip mptcp' parsing is a
> little fragile here but in a way that's not too harmful)

I don't know if it would help as the output would be very different from
pm_nl_ctl but you can also use:

  ip -json mptcp endpoint show

'jq' can be used to help with the parsing.

We could have a pm_nl_XXX() function already parsing the output.

Or we could also modify "./pm_nl_ctl dump" to output data in the same
json format.

But maybe the current version is enough for the moment.

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