[PATCH mptcp-next v7 10/11] mptcp: pm: make get_local_id helpers static

Geliang Tang posted 11 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH mptcp-next v7 10/11] mptcp: pm: make get_local_id helpers static
Posted by Geliang Tang 1 month, 1 week ago
From: Geliang Tang <tanggeliang@kylinos.cn>

Now mptcp_pm_get_local_id() can directly invoke get_local_id() interface
through "ops" of "msk->pm". Instead of using mptcp_pm_is_userspace() to
check which get_local_id() helper to invoke.

Then mptcp_pm_nl_get_local_id() and mptcp_userspace_pm_get_local_id()
helpers can be static.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c           | 4 +---
 net/mptcp/pm_kernel.c    | 4 ++--
 net/mptcp/pm_userspace.c | 4 ++--
 net/mptcp/protocol.h     | 4 ----
 4 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 1ce58d16370a..848393511997 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -872,9 +872,7 @@ int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
 	skc_local.addr.id = 0;
 	skc_local.flags = MPTCP_PM_ADDR_FLAG_IMPLICIT;
 
-	if (mptcp_pm_is_userspace(msk))
-		return mptcp_userspace_pm_get_local_id(msk, &skc_local);
-	return mptcp_pm_nl_get_local_id(msk, &skc_local);
+	return msk->pm.ops->get_local_id(msk, &skc_local);
 }
 
 bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 8a5966e6e3e3..3d5beaafc3ec 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -693,8 +693,8 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
 	return err;
 }
 
-int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk,
-			     struct mptcp_pm_addr_entry *skc)
+static int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk,
+				    struct mptcp_pm_addr_entry *skc)
 {
 	struct mptcp_pm_addr_entry *entry;
 	struct pm_nl_pernet *pernet;
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index d53f44df9641..05495f6e771a 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -127,8 +127,8 @@ mptcp_userspace_pm_lookup_addr_by_id(struct mptcp_sock *msk, unsigned int id)
 	return NULL;
 }
 
-int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
-				    struct mptcp_pm_addr_entry *skc)
+static int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
+					   struct mptcp_pm_addr_entry *skc)
 {
 	__be16 msk_sport =  ((struct inet_sock *)
 			     inet_sk((struct sock *)msk))->inet_sport;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3579c31650fa..776f164a21eb 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1137,10 +1137,6 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb,
 bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
 			     struct mptcp_rm_list *rm_list);
 int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
-int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk,
-			     struct mptcp_pm_addr_entry *skc);
-int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
-				    struct mptcp_pm_addr_entry *skc);
 bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc);
 bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
 bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
-- 
2.43.0
Re: [PATCH mptcp-next v7 10/11] mptcp: pm: make get_local_id helpers static
Posted by Matthieu Baerts 1 month, 1 week ago
Hi Geliang,

On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Now mptcp_pm_get_local_id() can directly invoke get_local_id() interface
> through "ops" of "msk->pm". Instead of using mptcp_pm_is_userspace() to
> check which get_local_id() helper to invoke.

As mentioned in a previous comment, I think it would be clearer to add
get_local_id() in the PM ops structure (+ the validation, + in the
different PM ops) here in this commit.

Same on the next patch, and for all future PM ops:
new/established/closed connections/subflow, ADD/RM_ADDR received, etc. →
everywhere mptcp_pm_is_userspace() or mptcp_pm_is_kernel() is used in pm.c.

Note that when it is needed to do actions from the worker, I guess it
would be better to schedule the worker if the corresponding pm->ops->XXX
is defined, then calling pm->ops->XXX from mptcp_pm_worker(). The idea
would be to get rid of __mptcp_pm_kernel_worker(), everything should be
done from mptcp_pm_worker(). That should also simplify the hooks for the
BPF PMs, only having hooks called while owning the MSK socket. WDYT?

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