[v1] fullmesh flag setting support

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

Posted by Geliang Tang 4 months, 2 weeks ago

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

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

Posted by Paolo Abeni 4 months, 2 weeks ago

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 2/7] mptcp: add clear_flags in pm_netlink

Posted by Geliang Tang 4 months, 2 weeks ago

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

Posted by Paolo Abeni 4 months, 2 weeks ago

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

[PATCH mptcp-next 1/7] mptcp: fix removing ids bitmap
[PATCH mptcp-next 2/7] mptcp: add clear_flags in pm_netlink
[PATCH mptcp-next 3/7] selftests: mptcp: add clear_flags in pm_nl_ctl
[PATCH mptcp-next 4/7] selftests: mptcp: add clear_flags in mptcp_join.sh
[PATCH mptcp-next 5/7] mptcp: set fullmesh flag in pm_netlink
[PATCH mptcp-next 6/7] selftests: mptcp: set fullmesh flag in pm_nl_ctl
[PATCH mptcp-next 7/7] selftests: mptcp: add fullmesh setting tests