[PATCH mptcp-next v1 2/6] mptcp: pm: userspace: drop is_userspace in free_local_addr_list

Geliang Tang posted 6 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH mptcp-next v1 2/6] mptcp: pm: userspace: drop is_userspace in free_local_addr_list
Posted by Geliang Tang 2 months, 2 weeks ago
From: Geliang Tang <tanggeliang@kylinos.cn>

To reduce the path manager's reliance on mptcp_pm_is_userspace()
and mptcp_pm_is_kernel() helpers, this patch drops the check for
mptcp_pm_is_userspace() in mptcp_free_local_addr_list() and
replaces it with a check to see if userspace_pm_local_addr_list
is empty.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_userspace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 5b3ee43130be..98fe8808d1e1 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -18,7 +18,7 @@ void mptcp_free_local_addr_list(struct mptcp_sock *msk)
 	struct sock *sk = (struct sock *)msk;
 	LIST_HEAD(free_list);
 
-	if (!mptcp_pm_is_userspace(msk))
+	if (list_empty(&msk->pm.userspace_pm_local_addr_list))
 		return;
 
 	spin_lock_bh(&msk->pm.lock);
-- 
2.43.0
Re: [PATCH mptcp-next v1 2/6] mptcp: pm: userspace: drop is_userspace in free_local_addr_list
Posted by Matthieu Baerts 2 months, 2 weeks ago
Hi Geliang,

On 27/02/2025 07:43, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> To reduce the path manager's reliance on mptcp_pm_is_userspace()
> and mptcp_pm_is_kernel() helpers, this patch drops the check for
> mptcp_pm_is_userspace() in mptcp_free_local_addr_list() and
> replaces it with a check to see if userspace_pm_local_addr_list
> is empty.

The existing design feels wrong: I see that mptcp_destroy_common()
directly calls specific PM code.

I think in general, MPTCP core should only call code from pm.c, then
redirected to the right PM (later using the 'pm->ops').

For the moment, I see mptcp_destroy_common() from protocol.c is calling:

 - mptcp_pm_free_anno_list(msk);
 - mptcp_free_local_addr_list(msk);

Instead, I suggest mptcp_destroy_common() calling mptcp_pm_destroy(),
which will always call mptcp_pm_free_anno_list() and only call
mptcp_free_local_addr_list() for the userspace pm.

Later on, mptcp_free_local_addr_list() could be called via
'pm->ops->destroy' I suppose, no?

BTW, mptcp_pm_free_anno_list() seems to be used by all PMs, right? I
think it would be better to move this code to pm.c. Same for all
"common" code. I can look at that if you prefer. But maybe you prefer to
do this after your changed or is that OK to rebase after?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.