Re: [PATCH net v2] tcp: Fix dup-ack detection on non-SACK bidirectional data transfer.

[Kuniyuki Iwashima <kuniyu@google.com>]

On Wed, 28 Oct 2026 21:32:35 +0100, Guillaume Nault wrote:
> When receiving a pure ack from a peer that also sends data, we can
> have SEG.SEQ > SND.WL1, even though the segment contains no data. This
> makes tcp_may_update_window() return true (because of the
> "after(ack_seq, tp->snd_wl1)" condition), and tcp_ack_update_window()
> then adds FLAG_WIN_UPDATE to the segment set of flags, even when
> there's no window update.
> 
> Since FLAG_WIN_UPDATE prevents the segment from being considered as
> dup-ack (as it's part of FLAG_NOT_DUP), this can inhibit fast
> retransmit on non-SACK connections, making recovery very slow as it
> now entirely relies on RTO.
> 
> The "DUPLICATE ACKNOWLEDGMENT" definition of RFC 5681, section 2,
> says that lack of window update is a necessary condition for a dup ack:
> "(e) the advertised window in the incoming acknowledgment equals the
> advertised window in the last incoming acknowledgment.". Spurious
> window updates can thus prevent dup ack detection and, in the end, fast
> retransmit.
> 
> Avoid this scenario by setting FLAG_WIN_UPDATE only when a window
> update actually occurs. Pure acks with SEG.SEQ > SND.WL1 can now
> be detected as dup-ack and trigger fast retransmission.
> 
> Also, stop setting FLAG_WIN_UPDATE in tcp_ack()'s header prediction
> path. One of the conditions for header prediction to happen is that
> received segments don't trigger a window update. This flag update was
> thus wrong to begin with. Now that tcp_ack_update_window() sets
> FLAG_WIN_UPDATE only if a window update actually occurred, we need the
> header prediction path to stop reporting spurious window updates too.
> Otherwise BPF programs would get inconsistent window update information
> (CA_ACK_WIN_UPDATE) between fast an slow paths.
> 
> Finally, add FLAG_SND_UNA_ADVANCED to the definition of FLAG_NOT_DUP.
> This is necessary for the case where an incoming segment acknowledges
> less than a full segment of the retransmit queue. Such segments were
> not considered dubious by tcp_ack_is_dubious() because of the spurious
> FLAG_WIN_UPDATE flag, which prevented the segment from being seen as a
> duplicate ack. Now that FLAG_WIN_UPDATE doesn't cover this scenario
> anymore, we need another way to mark these acks as non-duplicate.
> Adding FLAG_SND_UNA_ADVANCED to FLAG_NOT_DUP is the natural way of
> doing this, since, by definition, a segment advancing SND.UNA, even by
> a small amount, cannot be a duplicate ack.
> 
> A side effect of this patch is that repeated zero window acks received
> from peer can now be considered duplicate acks (because of the the
> "|| !win" condition in tcp_may_update_window()). Therefore, they can
> now trigger fast retransmission and fast recovery, including congestion
> window reduction, when the peer is stalled. This is RFC compliant and
> shouldn't have any negative effect.
> 
> Add packetdrill selftest. Before this patch, the 1001:2001 segment was
> retransmitted upon RTO, now it's sent right after reception of the
> third duplicate ack.
> 
> Thanks to Tom Williams for the investigation help and initial
> reproducer.
> 
> Reported-by: Tom Williams <Tom.D.Williams@MorganStanley.com>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Guillaume Nault <gnault@redhat.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>