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

On Thu, Oct 22, 2026 at 11:46:48AM +0200, Eric Dumazet wrote:

Le jeu. 22 oct. 2026 à 10:49, Guillaume Nault [off-list ref] a écrit :

quoted

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.

Nice catch. This also matches RFC 5681 dupack definition, (e):
"the advertised window in the incoming acknowledgment equals the
advertised window in the last incoming acknowledgment".
Maybe worth quoting in the changelog.

Good idea, I'll add the quote in v2.

quoted

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.

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.

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 <redacted>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Guillaume Nault <redacted>
---
 net/ipv4/tcp_input.c                          |  2 +-
 .../tcp_bidir_fast_retransmit_nosack.pkt      | 42 +++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_bidir_fast_retransmit_nosack.pkt

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 99baf14afdfd..ef20bb5a0523 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c

@@ -3959,11 +3959,11 @@ static int tcp_ack_update_window(struct sock *sk, const struct sk_buff *skb, u32
                nwin <<= tp->rx_opt.snd_wscale;

        if (tcp_may_update_window(tp, ack, ack_seq, nwin)) {
-               flag |= FLAG_WIN_UPDATE;
                tcp_update_wl(tp, ack_seq);

                if (tp->snd_wnd != nwin) {
                        tp->snd_wnd = nwin;
+                       flag |= FLAG_WIN_UPDATE;

One behavior change to keep in mind: because of the "|| !nwin" clause
in tcp_may_update_window(), every zero-window ACK used to carry
FLAG_WIN_UPDATE and was never considered as a dupack.
After this patch, repeated "win 0" ACKs at snd_una with outstanding
data (receiver dropping in-flight packets because its queue is full)
will be counted as dupacks and can trigger fast retransmit + cwnd
reduction while the peer is simply stalled.
This is RFC compliant, I think this is fine, but a packetdrill test
covering this case would be nice.

Indeed, I also think this new behaviour is valid.

Also, the header-prediction branch in tcp_ack() still sets
FLAG_WIN_UPDATE unconditionally, so CA_ACK_WIN_UPDATE (only visible to
BPF CC) is now inconsistent between fast and slow paths.

I feel I'm missing something about this header prediction code. The
comment says "Window is constant, pure forward advance.", so why does it
set "flag |= FLAG_WIN_UPDATE;" right afterwards? Do I misread the
comment?

My (theoritical) understanding of header prediction is that one of the
precoditions for it to happen is that SEQ.WND == SND.WND. In that case
we could just drop the "flag |= FLAG_WIN_UPDATE;" line. However, I
can't see any such guarantee in the code (but I may have missed
something).

In the end, should I just add a chunk like the following?

 		tcp_update_wl(tp, ack_seq);
 		tcp_snd_una_update(tp, ack);
-		flag |= FLAG_WIN_UPDATE;
+		if (tp->snd_wnd != ntohs(tcp_hdr(skb)->window))
+			flag |= FLAG_WIN_UPDATE;
 
 		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPHPACKS);

It is quite sad that some TCP stacks do not use SACK in 2026.
They will still be dealing with decade old bugs.

Yes, we have what we have :(.

I am unsure if this is a net candidate, perhaps Neal and Kuniyuki can
give their thoughts.

I posted it to net because I considered it's a bug in the duplicate ack
detection. But if maintainers think this should be for net-next, that's
okay too.