Re: [PATCH net 1/2] tcp: restore RACK list membership when undoing loss

[Kuniyuki Iwashima <kuniyu@google.com>]

From: Jiayuan Chen <jiayuan.chen@linux.dev>
Date: Wed, 30 Sep 2026 10:13:42 +0800
> On 9/26/26 8:25 AM, nramaswamy@openai.com wrote:
> > From: Neil Ramaswamy <nramaswamy@openai.com>
> >
> > Partial undo can clear the TCPCB_LOST flag on segments already removed from
> > RACK's list, which prevents subsequent RACK loss detection and can lead to
> > long retransmission delays. Restoring them to the RACK list allows them to
> > be reconsidered for fast retransmission in the future. To do this, we first
> > sort the segments whose lost flag is being cleared, and reinsert them into
> > the RACK list (which is sorted by transmission time).
> >
> > Fixes: 043b87d7599e ("tcp: more efficient RACK loss detection")
> > Signed-off-by: Neil Ramaswamy <nramaswamy@openai.com>
> > Assisted-by: LLM sparse
> > ---
> >   net/ipv4/tcp_input.c | 33 +++++++++++++++++++++++++++++++++
> >   1 file changed, 33 insertions(+)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index 92bc60716f33..38ac07c8b38f 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -69,6 +69,7 @@
> >   #include <linux/module.h>
> >   #include <linux/sysctl.h>
> >   #include <linux/kernel.h>
> > +#include <linux/list_sort.h>
> >   #include <linux/prefetch.h>
> >   #include <linux/bitops.h>
> >   #include <net/dst.h>
> > @@ -2840,16 +2841,48 @@ static void DBGUNDO(struct sock *sk, const char *msg)
> >   #endif
> >   }
> >   
> > +static int tcp_rack_skb_cmp(void *priv, const struct list_head *a,
> > +			    const struct list_head *b)
> > +{
> > +	const struct sk_buff *skb_a = list_entry(a, struct sk_buff,
> > +					       tcp_tsorted_anchor);
> > +	const struct sk_buff *skb_b = list_entry(b, struct sk_buff,
> > +					       tcp_tsorted_anchor);
> > +
> > +	return tcp_skb_sent_after(tcp_skb_timestamp_us(skb_a),
> > +				  tcp_skb_timestamp_us(skb_b),
> > +				  TCP_SKB_CB(skb_a)->end_seq,
> > +				  TCP_SKB_CB(skb_b)->end_seq);
> > +}
> > +
> >   static void tcp_undo_cwnd_reduction(struct sock *sk, bool unmark_loss)
> >   {
> >   	struct tcp_sock *tp = tcp_sk(sk);
> >   
> >   	if (unmark_loss) {
> > +		LIST_HEAD(restored);
> >   		struct sk_buff *skb;
> >   
> >   		skb_rbtree_walk(skb, &sk->tcp_rtx_queue) {
> > +			if ((TCP_SKB_CB(skb)->sacked & TCPCB_LOST) == TCPCB_LOST)

nit: "== TCPCB_LOST" is redundant


> > +				list_move_tail(&skb->tcp_tsorted_anchor, &restored);
> 
> 
> I think the simplest way to solve the problems is just drop 
> 'list_del_init(&skb->tcp_tsorted_anchor);' in tcp_rack_detect_loss(),
> although it will reduce the efficiency of RACK loss detection ?

It will almost revert the optimisation done by 043b87d7599e.

One sort + merging sorted lists during undo seems better
than reintroducing costs in the fast(er) path.


> 
> Leave it to maintainers.
> 
> 
> >   			TCP_SKB_CB(skb)->sacked &= ~TCPCB_LOST;
> >   		}
> > +		if (!list_empty(&restored)) {
> > +			struct list_head *pos = &tp->tsorted_sent_queue;
> > +
> > +			/* Ensure lost skbs are added in transmission order */
> > +			list_sort(NULL, &restored, tcp_rack_skb_cmp);
> > +			while (!list_empty(&restored)) {
> > +				struct list_head *entry = restored.next;
> > +
> > +				while (pos->next != &tp->tsorted_sent_queue &&
> > +				       !tcp_rack_skb_cmp(NULL, pos->next, entry))
> > +					pos = pos->next;
> > +				list_move(entry, pos);
> > +				pos = entry;
> > +			}
> > +		}
> >   		tp->lost_out = 0;
> >   		tcp_clear_all_retrans_hints(tp);
> >   	}