From: Geliang Tang <tanggeliang@kylinos.cn>

The MPTCP path manager event handler mptcp_pm_subflow_closed interface
is added in the commit "mptcp: Add path manager interface", but removed
in the commit "mptcp: do not block subflows creation on errors".

This patch adds it back and changes its 2nd parameter from "u8 id" to
"struct sock *ssk". Invoke mptcp_event with the MPTCP_EVENT_SUB_CLOSED
event type in it.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c       | 7 +++++++
 net/mptcp/protocol.c | 2 +-
 net/mptcp/protocol.h | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 16c336c51940..23f73f07a9e7 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -176,6 +176,13 @@ void mptcp_pm_subflow_established(struct mptcp_sock *msk)
 	spin_unlock_bh(&pm->lock);
 }
 
+void mptcp_pm_subflow_closed(struct mptcp_sock *msk, struct sock *ssk)
+{
+	pr_debug("msk=%p", msk);
+
+	mptcp_event(MPTCP_EVENT_SUB_CLOSED, msk, ssk, GFP_KERNEL);
+}
+
 void mptcp_pm_subflow_check_next(struct mptcp_sock *msk,
 				 const struct mptcp_subflow_context *subflow)
 {
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index a12149d8f718..69859eb2d891 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2515,7 +2515,7 @@ void mptcp_close_ssk(struct sock *sk, struct sock *ssk,
 	subflow->close_event_done = true;
 
 	if (sk->sk_state == TCP_ESTABLISHED)
-		mptcp_event(MPTCP_EVENT_SUB_CLOSED, mptcp_sk(sk), ssk, GFP_KERNEL);
+		mptcp_pm_subflow_closed(mptcp_sk(sk), ssk);
 
 	/* subflow aborted before reaching the fully_established status
 	 * attempt the creation of the next subflow
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index d25d2dac88a5..0c8ab6071a3f 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -996,6 +996,7 @@ void mptcp_pm_fully_established(struct mptcp_sock *msk, const struct sock *ssk);
 bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk);
 void mptcp_pm_connection_closed(struct mptcp_sock *msk);
 void mptcp_pm_subflow_established(struct mptcp_sock *msk);
+void mptcp_pm_subflow_closed(struct mptcp_sock *msk, struct sock *ssk);
 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);
-- 
2.43.0

Hi Geliang,

On 09/09/2024 05:45, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> The MPTCP path manager event handler mptcp_pm_subflow_closed interface
> is added in the commit "mptcp: Add path manager interface", but removed
> in the commit "mptcp: do not block subflows creation on errors".
> 
> This patch adds it back and changes its 2nd parameter from "u8 id" to
> "struct sock *ssk". Invoke mptcp_event with the MPTCP_EVENT_SUB_CLOSED
> event type in it.

Please *always* add a reason: why is better to add it back? Is it to
avoid duplicated code? To move have all the event triggered from pm.c? etc.

Also, I'm less sure about that one: mptcp_pm_subflow_check_next() is
called just after. Why not having only one call to the PM side and
handle both? (or leave it as it is for the moment if you don't need to
change stuff around)

I'm then going to apply only the first patch if that's OK.

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

On Mon, 2024-09-09 at 11:45 +0800, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> The MPTCP path manager event handler mptcp_pm_subflow_closed
> interface
> is added in the commit "mptcp: Add path manager interface", but
> removed
> in the commit "mptcp: do not block subflows creation on errors".
> 
> This patch adds it back and changes its 2nd parameter from "u8 id" to
> "struct sock *ssk". Invoke mptcp_event with the
> MPTCP_EVENT_SUB_CLOSED
> event type in it.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm.c       | 7 +++++++
>  net/mptcp/protocol.c | 2 +-
>  net/mptcp/protocol.h | 1 +
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 16c336c51940..23f73f07a9e7 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -176,6 +176,13 @@ void mptcp_pm_subflow_established(struct
> mptcp_sock *msk)
>  	spin_unlock_bh(&pm->lock);
>  }
>  
> +void mptcp_pm_subflow_closed(struct mptcp_sock *msk, struct sock
> *ssk)
> +{
> +	pr_debug("msk=%p", msk);

"\n" is missing, should be:

	pr_debug("msk=%p\n", msk);

Thanks,
-Geliang

> +
> +	mptcp_event(MPTCP_EVENT_SUB_CLOSED, msk, ssk, GFP_KERNEL);
> +}
> +
>  void mptcp_pm_subflow_check_next(struct mptcp_sock *msk,
>  				 const struct mptcp_subflow_context
> *subflow)
>  {
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index a12149d8f718..69859eb2d891 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2515,7 +2515,7 @@ void mptcp_close_ssk(struct sock *sk, struct
> sock *ssk,
>  	subflow->close_event_done = true;
>  
>  	if (sk->sk_state == TCP_ESTABLISHED)
> -		mptcp_event(MPTCP_EVENT_SUB_CLOSED, mptcp_sk(sk),
> ssk, GFP_KERNEL);
> +		mptcp_pm_subflow_closed(mptcp_sk(sk), ssk);
>  
>  	/* subflow aborted before reaching the fully_established
> status
>  	 * attempt the creation of the next subflow
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index d25d2dac88a5..0c8ab6071a3f 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -996,6 +996,7 @@ void mptcp_pm_fully_established(struct mptcp_sock
> *msk, const struct sock *ssk);
>  bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk);
>  void mptcp_pm_connection_closed(struct mptcp_sock *msk);
>  void mptcp_pm_subflow_established(struct mptcp_sock *msk);
> +void mptcp_pm_subflow_closed(struct mptcp_sock *msk, struct sock
> *ssk);
>  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);