lkml.org 
[lkml]   [2020]   [Dec]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 031/346] tcp: fix cwnd-limited bug for TSO deferral where we send nothing
    Date
    From: Neal Cardwell <ncardwell@google.com>

    [ Upstream commit 299bcb55ecd1412f6df606e9dc0912d55610029e ]

    When cwnd is not a multiple of the TSO skb size of N*MSS, we can get
    into persistent scenarios where we have the following sequence:

    (1) ACK for full-sized skb of N*MSS arrives
    -> tcp_write_xmit() transmit full-sized skb with N*MSS
    -> move pacing release time forward
    -> exit tcp_write_xmit() because pacing time is in the future

    (2) TSQ callback or TCP internal pacing timer fires
    -> try to transmit next skb, but TSO deferral finds remainder of
    available cwnd is not big enough to trigger an immediate send
    now, so we defer sending until the next ACK.

    (3) repeat...

    So we can get into a case where we never mark ourselves as
    cwnd-limited for many seconds at a time, even with
    bulk/infinite-backlog senders, because:

    o In case (1) above, every time in tcp_write_xmit() we have enough
    cwnd to send a full-sized skb, we are not fully using the cwnd
    (because cwnd is not a multiple of the TSO skb size). So every time we
    send data, we are not cwnd limited, and so in the cwnd-limited
    tracking code in tcp_cwnd_validate() we mark ourselves as not
    cwnd-limited.

    o In case (2) above, every time in tcp_write_xmit() that we try to
    transmit the "remainder" of the cwnd but defer, we set the local
    variable is_cwnd_limited to true, but we do not send any packets, so
    sent_pkts is zero, so we don't call the cwnd-limited logic to update
    tp->is_cwnd_limited.

    Fixes: ca8a22634381 ("tcp: make cwnd-limited checks measurement-based, and gentler")
    Reported-by: Ingemar Johansson <ingemar.s.johansson@ericsson.com>
    Signed-off-by: Neal Cardwell <ncardwell@google.com>
    Signed-off-by: Yuchung Cheng <ycheng@google.com>
    Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
    Signed-off-by: Eric Dumazet <edumazet@google.com>
    Link: https://lore.kernel.org/r/20201209035759.1225145-1-ncardwell.kernel@gmail.com
    Signed-off-by: Jakub Kicinski <kuba@kernel.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/ipv4/tcp_output.c | 9 ++++++---
    1 file changed, 6 insertions(+), 3 deletions(-)

    --- a/net/ipv4/tcp_output.c
    +++ b/net/ipv4/tcp_output.c
    @@ -1622,7 +1622,8 @@ static void tcp_cwnd_validate(struct soc
    * window, and remember whether we were cwnd-limited then.
    */
    if (!before(tp->snd_una, tp->max_packets_seq) ||
    - tp->packets_out > tp->max_packets_out) {
    + tp->packets_out > tp->max_packets_out ||
    + is_cwnd_limited) {
    tp->max_packets_out = tp->packets_out;
    tp->max_packets_seq = tp->snd_nxt;
    tp->is_cwnd_limited = is_cwnd_limited;
    @@ -2407,6 +2408,10 @@ repair:
    else
    tcp_chrono_stop(sk, TCP_CHRONO_RWND_LIMITED);

    + is_cwnd_limited |= (tcp_packets_in_flight(tp) >= tp->snd_cwnd);
    + if (likely(sent_pkts || is_cwnd_limited))
    + tcp_cwnd_validate(sk, is_cwnd_limited);
    +
    if (likely(sent_pkts)) {
    if (tcp_in_cwnd_reduction(sk))
    tp->prr_out += sent_pkts;
    @@ -2414,8 +2419,6 @@ repair:
    /* Send one loss probe per tail loss episode. */
    if (push_one != 2)
    tcp_schedule_loss_probe(sk, false);
    - is_cwnd_limited |= (tcp_packets_in_flight(tp) >= tp->snd_cwnd);
    - tcp_cwnd_validate(sk, is_cwnd_limited);
    return false;
    }
    return !tp->packets_out && !tcp_write_queue_empty(sk);

    \
     
     \ /
      Last update: 2020-12-28 14:21    [W:3.123 / U:1.168 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site