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

Commit Message

Geliang Tang Feb. 27, 2025, 6:43 a.m. UTC

From: Geliang Tang <[e-mail]>

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 <[e-mail]>
---
 net/mptcp/pm_userspace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Matthieu Baerts Feb. 27, 2025, 11:44 a.m. UTC | #1

Hi Geliang,

On 27/02/2025 07:43, Geliang Tang wrote:
> From: Geliang Tang <[e-mail]>
> 
> 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

Patch

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);