Any other comments? Anyone able to approve?

Fix text according to Ed's suggestion.

I think the change is useful. The RFC reference could be superseded, but that might also mean the code should be updated as well. It's reasonable to document which version was referenced for the code.

Re: avoiding ntohl(): that would be possible, but probably not within the scope of this change.

Moving the code down fixes a couple of problems, but it introduces a new one: multicasts from link-locals can be forwarded if we are a multicast router. Maybe add a separate check in that section? We should still receive such multicasts.

LGTM

Looks good to me, but this is not my area of expertise.

Looks better, thanks!

Sorry, I was thinking that the original text was gone; I see that it is here. It is still misleading, or at least poorly worded, to say that these commands "appeared" in FreeBSD 1.0. Essentially every command in 4.4BSD-Lite was also in FreeBSD 1.0, but we don't bother saying so; it adds no useful information. The reference to 1.0 should be deleted.

Looks good to me, I'm ready to approve. I'd also like to see a reviewer who knows the brgphy code.

Thanks, Conrad. I'm checking it out.

In fact, tcp_connect was able to use a port that had a wildcard bind. I can't think of a real-world problem this would cause, but it was easy to fix.

Do not allow a port with a wildcard bind to be used for connect.

In D24781#547768, @tuexen wrote:

Hi Mike,
let me ask you two questions:

Add sysctl to control new feature

In D24781#546975, @rscheff wrote:

What was the special handling of a inp_port == 0 doing, or why did you remove this in this patch?

The enterprise-grade Sidewinder firewall has had this feature for years. I don't know what other implementations work this way, but it seems like the obvious way to do this "right". A NAT box cannot serve multiple clients without handling port overlap, nor connections to multiple services (ports) on a server.

In D24436#539201, @manu wrote:

In D24436#538986, @karels wrote:

Address more review comments; fix bug with bpf and txcsum.

Notes:

I started a version with full detach; it is not working correctly yet.

What is the problem exactly ?

Address more review comments; fix bug with bpf and txcsum.

About doing a full detach: then code is needed to undo the init routine, and there are far more failure modes. I'll look at it though.

Is there an existing example of FDT / ACPI split to use as a model?

Updates based on review comments, including teardown after failed attach.

@greg: I'm not quite sure how the FDT/ACPI split should be done; I think that will have to wait for another time.

Arm seems most likely to want pre-loading, but I don't think it hurts anything. I think it could be enabled for everything. How about mips, powerpc?

Looks good, I didn't realize that the kernel already had validation code. Might as well use it!

Conrad: this fixes a current problem with live systems, debugging a multicast problem. I am not aware of any requests to get multicast stats from a kernel core.

Fix parens

Remove incomplete ifdefs.

Upload correct version with nested loops.

Change to use nested loops for procs and threads; deal with zombproc

I took yet another tack last night, and it is better than previous attempts. It seems to be working correctly, but needs more testing. The changes are much bigger, mostly due to code motions. (svn diff is now 286 lines, about twice as big.) I'll test more, then decide whether to proceed with it.

Thanks, John.

Conrad: it has now been more than a week since Marcel asked a question. If you can't or won't explain why you oppose this change, I think you need to drop your block on the review.

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.

@cem: phabricator says "cem requested changes to this revision.", but I see no changes requested.

minor corrections

@cem: Not if it is an improvement on another change, and makes better use of the existing space.

Although I have a couple of inline comments, I wouldn't object to committing this as-is. Touching up would be fine too.

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.

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.

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.).

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.