Re: [PATCH net v3 3/3] selftests: net: packetdrill: test RTO fallback after partial undo

Le ven. 9 oct. 2026 à 07:01, [off-list ref] a écrit :

From: Neil Ramaswamy <redacted>

Tests that a segment that is lost but also previously retransmitted is not
added back to the RACK list during partial undo, and is instead
retransmitted by the RTO. It utilizes the fact that a TLP doesn't increment
retrans_out, which needs to be 0 for partial undo (but this scenario may
also happen without the TLP).

Signed-off-by: Neil Ramaswamy <redacted>
Assisted-by: LLM
---

This test passes with or without patch 1/3, so it does not test the fix.

It records that a 3-segment flow with one TLP and one late ACK has to
wait for an RTO, and whoever fixes that later will have to rewrite it.

A single TLP probe is enough to get TCPCB_EVER_RETRANS on a segment, so
I am not convinced this case is rare.

Let me come back to tp->tsorted_lost_queue [1]. I was too terse there
("often") and it did not get discussed.

tcp_rack_detect_loss() walks tsorted_sent_queue from oldest to newest,
and whatever it leaves there was sent after what it just marked lost.
So if it does

list_move_tail(&skb->tcp_tsorted_anchor, &tp->tsorted_lost_queue);

instead of list_del_init(), the lost queue stays sorted by send time
across scans, for free. This also holds for skbs that were retransmitted
(TLP probe, lost retransmit), because they sit in tsorted_sent_queue at
their last transmit time. No TCPCB_EVER_RETRANS special case, no
comparator.

- Retransmit: tcp_update_skb_after_send() already moves the skb to the
  tail of tsorted_sent_queue.
- SACK and cumulative ACK: list_del_init()/list_del() are unchanged.
- tcp_fragment(): buff lands next to skb in the lost queue. Today, for
  an skb RACK has unlinked, list_add() builds a 2-node ring that belongs
  to no list.
- Undo in fast recovery: everything in the lost queue was sent before
  everything still in tsorted_sent_queue, so this is a
  list_splice_init() at the head. O(1), no walk, no timestamp compare.

The "often" is the RTO case: tcp_timeout_mark_lost() leaves the skbs it
marks in tsorted_sent_queue, and they can be older than what RACK moves
to the lost queue afterwards. So undo needs a check: splice at the head
if tsorted_sent_queue is empty or the lost tail was sent before the sent
head, otherwise merge the two sorted lists (the forward walk from Neal's
patch, without the EVER_RETRANS test).

Cost is one list_head in tcp_sock and an INIT_LIST_HEAD() in
tcp_init_sock(), tcp_create_openreq_child() and tcp_write_queue_purge().
Reneged skbs are on neither list and would stay as they are today.
Neal's point about per-socket state is fair, but I think 16 bytes is a
reasonable price for not needing the RTO fallback at all.

With that, B is back in tsorted_sent_queue after the undo at .490 in
your scenario. tcp_undo_cwnd_reduction() sets tp->rack.advanced, RACK
looks at B in the same ACK (sent around .413, rack.rtt_us 40ms from C,
reo_wnd around 10ms), marks it lost again and it is retransmitted
immediately. The test then has the same shape as 2/3 (untested):

  .490 < . 1:1(0) ack 1001 win 20000 <TS val 1390 ecr 210,sack 2001:3001>
   +0~+.05 > P. 1001:2001(1000) ack 1 <...>
   +0 %{ assert tcpi_retransmits == 0, tcpi_retransmits }%

and it fails on current kernels, which is what we want from a selftest
shipped with a fix.

If Neal and Yuchung still prefer the stateless version I can live with
it, but then the 3/3 changelog should say that the test passes without
1/3 and documents a known limitation.

[1] https://lore.kernel.org/netdev/CAL4WiipKWgA-EcFWDQd3t7nBwU7AnkLvqs6j--he-dR-SAgqiw@mail.gmail.com/ (local)

Thanks.