netinet: add SO_REUSEPORT_LB_CPU for receive-CPU socket affinity

[nick_spun.io]

Summary

A SO_REUSEPORT_LB group distributes incoming connections among its
members with a hash of the connection four-tuple. That hash is
unrelated to the receive queue the NIC steered the flow to, so a worker
pinned to the CPU that processes a given receive queue cannot be handed
the flows that land on its own CPU, and the locality that receive-side
scaling sets up is lost at the socket layer.

Add a SO_REUSEPORT_LB_CPU socket option that tags a member socket with a
receive-CPU affinity. When a packet is processed on that CPU,
in_pcblookup_lbgroup() delivers it to the matching member in preference
to the hash. Members without an affinity, and packets whose processing
CPU matches no affinitized member, keep the hashed distribution.

The group counts affinitized members, so the lookup scan is skipped for
groups that use none and the common path is unchanged. The match uses
curcpu at lookup time, the CPU the stack actually processes the flow on,
so it holds under both direct and deferred netisr dispatch and without
options RSS compiled in.

[glebius]

Pardon naive quick question, before reading deeper into the proposal: any downsides with proposed behavior? Can it be on by default for SO_REUSEPORT_LB?

[kbowling]

FWIW when we did LB we were really worried about breaking software, and in hindsight we maybe should have just aligned REUSEPORT across a major FreeBSD version with Linux to minimize third party software adaptation. Think hard before adding yet another non-portable sockopt; the LB_CPU_CURRENT behavior sounds desirable unless there is some corner you see. So I agree with Gleb.

[gallatin]

In D59501#1365171, @kbowling wrote:

FWIW when we did LB we were really worried about breaking software, and in hindsight we maybe should have just aligned REUSEPORT across a major FreeBSD version with Linux to minimize third party software adaptation. Think hard before adding yet another non-portable sockopt; the LB_CPU_CURRENT behavior sounds desirable unless there is some corner you see. So I agree with Gleb.

I've done the LB_CPU_CURRENT concept in a hack local branch at netflix months ago when experimenting with affinity and had planned to propose such a patch myself. It is a necessary step towards being able to achieve RSS-like affinity without RSS. The cool thing is that the NIC RSS hash + LACP partner hash puts the connect on the same core that normal incoming TCP traffic will land on. So by accepting with the current CPU (rather than a janky hash) you wind up having affinity throughout the life of the connection.

The problem is that for unconnected UDP, you really want a hash. Else traffic from the same sender can wind up mapped to different receiving threads.

I'm approving this *based on the concept*, but its been years since I've really read the inpcb hash code and I'd like you to wait to push until somebody more familiar with the inpcb hash code also approves.

[adrian]

I'm ok with in general (and I'm glad there's no push back so far, affinity is always one of those tricksy discussion points with respect to workload performance!)

I wonder if having CPU affinity here is the right thing to do or to have it go through some intermediary "thing" like a bucket ID where the load for all sockets on that "thing" can be moved CPUs.
I had a similar concern back when I was doing this for RSS; shuffling the "big ticket" items around at runtime became impossible.

(Don't try changing anything based on this; it's a larger discussion to have re: NIC RSS bucket/queue -> CPU selection, netisr CPU selection, userland thread CPU selection, etc, etc...)

sys/netinet/in_pcb.c

2346

how big can this array get? It's in the pcb lookup path so it has to be fast/bounded, right?

sys/netinet6/in6_pcb.c

1006

same here; how often is this being run?

[markj]

I don't have any strong feelings on the interface. Having each socket explicitly bound to a CPU (rather than some abstract flow ID/bucket identifier) seems kind of janky to me, but I'm also not familiar with the requirements of "typical" applications which might want to leverage this feature. Some description of such an application would be useful.

I made some comments on the implementation.

sys/netinet/in_pcb.c

585

What if two inps claim the same CPU? Shouldn't that be an error?

2346

This lookup is done once per connection, I believe, so it's not terrible, but yes a bit unsatisfying.

On the other side we could provide an array of MAXCPU pointers, indexed by cpuid, but that's 8KB per lbgroup.

Maybe we could provide a per-CPU lookup structure which maps the local port to a list of affinitized lbgroups.

2348

Do you actually need to check for NULL here? Note the assertion below.

sys/netinet/in_pcb.h

344

This could be a uint16_t instead, to take advantage of the existing 24-bit hole here.

[gallatin]

Looking at this more closely, I'm not sure it does what my hack did (and what I want).

Basically, what I want is for a daemon creating 1 worker per core to create listen sockets and call SO_REUSEPORT_LB_CPU_CURRENT on them and then have incoming connections assigned to a worker based on the current CPU that the **NIC ithread driving the incoming TCP connection is on.**

What I think this patch does is sets the CPU for each listener to whatever the current CPU was when they made the listen call. For people using nginx, this is not ideal, as nginx first opens the listen socket in the master proc, and then forks children for each listen socket. So the listen call (and setsockopts) all tend to happen on the same CPU. So we'd have 16 workers, all with listen sockets that have affinity to CPU 0.

My hack, which works the way that is best for daemons like nginx, is at: https://people.freebsd.org/~gallatin/affinity.diff

[gallatin]

Argh, i wish there was a way to retract approval but not go so far as to request changes..

[nick_spun.io]

In D59501#1366878, @gallatin wrote:

Looking at this more closely, I'm not sure it does what my hack did (and what I want).

Basically, what I want is for a daemon creating 1 worker per core to create listen sockets and call SO_REUSEPORT_LB_CPU_CURRENT on them and then have incoming connections assigned to a worker based on the current CPU that the **NIC ithread driving the incoming TCP connection is on.**

What I think this patch does is sets the CPU for each listener to whatever the current CPU was when they made the listen call. For people using nginx, this is not ideal, as nginx first opens the listen socket in the master proc, and then forks children for each listen socket. So the listen call (and setsockopts) all tend to happen on the same CPU. So we'd have 16 workers, all with listen sockets that have affinity to CPU 0.

My hack, which works the way that is best for daemons like nginx, is at: https://people.freebsd.org/~gallatin/affinity.diff

So this is actually at setsockopt() time and not listen() time. The argument is a CPU id and SO_REUSEPORT_LB_CPU_CURRENT is "the CPU I am on now", so the idea is that when a worker lands it can just setsockopt with SO_REUSEPORT_LB_CPU_CURRENT and affinity works that way

This works, and saves around 15% cycle time per flow in a crappy synthetic benchmark, and mostly mirrors an interface that Linux has, but it should really be a fallback to do explicit pinning rather than the golden path.

I'm starting to throw some rough PoCs together about what we could do at first accept() - I think we could be transparent to the caller there but it wouldn't cover everything and there's still a fair bit of plumbing to think about