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

Geliang Tang posted 3 patches 4 months, 1 week ago
Maintainers: Mat Martineau <mathew.j.martineau@linux.intel.com>, Jakub Kicinski <kuba@kernel.org>, "David S. Miller" <davem@davemloft.net>, Shuah Khan <shuah@kernel.org>, Matthieu Baerts <matthieu.baerts@tessares.net>
[PATCH mptcp-next v2 1/3] mptcp: set fullmesh flag in pm_netlink
Posted by Geliang Tang 4 months, 1 week ago
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


Re: [PATCH mptcp-next v2 1/3] mptcp: set fullmesh flag in pm_netlink
Posted by Paolo Abeni 4 months, 1 week ago
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;
> +			}
>  		}
>  	}
>