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.

Fix white space nit in usr.sbin/bhyve/bhyve.8
Remove extra comma in usr.sbin/bhyvectl/bhyvectl.c causing build failure

In D9930#306720, @bcr wrote:

Found a small whitespace nit in the man page. Also, you need to bump the .Dd to the date of the commit (when it is ready).

Update per Peter Grehan

In D9930#306330, @grehan wrote:

You might have to explain further why you think you are exempt from having to demonstrate testing with this.

In D9930#306276, @grehan wrote:

I'd like to see some guest test results (FreeBSD, Linux, Windows) for varying numbers of these, in particular, with sockets > 1, and comparing what Qemu shows for the same configuration.

Since your all here from the add of bhyve can you look at herald rule H97 https://reviews.freebsd.org/H97 , look for anything I might of missed in the list of file directories.
I am trying to get that upgraded to a global rule with an add non-blocking reviewer bhyve, but that requires higher access than I have.

If Peter accepts this and says I can go ahead with a commit I would do that, but I need his approval to commit it.

I dont know why it did that, I tried to simply "accept revision" and it removed all other reviewers, so put them back.

In D9930#301957, @grehan wrote:

Thanks for getting back to this Rod.

On the validation issue, AMD and Intel have their own limits on how these numbers are exposed, and even that can be model-specific. While they can be verified in the kernel, the error return is somewhat limited - it may be worth doing the check in user-space where a more useful error can be returned.

Correct issues and add man page changes

I missed that, overall I think this man page could use a rework in general form, gather all the syntax up in the SYNOPSIS, but if we keep pecking away at it at least the errors can be removed.

I have all the other comments corrected in my copy, seeking clarification on error check value for vm_set_topology before I correct that. Plan to delay depreciation of old sysctl method to allow simpler merge to stable/11. That is going to require a direct commit to stable/11, and a different commit to ^head

This is missing the associated man page change that changes gateway to [gateway] on the change syntax,
Though perhaps BSD did allow this in the past, the man page never said that the gateway was optional as far as I can recall.

I still find this as a gratuitous difference in FreeBSD that does not exist in NetBSD or OpenBSD, and appears to be BSD behavior back into the 80's, anyone that HAS already scripted to deal with symbolic names well be broken if this change is made.

In D14051#295081, @imp wrote:

In D14051#295069, @rgrimes wrote:

Only question I have before we remove this, is the tree void of code in this format?

If it is not then I think it would be unwise to remove this,if it is then yes, absolutely, remove this.

No new code has it.
There's odd corners of the tree that may still have it, but anybody changing it would remove the K&R style and go with full ANSI, so I'm not sure I understand how it would be useful to retain.

Only question I have before we remove this, is the tree void of code in this format?