Re: [PATCH mptcp-next 2/7] mptcp: move the stale logic out of retrans scheduler

[Geliang Tang <geliang@kernel.org>]

Hi Paolo,

Thank you for this new version of the series. I have rebased the MPTCP
KTLS code onto it, and all tests passed.

On Wed, 2026-08-05 at 18:17 +0200, Paolo Abeni wrote:
> This allow separating the stale logic invocation and the retrans
> scheduler, and will simplify the next patch.
> 
> It's also a cleaner design as the retrans scheduler has currently
> too many side effects. As a possible downside, the retrans work will
> now traverse the subflows list additional times; that does not matter
> much, as this is slowpath.

However, this patch makes the KTLS selftests significantly slower -
specifically, this test case now takes several hundred seconds to
complete, whereas it previously finished in just a few seconds:

	chunked_sendfile(_metadata, self, 1, 4096);

Is there any way we can make it run faster?

Thanks,
-Geliang

> 
> While at it, pick more accurate names for the involved helpers
> 
> Also note that the scheduler and the stale logic may observe
> different
> subflow statues, as no lock is acquired. This is intentional and not
> harmful, worst case leading to slower retransmissions.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  net/mptcp/pm.c       | 42 +++++++++++++++++++++++++++---------------
>  net/mptcp/protocol.c |  4 ++--
>  net/mptcp/protocol.h |  2 +-
>  3 files changed, 30 insertions(+), 18 deletions(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 5e499ec1c50a..9ce50e8a149d 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -1061,7 +1061,7 @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk,
> struct sock_common *skc)
>  	return msk->pm.ops->get_priority(msk, &skc_local);
>  }
>  
> -static void mptcp_pm_subflows_chk_stale(const struct mptcp_sock
> *msk, struct sock *ssk)
> +static void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk,
> struct sock *ssk)
>  {
>  	struct mptcp_subflow_context *iter, *subflow =
> mptcp_subflow_ctx(ssk);
>  	struct sock *sk = (struct sock *)msk;
> @@ -1098,22 +1098,34 @@ static void mptcp_pm_subflows_chk_stale(const
> struct mptcp_sock *msk, struct soc
>  	}
>  }
>  
> -void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct
> sock *ssk)
> +void mptcp_pm_chk_stale(const struct mptcp_sock *msk)
>  {
> -	struct mptcp_subflow_context *subflow =
> mptcp_subflow_ctx(ssk);
> -	u32 rcv_tstamp = READ_ONCE(tcp_sk(ssk)->rcv_tstamp);
> -
> -	/* keep track of rtx periods with no progress */
> -	if (!subflow->stale_count) {
> -		subflow->stale_rcv_tstamp = rcv_tstamp;
> -		subflow->stale_count++;
> -	} else if (subflow->stale_rcv_tstamp == rcv_tstamp) {
> -		if (subflow->stale_count < U8_MAX)
> +	struct mptcp_subflow_context *subflow;
> +
> +	mptcp_for_each_subflow(msk, subflow) {
> +		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +		u32 rcv_tstamp;
> +
> +		if (!__mptcp_subflow_active(subflow))
> +			continue;
> +
> +		/* No data outstanding at TCP level? not stale */
> +		if (tcp_rtx_and_write_queues_empty(ssk))
> +			continue;
> +
> +		/* keep track of rtx periods with no progress */
> +		rcv_tstamp = READ_ONCE(tcp_sk(ssk)->rcv_tstamp);
> +		if (!subflow->stale_count) {
> +			subflow->stale_rcv_tstamp = rcv_tstamp;
>  			subflow->stale_count++;
> -		mptcp_pm_subflows_chk_stale(msk, ssk);
> -	} else {
> -		subflow->stale_count = 0;
> -		mptcp_subflow_set_active(subflow);
> +		} else if (subflow->stale_rcv_tstamp == rcv_tstamp)
> {
> +			if (subflow->stale_count < U8_MAX)
> +				subflow->stale_count++;
> +			mptcp_pm_subflow_chk_stale(msk, ssk);
> +		} else {
> +			subflow->stale_count = 0;
> +			mptcp_subflow_set_active(subflow);
> +		}
>  	}
>  }
>  
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index a21b10a8c5d3..88167edc6598 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2469,7 +2469,6 @@ struct sock *mptcp_subflow_get_retrans(struct
> mptcp_sock *msk)
>  
>  		/* still data outstanding at TCP level? skip this */
>  		if (!tcp_rtx_and_write_queues_empty(ssk)) {
> -			mptcp_pm_subflow_chk_stale(msk, ssk);
>  			min_stale_count = min_t(int,
> min_stale_count, subflow->stale_count);
>  			continue;
>  		}
> @@ -2859,9 +2858,10 @@ static void __mptcp_retrans(struct sock *sk)
>  	struct mptcp_data_frag *dfrag;
>  	int err, len;
>  
> +	mptcp_pm_chk_stale(msk);
> +
>  	mptcp_clean_una_wakeup(sk);
>  
> -	/* first check ssk: need to kick "stale" logic */
>  	err = mptcp_sched_get_retrans(msk);
>  	dfrag = mptcp_rtx_head(sk);
>  	if (!dfrag) {
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 0042112f118a..c5a9c3d3f223 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -1104,7 +1104,7 @@ int mptcp_pm_parse_entry(struct nlattr *attr,
> struct genl_info *info,
>  bool mptcp_pm_addr_families_match(const struct sock *sk,
>  				  const struct mptcp_addr_info *loc,
>  				  const struct mptcp_addr_info
> *rem);
> -void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct
> sock *ssk);
> +void mptcp_pm_chk_stale(const struct mptcp_sock *msk);
>  void mptcp_pm_new_connection(struct mptcp_sock *msk, const struct
> sock *ssk, int server_side);
>  void mptcp_pm_fully_established(struct mptcp_sock *msk, const struct
> sock *ssk);
>  bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk);