Maybe add a comment for the else cases saying that they are chacha?

I've tested this on a Netflix server, and can confirm that it works fine, and eliminates spinlock contention on the gic cmd queue.

I've tested this on Netflix servers, and it seems to work.

Are legacy interrupts supported on this platform? Is this code also used for legacy interrupts?

I'm shocked that this feature was not present before.

This is nothing short of amazing. It cuts CPU use on an original Netflix 100G server (16 core Broadwell Xeon 2697A v4) from ~62% to ~52% at roughly 92Gb/s of a 100% ktls Netflix video serving workload.

In D28556#639376, @markj wrote:

In D28556#639295, @gallatin wrote:

This is pretty exciting. I can try this on amd64 and let you know how much it helps.

Thanks! I expect the improvement to be modest but that'll help inform whether this is worth doing.

It won't be until next week, as I have a few things going on this week, and it looks like I may need to change the ktls_isa-l_crypto-kmod to catch up to having iniovcnt != outiovcnt

I guess that's not in the tree? That would hopefully be a straightforward conversion.

This is pretty exciting. I can try this on amd64 and let you know how much it helps. It won't be until next week, as I have a few things going on this week, and it looks like I may need to change the ktls_isa-l_crypto-kmod to catch up to having iniovcnt != outiovcnt

The producer and consumer certainly may be on different CPUs.

This probably needs to be MFC'ed to 13

Note that we (Netflix) have been running this patch in evolving state since 2017 in production.

In D27797#629663, @rajesh1.kumar_amd.com wrote:

I see "iflib" group review is blocking here. Now that this patch is only driver specific and doesn't have any change in iflib, does this patch need any other action to get it pushed upstream?

Thank you for catching this.

OK, thanks that makes sense. I was not considering a multi-frag case.

Whats the path to having a null pointer as pf_rv ?

In D28102#628179, @markj wrote:

i386 should probably get the same patch?

There is already a mechanism drivers can use to build an array of packets which is used by LRO. Eg, tcp_lro_queue_mbuf(). It was quite helpful, not so much for the batching, but because it sorts the array to put packets from the same TCP connection adjacent to each other..

After talking with our AMD rep, the AMD advise for this is:

Might it be better to just make a new if_input method which takes an array of mbufs? List processing for a batch of packets seems like a recipe for cache misses..

I tested the kernel part of this on a Netflix server with an EPYC 7502P using TSC-low. It saves roughly 2% CPU and tsc disappears entirely from profiling output, making profiling output match that from our Xeons much more closely.

Thanks so much for reviewing. I've made the changes you requested in the patch I'm about to commit.

Rebase patch to r368767

Rebase patch as of r368767

This looks correct to me.

In D27344#611029, @kbowling wrote:

@gallatin I don't have the resources to fix it, so I'll just say of the things we were trying to do is get away from expert knob turning for common use cases, which I would lump a netapp filer and a CDN into for FreeBSD. iflib is supposed to not needlessly rearm interrupts while it is processing and there is no more work, so we should have some moderation of interrupts already or we need to add a callback into the framework so hardware like this can further modulate if necessary. I don't like bringing back a driver-only hack instead of holistically solving this. Nonetheless, if I am the only person that cares about that kind of thing I think my assumptions need to be retuned and I'll go away.

A few more thoughts based on a conversation I had with somebody: In order for mbuf sorting to be effective, you need to have a large number of packets received per irq. Sorting a dozen mbufs is not useful when you're already aggregating 64 different connections, for example. So AIM (or very large irq colaescing timer/packet settings) are required to get enough packets per irq to make mbuf sorting worthwhile.

Approving for just myself for now until the discussion regarding AIM / LRO is wrapped up.

In D27344#610766, @kbowling wrote:

I would rather see this fixed by implementing LRO mbuf sorting in iflib, a la cxgbe and mlx5en. It's a small code change like https://freshbsd.org/commit/freebsd/src/317041. Would you be willing to look at this or discuss with intel to do it on your behalf?

In D26973#609585, @gallatin wrote:

After this change, perl freaks out like this:

perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
LC_ALL = (unset),
LANG = "C.UTF-8"

are supported and installed on your system.

perl: warning: Falling back to the standard locale ("C").
panic: locale.c: 4560: Could not change LC_CTYPE locale to C.UTF-8, errno=22

Is this expected? This is perl5-5.32.0

Thanks

After this change, perl freaks out like this:

Isn't errno different in linux on different architectures? I know that at least alpha had a very different errno than x86..

Its in my branch, but not our main branch.

Awesome. Thank you!

Can we add some __predict_false() to aid the compiler (and the reader) to know these are rare?

In D26691#595529, @hselasky wrote:

Looks good from mlx5_en_main.c's point of view.
@gallatin: Did you test this with mlx5en?

Thank you for making these changes.

In D26409#591582, @rscheff wrote:

So, how about moving the PCP to become part of the inpcb, and use IPPROTO sockopts instead. Sounds like that may be the better approach; doing this in the socket struct was "just" the easiest way to get a proof-of-concept running for various reasons... While the PCP is part of the (extended) Ethernet Header, there are no setsockopt Protos that deal with data on that level...

Is there a file which is already perfectly style(9) compliant? Is it this file?

What command line arguments, or other clang format configuration was used here?