So, spent a few minutes looking into this, my testing was done on:

FreeBSD gate4 14.0-CURRENT FreeBSD 14.0-CURRENT #0 main-n255198-1907e1c07c3: Thu May  5 07:52:56 UTC 2022     root@releng1.nyi.freebsd.org:/usr/obj/usr/src/amd64.amd64/sys/GENERIC amd64

New patch has addressed a few of the comments.

Addresses a few of the comments.

Thanks, will address a few of the comments.

I'll second the usage of Em or Sy, I'm a bit nervous about Sy, being symbolic, and how it'll format when using a proportional font (e.g. printed).

Thanks for catching the compile issue. I hacked the modules to compile w/ debug, so missed the bug compiling w/o.

• make compile w/o USB_DEBUG defined...

update to support VLAN MTU...

Let me know more about the CDCE_IND_SIZE_MAX constant.

No, this needs to be updated to the new website.

looks good.

In D29748#667598, @trasz wrote:

Makes perfect sense, but growfs depends on awk(1), which might be on a separate /usr, which gets mounted by mountcritlocal, which runs after root.

I wonder if it would be possible to somehow extend geom(8) to make the walk through the hierarchy easier, to not require complicated text processing?

In D29748#667254, @trasz wrote:

Do you remember any details about the bug, or the testing setup?

The problem with current ordering is that it's not really defined when the sysctl script runs - it has no dependencies, so it's up to rcorder(8), and from what I've seen it's not very stable about sorting.

Looking at the comment in the original commit:

# TODO: Figure out where this should really be ordered.
# I suspect it should go just after fsck but before mountcritlocal
# but it's hard to tell for sure because of the bug described
# below.

the phrase "This file derived from" has no history in our tree, I'd prefer the phrase I proposed.

Thank you.

Ok, the results from an arm64 run (A53, Pine64 A64-LTS).

In D29007#650040, @cem wrote:

In D29007#649973, @jmg wrote:

In D29007#649700, @cem wrote:

Is 128 a reasonable chunk here? How does it relate to the internal buffering of FILE?

I'm going to run some perf numbers on ARM, but w/ the attached file w/ benchmarks, it looks like the chunk should be closer to 16.

That sounds about right if we're optimizing for single conversions (%d, %g, whatever) at the start of long input strings. Does the small size penalize larger conversions much? It seems like we'll be bouncing around _vsscanf_read every N bytes now. (I wonder how common the GTA use case actually is.)

In D29007#649700, @cem wrote:

Is 128 a reasonable chunk here? How does it relate to the internal buffering of FILE?

strlen tests pass:

freebsd@test:/usr/tests $ kyua test lib/libc/string/strlen_test
lib/libc/string/strlen_test:strlen_basic  ->  passed  [0.004s]
lib/libc/string/strlen_test:strlen_huge  ->  passed  [0.027s]
lib/libc/string/strlen_test:strnlen_basic  ->  passed  [0.004s]

address comments.

• address brooks's comments,

style(9) uses \0 exclusively, so I guess I'll change.

• use a more common spelling of NUL...

Attached is a set of benchmarks. First number is chunk size, second number is number of 0's in the string that is sscanf'd.
Bench (results from an amd64, AMD PRO A10-8770E, 2.8GHz):

The strnlen will be committed as a separate commit, likely before this change, as this change depends upon it for performance.

As "more" correct term was never provided, and phk also didn't like this change, no point in keeping this open.

I have no plans to work on this in the future. Someone else is free to take this over and get it committed.

In D26538#632510, @emaste wrote:

Compat with what? There is no code in the FreeBSD source tree that uses it, and no releases have this ioctl. It hasn't even been merged to 12-stable yet.

Future backwards compat - i.e. introduce the new ioctl if and when we extend if_data.

In D26538#632352, @markj wrote:

In D26538#632166, @jmg wrote:

I know this was closed and committed, but I want to get some more discussion on this.

I happen to be reviewing this for my own code (to get at _hwassist), and noticed that the code does not check how much space the program allocated for the if_data structure. This means that as the interface stands, it cannot be expanded, ever in the future, as it'll break old programs when they use this.

Why can't we change the ioctl number and keep the old one for compat?

I know this was closed and committed, but I want to get some more discussion on this.

can't say how correct this is, but it'll be great to have it fixed!

Yes, can we please get this committed ASAP? I just spent some time debugging this problem. (Other solution would be for base not to read /etc/make.conf, but I have a feeling this commit is more likely.)

• minor spelling and grammar fixups. (was originally included)
• revert this change... (release.sh)
• remove whitespace at end of the copyright lines..

I am planning on committing 1 and 2 individually.

In D26220#587460, @pnagato_protonmail.com wrote:

@jmg @imp I have seen other places where KASSERT was re-defined as

#define	KASSERT(exp,msg) assert((exp))

Might I suggest that you enable NetBSD's cp tests, and add necessary tests to prevent this breakage again?

minor update to include some new register definitions.. unused for now..

don't overwrite the RCR register, keep some bits set. This gets
MTU up to 4096 - mumble working w/o my devices crashing...

In D26220#584528, @pnagato_protonmail.com wrote:

In D26220#584521, @imp wrote:

In D26220#584195, @pnagato_protonmail.com wrote:

@jmg @imp Need your input how to proceed on the sbuf_negative test.

I'm having trouble understanding its purpose. Maybe you could give a one or two sentence summary of what it should test?

@imp I am assuming the sbuf_new_negative_test is supposed to test for cases for which sbuf_new fails to create a sbuf.

From sys/kern/subr_sbuf.c

	KASSERT(length >= 0,
	    ("attempt to create an sbuf of negative length (%d)", length));
	KASSERT((flags & ~SBUF_USRFLAGMSK) == 0,
	    ("%s called with invalid flags", __func__));

or when SBMALLOC fails..

move some debug prints to before the error...

Thanks for committing this. I've run the patch for a while, and it solves my panic, and didn't introduce any others.

commited in https://reviews.freebsd.org/rS363683

In D25874#573062, @zeising wrote:

Removed manual pages need to be added to ObsoleteFiles.inc as well.

didn't complain about missing file on command line, that being
rp's module dir..

forgot to remove from files..

too smart w/ command line, include the rest of the files..

I'll wait to get a few more reports of tests before committing..

ok, these should address all of your comments.

update to address hselasky's comments.

make it relative to HEAD.. Post white space commit..