On 2022-12-13 09:18, Andrew Gallatin wrote:
I was trying to improve the performance of in_pcblookup(), as it is a
very hot path for us (Netflix). One thing I noticed was the
prison_flag() check in in_pcblookup_hash_locked() can cause a cache
miss just by deref'ing the cred pointer, and it can also cause multiple
misses in tables with collisions by causing us to walk the entire chain
even after finding a perfect match.
I'm curious why this check is needed. Can you explain it to me? It
originated in this commit:
commit 413628a7e3d23a897cd959638d325395e4c9691b
Author: Bjoern A. Zeeb <b...@freebsd.org>
Date: Sat Nov 29 14:32:14 2008 +0000
MFp4:
Bring in updated jail support from bz_jail branch.
This enhances the current jail implementation to permit multiple
addresses per jail. In addtion to IPv4, IPv6 is supported as well.
My thinking is that a jail will either use the host IP, and share its
port space, or it will have its own IP entirely (but I know nothing
about jails). In either case, a perfect 4-tuple match should be enough
to uniquely identify the connection.
Even if this somehow is not the case and we have multiple connections
somehow sharing the same 4-tuple, how does checking the prison flag
help us? It would prefer the jailed connection over the non jailed,
but that would shadow a host connection. And if we had 2 jails sharing
the same 4-tuple, the first jail would win.
I can't see how this check is doing anything useful, so I'd very much
like to remove this check if possible. Untested patch attached.
For a complete 4-tuple, it should indeed be the case that a match would
only ever identify a single prison. The later part of the function that
examines wildcards definitely needs the check. I don't get the XXX
comment about both being bound with SO_REUSEPORT, because I would only
expect that to apply to listening, not to full connections. But I also
expect Bjoern to know more than I do here...
- Jamie
