Can I get some final accept on this, I updated the open issues

I like it, just nits, that for the most part you can ignore, other than $FreeBSD: vs $FreeBSD$

This appears to be used to deal with a core hang on specific CPU's, we need to see that we have mitigated that in the host, and the decide what the appropriate action here is, I am uncomfortable with just making this silently disappear. Deferning to jhb.

Though this does fix your issue by silencing the log, it does little to improve the situation. I think what we may want to do is to add yet another option to the bhyve command that along with -w to ignore invalid MSR access to do so silently so that we do not have to add code like this everything a new msr comes into play. John, Patrick your thoughts on this approach?

In D19426#417642, @emaste wrote:

I would be comfortable with simply "https://bugs.freebsd.org/12344" as that can likely work forever.

Note that https://bugs.freebsd.org/12344 does work - we should accept either https://bugs.freebsd.org/bugzilla/show_bug.cgi?id= or https://bugs.freebsd.org/[0-9]+

I believe what your wanting is web server re-write rules, that would rewrite the URL in and out of the web pages to have the preffered form. Beyond my area of expertise to implement, but I know it can be done and that is probably the easiest place to do it (ie, no modifying bugzilla should be required.)

I removed a redundant "to" in the description. Patch looks good to me. Copyright over to jhb@

I'll review if you ack your are done updating John

Close, very very close, but still slightly short of leaving no doubt as to what goes here.

Missing is the change to the commit message template to update it to say URL instead of PRxxxx

Convert nat64_check_ip4 in sys/netpfil/ipfw/nat64/nat64_translate.h to use IN_foo macros rather than hand rolled code.

In D19380#414840, @manu wrote:

I don't think this is a good approach.
My goal for this is :

• Finish u-boot-install (https://gist.github.com/evadot/1bcad7c4c1e5cc7f9a692f4d19ab5421) to handle multi file u-boot
• Do a rpi-fw-install script and include it in the rpi-firmware package
• Create to GENERICSD image for arm64 (like the armv7 one), one with MBR and one with GPT scheme.
• Remove board configs that have no quirks from release.
• re@ could then create only two images (GENERICSD-MBR, GENERICSD-GPT) and install u-boot on each one based on list somewhere.

In D19316#414645, @karels wrote:

I agree with John that we should do everything we can to eliminate class A/B/C rather than just shuffle it around. ..trimmed...

Some of the patches I have already posted do infact remove the direct use of IN_CLASSA by use of the proper macro IN_LOOPBACK. I shall keep an eye out as I look over the code for issues that effect my stated goals in cleaning up things such that IN_ZERONET, IN_LOOPBACK, and IN_EXPERIMENTAL can be used to fully effect all places that are trying to deal with these ranges should they ever be redefined.

The added kernel code seems to be mssing INET/INET6 ifdefs. None of the inline comments are marked done, but the code looks as if you have addressed them.

In D19316#413794, @karels wrote:

Re: Classs A/B/C: I'd be quite happy to see those definitions go away.

In my first pass audit of a search results on IN_CLASS[A-E] the common use cases here are the default calculation of netmasks, 2 minutes ago I would of told you that probably can not go away, but it just hit me, IN_DEFAULTMASKFOR(i) could be written that would incorporate the ancient class rules in a central place and cleaning up some code. Thoughts?

In D19316#413638, @karels wrote:

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.

The changes well need expansion, the present form is to make a patch available so that the other patches can be tested.
A walk down through the RFC's say a few things, one is that it is not called Class E anymore. It is still reserved.

Add some more places that hand crafted macros are used, ip_input, ip_output, netdump code.

Address review comments from Patrick Mooney. Found some additional places that caching would be helpful so update them too.

In the future it helps to upload full file diffs, not sure what vcs your using or what its command is, for svn I use svn diff -x U999999.

In D19218#412862, @rmacklem wrote:

Whether or not "#ifdef INET6" and "#ifdef INET" should be in the code is still an open question.
I was hoping bz@ could answer this?
(I, personally, was of the understanding that this was no longer required/recommended for
FreeBSD sources. If bz@ does not know the correct answer for this, I will ask on freebsd-net@.)
At this point I have made no changes to the Makefile.
To build/run it, there is a small kernel change required, but I did not include that in this review.

Add handling of cores or threads > 254 by truncating to 0, per smbios 2.6 spec

In D18998#412749, @jhb wrote:

FYI, you can get SMBIOS specs here: https://www.dmtf.org/standards/smbios

In D18815#402284, @araujo wrote:

It doesn't looks quite right! I need more time to articulate better arguments, but this patch as it is, must not be committed in.

Wrap comment text, provide details of MADT_SIZE formula

Update to incorporate feedback from bde@ (mentor), wrap long lines, sort prototypes within group vm_get* in sys/amd64/include/vmm.h.

I smashed the wrong thing. I do not see the Makefile in the diffs, nor do I see the inline #ifdef's.

In D19218#412405, @bz wrote:

I think doing DNS things for these is not a good thing. Especially given inside an IP jail a "localhost" will give you different results.

Could you elaborate on that "give you different results?" This may be the very thing I am trying to say, and it may be what could break this code if run inside a jail, or in my other cases test bed with modified localhost value.

In D18816#411819, @jhb wrote:

In D18816#411818, @jhb wrote:

Pasting a reply from Rod:

I am just trying to fix the current bug that this code does not
work correctly if you raise VM_MAXCPU, it starts to give you
problems at something like 24 due to the stats array not having
space for the ipi table that tries to get added.

This also documents the size dependancy and removes the
false /* arbitrary */ comment.

The problem is you can't change it. Changing the constant in vmm_dev.h changes the ABI since the size of the structure is assumed in vmm_stat_copy()'s current API and that is also encoded in the ioctl value since the size of the structure is part of the constant. One of the approaches I described above is the only way you can do this without breaking the ABI.

pfg I think you miss understood, and upon looking I was not clear, it is not you that I am unhappy with about "playing with copyrights and licenses", it is NetBSD and possibly Charles Hannum who as Robert points out modified a license without ALL authors approval. I would be very upset if someone disturbed my disclaimer in this manner.

The review does nothing about addressing your other issues, it simply corrects a current state of affairs that is wrong if you raise VM_MAXCPU above 24.

Ping

Fabian, I have done some other work that moves forward with this concept, mostly correcting stuff that puts limits in place on how high VM_MAXCPU can go, and have WIP that actually changes this to a run time per vm value. Can we either abandon and retire this differential, and I'll start a new one or can I commandeer this one and make all the other work subordanate to it?

Upon closer examination of this there are some issues, it alters a license post fact of copyrights asserted by some, and changes the clause in the license from Author to NetBSD, which effects the since of the copyright. I really wish the playing with Copyrights and Licenses would stop, this is a good example, of a well meant change, causing an ugly problem. For examle, the disclaimer is now in the name NetBSD, so what happened to the disclaim asserted by nCircle Network, and Robert Watson?

In D19218#411491, @rmacklem wrote:

W.r.t. the NFS root mount, you are sort of correct at this time.
What would happen for an NFSv4 root mount at this time is
that the NFS client code would have a miss on the "uid<->username"
cache and attempt an upcall to the nfsuserd, which would not
work. (I think it will eventually fail and use "nobody", but I'd have
to look at the code to be sure.
As such, NFSv4 root mounts are not currently recommended, but I
would like to resolve that someday.

In D19218#411391, @rmacklem wrote:

The problem with doing a name lookup in this daemon is that it assumes/expects it
to work. Often (an NFS mounted root fs is one example and a system configured to
use DNS before the local /etc/hosts file) are not yet working and an NFS mount is
expected to work.
(ie. In general I'd agree with you, but for NFS not so much...)

I can not speak much for all this code, other than to say I can read it and pretty much understand what is going on, however I would like to address the issue that this code is missing the conditional compilation on if I even want either or both of IPv4 or ipv6 supported. I believe most (all) our other code has #ifdef INET or #ifdef INET6 in it, see usr.bin/netstat as an example. I am not hardbound on using "localhost" as a grep of the code shows this has seriously been ignored, which means /etc/hosts is no longer the source of truth for the value of localhost :-(.

Address jhb style 9 issues

Move initialization of uopt to after declares addressing jhb style(9) issue