From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd42.google.com (mail-io1-xd42.google.com [IPv6:2607:f8b0:4864:20::d42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id 042173CB35 for ; Tue, 10 Sep 2019 05:40:04 -0400 (EDT) Received: by mail-io1-xd42.google.com with SMTP id b136so36029420iof.3 for ; Tue, 10 Sep 2019 02:40:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=s2jM2rPH13YleFbWVnnMMVNJNJrB7ClmDrFBZbPbUGQ=; b=Vhw+AEfTQLZNLDWluCoHKqbkwfSDXnIEcGyYa6CfsDG2Uxnf4DoARa9L+4neI8kakp 7KUorSJ2o/8ZlQXrMXiNPVT//1+3rlow47pI1UVAsJRIfYNGUv4msSamik9Dc4Zkhs5e I3nf/juJnC01I8XF69bealNto5lPqSlI4hZwmYqGy27s8a0N776WVTPjyglW5tsi+nfL AbMSt6e9UZadHZ7hER2JAyf16mHrBIUxBLLuN+sQBeuT9YOHvzaCYV4/4E93XqbVbaZw ZMSI3qcteTo/XjTrz67thS6BNyXhX04BQ7+te/fIO5kLB4fOqMgMQ0vckbicvS7UIyAo vCiA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=s2jM2rPH13YleFbWVnnMMVNJNJrB7ClmDrFBZbPbUGQ=; b=Szz5bT4AtAvVx0r/MCH0n7qiBSDe6X2/Bj0f0fpIdNOt2Mv9RUoLKrhHDTiaC3NB/Y NQ3cCF0ue/25qPhYLf03FCPmYyi/WZZEPw+547MDgLnlyKGllGwlRGaq6p52YIiuc8k5 UNwUJpEjMIvZUPwx6LpfJKqcq7qUm/9q08FGSoyrOWuzvFgDEoXfXMsdRT54R6MvmemW JakNt/58BkTi3vvnkPPJMXYRJXNTuZGTD7+kpQ5vW3+tcxB69QW4eTlMt5mPhWR+RZY3 xN4LyxpPArm8WzycXjh8VIdlawVjod8x22exNpZgq9dTKUUxiqCskNjhcYB9Nk4RvYKT 1bVw== X-Gm-Message-State: APjAAAVTIDqXs8M8DAWmf9FxNRXQP+Eg14JhAh0W+dVph9fPUSMOspHB yfJLXJRdSTAiFkhjIHf9WWzDee+eawBYnhEADDccdY7NJ1g= X-Google-Smtp-Source: APXvYqxxJFNb6q6DQRHCZnfhLyx8XQ3NeCgVJJPdit03LEvjsK2DP5Xr4r12hOzW6L27/abjont0f3H9xI7hdMxpoeQ= X-Received: by 2002:a6b:7a09:: with SMTP id h9mr29299266iom.0.1568108404228; Tue, 10 Sep 2019 02:40:04 -0700 (PDT) MIME-Version: 1.0 References: <20190909205602.248472-1-ncardwell@google.com> In-Reply-To: <20190909205602.248472-1-ncardwell@google.com> From: Dave Taht Date: Tue, 10 Sep 2019 10:39:52 +0100 Message-ID: To: ECN-Sane Content-Type: multipart/alternative; boundary="00000000000067454c05922fb0e8" Subject: [Ecn-sane] Fwd: [PATCH net] tcp: fix tcp_ecn_withdraw_cwr() to clear TCP_ECN_QUEUE_CWR X-BeenThere: ecn-sane@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion of explicit congestion notification's impact on the Internet List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Sep 2019 09:40:05 -0000 --00000000000067454c05922fb0e8 Content-Type: text/plain; charset="UTF-8" ---------- Forwarded message --------- From: Neal Cardwell Date: Tue, Sep 10, 2019, 10:32 AM Subject: [PATCH net] tcp: fix tcp_ecn_withdraw_cwr() to clear TCP_ECN_QUEUE_CWR To: David Miller Cc: , Neal Cardwell , Yuchung Cheng , Soheil Hassas Yeganeh , Eric Dumazet Fix tcp_ecn_withdraw_cwr() to clear the correct bit: TCP_ECN_QUEUE_CWR. Rationale: basically, TCP_ECN_DEMAND_CWR is a bit that is purely about the behavior of data receivers, and deciding whether to reflect incoming IP ECN CE marks as outgoing TCP th->ece marks. The TCP_ECN_QUEUE_CWR bit is purely about the behavior of data senders, and deciding whether to send CWR. The tcp_ecn_withdraw_cwr() function is only called from tcp_undo_cwnd_reduction() by data senders during an undo, so it should zero the sender-side state, TCP_ECN_QUEUE_CWR. It does not make sense to stop the reflection of incoming CE bits on incoming data packets just because outgoing packets were spuriously retransmitted. The bug has been reproduced with packetdrill to manifest in a scenario with RFC3168 ECN, with an incoming data packet with CE bit set and carrying a TCP timestamp value that causes cwnd undo. Before this fix, the IP CE bit was ignored and not reflected in the TCP ECE header bit, and sender sent a TCP CWR ('W') bit on the next outgoing data packet, even though the cwnd reduction had been undone. After this fix, the sender properly reflects the CE bit and does not set the W bit. Note: the bug actually predates 2005 git history; this Fixes footer is chosen to be the oldest SHA1 I have tested (from Sep 2007) for which the patch applies cleanly (since before this commit the code was in a .h file). Fixes: bdf1ee5d3bd3 ("[TCP]: Move code from tcp_ecn.h to tcp*.c and tcp.h & remove it") Signed-off-by: Neal Cardwell Acked-by: Yuchung Cheng Acked-by: Soheil Hassas Yeganeh Cc: Eric Dumazet --- net/ipv4/tcp_input.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index c21e8a22fb3b..8a1cd93dbb09 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -266,7 +266,7 @@ static void tcp_ecn_accept_cwr(struct sock *sk, const struct sk_buff *skb) static void tcp_ecn_withdraw_cwr(struct tcp_sock *tp) { - tp->ecn_flags &= ~TCP_ECN_DEMAND_CWR; + tp->ecn_flags &= ~TCP_ECN_QUEUE_CWR; } static void __tcp_ecn_check_ce(struct sock *sk, const struct sk_buff *skb) -- 2.23.0.162.g0b9fbb3734-goog --00000000000067454c05922fb0e8 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

---------- Forwarded message ---------
From: Neal Cardwell <ncardwell@google.com&= gt;
Date: Tue, Sep 10, 2019, 10:32 AM
Subject: [PATCH net] tcp= : fix tcp_ecn_withdraw_cwr() to clear TCP_ECN_QUEUE_CWR
To: David Miller= <davem@davemloft.net>
= Cc: <netdev@vger.kernel.org>, Neal Cardwell <ncardwell= @google.com>, Yuchung Cheng <ycheng@google.com>, Soheil Hassas Yeganeh <soheil@google.com>, Eric Dumazet <edumazet@google.com>


Fix tc= p_ecn_withdraw_cwr() to clear the correct bit:
TCP_ECN_QUEUE_CWR.

Rationale: basically, TCP_ECN_DEMAND_CWR is a bit that is purely about
the behavior of data receivers, and deciding whether to reflect
incoming IP ECN CE marks as outgoing TCP th->ece marks. The
TCP_ECN_QUEUE_CWR bit is purely about the behavior of data senders,
and deciding whether to send CWR. The tcp_ecn_withdraw_cwr() function
is only called from tcp_undo_cwnd_reduction() by data senders during
an undo, so it should zero the sender-side state,
TCP_ECN_QUEUE_CWR. It does not make sense to stop the reflection of
incoming CE bits on incoming data packets just because outgoing
packets were spuriously retransmitted.

The bug has been reproduced with packetdrill to manifest in a scenario
with RFC3168 ECN, with an incoming data packet with CE bit set and
carrying a TCP timestamp value that causes cwnd undo. Before this fix,
the IP CE bit was ignored and not reflected in the TCP ECE header bit,
and sender sent a TCP CWR ('W') bit on the next outgoing data packe= t,
even though the cwnd reduction had been undone.=C2=A0 After this fix, the sender properly reflects the CE bit and does not set the W bit.

Note: the bug actually predates 2005 git history; this Fixes footer is
chosen to be the oldest SHA1 I have tested (from Sep 2007) for which
the patch applies cleanly (since before this commit the code was in a
.h file).

Fixes: bdf1ee5d3bd3 ("[TCP]: Move code from tcp_ecn.h to tcp*.c and tc= p.h & remove it")
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Cc: Eric Dumazet <edumazet@google.com>
---
=C2=A0net/ipv4/tcp_input.c | 2 +-
=C2=A01 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c21e8a22fb3b..8a1cd93dbb09 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -266,7 +266,7 @@ static void tcp_ecn_accept_cwr(struct sock *sk, const s= truct sk_buff *skb)

=C2=A0static void tcp_ecn_withdraw_cwr(struct tcp_sock *tp)
=C2=A0{
-=C2=A0 =C2=A0 =C2=A0 =C2=A0tp->ecn_flags &=3D ~TCP_ECN_DEMAND_CWR;<= br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0tp->ecn_flags &=3D ~TCP_ECN_QUEUE_CWR; =C2=A0}

=C2=A0static void __tcp_ecn_check_ce(struct sock *sk, const struct sk_buff = *skb)
--
2.23.0.162.g0b9fbb3734-goog

--00000000000067454c05922fb0e8--