- User Since
- May 11 2014, 7:08 PM (499 w, 2 d)
Mar 2 2022
Dec 1 2021
I won't have the time to review this in the near future but would note that many of the things I flagged for the original newreno hystart++ implementation also apply to this implementation. However, I wouldn't call any of them blockers so don't have an objection to you committing this at your discretion.
Nov 24 2021
Nov 16 2021
The whole point of this was that there were absolutely no logs what so ever when tcp_respond() was used. So prior to this set of commits you *never* saw anything when we responded to a packet (and that happens in many places in all stacks). Now I would have to study when we use tcp_respond() with a INP_RLOCK() this is a very narrow case I would imagine, but if you are not willing to get a INP_WLOCK() that implies to me you are not changing the TCB... but thats what logging does.
Nov 15 2021
Nov 4 2021
Thanks for fixing!
Nov 3 2021
First pass feedback
Oct 22 2021
I haven't finished reviewing yet but some initial thoughts in addition to my inline comments:
- Nit: please submit diffs with full context as it makes reviewing easier
- You need to address how enabling Hystart++ when using cc_newreno with the default FreeBSD stack will work
- The implementation pessimises the common case where newreno will be used without ABE or Hystart++ which is probably undesirable. The module's lack of a cb_init() hook implementation was very deliberate to avoid such pessimisation and also as noted inline, assumptions are made elsewhere that cc_newreno can always be used as an algo of last resort so we cannot allow init to ever fail.
Apr 5 2021
Apr 2 2021
Mar 25 2021
Mar 24 2021
Mar 23 2021
Loic doesn't have a commit bit, so I've offered to commit on his behalf.
Dec 13 2020
Nov 24 2020
Nov 23 2020
Apologies for the delayed reaction...
Sorry for the tardy comments, but a few things in this changeset caught my eye during our internal Netflix upstream merge review.
Nov 4 2020
Nov 18 2019
Nov 5 2019
Thanks for the fix @brooks
Oct 14 2019
Oct 8 2019
Oct 2 2019
Sep 24 2019
Aug 12 2019
Jul 11 2019
Mar 28 2019
Feb 25 2019
Feb 7 2019
Agreed and apologies, I misread/miunderstood the code. I also touched base with one of the original CDG authors to sanity check and they agree this change is a good idea.
Feb 5 2019
Suggest moving stailq flush loops out of cdg_cb_destroy() into an inline function, changing smoothing_factor sysctl to a SYSCTL_PROC with custom handler similar to the exp_backoff_scale sysctl, and calling flush function from both cdg_cb_destroy() and sysctl handler when smoothing_factor is set to zero
Actually, we probably need to flush the qdiffmin_q and qdiffmax_q lists in the sample_q_size == 0 branch so that a change back to non-zero smoothing_factor doesn't start operating with stale data.
Thank you for fixing this.
Dec 3 2018
Thanks all for the feedback... I'm a bit rusty on working with ports. Will commit sometime this week with my src commit bit hat on and "review/approved by:" if no further feedback materialises.
Nov 26 2018
Checked with one of the authors. Preference is to reference the Bitbucket URL in pkg-descr if it can only list a single URL.
Full context diff, shuffle variable order and ditch DISTVERSION in favour of a custom variable to hold the commit hash used in the tarball directory name.
Oct 31 2018
@tuexen Any thoughts on the TFO case?
Oct 18 2018
Jul 21 2018
Jul 20 2018
Jun 8 2018
May 17 2018
May 15 2018
May 14 2018
Final commit candidate, rebased against FreeBSD head r333598, and with M_ZERO removed from malloc call.
May 10 2018
newreno_plugleak_v3.diff with getsockopt(2)/setsockopt(2) updated.
@wollman Many thanks for the historical and standards related context - greatly appreciated. I realised that I probably need to update getsockopt(2)/setsockopt(2) as well...
May 9 2018
Still to be tested, but I think something like this would address the leak and change memory allocation to being conditional on need: newreno_plugleak_v1.diff
Mar 19 2018
Feb 7 2018
@jason_eggnet.com I thought about this some more and while there is no doubt that overflow/underflow due to the function's inputs is possible and needs to be remedied, the cause of overflow in your test case is not in fact the time since congestion being too large, but the bogus value of K, which for a wmax of 1460 bytes (i.e. slightly more than 1 MSS) should be 205 per my quick check:
Oh and @jason_eggnet.com , regarding your test code, I structured things the way they are so that you can simply #include <netinet/cc/cc_cubic.h> into your userspace test.c to get access to the various window calculation inlines rather than copy/pasting them.
This fix doesn't make sense to me. If it has been a long time since the last congestion epoch resulting in an overflow in the calculated congestion window, collapsing the window from <something_large> to 1 segment is a terrible idea. There's also no sound reason that cwnd shouldn't be allowed to shrink below 1 segment, and if that causes problems elsewhere, those places should be fixed.
Aug 24 2017
Aug 18 2017
yoda be gone
Aug 17 2017
Update diff post r322614 commit.
Conrad: would you be willing to sanity check this sbuf change for me as well?
Aug 15 2017
Aug 8 2017
Rebase patch against current head and address review feedback.
The off-by-one error was incorrectly attributed to the condition that checks if vsnprintf() was successful at rendering all of the specified content into the sbuf.
Aug 2 2017
Thanks Tom, looking good. Will wait a few days to see if any further feedback materialises.
Jul 28 2017
Jul 27 2017
This looks good barring a couple of nits as noted. Please update tcp(4) and cc_newreno(4) appropriately.
Jul 17 2017
I'm pretty sure this wouldn't compile as proposed. Please remember to build test patches against FreeBSD's svn head branch, as all work always gets comitted to head first before potentially being backported later.
Jul 11 2017
Following up on some discussion held on the fringe of the recent developer summit at BSDCan, what this work is missing is some accompanying documentation to demonstrate the dynamic behaviour of the proposed implementation over some range of relevant network parameters, along with a critique of expected versus measured/observed behaviour. I'm not asking for a full blown academic paper - a stream of consciousness Google-doc with some data, a few X vs time plots and some comments would suffice. Happy to provide guidance if required - you know where to reach me.
May 24 2017
May 4 2017
Apr 7 2017
Mar 19 2017
Apologies for the delay in getting to this.