Message ID | 1342762841.2626.5633.camel@edumazet-glaptop |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
It might be clearer to instead introduce a temporary variable to calculate the snd_cwnd change in the while loop. That is: unsigned int snd_cwnd_delta = 0; ... tp->snd_cwnd_cnt += cnt; while (tp->snd_cwnd_cnt >= tp->snd_cwnd) { tp->snd_cwnd_cnt -= tp->snd_cwnd; snd_cwnd_delta++; } tp->snd_cwnd = min(tp->snd_cwnd + snd_cwnd_delta, tp->snd_cwnd_clamp); On Fri, Jul 20, 2012 at 1:40 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > From: Eric Dumazet <edumazet@google.com> > > When/if sysctl_tcp_abc > 1, we expect to increase cwnd by 2 if the > received ACK acknowledges more than 2*MSS bytes, in tcp_slow_start() > > Problem is this RFC 3465 statement is not correctly coded, as > the while () loop increases snd_cwnd one by one. > > So to reach the "cwnd += 2" goal, we need to use "cnt = 2*cnt + 1" > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Tom Herbert <therbert@google.com> > Cc: Yuchung Cheng <ycheng@google.com> > Cc: Neal Cardwell <ncardwell@google.com> > Cc: Nandita Dukkipati <nanditad@google.com> > Cc: John Heffner <johnwheffner@gmail.com> > Cc: Stephen Hemminger <shemminger@vyatta.com> > --- > net/ipv4/tcp_cong.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c > index 04dbd7a..486379a 100644 > --- a/net/ipv4/tcp_cong.c > +++ b/net/ipv4/tcp_cong.c > @@ -306,7 +306,7 @@ EXPORT_SYMBOL_GPL(tcp_is_cwnd_limited); > */ > void tcp_slow_start(struct tcp_sock *tp) > { > - int cnt; /* increase in packets */ > + unsigned int cnt; /* increase in packets */ > > /* RFC3465: ABC Slow start > * Increase only after a full MSS of bytes is acked > @@ -318,16 +318,19 @@ void tcp_slow_start(struct tcp_sock *tp) > if (sysctl_tcp_abc && tp->bytes_acked < tp->mss_cache) > return; > > - if (sysctl_tcp_max_ssthresh > 0 && tp->snd_cwnd > sysctl_tcp_max_ssthresh) > + cnt = tp->snd_cwnd; /* exponential increase */ > + if (sysctl_tcp_max_ssthresh > 0 && > + tp->snd_cwnd > sysctl_tcp_max_ssthresh) > cnt = sysctl_tcp_max_ssthresh >> 1; /* limited slow start */ > - else > - cnt = tp->snd_cwnd; /* exponential increase */ > > /* RFC3465: ABC > * We MAY increase by 2 if discovered delayed ack > + * The "+ 1" in the expression is needed if we want to increase > + * cwnd by 2, because the way is coded the following loop. > */ > if (sysctl_tcp_abc > 1 && tp->bytes_acked >= 2*tp->mss_cache) > - cnt <<= 1; > + cnt = 2*cnt + 1; > + > tp->bytes_acked = 0; > > tp->snd_cwnd_cnt += cnt; > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2012-07-20 at 10:41 -0400, John Heffner wrote: > It might be clearer to instead introduce a temporary variable to > calculate the snd_cwnd change in the while loop. That is: > > unsigned int snd_cwnd_delta = 0; > ... > tp->snd_cwnd_cnt += cnt; > while (tp->snd_cwnd_cnt >= tp->snd_cwnd) { > tp->snd_cwnd_cnt -= tp->snd_cwnd; > snd_cwnd_delta++; > } > tp->snd_cwnd = min(tp->snd_cwnd + snd_cwnd_delta, tp->snd_cwnd_clamp); > Good idea, thanks, I'll send a v2 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c index 04dbd7a..486379a 100644 --- a/net/ipv4/tcp_cong.c +++ b/net/ipv4/tcp_cong.c @@ -306,7 +306,7 @@ EXPORT_SYMBOL_GPL(tcp_is_cwnd_limited); */ void tcp_slow_start(struct tcp_sock *tp) { - int cnt; /* increase in packets */ + unsigned int cnt; /* increase in packets */ /* RFC3465: ABC Slow start * Increase only after a full MSS of bytes is acked @@ -318,16 +318,19 @@ void tcp_slow_start(struct tcp_sock *tp) if (sysctl_tcp_abc && tp->bytes_acked < tp->mss_cache) return; - if (sysctl_tcp_max_ssthresh > 0 && tp->snd_cwnd > sysctl_tcp_max_ssthresh) + cnt = tp->snd_cwnd; /* exponential increase */ + if (sysctl_tcp_max_ssthresh > 0 && + tp->snd_cwnd > sysctl_tcp_max_ssthresh) cnt = sysctl_tcp_max_ssthresh >> 1; /* limited slow start */ - else - cnt = tp->snd_cwnd; /* exponential increase */ /* RFC3465: ABC * We MAY increase by 2 if discovered delayed ack + * The "+ 1" in the expression is needed if we want to increase + * cwnd by 2, because the way is coded the following loop. */ if (sysctl_tcp_abc > 1 && tp->bytes_acked >= 2*tp->mss_cache) - cnt <<= 1; + cnt = 2*cnt + 1; + tp->bytes_acked = 0; tp->snd_cwnd_cnt += cnt;