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

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

[Geliang Tang]

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

 include/uapi/linux/mptcp.h                    |  1 +
 net/mptcp/pm_netlink.c                        | 27 +++++++++++++-----
 .../testing/selftests/net/mptcp/mptcp_join.sh | 28 +++++++++++++++----
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c |  6 +++-
 4 files changed, 49 insertions(+), 13 deletions(-)

-- 
2.31.1

[PATCH mptcp-next v2 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>
---
 include/uapi/linux/mptcp.h |  1 +
 net/mptcp/pm_netlink.c     | 27 ++++++++++++++++++++-------
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index f106a3941cdf..16758e124e51 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -81,6 +81,7 @@ enum {
 #define MPTCP_PM_ADDR_FLAG_SUBFLOW			(1 << 1)
 #define MPTCP_PM_ADDR_FLAG_BACKUP			(1 << 2)
 #define MPTCP_PM_ADDR_FLAG_FULLMESH			(1 << 3)
+#define MPTCP_PM_ADDR_FLAG_NOFULLMESH			(1 << 4)
 
 enum {
 	MPTCP_PM_CMD_UNSPEC,
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 47ad00d01cf2..1cdaa774cafc 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1743,13 +1743,17 @@ 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);
 	if (ret < 0)
 		return ret;
 
-	if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
+	if (addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH ||
+	    addr.flags & MPTCP_PM_ADDR_FLAG_NOFULLMESH)
+		fullmesh = 1;
+	else if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
 		bkup = 1;
 	if (addr.addr.family == AF_UNSPEC) {
 		lookup_by_id = 1;
@@ -1760,12 +1764,21 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
 	list_for_each_entry(entry, &pernet->local_addr_list, list) {
 		if ((!lookup_by_id && addresses_equal(&entry->addr, &addr.addr, true)) ||
 		    (lookup_by_id && entry->addr.id == addr.addr.id)) {
-			mptcp_nl_addr_backup(net, &entry->addr, bkup);
-
-			if (bkup)
-				entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
-			else
-				entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
+			if (fullmesh) {
+				mptcp_nl_remove_subflow_and_signal_addr(net, &entry->addr);
+				if (addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH)
+					entry->flags |= MPTCP_PM_ADDR_FLAG_FULLMESH;
+				else
+					entry->flags &= ~MPTCP_PM_ADDR_FLAG_FULLMESH;
+				mptcp_nl_add_subflow_or_signal_addr(net);
+			} else {
+				mptcp_nl_addr_backup(net, &entry->addr, bkup);
+
+				if (bkup)
+					entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
+				else
+					entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
+			}
 		}
 	}
 
-- 
2.31.1

[PATCH mptcp-next v2 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 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
index 354784512748..db8b9fdb49d6 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,6 +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, "fullmesh"))
+					flags |= MPTCP_PM_ADDR_FLAG_FULLMESH;
+				else if (!strcmp(tok, "nofullmesh"))
+					flags |= MPTCP_PM_ADDR_FLAG_NOFULLMESH;
 				else if (strcmp(tok, "nobackup"))
 					error(1, errno,
 					      "unknown flag %s", argv[arg]);
-- 
2.31.1

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

[Geliang Tang]

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

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index e48ce23d2386..2868e2657ca1 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,13 +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"
 				$backup
 			fi
 		done
@@ -545,7 +545,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 +574,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 +1921,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 1 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 1 1
 }
 
 userspace_tests()
-- 
2.31.1

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

[Paolo Abeni]

Hello,

On Tue, 2022-01-11 at 10:42 +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>
> ---
>  include/uapi/linux/mptcp.h |  1 +
>  net/mptcp/pm_netlink.c     | 27 ++++++++++++++++++++-------
>  2 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
> index f106a3941cdf..16758e124e51 100644
> --- a/include/uapi/linux/mptcp.h
> +++ b/include/uapi/linux/mptcp.h
> @@ -81,6 +81,7 @@ enum {
>  #define MPTCP_PM_ADDR_FLAG_SUBFLOW			(1 << 1)
>  #define MPTCP_PM_ADDR_FLAG_BACKUP			(1 << 2)
>  #define MPTCP_PM_ADDR_FLAG_FULLMESH			(1 << 3)
> +#define MPTCP_PM_ADDR_FLAG_NOFULLMESH			(1 << 4)
>  
>  enum {
>  	MPTCP_PM_CMD_UNSPEC,
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 47ad00d01cf2..1cdaa774cafc 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1743,13 +1743,17 @@ 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);
>  	if (ret < 0)
>  		return ret;
>  
> -	if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
> +	if (addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH ||
> +	    addr.flags & MPTCP_PM_ADDR_FLAG_NOFULLMESH)
> +		fullmesh = 1;
> +	else if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
>  		bkup = 1;

if I read correctly this is somewhat halfway the two ideas discussed
yday and additionally allows to change either the backup or the
fullmesh flag, but not both simultanusly. I think we we can support
changing both flags with a single command and I think it's better to
stuck with one schema. Just:

	if (addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH)
		fullmesh = 1;


>  	if (addr.addr.family == AF_UNSPEC) {
>  		lookup_by_id = 1;
> @@ -1760,12 +1764,21 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
>  	list_for_each_entry(entry, &pernet->local_addr_list, list) {

Here there is a pre-existing problem, I think. 'local_addr_list' is
protected by pernet->lock or RCU, but this chunk of code does not hold
any of them. The fix will conflict with this series.

>  		if ((!lookup_by_id && addresses_equal(&entry->addr, &addr.addr, true)) ||
>  		    (lookup_by_id && entry->addr.id == addr.addr.id)) {
> -			mptcp_nl_addr_backup(net, &entry->addr, bkup);
> -
> -			if (bkup)
> -				entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
> -			else
> -				entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
			
I think here it would be better to expand the mptcp_nl_addr_backup()
helper to cope with fullmesh, too (so we traverse the msk socket hash
only once);

static void mptcp_nl_addr_flags(struct net *net,
                                struct mptcp_addr_info *addr,
                                u8 bkup, u8 fullmesh)
{
        long s_slot = 0, s_num = 0;
        struct mptcp_sock *msk;

        while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
                struct sock *sk = (struct sock *)msk;

                if (list_empty(&msk->conn_list) || mptcp_pm_is_userspace(msk))
                        goto next;

                lock_sock(sk);
                spin_lock_bh(&msk->pm.lock);
                mptcp_pm_nl_mp_prio_send_ack(msk, addr, bkup);

		/* this should create/delete subflows as needed.
		  if 'addr' is MPTCP_PM_ADDR_FLAG_SIGNAL is a no-op 
*/
		mptcp_pm_nl_fullmesh(msk, addr, fullmesh);
                spin_unlock_bh(&msk->pm.lock);
                release_sock(sk);

next:
                sock_put(sk);
                cond_resched();
        }
}



> +			if (fullmesh) {
> +				mptcp_nl_remove_subflow_and_signal_addr(net, &entry->addr);
> +				if (addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH)
> +					entry->flags |= MPTCP_PM_ADDR_FLAG_FULLMESH;
> +				else
> +					entry->flags &= ~MPTCP_PM_ADDR_FLAG_FULLMESH;
> +				mptcp_nl_add_subflow_or_signal_addr(net);
> +			} else {
> +				mptcp_nl_addr_backup(net, &entry->addr, bkup);
> +
> +				if (bkup)
> +					entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
> +				else
> +					entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
> +			}
>  		}
>  	}
>