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

In D14995#315621, @imp wrote:

In D14995#315620, @rgrimes wrote:

The reason I harped on the "Before 2.0" is that technically all 1.x code is to be unpublished by Agreement with AT&T/USL.
The time frame you are looking at here is 2.0, very near its actual release.

Yet, it's widely available. For example, you can find it at https://github.com/dspinellis/unix-history-repo/tree/FreeBSD-release/1.0-Import as well as all the pre-history of FreeBSD in the 386BSD + patchkit era. We're long past the time where it really matters for everybody that wasn't a direct party to that agreement, since there's no legal entity that could make an agreement on my behalf, since I wasn't even part of the project at the time, and the agreement was secret for years.

In D14995#315589, @ali_mashtizadeh.com wrote:

In D14995#315582, @rgrimes wrote:

In D14995#315560, @ali_mashtizadeh.com wrote:

So this doesn't get lost this is the original change in SVN. It dates back before FreeBSD 2.0

https://svnweb.freebsd.org/base/head/sys/i386/i386/machdep.c?r1=4193&r2=4201

I find that hard to believe, given that the cvs versions before 2.0 do not exist in svn, or ncvs, and should only exist in an offline repository called cvs.

Well the SVN revisions might be bogus because of the transition from CVS. But I'm assuming the dates and the snapshots of the releases are correct.

r4201 is where the bzero() was introduced to setregs() on Nov. 6, 1994
r4762 is the creation of RELEASE_2_0 on Nov. 22, 1994

https://svnweb.freebsd.org/base/release/2.0/sys/i386/i386/machdep.c?revision=4762&view=markup#l865

In D14995#315560, @ali_mashtizadeh.com wrote:

So this doesn't get lost this is the original change in SVN. It dates back before FreeBSD 2.0

https://svnweb.freebsd.org/base/head/sys/i386/i386/machdep.c?r1=4193&r2=4201

Could it be this works on i386 only?

In D9930#315522, @bcr wrote:

I think we're still good on the man page!

In D14989#315514, @kib wrote:

In D14989#315498, @rgrimes wrote:

It still appears that you deprecated a function, you can not just do that in stable/11 as that breaks ABI It is also desirable to have a change to stable/11 that makes use of this function emit a deprecation warning.

What ABI does it break ?

In D14989#315495, @ali_mashtizadeh.com wrote:

@rgrimes With this version of the patch do we need to do anything about stable/11? The behavior now should most closely match the documentation.

Clean up the topology_parse() routing to catch more errors, and to properly match the documented syntax, update the bhyve.8 manual page to reflect the actual syntax implemented.

What are the plans/methods to be used for stable/11 informing consumers that this feature is being deprecated?

Make cpu topology sysctl's conditional on FreeBSD_version so that this patch can be merged to stable/11 and make these sysctl obsolete in >120057

The issue causing the cpu topology to come out wrong on Intel cpus has been found, it is a misplaced call to vm_get_topology that is only executed for the cpuid 0xb, ecx=0 case, causing the ecx=1 case to use uninitialized values.

In D14679#308658, @mckusick wrote:

A couple of points that I would like to add to this discussion:

1. The rerunning of fsck can happen when certain cleanups are needed that may undo some earlier changes. For example when a disconnected directory is added to lost+found, previously calculated link counts for the directory that it used to be in may need to be adjusted. Rather than add a lot of code to fsck to deal with these rare cases, it is easier to just rerun it. Worst case scenario, it could happen once for each level of directory hierarchy in the filesystem. In practice I have only ever been able to make it loop twice with carefully constructed damage to the filesystem. There are many places where a change could cause the need for a rerun (and thus it is requested) even though it actually does not which is why the most common examples where it reruns, the result is simply to mark the filesystem clean.

In D14679#308469, @ian wrote:

In D14679#308435, @rgrimes wrote:

[...]

I'll support any knobs you want to add to get to your goal, just please do not remove or change any knobs that prevent me from getting to my goal.

It sounds like maybe what we need is to add support for an "fsck_p_flags" rcvar that gets used on fsck -p runs. We can put fsck_p_flags="-T ffs:-R" in /etc/defaults/rc.conf so that by default we fully preen a filesystem automatically, but users/admins can override it to turn off that default as needed.

In D14679#308433, @markj wrote:

In D14679#308430, @rgrimes wrote:

If fsck -p internally does a restart I do have some concerns, as this usually is indicative of a decaying situation.

I've hit this problem dozens of times in my setup, and the second invocation does nothing but mark the superblock clean. I don't agree with your claim.

In D14679#308387, @ian wrote:

In D14679#308384, @markj wrote:

Ian reported in the thread that -R has no effect if -p is specified, but I'm not seeing how that's true (and that behaviour doesn't really make sense if it is true).

I just looked at the code again, and I think maybe I was confused between ERESTART vs ERERUN or something, because now it looks to me like the -R option does restart internally in preen mode. Sorry for the misinformation.

Given that, I think "fsck -p -T ffs:-R" should be the standard set of flags in defaults/rc.conf. If preen is safe to run once, it should be safe to run multiple times as needed.

In D14679#308384, @markj wrote:

Since fsck_y_enable is "NO" by default, this leaves us with fragile out-of-the-box behaviour. I would really like us to try harder to not leave a system stuck in single-user mode.

Ian reported in the thread that -R has no effect if -p is specified, but I'm not seeing how that's true (and that behaviour doesn't really make sense if it is true).

Rodney's claim that we wouldn't want to run fsck_ffs -p multiple times if requested also doesn't make sense to me: why would you be ok with running it once, unprompted, but not multiple times?

Let me try again about the net.inet.ip.fw.one_pass. This patch leaves that value alone, that value is 1 by default. The added rules shall reassemble all UDP packets, and since one_pass is set it well at that point PASS THE PACKET. This is a huge hole in the firewall in effect allowing all UDP traffic to pass inward without any port or state being checked. With the added rule that reassembles udp packets it is a MUST that net.inet.ip.fw.one_pass be set to 0 so that the additional checks later in the firewall can be checked. It is also a must that the rule be moved before the check-state.

In D9920#307806, @freebsd_oldach.net wrote:

In D9920#307797, @rgrimes wrote:

I am not so sure if we want to turn on net.inet.ip.fw.one_pass

Indeed we should not do that. It's also not necessary. The addition of a reass line suffices to fix the issue.

I agree that the rule order is wrong here, reass should be done before a check state,
in general you want reass to occur very early, so that other rules are checking the
reassembled packet. reass should occur before any rules that list ports.

Update bhyve -h usage output to include new cpu topology options.
Pointed out by Roman Bogorodskiy (novel@).

Translate reviewer rgrimes -> bhyve triggered by touching stand/userboot.