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

[Jiayuan Chen <jiayuan.chen@linux.dev>]

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)
> +				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 ?

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);
>   	}