I really think this belongs in tcp_lro_low_level_parser(). The function you've hooked exists simply to deal with IPv4 IP checksums, that's why there was no analog for V6. If you do push this, you need to remove the stat increments.

In D24645#889225, @mjg wrote:

Is this still an issue? I think some work by mav@ managed to depessimize some of cpu search in 2668bb2add8d11c56524ce9014b510412f8f6aa9 and aefe0a8c32d370f2fdd0d0771eb59f8845beda17, perhaps to the point where the above is moot.

If stealing right now reduces your perf, I think a ratelimit on limit is only papering over the real problem.

In D24645#889225, @mjg wrote:

Is this still an issue? I think some work by mav@ managed to depessimize some of cpu search in 2668bb2add8d11c56524ce9014b510412f8f6aa9 and aefe0a8c32d370f2fdd0d0771eb59f8845beda17, perhaps to the point where the above is moot.

If stealing right now reduces your perf, I think a ratelimit on limit is only papering over the real problem.

In D38504#877161, @markj wrote:

In D38504#876841, @gallatin wrote:

It might be nice to document why the SOLISTENTING() check is not needed in the tx case..

Why exactly do we acquire the SEND_IO lock in enable_tx but we don't acquire the RECV_IO lock in enable_rx? It's a bit hard to write a useful comment without that info.

It might be nice to document why the SOLISTENTING() check is not needed in the tx case..

In D38380#875354, @gallatin wrote:

In D38380#875352, @jhb wrote:

I'm trying to remember the edge case for why the flag wasn't cleared when moving to a software session. I'm not sure this doesn't open whatever race that was back up. I think the problem might have been that when you switch to SW TLS you might still have existing mbufs in the socket buffer that were framed with the NIC TLS session. We don't go back and try to do software encryption of those mbufs, and even though you've changed the TLS session for "new" requests to send data in the future, those previously existing mbufs still have the old snd_tag and need NIC TLS behavior in TCP. I think you've now broken that case again. In theory the right answer is to check the mbufs you are planning to send to see if they have a send tag and do the split in tcp_m based on that, but that means walking the mbuf chain I think all the time, and the global flag in the sockbuf seemed less expensive.

When you switch to SW TLS, the existing mbufs that were framed with NIC TLS still reference the NIC TLS session. The t_nic_ktls_xmit flag remains set on the tcbcb until the NIC TLS session is released, meaning that TCP has moved past all those mbufs,

In D38380#875352, @jhb wrote:

I'm trying to remember the edge case for why the flag wasn't cleared when moving to a software session. I'm not sure this doesn't open whatever race that was back up. I think the problem might have been that when you switch to SW TLS you might still have existing mbufs in the socket buffer that were framed with the NIC TLS session. We don't go back and try to do software encryption of those mbufs, and even though you've changed the TLS session for "new" requests to send data in the future, those previously existing mbufs still have the old snd_tag and need NIC TLS behavior in TCP. I think you've now broken that case again. In theory the right answer is to check the mbufs you are planning to send to see if they have a send tag and do the split in tcp_m based on that, but that means walking the mbuf chain I think all the time, and the global flag in the sockbuf seemed less expensive.

Updated patch to restore and document the td_rw_rlocks hack to detect if we might hold an rlock on the inp. Without this, we end up scheduling a taskqueue in about 18% of cases due to other threads holding locks.

Update to hold the wlock in ktls_destroy for transmit ktls, as suggested by Mark.

Run ktls_destroy() if we are called by a thread holding an rlock. There is no way to know if the rlock held by the thread is the inpcb lock, but just assume it is for safety.

In D37977#873536, @mjg wrote:

What cache-misses? I did not see any point in benching this change, it is the original minus some branches and no extra memory accesses.

In D37977#873507, @mjg wrote:

that's probably because the branches you would normally ran into got previously sorted out with 0b70e3e78b0279c66be06dea27bcdaf5eadf663d , but chances are there is enough other slowdown for that to not matter on its own

Update diff to use ifdefs and SIZEOF_LONG as suggested by reviewers.

Remove now unneeded ifndef VM_BATCHQUEUE_SIZE as suggested by @jhb

Updated patch to use SIZEOF_LONG, based on review feedback from @jhb and @markj

Updated patch to load the proper systrace module when running on a 32-bit system.

Rebased to today's current.

In D37903#861938, @gbe wrote:

@gallatin transport Has had anyone the time to look at https://reviews.freebsd.org/D36538? At least TCPHPTS should be included somewhere and the documentation should also be updated!

You currently also need RATELIMIT if you want to actually use the stacks. I've submitted a patch privately to @rrs which removes this requirement.

In D37903#861260, @glebius wrote:

I actually prefer D37715. Isn't that what we have discussed on the TCP call on December 15th?

Add WITH_EXTRA_TCP_STACKS to common NOTES file, as suggested by @imp

Abandoning this in favor of D37903

In D37715#857714, @gallatin wrote:

In D37715#857654, @jtl wrote:

This causes them to be left out of CI build testing

The stacks should be build as part of a LINT kernel or tinderbox build. Is the CI test not doing that?

I think they are not being built, as they didn't compile with the various LINT_NO{IP,INET,INET6} and I didn't see any warnings about it. I'm not sure how they could be, as they are built when MK_EXTRA_TCP_STACKS=yes exists in /etc/src.conf. Is there a way to specify this inside a kernel config file, rather than in src.conf?

In D37715#857654, @jtl wrote:

In D37305#853896, @markj wrote:

The patch seems ok to me, but:

• In vm_pageout_reinsert_active(), we should change the call to read if (vm_batchqueue_insert(bq, m) != 0), even though it's just a stylistic change.

Thanks for this. I tested this and confirmed that it works as described.

In D37305#851659, @gallatin wrote:

Update diff to restore missing hunk that causes vm_batchqueue_insert() to return the number of free slots. As pointed out by Markj

Update diff to restore missing hunk that causes vm_batchqueue_insert() to return the number of free slots. As pointed out by Markj

Why not just use a mutex, using mtx_trylock() for the acquire? Mutexes already check the panic special case..

Neat. I was not aware of seqc(9) until now!

Move NET_EPOCH_EXIT() to after the prints.

• Null'ed drv_ioctl_data.nvcap as suggested by @kib

Since all bits may be set, use the fact that the list is empty, rather than all bits remaining set, to zero caps and enables, as suggested by Hans