Bug #376
Fwd: [PATCH] tcp: fix infinite cwnd in tcp_complete_cwr()
| Status: | New | Start date: | ||
|---|---|---|---|---|
| Priority: | High | Due date: | ||
| Assignee: |  David Taht |
% Done: | 0% |
|
| Category: | Linux Kernel | Spent time: | 2.00 hours | |
| Target version: | 1st Public Cerowrt release |
Description
grumble
---------- Forwarded message ----------
From: Yuchung Cheng <ycheng@google.com>
Date: Sat, Apr 28, 2012 at 12:04 PM
Subject: [PATCH] tcp: fix infinite cwnd in tcp_complete_cwr()
To: davem@davemloft.net, ilpo.jarvinen@helsinki.fi
Cc: ncardwell@google.com, nanditad@google.com, netdev@vger.kernel.org,
Yuchung Cheng <ycheng@google.com>
When the cwnd reduction is done, ssthresh may be infinite
if TCP enters CWR via ECN or F-RTO. If cwnd is not undone, i.e.,
undo_marker is set, tcp_complete_cwr() falsely set cwnd to the
infinite ssthresh value. The correct operation is to keep cwnd
intact because it has been updated in ECN or F-RTO.
Signed-off-by: Yuchung Cheng <ycheng@google.com>
---
net/ipv4/tcp_input.c | 8 +++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c93b0cb..ac417de 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c@ -2868,11 +2868,13 @ static inline void tcp_complete_cwr(struct sock *sk)
/* Do not moderate cwnd if it's already undone in cwr or recovery. /
if (tp->undo_marker) {
- if (inet_csk(sk)->icsk_ca_state TCP_CA_CWR)
+ if (inet_csk(sk)->icsk_ca_state TCP_CA_CWR) {
tp->snd_cwnd = min(tp->snd_cwnd, tp->snd_ssthresh);
- else / PRR /
+ } else if (tp->snd_ssthresh < TCP_INFINITE_SSTHRESH) {
+ / PRR algorithm. */
tp->snd_cwnd = tp->snd_ssthresh;
- tp->snd_cwnd_stamp = tcp_time_stamp;
+ tp->snd_cwnd_stamp = tcp_time_stamp;
+ }
}
tcp_ca_event(sk, CA_EVENT_COMPLETE_CWR);
}
Related issues
| duplicated by Cerowrt - Bug #378: [#376 ] Fwd: [PATCH] tcp: fix infinite cwnd in tcp_comple... | New |
History
Updated by Dave Täht about 1 year ago
- Category set to Linux Kernel
- Assignee set to David Taht
- Priority changed from Normal to High
- Target version set to 1st Public Cerowrt release
---------- Forwarded message ----------
From: Neal Cardwell <ncardwell@google.com>
Date: Sun, Apr 29, 2012 at 8:49 PM
Subject: Re: [PATCH] tcp: fix infinite cwnd in tcp_complete_cwr()
To: Yuchung Cheng <ycheng@google.com>
Cc: davem@davemloft.net, ilpo.jarvinen@helsinki.fi,
nanditad@google.com, netdev@vger.kernel.org
On Sat, Apr 28, 2012 at 3:04 PM, Yuchung Cheng <ycheng@google.com> wrote:
When the cwnd reduction is done, ssthresh may be infinite if TCP enters CWR via ECN or F-RTO. If cwnd is not undone, i.e., undo_marker is set, tcp_complete_cwr() falsely set cwnd to the infinite ssthresh value. The correct operation is to keep cwnd intact because it has been updated in ECN or F-RTO.
Signed-off-by: Yuchung Cheng <ycheng@google.com> net/ipv4/tcp_input.c | 8 ++--- 1 files changed, 5 insertions(), 3 deletions(-)diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index c93b0cb..ac417de 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c -2868,11 +2868,13 static inline void tcp_complete_cwr(struct sock *sk)/* Do not moderate cwnd if it's already undone in cwr or recovery. */ if (tp->undo_marker) { - if (inet_csk(sk)->icsk_ca_state TCP_CA_CWR) + if (inet_csk(sk)->icsk_ca_state TCP_CA_CWR) { tp->snd_cwnd = min(tp->snd_cwnd, tp->snd_ssthresh);In this TCP_CA_CWR case the patch needs to add the assignment:
tp->snd_cwnd_stamp = tcp_time_stamp;I know that was in earlier versions of your patch; looks like it just
got accidentally dropped on this latest version.
Otherwise, looks good to me.
Updated by David Taht about 1 year ago
final patch
---------- Forwarded message ----------
From: Yuchung Cheng <ycheng@google.com>
Date: Mon, Apr 30, 2012 at 9:00 AM
Subject: [PATCH v2] tcp: fix infinite cwnd in tcp_complete_cwr()
To: davem@davemloft.net, ilpo.jarvinen@helsinki.fi, ncardwell@google.com
Cc: nanditad@google.com, netdev@vger.kernel.org, Yuchung Cheng
<ycheng@google.com>
When the cwnd reduction is done, ssthresh may be infinite
if TCP enters CWR via ECN or F-RTO. If cwnd is not undone, i.e.,
undo_marker is set, tcp_complete_cwr() falsely set cwnd to the
infinite ssthresh value. The correct operation is to keep cwnd
intact because it has been updated in ECN or F-RTO.
Signed-off-by: Yuchung Cheng <ycheng@google.com>
---
ChangeLog since v1:
- Add snd_cwnd_stamp timestamping in CWR mode
net/ipv4/tcp_input.c | 9 ++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c93b0cb..e0a9b89 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c@ -2868,11 +2868,14 @ static inline void tcp_complete_cwr(struct sock *sk)
/* Do not moderate cwnd if it's already undone in cwr or recovery. /
if (tp->undo_marker) {
- if (inet_csk(sk)->icsk_ca_state TCP_CA_CWR)
+ if (inet_csk(sk)->icsk_ca_state TCP_CA_CWR) {
tp->snd_cwnd = min(tp->snd_cwnd, tp->snd_ssthresh);
- else / PRR /
+ tp->snd_cwnd_stamp = tcp_time_stamp;
+ } else if (tp->snd_ssthresh < TCP_INFINITE_SSTHRESH) {
+ / PRR algorithm. */
tp->snd_cwnd = tp->snd_ssthresh;
- tp->snd_cwnd_stamp = tcp_time_stamp;
+ tp->snd_cwnd_stamp = tcp_time_stamp;
+ }
}
tcp_ca_event(sk, CA_EVENT_COMPLETE_CWR);
}