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

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

[Geliang Tang]

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

Geliang Tang (7):
  mptcp: fix removing ids bitmap
  mptcp: add clear_flags in pm_netlink
  selftests: mptcp: add clear_flags in pm_nl_ctl
  selftests: mptcp: add clear_flags in mptcp_join.sh
  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                        | 45 ++++++++++++++-----
 .../testing/selftests/net/mptcp/mptcp_join.sh | 42 ++++++++++++++---
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 16 ++++---
 4 files changed, 82 insertions(+), 22 deletions(-)

-- 
2.31.1

[PATCH mptcp-next 1/7] mptcp: fix removing ids bitmap

[Geliang Tang]

In mptcp_pm_nl_rm_addr_or_subflow(), the bit of rm_list->ids[i] in the
id_avail_bitmap should be set, not rm_list->ids[1]. This patch fixed it.

Fixes: 86e39e04482b ("mptcp: keep track of local endpoint still available for each msk")
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/pm_netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index f12effa71942..47ad00d01cf2 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -777,7 +777,7 @@ static void mptcp_pm_nl_rm_addr_or_subflow(struct mptcp_sock *msk,
 			removed = true;
 			__MPTCP_INC_STATS(sock_net(sk), rm_type);
 		}
-		__set_bit(rm_list->ids[1], msk->pm.id_avail_bitmap);
+		__set_bit(rm_list->ids[i], msk->pm.id_avail_bitmap);
 		if (!removed)
 			continue;
 
-- 
2.31.1

[PATCH mptcp-next 2/7] mptcp: add clear_flags in pm_netlink

[Geliang Tang]

Splite set_flags() into two parts, set_flags() and clear_flags(), make it
easy to add new flags to set or clear.

This patch added a new PM command MPTCP_PM_CMD_CLEAR_FLAGS, and a new
function mptcp_nl_cmd_clear_flags().

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 include/uapi/linux/mptcp.h |  1 +
 net/mptcp/pm_netlink.c     | 36 ++++++++++++++++++++++++++----------
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index f106a3941cdf..1ca9b13c2ed0 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -92,6 +92,7 @@ enum {
 	MPTCP_PM_CMD_SET_LIMITS,
 	MPTCP_PM_CMD_GET_LIMITS,
 	MPTCP_PM_CMD_SET_FLAGS,
+	MPTCP_PM_CMD_CLEAR_FLAGS,
 
 	__MPTCP_PM_CMD_AFTER_LAST
 };
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 47ad00d01cf2..da1bef34e8e6 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1736,21 +1736,21 @@ static int mptcp_nl_addr_backup(struct net *net,
 	return ret;
 }
 
-static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
+static int __mptcp_nl_cmd_set_flags(struct sk_buff *skb,
+				    struct genl_info *info,
+				    int clear_flags)
 {
 	struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, }, *entry;
 	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)
@@ -1760,18 +1760,29 @@ 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 (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP) {
+				mptcp_nl_addr_backup(net, &entry->addr, !clear_flags);
+				if (clear_flags)
+					entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
+				else
+					entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
+			}
 		}
 	}
 
 	return 0;
 }
 
+static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
+{
+	return __mptcp_nl_cmd_set_flags(skb, info, 0);
+}
+
+static int mptcp_nl_cmd_clear_flags(struct sk_buff *skb, struct genl_info *info)
+{
+	return __mptcp_nl_cmd_set_flags(skb, info, 1);
+}
+
 static void mptcp_nl_mcast_send(struct net *net, struct sk_buff *nlskb, gfp_t gfp)
 {
 	genlmsg_multicast_netns(&mptcp_genl_family, net,
@@ -2074,6 +2085,11 @@ static const struct genl_small_ops mptcp_pm_ops[] = {
 		.doit   = mptcp_nl_cmd_set_flags,
 		.flags  = GENL_ADMIN_PERM,
 	},
+	{
+		.cmd    = MPTCP_PM_CMD_CLEAR_FLAGS,
+		.doit   = mptcp_nl_cmd_clear_flags,
+		.flags  = GENL_ADMIN_PERM,
+	},
 };
 
 static struct genl_family mptcp_genl_family __ro_after_init = {
-- 
2.31.1

[PATCH mptcp-next 3/7] selftests: mptcp: add clear_flags in pm_nl_ctl

[Geliang Tang]

This patch added 'clear' command for pm_nl_ctl, to replace the command
'pm_nl_ctl set ip flags nobackup' with 'pm_nl_ctl clear ip flags backup'.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 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..85ca37e4b34c 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]\n");
+	fprintf(stderr, "\tset <ip> [flags backup]\n");
+	fprintf(stderr, "\tclear <ip> [flags backup]\n");
 	fprintf(stderr, "\tflush\n");
 	fprintf(stderr, "\tdump\n");
 	fprintf(stderr, "\tlimits [<rcv addr max> <subflow max>]\n");
@@ -647,7 +648,7 @@ int get_set_limits(int fd, int pm_family, int argc, char *argv[])
 	return 0;
 }
 
-int set_flags(int fd, int pm_family, int argc, char *argv[])
+int set_flags(int fd, int pm_family, int clear_flags, int argc, char *argv[])
 {
 	char data[NLMSG_ALIGN(sizeof(struct nlmsghdr)) +
 		  NLMSG_ALIGN(sizeof(struct genlmsghdr)) +
@@ -662,7 +663,8 @@ int set_flags(int fd, int pm_family, int argc, char *argv[])
 
 	memset(data, 0, sizeof(data));
 	nh = (void *)data;
-	off = init_genl_req(data, pm_family, MPTCP_PM_CMD_SET_FLAGS,
+	off = init_genl_req(data, pm_family,
+			    clear_flags ? MPTCP_PM_CMD_CLEAR_FLAGS : MPTCP_PM_CMD_SET_FLAGS,
 			    MPTCP_PM_VER);
 
 	if (argc < 3)
@@ -755,7 +757,9 @@ int main(int argc, char *argv[])
 	else if (!strcmp(argv[1], "limits"))
 		return get_set_limits(fd, pm_family, argc, argv);
 	else if (!strcmp(argv[1], "set"))
-		return set_flags(fd, pm_family, argc, argv);
+		return set_flags(fd, pm_family, 0, argc, argv);
+	else if (!strcmp(argv[1], "clear"))
+		return set_flags(fd, pm_family, 1, argc, argv);
 
 	fprintf(stderr, "unknown sub-command: %s", argv[1]);
 	syntax(argv);
-- 
2.31.1

[PATCH mptcp-next 4/7] selftests: mptcp: add clear_flags in mptcp_join.sh

[Geliang Tang]

This patch changed the command 'pm_nl_ctl set $addr flags nobackup' to
'pm_nl_ctl clear $addr flags backup', and renamed bkup to setflags.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh  | 18 ++++++++++++------
 1 file changed, 12 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..7dbd36c558cd 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}"
+	setflags="${10}"
 
 	port=$((10000+$TEST_COUNT))
 	TEST_COUNT=$((TEST_COUNT+1))
@@ -461,14 +461,20 @@ do_transfer()
 		fi
 	fi
 
-	if [ ! -z $bkup ]; then
+	if [ ! -z $setflags ]; 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
+				if [ $setflags = "backup" ]; then
+					cmd=set
+					flags=backup
+				elif [ $setflags = "nobackup" ]; then
+					cmd=clear
+					flags=backup
+				fi
+				ip netns exec $netns ./pm_nl_ctl $cmd $addr flags $flags
 			fi
 		done
 	fi
@@ -545,7 +551,7 @@ run_tests()
 	addr_nr_ns1="${5:-0}"
 	addr_nr_ns2="${6:-0}"
 	speed="${7:-fast}"
-	bkup="${8:-""}"
+	setflags="${8:-""}"
 	lret=0
 	oldin=""
 
@@ -574,7 +580,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} ${setflags}
 	lret=$?
 }
 
-- 
2.31.1

[PATCH mptcp-next 5/7] mptcp: set fullmesh flag in pm_netlink

[Geliang Tang]

This patch added the fullmesh flag setting and clearing support in
pm_netlink.

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

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index da1bef34e8e6..5fcf0f979579 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1766,6 +1766,13 @@ static int __mptcp_nl_cmd_set_flags(struct sk_buff *skb,
 					entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
 				else
 					entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
+			} else if (addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) {
+				mptcp_nl_remove_subflow_and_signal_addr(net, &entry->addr);
+				if (clear_flags)
+					entry->flags &= ~MPTCP_PM_ADDR_FLAG_FULLMESH;
+				else
+					entry->flags |= MPTCP_PM_ADDR_FLAG_FULLMESH;
+				mptcp_nl_add_subflow_or_signal_addr(net);
 			}
 		}
 	}
-- 
2.31.1

[PATCH mptcp-next 6/7] 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 clear ip flags fullmesh

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

diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
index 85ca37e4b34c..396d53d8fcec 100644
--- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
+++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
@@ -28,8 +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]\n");
-	fprintf(stderr, "\tclear <ip> [flags backup]\n");
+	fprintf(stderr, "\tset <ip> [flags backup|fullmesh]\n");
+	fprintf(stderr, "\tclear <ip> [flags backup|fullmesh]\n");
 	fprintf(stderr, "\tflush\n");
 	fprintf(stderr, "\tdump\n");
 	fprintf(stderr, "\tlimits [<rcv addr max> <subflow max>]\n");
@@ -711,7 +711,9 @@ int set_flags(int fd, int pm_family, int clear_flags, 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
 					error(1, errno,
 					      "unknown flag %s", argv[arg]);
 			}
-- 
2.31.1

[PATCH mptcp-next 7/7] 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 | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 7dbd36c558cd..8c4ee9092e83 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -473,6 +473,12 @@ do_transfer()
 				elif [ $setflags = "nobackup" ]; then
 					cmd=clear
 					flags=backup
+				elif [ $setflags = "fullmesh" ]; then
+					cmd=set
+					flags=fullmesh
+				elif [ $setflags = "nofullmesh" ]; then
+					cmd=clear
+					flags=fullmesh
 				fi
 				ip netns exec $netns ./pm_nl_ctl $cmd $addr flags $flags
 			fi
@@ -1927,6 +1933,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
+
+	# fullmesh flag setting test
+	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 "fullmesh flag setting test" 2 2 2
+	chk_rm_nr 1 1
+
+	# fullmesh flag clearing test
+	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 "fullmesh flag clearing test" 2 2 2
+	chk_rm_nr 1 1
 }
 
 userspace_tests()
-- 
2.31.1

Re: [PATCH mptcp-next 2/7] mptcp: add clear_flags in pm_netlink

[Paolo Abeni]

Hello,

On Mon, 2022-01-10 at 11:30 +0800, Geliang Tang wrote:
> Splite set_flags() into two parts, set_flags() and clear_flags(), make it
> easy to add new flags to set or clear.
> 
> This patch added a new PM command MPTCP_PM_CMD_CLEAR_FLAGS, and a new
> function mptcp_nl_cmd_clear_flags().
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  include/uapi/linux/mptcp.h |  1 +
>  net/mptcp/pm_netlink.c     | 36 ++++++++++++++++++++++++++----------
>  2 files changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
> index f106a3941cdf..1ca9b13c2ed0 100644
> --- a/include/uapi/linux/mptcp.h
> +++ b/include/uapi/linux/mptcp.h
> @@ -92,6 +92,7 @@ enum {
>  	MPTCP_PM_CMD_SET_LIMITS,
>  	MPTCP_PM_CMD_GET_LIMITS,
>  	MPTCP_PM_CMD_SET_FLAGS,
> +	MPTCP_PM_CMD_CLEAR_FLAGS,
>  
>  	__MPTCP_PM_CMD_AFTER_LAST
>  };

I'm unsure if this has already been discussed in some mtg I missed. If
so, I'm sorry for the late feedback :(

Looking at the existing netlink interfaces, I *think* the preferred way
of handling this would be adding instead a couple of MPTCP address
flags:

	MPTCP_PM_ADDR_FLAG_NOBACKUP,
	MPTCP_PM_ADDR_FLAG_NOFULLMESH

As e.g. IFA_F_NOPREFIXROUTE, IFA_F_NODAD,...

The above should require a similar amount of code and will make it
clear that we can't/wan't to flip the SUBFLOW or SIGNAL flags.

WDYT?

Thanks!

Paolo

Re: [PATCH mptcp-next 1/7] mptcp: fix removing ids bitmap

[Paolo Abeni]

On Mon, 2022-01-10 at 11:29 +0800, Geliang Tang wrote:
> In mptcp_pm_nl_rm_addr_or_subflow(), the bit of rm_list->ids[i] in the
> id_avail_bitmap should be set, not rm_list->ids[1]. This patch fixed it.
> 
> Fixes: 86e39e04482b ("mptcp: keep track of local endpoint still available for each msk")
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  net/mptcp/pm_netlink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index f12effa71942..47ad00d01cf2 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -777,7 +777,7 @@ static void mptcp_pm_nl_rm_addr_or_subflow(struct mptcp_sock *msk,
>  			removed = true;
>  			__MPTCP_INC_STATS(sock_net(sk), rm_type);
>  		}
> -		__set_bit(rm_list->ids[1], msk->pm.id_avail_bitmap);
> +		__set_bit(rm_list->ids[i], msk->pm.id_avail_bitmap);
>  		if (!removed)
>  			continue;
>  

LGTM!

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

I think this one could be spooled independently from the other patches,
so it could land on -net early.

WDYT?

Thanks!

Paolo

Re: [PATCH mptcp-next 1/7] mptcp: fix removing ids bitmap

[Geliang Tang]

On Mon, Jan 10, 2022 at 12:40:14PM +0100, Paolo Abeni wrote:
> On Mon, 2022-01-10 at 11:29 +0800, Geliang Tang wrote:
> > In mptcp_pm_nl_rm_addr_or_subflow(), the bit of rm_list->ids[i] in the
> > id_avail_bitmap should be set, not rm_list->ids[1]. This patch fixed it.
> > 
> > Fixes: 86e39e04482b ("mptcp: keep track of local endpoint still available for each msk")
> > Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> > ---
> >  net/mptcp/pm_netlink.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> > index f12effa71942..47ad00d01cf2 100644
> > --- a/net/mptcp/pm_netlink.c
> > +++ b/net/mptcp/pm_netlink.c
> > @@ -777,7 +777,7 @@ static void mptcp_pm_nl_rm_addr_or_subflow(struct mptcp_sock *msk,
> >  			removed = true;
> >  			__MPTCP_INC_STATS(sock_net(sk), rm_type);
> >  		}
> > -		__set_bit(rm_list->ids[1], msk->pm.id_avail_bitmap);
> > +		__set_bit(rm_list->ids[i], msk->pm.id_avail_bitmap);
> >  		if (!removed)
> >  			continue;
> >  
> 
> LGTM!
> 
> Acked-by: Paolo Abeni <pabeni@redhat.com>
> 
> I think this one could be spooled independently from the other patches,
> so it could land on -net early.
> 
> WDYT?

Great!

Matt, could you please help me to update the subject of this patch to
"mptcp: fix removing ids bitmap setting" when merging it?

Thanks,
-Geliang

> 
> Thanks!
> 
> Paolo
> 

Re: [PATCH mptcp-next 2/7] mptcp: add clear_flags in pm_netlink

[Geliang Tang]

On Mon, Jan 10, 2022 at 12:38:14PM +0100, Paolo Abeni wrote:
> Hello,
> 
> On Mon, 2022-01-10 at 11:30 +0800, Geliang Tang wrote:
> > Splite set_flags() into two parts, set_flags() and clear_flags(), make it
> > easy to add new flags to set or clear.
> > 
> > This patch added a new PM command MPTCP_PM_CMD_CLEAR_FLAGS, and a new
> > function mptcp_nl_cmd_clear_flags().
> > 
> > Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> > ---
> >  include/uapi/linux/mptcp.h |  1 +
> >  net/mptcp/pm_netlink.c     | 36 ++++++++++++++++++++++++++----------
> >  2 files changed, 27 insertions(+), 10 deletions(-)
> > 
> > diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
> > index f106a3941cdf..1ca9b13c2ed0 100644
> > --- a/include/uapi/linux/mptcp.h
> > +++ b/include/uapi/linux/mptcp.h
> > @@ -92,6 +92,7 @@ enum {
> >  	MPTCP_PM_CMD_SET_LIMITS,
> >  	MPTCP_PM_CMD_GET_LIMITS,
> >  	MPTCP_PM_CMD_SET_FLAGS,
> > +	MPTCP_PM_CMD_CLEAR_FLAGS,
> >  
> >  	__MPTCP_PM_CMD_AFTER_LAST
> >  };
> 
> I'm unsure if this has already been discussed in some mtg I missed. If
> so, I'm sorry for the late feedback :(
> 
> Looking at the existing netlink interfaces, I *think* the preferred way
> of handling this would be adding instead a couple of MPTCP address
> flags:
> 
> 	MPTCP_PM_ADDR_FLAG_NOBACKUP,
> 	MPTCP_PM_ADDR_FLAG_NOFULLMESH
> 
> As e.g. IFA_F_NOPREFIXROUTE, IFA_F_NODAD,...
> 
> The above should require a similar amount of code and will make it
> clear that we can't/wan't to flip the SUBFLOW or SIGNAL flags.
> 
> WDYT?

Agree, will update in v2.

Thanks,
-Geliang

> 
> Thanks!
> 
> Paolo
> 

Re: [PATCH mptcp-next 2/7] mptcp: add clear_flags in pm_netlink

[Paolo Abeni]

On Mon, 2022-01-10 at 20:35 +0800, Geliang Tang wrote:
> On Mon, Jan 10, 2022 at 12:38:14PM +0100, Paolo Abeni wrote:
> > Hello,
> > 
> > On Mon, 2022-01-10 at 11:30 +0800, Geliang Tang wrote:
> > > Splite set_flags() into two parts, set_flags() and clear_flags(), make it
> > > easy to add new flags to set or clear.
> > > 
> > > This patch added a new PM command MPTCP_PM_CMD_CLEAR_FLAGS, and a new
> > > function mptcp_nl_cmd_clear_flags().
> > > 
> > > Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> > > ---
> > >  include/uapi/linux/mptcp.h |  1 +
> > >  net/mptcp/pm_netlink.c     | 36 ++++++++++++++++++++++++++----------
> > >  2 files changed, 27 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
> > > index f106a3941cdf..1ca9b13c2ed0 100644
> > > --- a/include/uapi/linux/mptcp.h
> > > +++ b/include/uapi/linux/mptcp.h
> > > @@ -92,6 +92,7 @@ enum {
> > >  	MPTCP_PM_CMD_SET_LIMITS,
> > >  	MPTCP_PM_CMD_GET_LIMITS,
> > >  	MPTCP_PM_CMD_SET_FLAGS,
> > > +	MPTCP_PM_CMD_CLEAR_FLAGS,
> > >  
> > >  	__MPTCP_PM_CMD_AFTER_LAST
> > >  };
> > 
> > I'm unsure if this has already been discussed in some mtg I missed. If
> > so, I'm sorry for the late feedback :(
> > 
> > Looking at the existing netlink interfaces, I *think* the preferred way
> > of handling this would be adding instead a couple of MPTCP address
> > flags:
> > 
> > 	MPTCP_PM_ADDR_FLAG_NOBACKUP,
> > 	MPTCP_PM_ADDR_FLAG_NOFULLMESH
> > 
> > As e.g. IFA_F_NOPREFIXROUTE, IFA_F_NODAD,...
> > 
> > The above should require a similar amount of code and will make it
> > clear that we can't/wan't to flip the SUBFLOW or SIGNAL flags.
> > 
> > WDYT?
> 
> Agree, will update in v2.

[ mostly C&P from IRC ]

I'm sorry for being self-contradictory on this point. Think again about
the above even the 

	MPTCP_PM_ADDR_FLAG_NOBACKUP,
	MPTCP_PM_ADDR_FLAG_NOFULLMESH

thing does look great.

I'm wondering if we could simply extend the current
MPTCP_PM_ADDR_FLAG_BACKUP handling to the MPTCP_PM_ADDR_FLAG_FULLMESH
flag? e.g. set or clear the corresponding entry->flags bit according to
the provided address flag attribute.

Cheers,

Paolo