- User Since
- Oct 29 2015, 5:25 PM (168 w, 2 d)
Sun, Jan 13
Please commit with "Approved by: jtl"
Fri, Jan 11
Has this landed yet?
By the way, in the commit message, please make it clear that this simple "allows" you to more easily use arc if you want your changes to be reviewed. It doesn't require it in any way, shape, or form. And, we're not changing any expectations regarding when someone will get review.
I'm OK with importing the code as an option for developers to use. I'd be wary of specifying that there is a "plan" to convert existing test cases unless there is general buy in to do that.
Regardless of what the group says, it seems like we'll probably want to support both for a transitional period. Given that, this change seems OK. @emaste , do you agree?
Since make clean runs first, I'm assuming the only things left will be things with flags set? If so, what you propose is probably correct (and we don't need the rm/chflags/rm "dance" optimization).
Dec 17 2018
LGTM; would appreciate also hearing @rrs's opinion.
Nov 16 2018
Nov 9 2018
Yeah, I saw this shortly before seeing this review. I agree we should fix it.
Nov 8 2018
If alignment is important, we can (theoretically) do something more intelligent to maintain it. However, I think this is both safe and good enough for an emergency fix.
Nov 2 2018
Oct 18 2018
Oct 16 2018
Fix stupid syntax error.
Oct 13 2018
Please make the two changes suggested by bz@ and then feel free to commit this. I think its OK to ask re@ to commit this during the freeze. (I also won't be upset if they say "no".)
Oct 12 2018
Oct 11 2018
Oct 10 2018
The other half of the original diff is now in D17503.
Split the commit into two parts. In this part, just fix the early epoch calls.
Oct 9 2018
Oct 6 2018
Oct 5 2018
FYI, it looks like there was a typo in the description:
Oct 4 2018
Looks good. Please commit with "Approved: jtl (mentor)".
Sep 8 2018
Hi. I know I'm late to the party, but I have three comments:
a) I could be wrong, but I don't think there is any guarantee this won't be called simultaneously for two different groups at the same time. (The groups could be in different VNETs, for example.) In that case, two different invocations could be working on the function's static variables at the same time. That may produce unexpected results. (Granted, it would take an unusual series of events. But, I think we've all seen highly unusual events occur.)
b) I don't think the const variable also needs to be static.
c) It seems like the rate limiter should really be per-group, so I would suggest adding the lastprint variable to the inpcblbgroup struct.
Aug 23 2018
FYI, something didn't look right doing my tests. So, I'm going to delay committing this until I can satisfy myself that it behaves correctly. That will almost certainly mean I miss the code freeze. C'est la vie!
- Limit the ipq structure to the kernel to eliminate a buildworld failure. (And, why should we make userspace code import the sys/queue.h header for a structure they don't need anyway?)
- Address @jhb's nit.
Aug 22 2018
Aug 21 2018
Aug 18 2018
LGTM (with minor change noted in-line).