Message ID | 20190909205602.248472-1-ncardwell@google.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [net] tcp: fix tcp_ecn_withdraw_cwr() to clear TCP_ECN_QUEUE_CWR | expand |
On Mon, Sep 9, 2019 at 10:56 PM Neal Cardwell <ncardwell@google.com> wrote: > > 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 <ncardwell@google.com> > Acked-by: Yuchung Cheng <ycheng@google.com> > Acked-by: Soheil Hassas Yeganeh <soheil@google.com> > Cc: Eric Dumazet <edumazet@google.com> Signed-off-by: Eric Dumazet <edumazet@google.com> Thanks Neal
From: Neal Cardwell <ncardwell@google.com> Date: Mon, 9 Sep 2019 16:56:02 -0400 > 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 <ncardwell@google.com> > Acked-by: Yuchung Cheng <ycheng@google.com> > Acked-by: Soheil Hassas Yeganeh <soheil@google.com> > Cc: Eric Dumazet <edumazet@google.com> Applied and queued up for -stable, thanks Neal.
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)