[PATCH mptcp-next v3 0/3] fullmesh flag setting support

[PATCH mptcp-next v3 0/3] fullmesh flag setting support

[Geliang Tang]

v3:
 - drop MPTCP_PM_ADDR_FLAG_NOFULLMESH.
 - clear both backup and fullmesh flags as Paolo suggested.
 - depend on:
  mptcp: fix removing ids bitmap setting
  mptcp: fix msk traversal in mptcp_nl_cmd_set_flags()

v2:
 - add MPTCP_PM_ADDR_FLAG_NOFULLMESH instead of MPTCP_PM_CMD_CLEAR_FLAGS

Added the fullmesh flag setting support:
 # pm_nl_ctl set 10.0.1.1 flags fullmesh
 # pm_nl_ctl set 10.0.1.1 flags nofullmesh

Geliang Tang (3):
  mptcp: set fullmesh flag in pm_netlink
  selftests: mptcp: set fullmesh flag in pm_nl_ctl
  selftests: mptcp: add fullmesh setting tests

 net/mptcp/pm_netlink.c                        | 30 +++++++++++++++++--
 .../testing/selftests/net/mptcp/mptcp_join.sh | 29 ++++++++++++++----
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c |  7 +++--
 3 files changed, 56 insertions(+), 10 deletions(-)

-- 
2.31.1

[PATCH mptcp-next v3 1/3] mptcp: set fullmesh flag in pm_netlink

[Geliang Tang]

This patch added the fullmesh flag setting support in pm_netlink.

If the fullmesh flag of the address is changed, remove all the related
subflows, update the fullmesh flag and create subflows again.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/pm_netlink.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 4c06cd6381ef..ef1d58373ef2 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1722,7 +1722,18 @@ mptcp_nl_cmd_get_limits(struct sk_buff *skb, struct genl_info *info)
 	return -EMSGSIZE;
 }
 
-static int mptcp_nl_addr_backup(struct net *net,
+static void mptcp_pm_nl_fullmesh(struct mptcp_sock *msk,
+				 struct mptcp_addr_info *addr)
+{
+	struct mptcp_rm_list list = { .nr = 0 };
+
+	list.ids[list.nr++] = addr->id;
+
+	mptcp_pm_nl_rm_subflow_received(msk, &list);
+	mptcp_pm_create_subflow_or_signal_addr(msk);
+}
+
+static int mptcp_nl_addr_flags(struct net *net,
 				struct mptcp_addr_info *addr,
 				u8 bkup)
 {
@@ -1739,6 +1750,7 @@ static int mptcp_nl_addr_backup(struct net *net,
 		lock_sock(sk);
 		spin_lock_bh(&msk->pm.lock);
 		ret = mptcp_pm_nl_mp_prio_send_ack(msk, addr, bkup);
+		mptcp_pm_nl_fullmesh(msk, addr);
 		spin_unlock_bh(&msk->pm.lock);
 		release_sock(sk);
 
@@ -1757,6 +1769,7 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
 	struct net *net = sock_net(skb->sk);
 	u8 bkup = 0, lookup_by_id = 0;
+	u8 fullmesh = 0;
 	int ret;
 
 	ret = mptcp_pm_parse_addr(attr, info, false, &addr);
@@ -1765,6 +1778,8 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
 
 	if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
 		bkup = 1;
+	else if (addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH)
+		fullmesh = 1;
 	if (addr.addr.family == AF_UNSPEC) {
 		lookup_by_id = 1;
 		if (!addr.addr.id)
@@ -1778,14 +1793,25 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
 		return -EINVAL;
 	}
 
+	if (fullmesh && (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
+		spin_unlock_bh(&pernet->lock);
+		return -EINVAL;
+	}
+
 	if (bkup)
 		entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
 	else
 		entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
+
+	if (fullmesh)
+		entry->flags |= MPTCP_PM_ADDR_FLAG_FULLMESH;
+	else
+		entry->flags &= ~MPTCP_PM_ADDR_FLAG_FULLMESH;
+
 	addr = *entry;
 	spin_unlock_bh(&pernet->lock);
 
-	mptcp_nl_addr_backup(net, &addr.addr, bkup);
+	mptcp_nl_addr_flags(net, &addr.addr, bkup);
 	return 0;
 }
 
-- 
2.31.1

[PATCH mptcp-next v3 2/3] selftests: mptcp: set fullmesh flag in pm_nl_ctl

[Geliang Tang]

This patch added the fullmesh flag setting and clearing support in
pm_nl_ctl:

 # pm_nl_ctl set ip flags fullmesh
 # pm_nl_ctl set ip flags nofullmesh

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

diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
index 354784512748..9b6db9d77223 100644
--- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
+++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
@@ -28,7 +28,7 @@ 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]\n");
+	fprintf(stderr, "\tset <ip> [flags backup|nobackup|fullmesh|nofullmesh]\n");
 	fprintf(stderr, "\tflush\n");
 	fprintf(stderr, "\tdump\n");
 	fprintf(stderr, "\tlimits [<rcv addr max> <subflow max>]\n");
@@ -709,7 +709,10 @@ int set_flags(int fd, int pm_family, int argc, char *argv[])
 			     str = NULL) {
 				if (!strcmp(tok, "backup"))
 					flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
-				else if (strcmp(tok, "nobackup"))
+				else if (!strcmp(tok, "fullmesh"))
+					flags |= MPTCP_PM_ADDR_FLAG_FULLMESH;
+				else if (strcmp(tok, "nobackup") &&
+					 strcmp(tok, "nofullmesh"))
 					error(1, errno,
 					      "unknown flag %s", argv[arg]);
 			}
-- 
2.31.1

[PATCH mptcp-next v3 3/3] selftests: mptcp: add fullmesh setting tests

[Geliang Tang]

This patch added the fullmesh setting and clearing selftests in
mptcp_join.sh.

Now we can set both backup and fullmesh flags, so avoid using the
words 'backup' and 'bkup'.

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index e48ce23d2386..3060da3f7c04 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -289,7 +289,7 @@ do_transfer()
 	addr_nr_ns1="$7"
 	addr_nr_ns2="$8"
 	speed="$9"
-	bkup="${10}"
+	sflags="${10}"
 
 	port=$((10000+$TEST_COUNT))
 	TEST_COUNT=$((TEST_COUNT+1))
@@ -461,14 +461,13 @@ do_transfer()
 		fi
 	fi
 
-	if [ ! -z $bkup ]; then
+	if [ ! -z $sflags ]; then
 		sleep 1
 		for netns in "$ns1" "$ns2"; do
 			dump=(`ip netns exec $netns ./pm_nl_ctl dump`)
 			if [ ${#dump[@]} -gt 0 ]; then
 				addr=${dump[${#dump[@]} - 1]}
-				backup="ip netns exec $netns ./pm_nl_ctl set $addr flags $bkup"
-				$backup
+				ip netns exec $netns ./pm_nl_ctl set $addr flags $sflags
 			fi
 		done
 	fi
@@ -545,7 +544,7 @@ run_tests()
 	addr_nr_ns1="${5:-0}"
 	addr_nr_ns2="${6:-0}"
 	speed="${7:-fast}"
-	bkup="${8:-""}"
+	sflags="${8:-""}"
 	lret=0
 	oldin=""
 
@@ -574,7 +573,7 @@ run_tests()
 	fi
 
 	do_transfer ${listener_ns} ${connector_ns} MPTCP MPTCP ${connect_addr} \
-		${test_linkfail} ${addr_nr_ns1} ${addr_nr_ns2} ${speed} ${bkup}
+		${test_linkfail} ${addr_nr_ns1} ${addr_nr_ns2} ${speed} ${sflags}
 	lret=$?
 }
 
@@ -1921,6 +1920,24 @@ fullmesh_tests()
 	run_tests $ns1 $ns2 10.0.1.1 0 0 fullmesh_2 slow
 	chk_join_nr "fullmesh test 1x2, limited" 4 4 4
 	chk_add_nr 1 1
+
+	# set fullmesh flag
+	reset
+	ip netns exec $ns1 ./pm_nl_ctl limits 4 4
+	ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags subflow
+	ip netns exec $ns2 ./pm_nl_ctl limits 4 4
+	run_tests $ns1 $ns2 10.0.1.1 0 0 1 slow fullmesh
+	chk_join_nr "set fullmesh flag test" 2 2 2
+	chk_rm_nr 0 1
+
+	# set nofullmesh flag
+	reset
+	ip netns exec $ns1 ./pm_nl_ctl limits 4 4
+	ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags subflow,fullmesh
+	ip netns exec $ns2 ./pm_nl_ctl limits 4 4
+	run_tests $ns1 $ns2 10.0.1.1 0 0 fullmesh_1 slow nofullmesh
+	chk_join_nr "set nofullmesh flag test" 2 2 2
+	chk_rm_nr 0 1
 }
 
 userspace_tests()
-- 
2.31.1

[PATCH iproute2-next v2] mptcp: add the fullmesh flag setting support

[Geliang Tang]

This patch added the fullmesh flag setting support.

 ip mptcp endpoint change id 1 fullmesh
 ip mptcp endpoint change id 1 nofullmesh

Added the fullmesh flag check for the adding address too.

Needs to apply the patch "mptcp: add id check for deleting address"
first.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 v2:
 - drop MPTCP_PM_ADDR_FLAG_NOFULLMESH.
 - rename MPTCP_PM_ADDR_FLAG_NOBACKUP to MPTCP_PM_ADDR_FLAG_NONE.
---
 ip/ipmptcp.c        | 16 ++++++++++++----
 man/man8/ip-mptcp.8 |  8 ++++++--
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
index f85c49a8..564500a8 100644
--- a/ip/ipmptcp.c
+++ b/ip/ipmptcp.c
@@ -25,7 +25,8 @@ static void usage(void)
 		"Usage:	ip mptcp endpoint add ADDRESS [ dev NAME ] [ id ID ]\n"
 		"				      [ port NR ] [ FLAG-LIST ]\n"
 		"	ip mptcp endpoint delete id ID [ ADDRESS ]\n"
-		"	ip mptcp endpoint change id ID [ backup | nobackup ]\n"
+		"	ip mptcp endpoint change id ID [ backup | nobackup |\n"
+		"					 fullmesh | nofullmesh ]\n"
 		"	ip mptcp endpoint show [ id ID ]\n"
 		"	ip mptcp endpoint flush\n"
 		"	ip mptcp limits set [ subflows NR ] [ add_addr_accepted NR ]\n"
@@ -46,7 +47,7 @@ static int genl_family = -1;
 	GENL_REQUEST(_req, MPTCP_BUFLEN, genl_family, 0,	\
 		     MPTCP_PM_VER, _cmd, _flags)
 
-#define MPTCP_PM_ADDR_FLAG_NOBACKUP 0x0
+#define MPTCP_PM_ADDR_FLAG_NONE 0x0
 
 /* Mapping from argument to address flag mask */
 static const struct {
@@ -57,7 +58,8 @@ static const struct {
 	{ "subflow",		MPTCP_PM_ADDR_FLAG_SUBFLOW },
 	{ "backup",		MPTCP_PM_ADDR_FLAG_BACKUP },
 	{ "fullmesh",		MPTCP_PM_ADDR_FLAG_FULLMESH },
-	{ "nobackup",		MPTCP_PM_ADDR_FLAG_NOBACKUP }
+	{ "nobackup",		MPTCP_PM_ADDR_FLAG_NONE },
+	{ "nofullmesh",		MPTCP_PM_ADDR_FLAG_NONE }
 };
 
 static void print_mptcp_addr_flags(unsigned int flags)
@@ -116,9 +118,15 @@ static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n, int cmd)
 	ll_init_map(&rth);
 	while (argc > 0) {
 		if (get_flags(*argv, &flags) == 0) {
+			if (adding &&
+			    (flags & MPTCP_PM_ADDR_FLAG_SIGNAL) &&
+			    (flags & MPTCP_PM_ADDR_FLAG_FULLMESH))
+				invarg("invalid flags\n", *argv);
+
 			/* allow changing the 'backup' flag only */
 			if (cmd == MPTCP_PM_CMD_SET_FLAGS &&
-			    (flags & ~MPTCP_PM_ADDR_FLAG_BACKUP))
+			    (flags & ~MPTCP_PM_ADDR_FLAG_BACKUP) &&
+			    (flags & ~MPTCP_PM_ADDR_FLAG_FULLMESH))
 				invarg("invalid flags\n", *argv);
 
 		} else if (matches(*argv, "id") == 0) {
diff --git a/man/man8/ip-mptcp.8 b/man/man8/ip-mptcp.8
index 0e789225..bddbff3c 100644
--- a/man/man8/ip-mptcp.8
+++ b/man/man8/ip-mptcp.8
@@ -41,7 +41,7 @@ ip-mptcp \- MPTCP path manager configuration
 .BR "ip mptcp endpoint change id "
 .I ID
 .RB "[ "
-.I BACKUP-OPT
+.I CHANGE-OPT
 .RB "] "
 
 .ti -8
@@ -68,10 +68,14 @@ ip-mptcp \- MPTCP path manager configuration
 .RB  "]"
 
 .ti -8
-.IR BACKUP-OPT " := ["
+.IR CHANGE-OPT " := ["
 .B backup
 .RB "|"
 .B nobackup
+.RB "|"
+.B fullmesh
+.RB "|"
+.B nofullmesh
 .RB  "]"
 
 .ti -8
-- 
2.31.1

Re: [PATCH mptcp-next v3 1/3] mptcp: set fullmesh flag in pm_netlink

[Paolo Abeni]

On Tue, 2022-01-11 at 23:06 +0800, Geliang Tang wrote:
> This patch added the fullmesh flag setting support in pm_netlink.
> 
> If the fullmesh flag of the address is changed, remove all the related
> subflows, update the fullmesh flag and create subflows again.
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  net/mptcp/pm_netlink.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 4c06cd6381ef..ef1d58373ef2 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1722,7 +1722,18 @@ mptcp_nl_cmd_get_limits(struct sk_buff *skb, struct genl_info *info)
>  	return -EMSGSIZE;
>  }
>  
> -static int mptcp_nl_addr_backup(struct net *net,
> +static void mptcp_pm_nl_fullmesh(struct mptcp_sock *msk,
> +				 struct mptcp_addr_info *addr)
> +{
> +	struct mptcp_rm_list list = { .nr = 0 };
> +
> +	list.ids[list.nr++] = addr->id;
> +
> +	mptcp_pm_nl_rm_subflow_received(msk, &list);
> +	mptcp_pm_create_subflow_or_signal_addr(msk);
> +}
> +
> +static int mptcp_nl_addr_flags(struct net *net,
>  				struct mptcp_addr_info *addr,
>  				u8 bkup)
>  {
> @@ -1739,6 +1750,7 @@ static int mptcp_nl_addr_backup(struct net *net,
>  		lock_sock(sk);
>  		spin_lock_bh(&msk->pm.lock);
>  		ret = mptcp_pm_nl_mp_prio_send_ack(msk, addr, bkup);
> +		mptcp_pm_nl_fullmesh(msk, addr);

I think it would be better moving the check for
MPTCP_PM_ADDR_FLAG_SIGNAL here (the following assumes
mptcp_nl_addr_flags() get a mptcp_pm_addr_entry argument, see below):

		if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
			mptcp_pm_nl_fullmesh(msk, &entry->addr);

>  		spin_unlock_bh(&msk->pm.lock);
>  		release_sock(sk);
>  
> @@ -1757,6 +1769,7 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
>  	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
>  	struct net *net = sock_net(skb->sk);
>  	u8 bkup = 0, lookup_by_id = 0;
> +	u8 fullmesh = 0;
>  	int ret;
>  
>  	ret = mptcp_pm_parse_addr(attr, info, false, &addr);
> @@ -1765,6 +1778,8 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
>  
>  	if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
>  		bkup = 1;
> +	else if (addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH)
> +		fullmesh = 1;
>  	if (addr.addr.family == AF_UNSPEC) {
>  		lookup_by_id = 1;
>  		if (!addr.addr.id)
> @@ -1778,14 +1793,25 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
>  		return -EINVAL;
>  	}
>  
> +	if (fullmesh && (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
> +		spin_unlock_bh(&pernet->lock);
> +		return -EINVAL;

With the change proposed above we can drop this chunk, and avoiding
returning an error.

> +	}
> +
>  	if (bkup)
>  		entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
>  	else
>  		entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
> +
> +	if (fullmesh)
> +		entry->flags |= MPTCP_PM_ADDR_FLAG_FULLMESH;
> +	else
> +		entry->flags &= ~MPTCP_PM_ADDR_FLAG_FULLMESH;
> +
>  	addr = *entry;
>  	spin_unlock_bh(&pernet->lock);
>  
> -	mptcp_nl_addr_backup(net, &addr.addr, bkup);
> +	mptcp_nl_addr_flags(net, &addr.addr, bkup);

If we change the 2# arguments of mptcp_nl_addr_flags() to
'mptcp_pm_addr_entry' and we pass '&addr' here, we can drop the 'bkup'
argument. It's a bit strange to provide 'bkup' but not 'fullmesh'.

I think all the mentioned changes could land in a squash-to patch, if
that fits others.

Thanks!

Paolo