<div dir="auto"></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">---------- Forwarded message ---------<br>From: <strong class="gmail_sendername" dir="auto">Neal Cardwell</strong> <span dir="auto"><<a href="mailto:ncardwell@google.com">ncardwell@google.com</a>></span><br>Date: Tue, Sep 10, 2019, 10:32 AM<br>Subject: [PATCH net] tcp: fix tcp_ecn_withdraw_cwr() to clear TCP_ECN_QUEUE_CWR<br>To: David Miller <<a href="mailto:davem@davemloft.net">davem@davemloft.net</a>><br>Cc:  <<a href="mailto:netdev@vger.kernel.org">netdev@vger.kernel.org</a>>, Neal Cardwell <<a href="mailto:ncardwell@google.com">ncardwell@google.com</a>>, Yuchung Cheng <<a href="mailto:ycheng@google.com">ycheng@google.com</a>>, Soheil Hassas Yeganeh <<a href="mailto:soheil@google.com">soheil@google.com</a>>, Eric Dumazet <<a href="mailto:edumazet@google.com">edumazet@google.com</a>><br></div><br><br>Fix tcp_ecn_withdraw_cwr() to clear the correct bit:<br>
TCP_ECN_QUEUE_CWR.<br>
<br>
Rationale: basically, TCP_ECN_DEMAND_CWR is a bit that is purely about<br>
the behavior of data receivers, and deciding whether to reflect<br>
incoming IP ECN CE marks as outgoing TCP th->ece marks. The<br>
TCP_ECN_QUEUE_CWR bit is purely about the behavior of data senders,<br>
and deciding whether to send CWR. The tcp_ecn_withdraw_cwr() function<br>
is only called from tcp_undo_cwnd_reduction() by data senders during<br>
an undo, so it should zero the sender-side state,<br>
TCP_ECN_QUEUE_CWR. It does not make sense to stop the reflection of<br>
incoming CE bits on incoming data packets just because outgoing<br>
packets were spuriously retransmitted.<br>
<br>
The bug has been reproduced with packetdrill to manifest in a scenario<br>
with RFC3168 ECN, with an incoming data packet with CE bit set and<br>
carrying a TCP timestamp value that causes cwnd undo. Before this fix,<br>
the IP CE bit was ignored and not reflected in the TCP ECE header bit,<br>
and sender sent a TCP CWR ('W') bit on the next outgoing data packet,<br>
even though the cwnd reduction had been undone.  After this fix, the<br>
sender properly reflects the CE bit and does not set the W bit.<br>
<br>
Note: the bug actually predates 2005 git history; this Fixes footer is<br>
chosen to be the oldest SHA1 I have tested (from Sep 2007) for which<br>
the patch applies cleanly (since before this commit the code was in a<br>
.h file).<br>
<br>
Fixes: bdf1ee5d3bd3 ("[TCP]: Move code from tcp_ecn.h to tcp*.c and tcp.h & remove it")<br>
Signed-off-by: Neal Cardwell <<a href="mailto:ncardwell@google.com" target="_blank" rel="noreferrer">ncardwell@google.com</a>><br>
Acked-by: Yuchung Cheng <<a href="mailto:ycheng@google.com" target="_blank" rel="noreferrer">ycheng@google.com</a>><br>
Acked-by: Soheil Hassas Yeganeh <<a href="mailto:soheil@google.com" target="_blank" rel="noreferrer">soheil@google.com</a>><br>
Cc: Eric Dumazet <<a href="mailto:edumazet@google.com" target="_blank" rel="noreferrer">edumazet@google.com</a>><br>
---<br>
 net/ipv4/tcp_input.c | 2 +-<br>
 1 file changed, 1 insertion(+), 1 deletion(-)<br>
<br>
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c<br>
index c21e8a22fb3b..8a1cd93dbb09 100644<br>
--- a/net/ipv4/tcp_input.c<br>
+++ b/net/ipv4/tcp_input.c<br>
@@ -266,7 +266,7 @@ static void tcp_ecn_accept_cwr(struct sock *sk, const struct sk_buff *skb)<br>
<br>
 static void tcp_ecn_withdraw_cwr(struct tcp_sock *tp)<br>
 {<br>
-       tp->ecn_flags &= ~TCP_ECN_DEMAND_CWR;<br>
+       tp->ecn_flags &= ~TCP_ECN_QUEUE_CWR;<br>
 }<br>
<br>
 static void __tcp_ecn_check_ce(struct sock *sk, const struct sk_buff *skb)<br>
-- <br>
2.23.0.162.g0b9fbb3734-goog<br>
<br>
</div>