Skip to content

Commit f921a4a

Browse files
Eric Dumazetkuba-moo
Eric Dumazet
authored andcommitted
tcp: tsq: relax tcp_small_queue_check() when rtx queue contains a single skb
In commit 75eefc6 ("tcp: tsq: add a shortcut in tcp_small_queue_check()") we allowed to send an skb regardless of TSQ limits being hit if rtx queue was empty or had a single skb, in order to better fill the pipe when/if TX completions were slow. Then later, commit 75c119a ("tcp: implement rb-tree based retransmit queue") accidentally removed the special case for one skb in rtx queue. Stefan Wahren reported a regression in single TCP flow throughput using a 100Mbit fec link, starting from commit 6546690 ("tcp: adjust TSO packet sizes based on min_rtt"). This last commit only made the regression more visible, because it locked the TCP flow on a particular behavior where TSQ prevented two skbs being pushed downstream, adding silences on the wire between each TSO packet. Many thanks to Stefan for his invaluable help ! Fixes: 75c119a ("tcp: implement rb-tree based retransmit queue") Link: https://lore.kernel.org/netdev/[email protected]/ Reported-by: Stefan Wahren <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Eric Dumazet <[email protected]> Acked-by: Neal Cardwell <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent a0ca6b9 commit f921a4a

File tree

1 file changed

+14
-2
lines changed

1 file changed

+14
-2
lines changed

net/ipv4/tcp_output.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2542,6 +2542,18 @@ static bool tcp_pacing_check(struct sock *sk)
25422542
return true;
25432543
}
25442544

2545+
static bool tcp_rtx_queue_empty_or_single_skb(const struct sock *sk)
2546+
{
2547+
const struct rb_node *node = sk->tcp_rtx_queue.rb_node;
2548+
2549+
/* No skb in the rtx queue. */
2550+
if (!node)
2551+
return true;
2552+
2553+
/* Only one skb in rtx queue. */
2554+
return !node->rb_left && !node->rb_right;
2555+
}
2556+
25452557
/* TCP Small Queues :
25462558
* Control number of packets in qdisc/devices to two packets / or ~1 ms.
25472559
* (These limits are doubled for retransmits)
@@ -2579,12 +2591,12 @@ static bool tcp_small_queue_check(struct sock *sk, const struct sk_buff *skb,
25792591
limit += extra_bytes;
25802592
}
25812593
if (refcount_read(&sk->sk_wmem_alloc) > limit) {
2582-
/* Always send skb if rtx queue is empty.
2594+
/* Always send skb if rtx queue is empty or has one skb.
25832595
* No need to wait for TX completion to call us back,
25842596
* after softirq/tasklet schedule.
25852597
* This helps when TX completions are delayed too much.
25862598
*/
2587-
if (tcp_rtx_queue_empty(sk))
2599+
if (tcp_rtx_queue_empty_or_single_skb(sk))
25882600
return false;
25892601

25902602
set_bit(TSQ_THROTTLED, &sk->sk_tsq_flags);

0 commit comments

Comments
 (0)