[PATCH mptcp-next v4 0/4] fullmesh flag setting support

[PATCH mptcp-next v4 0/4] fullmesh flag setting support

[Geliang Tang]

v4:
 - update patch 1 as Paolo suggested.
 - put the iproute2 patch into this series too.
 - depend on:
  mptcp: fix removing ids bitmap setting
  mptcp: fix msk traversal in mptcp_nl_cmd_set_flags()

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                        | 35 ++++++++++++++-----
 .../testing/selftests/net/mptcp/mptcp_join.sh | 33 +++++++++++++----
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c |  7 ++--
 3 files changed, 58 insertions(+), 17 deletions(-)

Geliang Tang (1):
  mptcp: add the fullmesh flag setting support

 ip/ipmptcp.c        | 16 ++++++++++++----
 man/man8/ip-mptcp.8 |  8 ++++++--
 2 files changed, 18 insertions(+), 6 deletions(-)

-- 
2.31.1

[PATCH mptcp-next v4 1/4] 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 | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 4c06cd6381ef..8c5a4be21e3d 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1722,9 +1722,19 @@ mptcp_nl_cmd_get_limits(struct sk_buff *skb, struct genl_info *info)
 	return -EMSGSIZE;
 }
 
-static int mptcp_nl_addr_backup(struct net *net,
-				struct mptcp_addr_info *addr,
-				u8 bkup)
+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_set_flags(struct net *net,
+			      struct mptcp_pm_addr_entry *entry)
 {
 	long s_slot = 0, s_num = 0;
 	struct mptcp_sock *msk;
@@ -1738,7 +1748,10 @@ 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);
+		ret = mptcp_pm_nl_mp_prio_send_ack(msk, &entry->addr,
+						   entry->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
+		if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
+			mptcp_pm_nl_fullmesh(msk, &entry->addr);
 		spin_unlock_bh(&msk->pm.lock);
 		release_sock(sk);
 
@@ -1756,15 +1769,13 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
 	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	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 lookup_by_id = 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)
-		bkup = 1;
 	if (addr.addr.family == AF_UNSPEC) {
 		lookup_by_id = 1;
 		if (!addr.addr.id)
@@ -1778,14 +1789,20 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
 		return -EINVAL;
 	}
 
-	if (bkup)
+	if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
 		entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
 	else
 		entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
+
+	if (addr.flags & MPTCP_PM_ADDR_FLAG_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_set_flags(net, &addr);
 	return 0;
 }
 
-- 
2.31.1

[PATCH mptcp-next v4 2/4] 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 v4 3/4] 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 | 33 +++++++++++++++----
 1 file changed, 27 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..25725be98342 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=$?
 }
 
@@ -1640,6 +1639,7 @@ backup_tests()
 	run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow nobackup
 	chk_join_nr "single subflow, backup" 1 1 1
 	chk_prio_nr 0 1
+	chk_rm_nr 0 1
 
 	# single address, backup
 	reset
@@ -1650,6 +1650,7 @@ backup_tests()
 	chk_join_nr "single address, backup" 1 1 1
 	chk_add_nr 1 1
 	chk_prio_nr 1 0
+	chk_rm_nr 0 0
 }
 
 add_addr_ports_tests()
@@ -1921,6 +1922,26 @@ 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_prio_nr 0 1
+	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_prio_nr 0 1
+	chk_rm_nr 0 1
 }
 
 userspace_tests()
-- 
2.31.1

[PATCH iproute-next v4 4/4] 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.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 v4:
 - put into 'fullmesh flag setting support' serirs.
 - No code changed.

 v2:
 - drop MPTCP_PM_ADDR_FLAG_NOFULLMESH.
 - rename MPTCP_PM_ADDR_FLAG_NOBACKUP to MPTCP_PM_ADDR_FLAG_NONE.
 - Needs to apply the patch "mptcp: add id check for deleting address"
first.
---
 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 v4 0/4] fullmesh flag setting support

[Paolo Abeni]

On Wed, 2022-01-12 at 17:33 +0800, Geliang Tang wrote:
> v4:
>  - update patch 1 as Paolo suggested.
>  - put the iproute2 patch into this series too.
>  - depend on:
>   mptcp: fix removing ids bitmap setting
>   mptcp: fix msk traversal in mptcp_nl_cmd_set_flags()
> 
> 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                        | 35 ++++++++++++++-----
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 33 +++++++++++++----
>  tools/testing/selftests/net/mptcp/pm_nl_ctl.c |  7 ++--
>  3 files changed, 58 insertions(+), 17 deletions(-)
> 
> Geliang Tang (1):
>   mptcp: add the fullmesh flag setting support
> 
>  ip/ipmptcp.c        | 16 ++++++++++++----
>  man/man8/ip-mptcp.8 |  8 ++++++--
>  2 files changed, 18 insertions(+), 6 deletions(-)
> 
LGTM, thanks!

Acked-by: Paolo Abeni <pabeni@redhat.com>

Re: [PATCH mptcp-next v4 1/4] mptcp: set fullmesh flag in pm_netlink

[Mat Martineau]

On Wed, 12 Jan 2022, 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 | 35 ++++++++++++++++++++++++++---------
> 1 file changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 4c06cd6381ef..8c5a4be21e3d 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1722,9 +1722,19 @@ mptcp_nl_cmd_get_limits(struct sk_buff *skb, struct genl_info *info)
> 	return -EMSGSIZE;
> }
>
> -static int mptcp_nl_addr_backup(struct net *net,
> -				struct mptcp_addr_info *addr,
> -				u8 bkup)
> +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_set_flags(struct net *net,
> +			      struct mptcp_pm_addr_entry *entry)
> {
> 	long s_slot = 0, s_num = 0;
> 	struct mptcp_sock *msk;
> @@ -1738,7 +1748,10 @@ 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);
> +		ret = mptcp_pm_nl_mp_prio_send_ack(msk, &entry->addr,
> +						   entry->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
> +		if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
> +			mptcp_pm_nl_fullmesh(msk, &entry->addr);

My concern here is that if MPTCP_PM_ADDR_FLAG_SIGNAL is not set for 
this entry, mptcp_pm_nl_fullmesh() gets called every time there's a 
MPTCP_PM_CMD_SET_FLAGS command for a valid endpoint.

Would it be better to pass a mask of *changed* flags to 
mptcp_nl_set_flags() so every change to a backup flag doesn't trigger a 
bunch of fullmesh processing, and vice-versa?

- Mat

> 		spin_unlock_bh(&msk->pm.lock);
> 		release_sock(sk);
>
> @@ -1756,15 +1769,13 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
> 	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
> 	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 lookup_by_id = 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)
> -		bkup = 1;
> 	if (addr.addr.family == AF_UNSPEC) {
> 		lookup_by_id = 1;
> 		if (!addr.addr.id)
> @@ -1778,14 +1789,20 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
> 		return -EINVAL;
> 	}
>
> -	if (bkup)
> +	if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
> 		entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
> 	else
> 		entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
> +
> +	if (addr.flags & MPTCP_PM_ADDR_FLAG_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_set_flags(net, &addr);
> 	return 0;
> }
>
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel

Re: [PATCH iproute-next v4 4/4] mptcp: add the fullmesh flag setting support

[Mat Martineau]

On Wed, 12 Jan 2022, Geliang Tang wrote:

> 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.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> v4:
> - put into 'fullmesh flag setting support' serirs.
> - No code changed.
>

The patch looks good to me:

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


For future patches that might involve more than one repository, it is 
helpful to group them in a separate series for each repository (for 
example, posting separately for this patch and the previous 3 mptcp-next 
patches). Noting the dependencies in the cover letters or notes section 
of the commit message is very helpful, thanks for doing that. Keeping the 
series separated helps with using tools like patchew and b4.

Thanks!

-Mat

> v2:
> - drop MPTCP_PM_ADDR_FLAG_NOFULLMESH.
> - rename MPTCP_PM_ADDR_FLAG_NOBACKUP to MPTCP_PM_ADDR_FLAG_NONE.
> - Needs to apply the patch "mptcp: add id check for deleting address"
> first.
> ---
> 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
>
>
>

--
Mat Martineau
Intel

Re: [PATCH iproute-next v4 4/4] mptcp: add the fullmesh flag setting support

[Geliang Tang]

On Thu, Jan 13, 2022 at 03:44:42PM -0800, Mat Martineau wrote:
> On Wed, 12 Jan 2022, Geliang Tang wrote:
> 
> > 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.
> > 
> > Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> > ---
> > v4:
> > - put into 'fullmesh flag setting support' serirs.
> > - No code changed.
> > 
> 
> The patch looks good to me:
> 
> Acked-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

Hi Mat,

This patch is updated for support to set backup and fullmesh flags
together. I just sent out a v5.

> 
> 
> For future patches that might involve more than one repository, it is
> helpful to group them in a separate series for each repository (for example,
> posting separately for this patch and the previous 3 mptcp-next patches).
> Noting the dependencies in the cover letters or notes section of the commit
> message is very helpful, thanks for doing that. Keeping the series separated
> helps with using tools like patchew and b4.
> 
> Thanks!
> 
> -Mat
> 
> > v2:
> > - drop MPTCP_PM_ADDR_FLAG_NOFULLMESH.
> > - rename MPTCP_PM_ADDR_FLAG_NOBACKUP to MPTCP_PM_ADDR_FLAG_NONE.
> > - Needs to apply the patch "mptcp: add id check for deleting address"
> > first.
> > ---
> > 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 */

Update this comment to:

	/* allow changing the 'backup' and 'fullmesh' flags 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))

This two line change to:

	(flags & ~MPTCP_PM_ADDR_FLAG_BACKUP & ~MPTCP_PM_ADDR_FLAG_FULLMESH))

Thanks,
-Geliang

> > 				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
> > 
> > 
> > 
> 
> --
> Mat Martineau
> Intel
>