[PATCH mptcp-next v7 2/7] mptcp: only remove addrs in nl_cmd_remove

Geliang Tang posted 7 patches 2 years, 8 months ago
Maintainers: Matthieu Baerts <matthieu.baerts@tessares.net>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Shuah Khan <shuah@kernel.org>
[PATCH mptcp-next v7 2/7] mptcp: only remove addrs in nl_cmd_remove
Posted by Geliang Tang 2 years, 8 months ago
Only remove addrs in mptcp_nl_cmd_remove(), add a new helper
mptcp_pm_remove_addrs() to do this.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/pm_netlink.c   | 17 +++++++++++++++++
 net/mptcp/pm_userspace.c |  2 +-
 net/mptcp/protocol.h     |  2 ++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index a02822111218..dd15ed96fae8 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1555,6 +1555,23 @@ static int mptcp_nl_cmd_del_addr(struct sk_buff *skb, struct genl_info *info)
 	return ret;
 }
 
+void mptcp_pm_remove_addrs(struct mptcp_sock *msk,
+			   struct list_head *rm_list)
+{
+	struct mptcp_rm_list alist = { .nr = 0 };
+	struct mptcp_pm_addr_entry *entry;
+
+	list_for_each_entry(entry, rm_list, list) {
+		if (remove_anno_list_by_saddr(msk, &entry->addr) &&
+		    alist.nr < MPTCP_RM_IDS_MAX) {
+			alist.ids[alist.nr++] = entry->addr.id;
+			spin_lock_bh(&msk->pm.lock);
+			mptcp_pm_remove_addr(msk, &alist);
+			spin_unlock_bh(&msk->pm.lock);
+		}
+	}
+}
+
 void mptcp_pm_remove_addrs_and_subflows(struct mptcp_sock *msk,
 					struct list_head *rm_list)
 {
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 4d0e54fab5cf..07714edb9086 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -232,7 +232,7 @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info)
 
 	list_move(&match->list, &free_list);
 
-	mptcp_pm_remove_addrs_and_subflows(msk, &free_list);
+	mptcp_pm_remove_addrs(msk, &free_list);
 
 	release_sock((struct sock *)msk);
 
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 990c21a97975..535c1b3ae6ed 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -845,6 +845,8 @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk,
 			   bool echo);
 int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_list);
 int mptcp_pm_remove_subflow(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_list);
+void mptcp_pm_remove_addrs(struct mptcp_sock *msk,
+			   struct list_head *rm_list);
 void mptcp_pm_remove_addrs_and_subflows(struct mptcp_sock *msk,
 					struct list_head *rm_list);
 
-- 
2.35.3
Re: [PATCH mptcp-next v7 2/7] mptcp: only remove addrs in nl_cmd_remove
Posted by Matthieu Baerts 2 years, 8 months ago
Hi Geliang,

On 14/04/2023 11:11, Geliang Tang wrote:
> Only remove addrs in mptcp_nl_cmd_remove(), add a new helper
> mptcp_pm_remove_addrs() to do this.

Thank you, the patch looks good to me but here as well, I think it is
important to explain the reason why you are doing that, e.g. according
to the specs, the MPTCP_CMD_REMOVE Netlink command should only send
RM_ADDRs, not deleting associated subflows.

Because it was not supposed to delete subflows, I guess you can add:

  Fixes: 9ab4807c84a4 ("mptcp: netlink: Add MPTCP_PM_CMD_ANNOUNCE")


Also, do you not need to modify the selftests because you changed the
behaviour? I guess "userspace pm add & remove address" and "userspace pm
create destroy subflow" are affected by this modification, no? (it seems
yes, when looking at patch 7/7)
Do you mind doing this modification in the same commit please? (or in a
dedicated commit after this one here explaining the link -- but maybe
clearer to do that in the same commit)


(...)

> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 990c21a97975..535c1b3ae6ed 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -845,6 +845,8 @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk,
>  			   bool echo);
>  int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_list);
>  int mptcp_pm_remove_subflow(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_list);
> +void mptcp_pm_remove_addrs(struct mptcp_sock *msk,
> +			   struct list_head *rm_list);

Small detail: could this go to the previous line please?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net