[PATCH mptcp-next 0/2] add the id argument for set_flags

[PATCH mptcp-next 0/2] add the id argument for set_flags

[Geliang Tang]

Add the id argument for 'pm_nl_ctl set', and allow to use the port keyword
with the non-signal flags for the setting flags in 'ip mptcp endpoint
change', more commands can be used to set the address flags:

 pm_nl_ctl set 10.0.2.1 flags backup port 10100
 pm_nl_ctl set id 1 flags backup
 ip mptcp endpoint change id 1 backup
 ip mptcp endpoint change 10.0.2.1 backup port 10100

Geliang Tang (2):
  Squash to "selftests: mptcp: add the port argument for set_flags"
  Squash to "selftests: mptcp: add wrapper for setting flags"

 .../testing/selftests/net/mptcp/mptcp_join.sh | 16 ++++-
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 64 +++++++++++++------
 2 files changed, 56 insertions(+), 24 deletions(-)

-- 
2.31.1

[PATCH mptcp-next 1/2] Squash to "selftests: mptcp: add the port argument for set_flags"

[Geliang Tang]

Please update the subject and the commit message:

'''
selftests: mptcp: add port and id for set_flags

This patch added the port and id arguments for setting the address flags
in pm_nl_ctl.

Usage:

    pm_nl_ctl set 10.0.2.1 flags backup port 10100
    pm_nl_ctl set id 1 flags backup
'''

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 64 +++++++++++++------
 1 file changed, 43 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
index 2a57462764d0..e554928487f8 100644
--- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
+++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
@@ -28,7 +28,8 @@ static void syntax(char *argv[])
 	fprintf(stderr, "\tadd [flags signal|subflow|backup|fullmesh] [id <nr>] [dev <name>] <ip>\n");
 	fprintf(stderr, "\tdel <id> [<ip>]\n");
 	fprintf(stderr, "\tget <id>\n");
-	fprintf(stderr, "\tset <ip> [flags backup|nobackup|fullmesh|nofullmesh] [port <nr>]\n");
+	fprintf(stderr, "\tset [<ip>] [id <nr>] [flags backup|nobackup|fullmesh|nofullmesh]\n");
+	fprintf(stderr, "\t                                                     [port <nr>]\n");
 	fprintf(stderr, "\tflush\n");
 	fprintf(stderr, "\tdump\n");
 	fprintf(stderr, "\tlimits [<rcv addr max> <subflow max>]\n");
@@ -657,8 +658,10 @@ int set_flags(int fd, int pm_family, int argc, char *argv[])
 	u_int32_t flags = 0;
 	u_int16_t family;
 	int nest_start;
+	int use_id = 0;
+	u_int8_t id;
 	int off = 0;
-	int arg;
+	int arg = 2;
 
 	memset(data, 0, sizeof(data));
 	nh = (void *)data;
@@ -674,29 +677,45 @@ int set_flags(int fd, int pm_family, int argc, char *argv[])
 	nest->rta_len = RTA_LENGTH(0);
 	off += NLMSG_ALIGN(nest->rta_len);
 
-	/* addr data */
-	rta = (void *)(data + off);
-	if (inet_pton(AF_INET, argv[2], RTA_DATA(rta))) {
-		family = AF_INET;
-		rta->rta_type = MPTCP_PM_ADDR_ATTR_ADDR4;
-		rta->rta_len = RTA_LENGTH(4);
-	} else if (inet_pton(AF_INET6, argv[2], RTA_DATA(rta))) {
-		family = AF_INET6;
-		rta->rta_type = MPTCP_PM_ADDR_ATTR_ADDR6;
-		rta->rta_len = RTA_LENGTH(16);
+	if (!strcmp(argv[arg], "id")) {
+		if (++arg >= argc)
+			error(1, 0, " missing id value");
+
+		use_id = 1;
+		id = atoi(argv[arg]);
+		rta = (void *)(data + off);
+		rta->rta_type = MPTCP_PM_ADDR_ATTR_ID;
+		rta->rta_len = RTA_LENGTH(1);
+		memcpy(RTA_DATA(rta), &id, 1);
+		off += NLMSG_ALIGN(rta->rta_len);
 	} else {
-		error(1, errno, "can't parse ip %s", argv[2]);
+		/* addr data */
+		rta = (void *)(data + off);
+		if (inet_pton(AF_INET, argv[arg], RTA_DATA(rta))) {
+			family = AF_INET;
+			rta->rta_type = MPTCP_PM_ADDR_ATTR_ADDR4;
+			rta->rta_len = RTA_LENGTH(4);
+		} else if (inet_pton(AF_INET6, argv[arg], RTA_DATA(rta))) {
+			family = AF_INET6;
+			rta->rta_type = MPTCP_PM_ADDR_ATTR_ADDR6;
+			rta->rta_len = RTA_LENGTH(16);
+		} else {
+			error(1, errno, "can't parse ip %s", argv[arg]);
+		}
+		off += NLMSG_ALIGN(rta->rta_len);
+
+		/* family */
+		rta = (void *)(data + off);
+		rta->rta_type = MPTCP_PM_ADDR_ATTR_FAMILY;
+		rta->rta_len = RTA_LENGTH(2);
+		memcpy(RTA_DATA(rta), &family, 2);
+		off += NLMSG_ALIGN(rta->rta_len);
 	}
-	off += NLMSG_ALIGN(rta->rta_len);
 
-	/* family */
-	rta = (void *)(data + off);
-	rta->rta_type = MPTCP_PM_ADDR_ATTR_FAMILY;
-	rta->rta_len = RTA_LENGTH(2);
-	memcpy(RTA_DATA(rta), &family, 2);
-	off += NLMSG_ALIGN(rta->rta_len);
+	if (++arg >= argc)
+		error(1, 0, " missing flags keyword");
 
-	for (arg = 3; arg < argc; arg++) {
+	for (; arg < argc; arg++) {
 		if (!strcmp(argv[arg], "flags")) {
 			char *tok, *str;
 
@@ -724,6 +743,9 @@ int set_flags(int fd, int pm_family, int argc, char *argv[])
 		} else if (!strcmp(argv[arg], "port")) {
 			u_int16_t port;
 
+			if (use_id)
+				error(1, 0, " port can't be used with id");
+
 			if (++arg >= argc)
 				error(1, 0, " missing port value");
 
-- 
2.31.1

[PATCH mptcp-next 2/2] Squash to "selftests: mptcp: add wrapper for setting flags"

[Geliang Tang]

Please update the commit message:

'''
This patch implemented a new function named pm_nl_set_endpoint(), wrap
the PM netlink commands 'ip mptcp endpoint change flags' and 'pm_nl_ctl
set flags' in it, use a new argument 'ip_mptcp' to choose which one to
use to set the flags of the PM endpoint. Randomly choose the 'addr' or
'id' as arguments for each command.

Parse the address ID from the PM dump output as well as the address and
port number, pass them to this wrapper in do_transfer() instead of using
the pm_nl_ctl command directly.
'''

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index cd45b854ff6e..ff82ce6bf81f 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -457,12 +457,22 @@ pm_nl_change_endpoint()
 	local id=$3
 	local addr=$4
 	local port=""
+	local use_id=$((RANDOM%2))
+
+	if [ $5 -ne 0 ]; then port="port $5"; fi
 
 	if [ $ip_mptcp -eq 1 ]; then
-		ip -n $ns mptcp endpoint change id $id ${flags//","/" "}
+		if [ $use_id -eq 1 ]; then
+			ip -n $ns mptcp endpoint change id $id ${flags//","/" "}
+		else
+			ip -n $ns mptcp endpoint change $addr ${flags//","/" "} $port
+		fi
 	else
-		if [ $5 -ne 0 ]; then port="port $5"; fi
-		ip netns exec $ns ./pm_nl_ctl set $addr flags $flags $port
+		if [ $use_id -eq 1 ]; then
+			ip netns exec $ns ./pm_nl_ctl set id $id flags $flags
+		else
+			ip netns exec $ns ./pm_nl_ctl set $addr flags $flags $port
+		fi
 	fi
 }
 
-- 
2.31.1

Re: [PATCH mptcp-next 2/2] Squash to "selftests: mptcp: add wrapper for setting flags"

[Mat Martineau]

On Fri, 21 Jan 2022, Geliang Tang wrote:

> Please update the commit message:
>
> '''
> This patch implemented a new function named pm_nl_set_endpoint(), wrap
> the PM netlink commands 'ip mptcp endpoint change flags' and 'pm_nl_ctl
> set flags' in it, use a new argument 'ip_mptcp' to choose which one to
> use to set the flags of the PM endpoint. Randomly choose the 'addr' or
> 'id' as arguments for each command.
>
> Parse the address ID from the PM dump output as well as the address and
> port number, pass them to this wrapper in do_transfer() instead of using
> the pm_nl_ctl command directly.
> '''
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index cd45b854ff6e..ff82ce6bf81f 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -457,12 +457,22 @@ pm_nl_change_endpoint()
> 	local id=$3
> 	local addr=$4
> 	local port=""
> +	local use_id=$((RANDOM%2))

I know the full set of mptcp_join.sh test cases takes a long time to run 
and I'm guessing this randomization is added to cover both id- and 
addr-based lookup without increasing the number of test cases. Other uses 
of RANDOM are mostly for sizes, but it seems like using it here would 
unnecessarily make the tests less repeatable.

In this script, both $addr and $id are available in 
pm_nl_change_endpoint() - my suggestion is to pick one of them to use.

pm_netlink.sh might be a better place to add test cases to get code 
coverage for both addr-based and id-based lookups.


-Mat

> +
> +	if [ $5 -ne 0 ]; then port="port $5"; fi
>
> 	if [ $ip_mptcp -eq 1 ]; then
> -		ip -n $ns mptcp endpoint change id $id ${flags//","/" "}
> +		if [ $use_id -eq 1 ]; then
> +			ip -n $ns mptcp endpoint change id $id ${flags//","/" "}
> +		else
> +			ip -n $ns mptcp endpoint change $addr ${flags//","/" "} $port
> +		fi
> 	else
> -		if [ $5 -ne 0 ]; then port="port $5"; fi
> -		ip netns exec $ns ./pm_nl_ctl set $addr flags $flags $port
> +		if [ $use_id -eq 1 ]; then
> +			ip netns exec $ns ./pm_nl_ctl set id $id flags $flags
> +		else
> +			ip netns exec $ns ./pm_nl_ctl set $addr flags $flags $port
> +		fi
> 	fi
> }
>
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel