Re: [Devel] [PATCH hci-8.1 1/8] ms/mptcp: fix retransmission loop when csum is enabled

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mptcp: __mptcp_retrans() advances snd_nxt without updating bytes_sent
--

commit 4d64a3ef806956ed9e7612432b9f24e9542dcd61
Author: Paolo Abeni <pabeni@redhat.com>

ms/mptcp: fix retransmission loop when csum is enabled

This commit raises msk->snd_nxt in __mptcp_retrans() when a retransmission
with csum enabled sends data past the old dfrag->already_sent. The goal is
to stop incoming ACKs from being dropped, which caused an endless
retransmission loop.

Link: https://patch.msgid.link/20260602-net-mptcp-misc-fixes-7-1-rc7-v2-2-856831229976@kernel.org

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c

[ ... ]

> @@ -2666,6 +2666,10 @@ static void __mptcp_retrans(struct sock *sk)
>  	msk->bytes_retrans += len;
>  	dfrag->already_sent = max(dfrag->already_sent, len);
>  
> +	/* With csum enabled retransmission can send new data. */
> +	if (after64(dfrag->already_sent + dfrag->data_seq, msk->snd_nxt))
> +		WRITE_ONCE(msk->snd_nxt, dfrag->already_sent + dfrag->data_seq);
> +

[Severity: Medium]
Should msk->bytes_sent be updated here as well?

When csum is enabled, __mptcp_retrans() sets info.limit to dfrag->data_len,
so the loop can send bytes beyond the old dfrag->already_sent.  Those bytes
are new data.

This hunk advances msk->snd_nxt for them, but it does not do the matching
accounting that mptcp_update_post_push() does on the normal push path:

mptcp_update_post_push() {
    ...
		msk->bytes_sent += snd_nxt_new - msk->snd_nxt;
		WRITE_ONCE(msk->snd_nxt, snd_nxt_new);
    ...
}

As a result, msk->bytes_sent, which is reported through MPTCP_INFO,
under-reports the new data sent from __mptcp_retrans().

The earlier msk->bytes_retrans += len in the same function also counts those
new bytes as retransmitted, so bytes_retrans appears to over-report.

Is msk->snd_burst also expected to be reduced for this new data, as
mptcp_update_post_push() does on the normal push path?

>  reset_timer:
>  	mptcp_check_and_set_pending(sk);
>

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261106030306.16014-1-eva.kurchatova@virtuozzo.com?part=1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel