Is there anyway to NOT have the 141,000 line xml version of the file stored in SVN?
Also we need to make absolutely sure that any services used by ports are not effected by anything this changes.

In D17040#364538, @emaste wrote:

Some more comments inline - as mentioned earlier we should try to address the root causes, as in D17074. That said, I'm fine with this approach pending root cause fixes (given that it doesn't build today), but want to make sure we have some way to track these outstanding issues such that they'll get fixed after the release.

It just hit me and I have to ask, why isnt libcasper avaliable in a static configuration? I certainly see a /usr/lib/libcasper.a on my systems.

In D16945#361418, @kevans wrote:

(Note to Rod: this is only partially what's going to end up in the review... this isn't a final product for review yet)

Allan, would you update the diff please

To add information, I have email from Paul Vixie and in fact he is wanting to break the reboot loop with -x so that it can be handled externally.

I do not believe this is a desirable change, at least it does not do what it claims it does.

Looks good, my minor nits can be ignored, up to you.
Oh, I see you changed the return code of the module load, I take it that is to lower its preference???

I am sorry for not getting to this today, I was side lined.

I respectfully request that a straight revert of the commits be done rather than this, for one this does not allow us to do the proper deprecation commits, nor to merge those commits back to stable/11. This is still a major violation of deprecation policy. Warner, would you new policy allow this type of thing to be accepted? I would hope not.

Looks ok, though I wonder about a shell script that writes a Makefile, it should be possible to do that in a Makefile by itself, there is the .for/.endfor operator to make which can duplicate what the shell script does.

Can I please have atleast 24 hours to review the text of this.

Im no perl fan so I wont comment on that aspect.

Can you please break the change to share/mk/bsd.confs.mk out into a separate review, though it is required by the moving of the profile files, it is actually not a part of that change directly. I would also like to ask that you tag bdrewery in on the review of the .mk change. I do not see how file owner/group/mode are set in the new Makefile.

Though I disagree with the relocation of this to libc as it is going to be installed with the absolute minimal system anyway so this delta just creates src tree churn, and user finger memory churn. You site your reason for moving it is to put it close to the sources, well, traditionally BSD sources are layed out to match the installed DESTDIR tree. I understand things like csh and sh conf files moving, that makes since in a world where csh or sh may or may not be installed by a pkgbase, however that makes no since in a world where libc and hence master.passwd shall always be installed.

A question has come up, isnt libc part of base, ie even in a pkgbase system libc must be installed? If that is the case then master.passwd must be installed, and I do not see a good compelling reason to move it from its current orthagonal to DESTDIR locatoin in the src tree.

In D16775#356743, @ian wrote:

If using option IPSEC (and I guess GELI and maybe other things) requires explicitly specifying device crypto, then shouldn't this change also cleanup and simplify sys/conf/files to remove "| ipsec | ipsec_support" from all the files that are optional on "crypto"?

And as long as I'm on about consistancy, if we're going to start specifying devices in std.arm* then shouldn't we comb the existing arm kernel config files to find all the common [pseudo-]devices and move them into std.arm* as well? I don't remember why we decided that the std.arm* files would contain only options, but I do vaguely remember discussing it; it wasn't by accident.

This looks okay to me for just the moving of master.passwd, though I am not found of its landing location, but can I ask for 24 hours to put together the net effect patch of what was commited, and partially reverted in combination with this patch to look for any of the pointy sticks that came up. Thanks, Rod

In D16705#355079, @rgrimes wrote:

In D16705#355071, @imp wrote:

In D16705#355069, @rgrimes wrote:

Would there be an objection to using SUBDIR names of loader.4th, loader.lua and loader.simp so that they sort next to each other vs the current names that scattter them about the directory? This also means that loader.foo/Makefile simply includes ../loader/Makefile all nice and orderly :-)

I would object.

We can't install them as loader.lua, loader.4th loader.simp because then we'd have loader.lua.efi, loader.4th.efi etc. Also the .4th suffix is what we use for actual 4th code. So that leaves us with a prefix, even if it is a bit less orderly.

Ack, ok simple enough reasoning.

In D16705#355071, @imp wrote:

In D16705#355069, @rgrimes wrote:

Would there be an objection to using SUBDIR names of loader.4th, loader.lua and loader.simp so that they sort next to each other vs the current names that scattter them about the directory? This also means that loader.foo/Makefile simply includes ../loader/Makefile all nice and orderly :-)

I would object.

We can't install them as loader.lua, loader.4th loader.simp because then we'd have loader.lua.efi, loader.4th.efi etc. Also the .4th suffix is what we use for actual 4th code. So that leaves us with a prefix, even if it is a bit less orderly.

Would there be an objection to using SUBDIR names of loader.4th, loader.lua and loader.simp so that they sort next to each other vs the current names that scattter them about the directory? This also means that loader.foo/Makefile simply includes ../loader/Makefile all nice and orderly :-)

Well, I am still on the fence with this one, seems someone has already done it to the route get -n command.

This is also going to require a man page update, as now there are more than the 4 exit codes listed in the man page.

In D16164#342949, @sbruno wrote:

In D16164#342938, @rgrimes wrote:

Does this not also make it so that these drivers require IFLIB, so they need to be marked as such in GENERIC/NOTES/etc and man pages?

Yeah, definitely. It also means that out of tree drivers (intel, bnxt) wouldn't work if a kernel is built without IFLIB. Is this ok to do?

Does this not also make it so that these drivers require IFLIB, so they need to be marked as such in GENERIC/NOTES/etc and man pages?

Looks good over all, just some nits on errx vs err when it appears that there is a proper errno avaliable.

I think it might be better to have 3 conv types, IDIRECT, ODIRECT, and DIRECT==IDIRECT|ODIRECT. You may or may not want vm effects on specific files.

I actually worry less about man page DEPRECATION, and more about having the binary spit something out so anyone using it for who knows what gets a heads up that it is/may be going away soon.

@brooks a fully sgree, infact it can be done in a seperate review, and the asf binary should spit out a warning, not just a man page change.

Does it make since to first commit a deprecation notice to this, which can be merged to stable/11, then actually remove it from ^/head after that? This avoids the need for a direct commit to stable/11

I think you have mis understood what I asked for in the other review. It only needs to be split into 2 parts, one containing the SPDX tags, and another containing all the white space cleanup.

It is normal to separate all whitespace changes from other. It is preferable to separate all SPDX changes from others when it is sweeping. I strongly assert that as the person now cleaning up a 180k lines and 16 commits of diff involving SPDX tags that this separation occur before this is commited.

Please seperate this into 2 reviews, one for the white space/tab cleanup and one for the SPDX tag addition.

Unclear why phabricator does the remove of reviewers when I try to add group bhyve, restore grehan caused by that in last change.

Copy over subscribers from D15628 and describe this review as a fix for an issue found during that review.

Isnt there some documentation some place that also needs to be updated?

Please mark the commit as MFC-X with https://svnweb.freebsd.org/changeset/base/332298

I am ok with this for now, but I think the values 100000uS and 2x are going to need some "experience" tuning.

I see this problem often on BPG routers. I usually use the birdc as a better and more reliabe interface to the routing information, though on occasion you do have to sanity check with what the kernel has.
Also birdc seems to be able to give me a routing table 10x faster than netstat -rn and without this issue.

Thanks

In D15465#326442, @markj wrote:

In D15465#326440, @rgrimes wrote:

Can we not split KDB and DDB with NETDUMP?
Ie, put it after the options MALLOC_DEBUG_MAXZONES=8 # Separate malloc(9) zones
would keep all the KDB, GDB, DDB, etc in one lump, followed by dumping stuff, perhaps with one line of white space

Wouldn't that make it look like the "turn off in stable branch" comment applies to the new options? Or am I misunderstanding your suggestion?

Can we not split KDB and DDB with NETDUMP?
Ie, put it after the options MALLOC_DEBUG_MAXZONES=8 # Separate malloc(9) zones
would keep all the KDB, GDB, DDB, etc in one lump, followed by dumping stuff, perhaps with one line of white space

In D15421#326315, @araujo wrote:

In D15421#325260, @rgrimes wrote:

Thanks for catch Roman, I have prepared a commit and sent it to my mentors for approval, with a fast track 3 day MFC as this made it into stable/11.

Wait! Do you just committed a patch sent by another developer?

@novel have you approved that?

Best

In D15459#326241, @jhb wrote:

Yeah, I actually meant to post a comment asking about the blank line. I would be fine with collapsing it now that the ID isn't surrounded by N layers of #ifdef.

Thanks for catch Roman, I have prepared a commit and sent it to my mentors for approval, with a fast track 3 day MFC as this made it into stable/11.

I have changed the transport reviewer to non-blocking, and added glebius as a reviewer since he seems to have some stake in this. Hopefully this can help it move forward.

How do you trigger a call to this DEVMETHOD?

Is this the workaround developed with macy?

It is a bit confusing to still have it be T_TRCTRAP, but all the comments now say debug exception...