Make congestion control algorithms take initcwnd_segments value into account after resuming from idle state while trying to decide the cwnd value.
Updating only newreno one as that gets used by other CCs.
Differential D5124
Update <cc>_after_idle to take initcwnd_segments into account.
hiren on Jan 29 2016, 10:40 PM. Authored by Tags None Referenced Files
Details
Diff Detail
Event TimelineComment Actions I would suggest that the code to handle RFC3390 should be merged with the new code i.e. the net.inet.tcp.rfc3390 sysctl should become a SYSCTL_PROC and simply set V_tcp_initcwnd_segments=3 behind the scenes, and return the evaluated result of "V_tcp_initcwnd_segments==3" as the sysctl value. Comment Actions Oops, that should of course be 4 segments, not 3 (though recall that we really need an initcwnd_bytes variable as well in order to fully capture the spirit of the RFC 3390 and later RFCs - something perhaps you can add as part of this work). Comment Actions @lstewart I agree and I think its time to improve the initcwnd handling code. But that'd be a separate commit. What is your take on the problem at hand? Are you okay with the diffs? I'd like to get this in and possibly MFC for 10.3. Now, coming to the cleanup/improvement part: tcp_initcwnd() would return snd_cwnd based on different rfcs and what not. About cwnd_bytes, iirc, we discussed something along the lines of having a similar sysctl named initcwnd_bytes where you can specify initial congestion window in bytes. Now how should we handle the new sysctls default value? should we initialize it to 14600 as we initialize initcwnd_segments to 10? I'd like to start discussing this on separate review/commit. Comment Actions The diff doesn't make a whole lot of sense to me in the current world order with defaults net.inet.tcp.rfc3390=1 and net.inet.tcp.initcwnd_segments=10 given that they are in effect mutually exclusive, and setting net.inet.tcp.initcwnd_segments=0 in order to get 3390 behaviour is just plain bizarre.
Ok well I'll draw a line under it in this review for the moment until we discuss elsewhere. Comment Actions https://reviews.freebsd.org/D5173 is what is in the works now. I still suggest we should fix 10.3R to not have this particular problem. I patched our inhouse tree with something like this for stable/10: --- a/sys/netinet/cc/cc_newreno.c +++ b/sys/netinet/cc/cc_newreno.c @@ -166,7 +166,10 @@ newreno_after_idle(struct cc_var *ccv) * * See RFC5681 Section 4.1. "Restarting Idle Connections". */ - if (V_tcp_do_rfc3390) + if (V_tcp_do_initcwnd10) + rw = min(10 * CCV(ccv, t_maxseg), + max(2 * CCV(ccv, t_maxseg), 14600)); + else if (V_tcp_do_rfc3390) rw = min(4 * CCV(ccv, t_maxseg), max(2 * CCV(ccv, t_maxseg), 4380)); else |