Re: [PATCH] tcp: Fix subtle bug in fast re-transmit path

[Stefano Brivio <sbrivio@redhat.com>]

On Wed,  7 Feb 2024 23:57:21 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> When a duplicate ack from the tap side triggers a fast re-transmit, we set
> both conn->seq_ack_from_tap and conn->seq_to_tap to the sequence number of
> the duplicate ack.  Setting seq_to_tap is correct: this is what triggers
> the retransmit from this point onwards.  Setting seq_ack_from_tap is
> not correct, though.
> 
> In most cases setting seq_ack_from_tap will be redundant but harmless:
> it will have already been updated to the same value by
> tcp_update_seqack_from_tap() a few lines above.  However that call can
> be skipped if tcp_sock_consume() fails, which is rare but possible.  In
> that case this update will cause problems.
> 
> We use seq_ack_from_tap to track two logically distinct things: how much of
> the stream has been acked by the guest, and how much of the stream from the
> socket has been read and discarded (as opposed to MSG_PEEKed).  We attempt
> to keep those values the same, because we discard data exactly when it is
> acked by the guest.  However tcp_sock_consume() failing means we weren't
> able to disard the acked data.  To handle that case, we skip the usual
> update of seq_ack_from_tap, effectively ignoring the ack assuming we'll get
> one which supersedes it soon enough.  Setting seq_ack_from_tap in the
> fast retransmit path, however, means we now really will have the
> read/discard point in the stream out of sync with seq_ack_from_tap.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Applied.

-- 
Stefano