We have tested PF, and verified that the sysctls are still there and have not changed.

ping, any update on this?

In D52326#1196472, @kgalazka wrote:

@jmg thank you for the patch! I started working on that issue, but a family emergency put me AFK for almost 2 weeks. To avoid further delay I think we should proceed with your version. It looks good to me, but let me run some basic tests on Monday. I'll approve this review as soon as I'm done.

fixed comments, commit message to be:

e1000: fix igb VF stats

address comments, add comment about union usage

Note that this is only compiled tested. This was mainly to get a preliminary test patch out as it might be a couple days before I can do my own testing, and w/ 15 branch coming up, thought if someone had time to test/look at it in the meantime, we can compress the timeline.

The attached program can be used to test/verify the problem/fix. Compile w/ -m32.

I forgot to tag the commit. This was committed in: https://cgit.freebsd.org/src/commit/?id=740b879c6ade531adebeba7cd2f261bbe650797f

In D38898#885672, @freebsd_igalic.co wrote:

In D38898#885654, @jmg wrote:

So, I think what might be wrong is that you don't have another virtio driver to "unstick" the virtio queue. FreeBSD only has drivers for balloon, block, console, network, entropy and scsi. I looked at your config, and the only other device you define that is supported by FreeBSD is balloon. In my config, I'm using both network and block so it's possible that it's working here only because there is additional disk/network traffic causing the rng events to get dequeued...

you can see (part? of) my config at https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=269874
so there is a network driver, but there might not have been much going on

regardless, the fact that virtio_random requires some other driver to unstick its poll queue sounds like a bug in our virtio implementation.

In D38898#885646, @freebsd_igalic.co wrote:

rebuilt with this patch, here's the disas:

`Dump of assembler code for function vtrnd_read:

0xffffffff82b05340 <+0>:     55      push   %rbp
0xffffffff82b05341 <+1>:     48 89 e5        mov    %rsp,%rbp
0xffffffff82b05344 <+4>:     41 57   push   %r15
0xffffffff82b05346 <+6>:     41 56   push   %r14
0xffffffff82b05348 <+8>:     41 54   push   %r12
0xffffffff82b0534a <+10>:    53      push   %rbx
0xffffffff82b0534b <+11>:    48 83 ec 10     sub    $0x10,%rsp
0xffffffff82b0534f <+15>:    48 8b 1d 3a 1f 00 00    mov    0x1f3a(%rip),%rbx        # 0xffffffff82b07290 <g_vtrnd_softc>
0xffffffff82b05356 <+22>:    45 31 f6        xor    %r14d,%r14d
0xffffffff82b05359 <+25>:    48 85 db        test   %rbx,%rbx
0xffffffff82b0535c <+28>:    74 4e   je     0xffffffff82b053ac <vtrnd_read+108>
0xffffffff82b0535e <+30>:    80 7b 20 00     cmpb   $0x0,0x20(%rbx)
0xffffffff82b05362 <+34>:    75 48   jne    0xffffffff82b053ac <vtrnd_read+108>
0xffffffff82b05364 <+36>:    41 89 f4        mov    %esi,%r12d
0xffffffff82b05367 <+39>:    49 89 ff        mov    %rdi,%r15
0xffffffff82b0536a <+42>:    48 8b 7b 10     mov    0x10(%rbx),%rdi
0xffffffff82b0536e <+46>:    48 8d 75 dc     lea    -0x24(%rbp),%rsi
0xffffffff82b05372 <+50>:    e8 29 be f0 fd  call   0xffffffff80a111a0 <virtqueue_dequeue>
0xffffffff82b05377 <+55>:    48 85 c0        test   %rax,%rax
0xffffffff82b0537a <+58>:    74 30   je     0xffffffff82b053ac <vtrnd_read+108>

[...]

0xffffffff82b053ac <+108>:   44 89 f0        mov    %r14d,%eax
0xffffffff82b053af <+111>:   48 83 c4 10     add    $0x10,%rsp
0xffffffff82b053b3 <+115>:   5b      pop    %rbx
0xffffffff82b053b4 <+116>:   41 5c   pop    %r12
0xffffffff82b053b6 <+118>:   41 5e   pop    %r14
0xffffffff82b053b8 <+120>:   41 5f   pop    %r15
0xffffffff82b053ba <+122>:   5d      pop    %rbp
0xffffffff82b053bb <+123>:   c3      ret

End of assembler dump.

This change was inspired by noticing that CPU usage was significantly increased when running w/ the virtio_random module loaded. W/o it loaded, the harvest thread was consuming ~.02% cpu, but when it was loaded, it was consuming ~1% CPU (and there's a bug report of it consuming 100% cpu: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=269823 which this would partially fix).

not sure how it included a file AFTER the commit hash, but arc is
weird.. hopefully this will work..

done.

implement kib's comments

wrong change made it, this fully addresses comments

address style comments

Ok, will update the patch in a second, also forgot to include the output of the result:

root@freebsd:~ # kenv | grep ^smbios
smbios.bios.reldate="02/06/2015"
smbios.bios.revision="0.0"
smbios.bios.vendor="EFI Development Kit II / OVMF"
smbios.bios.version="0.0.0"
smbios.chassis.maker="QEMU"
smbios.chassis.type="Other"
smbios.chassis.version="virt-7.1"
smbios.memory.enabled="4194304"
smbios.socket.enabled="1"
smbios.socket.populated="1"
smbios.system.maker="QEMU"
smbios.system.product="QEMU Virtual Machine"
smbios.system.version="virt-7.1"
smbios.version="3.0"

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.