diff mbox series

[mptcp-next,v3,07/10] mptcp: pm: add add_addr_received() interface

Message ID 2d7bb2ff8960c244aee00ec314f23edd0f5577df.1742521397.git.tanggeliang@kylinos.cn (mailing list archive)
State Superseded, archived
Delegated to: Matthieu Baerts
Headers show
Series BPF path manager, part 6 | expand

Checks

Context Check Description
matttbe/checkpatch success total: 0 errors, 0 warnings, 0 checks, 65 lines checked
matttbe/shellcheck success MPTCP selftests files have not been modified
matttbe/build success Build and static analysis OK
matttbe/KVM_Validation__normal success Success! ✅
matttbe/KVM_Validation__debug success Success! ✅
matttbe/KVM_Validation__btf-normal__only_bpftest_all_ success Success! ✅
matttbe/KVM_Validation__btf-debug__only_bpftest_all_ success Success! ✅

Commit Message

Geliang Tang March 21, 2025, 1:45 a.m. UTC 
From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds an optional .add_addr_received interface for struct
mptcp_pm_ops and invokes it in mptcp_pm_add_addr_received(). A new helper
mptcp_pm_add_addr_recv() is added to allow the MPTCP_PM_ADD_ADDR_RECEIVED
worker can be invoke from the in-kernel PM.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 include/net/mptcp.h   |  4 ++++
 net/mptcp/pm.c        |  9 +++++++--
 net/mptcp/pm_kernel.c | 13 +++++++++++++
 net/mptcp/protocol.h  |  1 +
 4 files changed, 25 insertions(+), 2 deletions(-)

Comments

Matthieu Baerts March 21, 2025, 10:27 a.m. UTC | #1 
Hi Geliang,

On 21/03/2025 02:45, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch adds an optional .add_addr_received interface for struct
> mptcp_pm_ops and invokes it in mptcp_pm_add_addr_received(). A new helper
> mptcp_pm_add_addr_recv() is added to allow the MPTCP_PM_ADD_ADDR_RECEIVED
> worker can be invoke from the in-kernel PM.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  include/net/mptcp.h   |  4 ++++
>  net/mptcp/pm.c        |  9 +++++++--
>  net/mptcp/pm_kernel.c | 13 +++++++++++++
>  net/mptcp/protocol.h  |  1 +
>  4 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index de9838ea37c4..37a84b4c661e 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -132,6 +132,10 @@ struct mptcp_pm_ops {
>  	bool (*allow_new_subflow)(struct mptcp_sock *msk);
>  	bool (*accept_new_subflow)(const struct mptcp_sock *msk);
>  
> +	/* optional */
> +	int (*add_addr_received)(struct mptcp_sock *msk,
> +				 const struct mptcp_addr_info *addr);
> +
>  	char			name[MPTCP_PM_NAME_MAX];
>  	struct module		*owner;
>  	struct list_head	list;
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index d2e72e36d80e..28bd91819ec1 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -559,6 +559,11 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk,
>  	spin_unlock_bh(&pm->lock);
>  }
>  
> +bool mptcp_pm_add_addr_recv(struct mptcp_sock *msk)
> +{
> +	return mptcp_pm_schedule_work(msk, MPTCP_PM_ADD_ADDR_RECEIVED);
> +}
> +
>  void mptcp_pm_add_addr_received(const struct sock *ssk,
>  				const struct mptcp_addr_info *addr)
>  {
> @@ -586,10 +591,10 @@ void mptcp_pm_add_addr_received(const struct sock *ssk,
>  		   (addr->id > 0 && !READ_ONCE(pm->accept_addr))) {
>  		mptcp_pm_announce_addr(msk, addr, true);
>  		mptcp_pm_add_addr_send_ack(msk);
> -	} else if (mptcp_pm_schedule_work(msk, MPTCP_PM_ADD_ADDR_RECEIVED)) {
> -		pm->remote = *addr;
>  	} else {
>  		ret = -EINVAL;
> +		if (pm->ops->add_addr_received)
> +			ret = pm->ops->add_addr_received(msk, addr);

Please see my comment from:

 https://lore.kernel.org/bf2ad4ed-bf4a-47c7-a7d5-eb23c0825198@kernel.org

This part:

> For add_addr_received, I think it is safer to have the callback from the
> worker context. In other words, when an ADD_ADDR received on a subflow:
> 
> - the ADD_ADDR echo should be sent: I don't think it is worth it letting
> the other peer resending it just in case the userspace PM was not "ready"
> 
> - if pm->ops->add_addr_received is set, schedule the worker and set
>   msk->pm.remote
> 
> - then pm->ops->add_addr_received will be called from the worker.

WDYT?

I don't think a BPF PM should schedule the worker and wait to get called
from another callback.

Instead, we should have something similar than .established, no?

Cheers,
Matt
diff mbox series

Patch

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index de9838ea37c4..37a84b4c661e 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -132,6 +132,10 @@  struct mptcp_pm_ops {
 	bool (*allow_new_subflow)(struct mptcp_sock *msk);
 	bool (*accept_new_subflow)(const struct mptcp_sock *msk);
 
+	/* optional */
+	int (*add_addr_received)(struct mptcp_sock *msk,
+				 const struct mptcp_addr_info *addr);
+
 	char			name[MPTCP_PM_NAME_MAX];
 	struct module		*owner;
 	struct list_head	list;
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index d2e72e36d80e..28bd91819ec1 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -559,6 +559,11 @@  void mptcp_pm_subflow_check_next(struct mptcp_sock *msk,
 	spin_unlock_bh(&pm->lock);
 }
 
+bool mptcp_pm_add_addr_recv(struct mptcp_sock *msk)
+{
+	return mptcp_pm_schedule_work(msk, MPTCP_PM_ADD_ADDR_RECEIVED);
+}
+
 void mptcp_pm_add_addr_received(const struct sock *ssk,
 				const struct mptcp_addr_info *addr)
 {
@@ -586,10 +591,10 @@  void mptcp_pm_add_addr_received(const struct sock *ssk,
 		   (addr->id > 0 && !READ_ONCE(pm->accept_addr))) {
 		mptcp_pm_announce_addr(msk, addr, true);
 		mptcp_pm_add_addr_send_ack(msk);
-	} else if (mptcp_pm_schedule_work(msk, MPTCP_PM_ADD_ADDR_RECEIVED)) {
-		pm->remote = *addr;
 	} else {
 		ret = -EINVAL;
+		if (pm->ops->add_addr_received)
+			ret = pm->ops->add_addr_received(msk, addr);
 	}
 
 	if (ret)
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index c89a8524e70e..314e62da84c4 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -1426,6 +1426,18 @@  static bool mptcp_pm_kernel_accept_new_subflow(const struct mptcp_sock *msk)
 	return READ_ONCE(msk->pm.accept_subflow);
 }
 
+static int mptcp_pm_kernel_add_addr_received(struct mptcp_sock *msk,
+					     const struct mptcp_addr_info *addr)
+{
+	int ret = 0;
+
+	if (mptcp_pm_add_addr_recv(msk))
+		msk->pm.remote = *addr;
+	else
+		ret = -EINVAL;
+	return ret;
+}
+
 static void mptcp_pm_kernel_init(struct mptcp_sock *msk)
 {
 	bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk);
@@ -1453,6 +1465,7 @@  struct mptcp_pm_ops mptcp_pm_kernel = {
 	.subflow_established	= mptcp_pm_kernel_subflow_established,
 	.allow_new_subflow	= mptcp_pm_kernel_allow_new_subflow,
 	.accept_new_subflow	= mptcp_pm_kernel_accept_new_subflow,
+	.add_addr_received	= mptcp_pm_kernel_add_addr_received,
 	.init			= mptcp_pm_kernel_init,
 	.name			= "kernel",
 	.owner			= THIS_MODULE,
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 64aa091cb685..ea44f552b3a4 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1014,6 +1014,7 @@  void mptcp_pm_subflow_established(struct mptcp_sock *msk);
 bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk);
 void mptcp_pm_subflow_check_next(struct mptcp_sock *msk,
 				 const struct mptcp_subflow_context *subflow);
+bool mptcp_pm_add_addr_recv(struct mptcp_sock *msk);
 void mptcp_pm_add_addr_received(const struct sock *ssk,
 				const struct mptcp_addr_info *addr);
 void mptcp_pm_add_addr_echoed(struct mptcp_sock *msk,