[PATCH mptcp-next v3 19/29] mptcp: add netlink pm addr entry refcount

Geliang Tang posted 29 patches 2 years, 4 months ago
Maintainers: Matthieu Baerts <matthieu.baerts@tessares.net>, Mat Martineau <martineau@kernel.org>, "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>, Florian Westphal <fw@strlen.de>, Kishen Maloor <kishen.maloor@intel.com>
There is a newer version of this series
[PATCH mptcp-next v3 19/29] mptcp: add netlink pm addr entry refcount
Posted by Geliang Tang 2 years, 4 months ago
This patch adds netlink PM address entry refcount. Init 'refcont' of
every address entry to 1. And add a new filed 'subflows' in struct
mptcp_pm_addr_entry, inited to 0, to store how many subflows have
been established on this address entry.

Increase both values in mptcp_pm_create_subflow_or_signal_addr() and
fill_local_addresses_vec(), and decrease the counter 'refcont' in
__mptcp_pm_release_addr_entry() according its 'subflows' value.

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

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 1529ec358815..7d1a4922e931 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -603,8 +603,12 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 			continue;
 
 		spin_unlock_bh(&msk->pm.lock);
-		for (i = 0; i < nr; i++)
-			__mptcp_subflow_connect(sk, &local->addr, &addrs[i]);
+		for (i = 0; i < nr; i++) {
+			if (refcount_inc_not_zero(&local->refcnt)) {
+				__mptcp_subflow_connect(sk, &local->addr, &addrs[i]);
+				local->subflows++;
+			}
+		}
 		spin_lock_bh(&msk->pm.lock);
 	}
 	mptcp_pm_nl_check_work_pending(msk);
@@ -644,9 +648,11 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
 		if (!mptcp_pm_addr_families_match(sk, &entry->addr, remote))
 			continue;
 
-		if (msk->pm.subflows < subflows_max) {
+		if (msk->pm.subflows < subflows_max &&
+		    refcount_inc_not_zero(&entry->refcnt)) {
 			msk->pm.subflows++;
 			addrs[i++] = entry->addr;
+			entry->subflows++;
 		}
 	}
 	rcu_read_unlock();
@@ -895,6 +901,16 @@ static bool address_use_port(struct mptcp_pm_addr_entry *entry)
 /* caller must ensure the RCU grace period is already elapsed */
 static void __mptcp_pm_release_addr_entry(struct mptcp_pm_addr_entry *entry)
 {
+	int i;
+
+	for (i = 0; i < entry->subflows; i++) {
+		if (!refcount_dec_not_one(&entry->refcnt)) {
+			pr_debug("netlink refcount error: refcnt=%d, subflows=%d",
+				 refcount_read(&entry->refcnt), entry->subflows);
+			return;
+		}
+	}
+
 	if (entry->lsk)
 		sock_release(entry->lsk);
 	kfree(entry);
@@ -1087,6 +1103,8 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc
 	entry->ifindex = 0;
 	entry->flags = MPTCP_PM_ADDR_FLAG_IMPLICIT;
 	entry->lsk = NULL;
+	entry->subflows = 0;
+	refcount_set(&entry->refcnt, 1);
 	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
 	if (ret < 0)
 		kfree(entry);
@@ -1314,6 +1332,7 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	*entry = addr;
+	refcount_set(&entry->refcnt, 1);
 	if (entry->addr.port) {
 		ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
 		if (ret) {
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index a71b64565e04..2194e53070d8 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -245,6 +245,7 @@ struct mptcp_pm_addr_entry {
 	u8			flags;
 	int			ifindex;
 	struct socket		*lsk;
+	u8			subflows;
 	refcount_t		refcnt;
 };
 
-- 
2.35.3
Re: [PATCH mptcp-next v3 19/29] mptcp: add netlink pm addr entry refcount
Posted by Matthieu Baerts 2 years, 4 months ago
Hi Geliang,

On 25/09/2023 10:41, Geliang Tang wrote:
> This patch adds netlink PM address entry refcount. Init 'refcont' of
> every address entry to 1. And add a new filed 'subflows' in struct
> mptcp_pm_addr_entry, inited to 0, to store how many subflows have
> been established on this address entry.
> 
> Increase both values in mptcp_pm_create_subflow_or_signal_addr() and
> fill_local_addresses_vec(), and decrease the counter 'refcont' in
> __mptcp_pm_release_addr_entry() according its 'subflows' value.

Please *always* explain WHY this patch is needed.

When reading the code, it is not clear to me what you are trying to fix
here: with the Netlink PM, the userspace adds and deletes endpoints, not
specific subflows. I mean: we don't have the same issue as we have with
the userspace PM. Or is there a different issue?

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