- User Since
- Mar 1 2015, 6:19 PM (232 w, 6 d)
Mon, Aug 12
change macro to take parameter
Sorry that this was unclear. There is a limitation in phabricator to the kinds of negative feedback metadata one can provide. It's "request changes" or nothing.
Sun, Aug 11
@cem: phabricator says "cem requested changes to this revision.", but I see no changes requested.
@cem: Not if it is an improvement on another change, and makes better use of the existing space.
Apr 22 2019
Mar 19 2019
Although I have a couple of inline comments, I wouldn't object to committing this as-is. Touching up would be fine too.
Mar 7 2019
This seems good to me. If this is not to be MFC'd, compatibility wouldn't matter (in which case I wouldn't have reduced the number of spares). It's nice to have things in order, but maybe this isn't the time to break compatibility given the small scope.
Feb 26 2019
I agree with John that we should do everything we can to eliminate class A/B/C rather than just shuffle it around. Removing those macros will cause some turmoil; e.g. libc's inet_netof and inet_lnaof assume classes, and don't know about local netmasks. They should probably go away. I don't know how painful that will be. But we are way overdue to get rid of this stuff, and it appears that Linux (Ubuntu) doesn't have them. But the old A/B/C are a somewhat different problem than making Class E usable.
Feb 25 2019
Hi, John! Very, very long time no (anything). I am very happy to see tests in this area. I certainly expect this to work with minimal changes (i.e. this one). Re: Classs A/B/C: I'd be quite happy to see those definitions go away. Re: "loopback net": I'm aware of a small number of things using that other than 127.0.0.1 (including one of my company's legacy products), but they are minor. I still remember driving a Sun workstation crazy by pinging 127.0.0.2 (there was a network route, but obviously only one address recognized.).
Feb 24 2019
Sorry, that reference should have been RFC 1112, not RFC 112.
It looks like I am the guilty party in calling Class E IN_EXPERIMENTAL, which went in when "Class D" became multicast. I suspect it was actually called that in some document, although it doesn't really matter. https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml doesn't call it Class E, but refers to RFC112, which does. In any case, assuming that https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml is still current, the treatment is the same.
I don't understand the purpose of this review. As it says, these changes should not be committed. I'm not actually sure of the official status of Class E, although I suspect it is still experimental/not for production use.
Generally looks good, one small comment inline.
Jan 26 2019
I agree that arprequest_int is rather opaque. At first I thought that is was just the version that returns int, which is not useful. arprequest_internal would be better. Otherwise, I have no objection; returning any errors seems like a good thing. And yes, I did not see any of the earlier (presumed) emails for this review.
Dec 8 2018
Nov 16 2018
Oct 30 2018
Sep 22 2018
Sep 19 2018
It would be good to see if the crashes stop before committing this, but it looks right to me.
Sep 6 2018
Sep 3 2018
Sep 1 2018
I believe I understand the issue with route caching. in6_selectroute_fib checks for loopback,and substitutes the interface with the local address. ip6_output doesn't need to know that interface, it should just skip the scope check in that case. I'll email a possible kernel patch to Andrey and Harti.
Aug 28 2018
You are right, inp_route6 would be better. I'm sure I copied and pasted, then didn't make as many changes as I should have. I will happily approve a review to make that change if you test that it compiles :-).
May 17 2018
Looks good to me. I could go either way on the blank line.
Mar 10 2018
Mar 9 2018
Thanks for catching the typo.
Mar 8 2018
Feb 28 2018
Feb 27 2018
Feb 10 2018
I agree with the change as well; the regression seems to have happened when the code was refactored and the "add" and "change" code became joined.
Jan 23 2018
Jan 22 2018
Got it. I didn't realize it, but Phabricator lists the reviews in reverse order, so that's the order I looked at them.
Jan 20 2018
I just realized that this is necessary when the L3 gateway changes. The L3 cache doesn't need to be invalidated, but L2 does. A comment to this effect would be helpful.
Could this use the new invalidate macro?
Was there a place where only L3 was invalidated?
Why is this necessary, or desirable? If a route that is cached is modified, that doesn't invalidate it. It should still be the best route if it was before.
Jan 13 2018
Jan 3 2018
Jan 2 2018
Jan 1 2018
A list of ioctl names didn't seem very useful to me, so I extracted the information from the header file.
IIRC, there are ten of these ioctls, although I didn't add any. I thought I'd correct the obvious out-of-date statement, but wasn't sure I could correctly document all of those added over time. Suggestions?
Dec 31 2017
Rod, perhaps you have never needed this functionality; but have you ever had a system hang? Along the same lines, there are probably about 1000 bits of code that you haven't needed in the kernel; should we add options to remove each of them? This way lies madness. There are two hardclock routines; which are you using? Should we ifdef out the other?
Sorry, my last comment was responding to imp@; it crossed with the comment from rgrimes.
Thanks. About loadable hardware watchdogs: that should work if they attach before watchdogd starts.
Dec 18 2017
Dec 9 2017
Nov 27 2017
Nov 25 2017
Fix macro usage, update text
Nov 24 2017
Nov 18 2017
Another attempt to clarify, flesh out example.
Nov 17 2017
Update Dd date
Sep 26 2017
Sep 24 2017
I hadn't noticed this snippet of code before. It was definitely wrong.
Aug 29 2017
Commited in https://reviews.freebsd.org/rS316065
Apr 22 2017
Apr 10 2017
Mar 27 2017
Mar 25 2017
Mar 20 2017
Dec 31 2016
I think the change is a step in the right direction. Certainly, "ifconfig xxN down" followed by an implicit UP should not cause any change to the routing table. Does anyone know why the "down" is removing the route? That seems wrong to me.
Seems fine to me; will let someone else approve.
Sep 21 2016
Aug 24 2016
Aug 22 2016
btw, the second chunk (first after include) is new since original version.
This patch has been tested by Mike Andrews, who will continue monitoring. I've asked Peter if he can test.