From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B7044C004C0 for ; Mon, 23 Oct 2023 11:31:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233833AbjJWLbq (ORCPT ); Mon, 23 Oct 2023 07:31:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51228 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233832AbjJWLbp (ORCPT ); Mon, 23 Oct 2023 07:31:45 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E4B33C1 for ; Mon, 23 Oct 2023 04:31:43 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 36B36C433C7; Mon, 23 Oct 2023 11:31:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1698060703; bh=CBUeou0icsmDTXAgKugx7alYaBVMuJfxPkLZGA74fWA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=2QbpUhpiap0242Mf7HDWjYF3xdLlq0agTjRc1hVjRG+wRRh51Yj7DBfLm72POhmIY OrLb4AXhg0BJmcPG0DPdWaVYuAk5J2WnE4A7S4dwjTs/M6UZNRDlTEL9hotptkWXDK xg+uxBJVFG6so4O/Ffn5vkQlqX1EVtMfnn7wyNko= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Stefan Wahren , Eric Dumazet , Neal Cardwell , Jakub Kicinski Subject: [PATCH 5.4 064/123] tcp: tsq: relax tcp_small_queue_check() when rtx queue contains a single skb Date: Mon, 23 Oct 2023 12:57:02 +0200 Message-ID: <20231023104819.829289116@linuxfoundation.org> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231023104817.691299567@linuxfoundation.org> References: <20231023104817.691299567@linuxfoundation.org> User-Agent: quilt/0.67 X-stable: review X-Patchwork-Hint: ignore MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org 5.4-stable review patch. If anyone has any objections, please let me know. ------------------ From: Eric Dumazet commit f921a4a5bffa8a0005b190fb9421a7fc1fd716b6 upstream. In commit 75eefc6c59fd ("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 75c119afe14f ("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 65466904b015 ("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: 75c119afe14f ("tcp: implement rb-tree based retransmit queue") Link: https://lore.kernel.org/netdev/7f31ddc8-9971-495e-a1f6-819df542e0af@gmx.net/ Reported-by: Stefan Wahren Tested-by: Stefan Wahren Signed-off-by: Eric Dumazet Acked-by: Neal Cardwell Link: https://lore.kernel.org/r/20231017124526.4060202-1-edumazet@google.com Signed-off-by: Jakub Kicinski Signed-off-by: Greg Kroah-Hartman --- net/ipv4/tcp_output.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -2259,6 +2259,18 @@ static bool tcp_pacing_check(struct sock return true; } +static bool tcp_rtx_queue_empty_or_single_skb(const struct sock *sk) +{ + const struct rb_node *node = sk->tcp_rtx_queue.rb_node; + + /* No skb in the rtx queue. */ + if (!node) + return true; + + /* Only one skb in rtx queue. */ + return !node->rb_left && !node->rb_right; +} + /* TCP Small Queues : * Control number of packets in qdisc/devices to two packets / or ~1 ms. * (These limits are doubled for retransmits) @@ -2296,12 +2308,12 @@ static bool tcp_small_queue_check(struct limit += extra_bytes; } if (refcount_read(&sk->sk_wmem_alloc) > limit) { - /* Always send skb if rtx queue is empty. + /* Always send skb if rtx queue is empty or has one skb. * No need to wait for TX completion to call us back, * after softirq/tasklet schedule. * This helps when TX completions are delayed too much. */ - if (tcp_rtx_queue_empty(sk)) + if (tcp_rtx_queue_empty_or_single_skb(sk)) return false; set_bit(TSQ_THROTTLED, &sk->sk_tsq_flags);