Re: [PATCH mptcp-net] mptcp: fix ack generation for fallback msk

[Geliang Tang <geliang@kernel.org>]

Hi Paolo,

Thanks for this fix.

On Fri, 2025-11-07 at 01:10 +0100, Paolo Abeni wrote:
> mptcp_cleanup_rbuf() need to know the last most recent, mptcp-level
> rcv_wnd sent, and such information is tracked into the msk-
> >old_wspace
> field, updated at ack transmission time by mptcp_write_options().
> 
> Fallback socket do not add any mptcp options, such helper is never
> invoked, and msk->old_wspace value remain stale. That in turn makes
> ack generation at recvmsg() time quite randomic.
> 
> Address the issue ensuring mptcp_write_options() is invoked even
> for fallback sockets, and just update the needed info in such a
> case.
> 
> The issue went unnoticed for a long time, as mptcp overshot the
> fallback socket receive buffer autotune significantly.
> 
> Fixes: e3859603ba13 ("mptcp: better msk receive window updates")
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/594
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

This patch looks good to me!

    Reviewed-by: Geliang Tang <geliang@kernel.org>

-Geliang

> ---
>  net/mptcp/options.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index ad51dcf18984..8d3b0f9754dc 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -838,8 +838,11 @@ bool mptcp_established_options(struct sock *sk,
> struct sk_buff *skb,
>  
>  	opts->suboptions = 0;
>  
> +	/* Force later mptcp_write_options(), but do not use any
> actual
> +	 * option space.
> +	 */
>  	if (unlikely(__mptcp_check_fallback(msk) &&
> !mptcp_check_infinite_map(skb)))
> -		return false;
> +		return true;
>  
>  	if (unlikely(skb && TCP_SKB_CB(skb)->tcp_flags &
> TCPHDR_RST)) {
>  		if (mptcp_established_options_fastclose(sk,
> &opt_size, remaining, opts) ||
> @@ -1351,6 +1354,20 @@ static void mptcp_set_rwin(struct tcp_sock
> *tp, struct tcphdr *th)
>  	subflow->rcv_wnd_sent = rcv_wnd_new;
>  }
>  
> +static void mptcp_track_rwin(struct tcp_sock *tp)
> +{
> +	const struct sock *ssk = (const struct sock *)tp;
> +	struct mptcp_subflow_context *subflow;
> +	struct mptcp_sock *msk;
> +
> +	if (!ssk)
> +		return;
> +
> +	subflow = mptcp_subflow_ctx(ssk);
> +	msk = mptcp_sk(subflow->conn);
> +	WRITE_ONCE(msk->old_wspace, tp->rcv_wnd);
> +}
> +
>  __sum16 __mptcp_make_csum(u64 data_seq, u32 subflow_seq, u16
> data_len, __wsum sum)
>  {
>  	struct csum_pseudo_header header;
> @@ -1643,6 +1660,9 @@ void mptcp_write_options(struct tcphdr *th,
> __be32 *ptr, struct tcp_sock *tp,
>  				      opts->reset_transient,
>  				      opts->reset_reason);
>  		return;
> +	} else if (unlikely(!opts->suboptions)) {
> +		mptcp_track_rwin(tp);
> +		return;
>  	}
>  
>  	if (OPTION_MPTCP_PRIO & opts->suboptions) {