in_broadcast() called for almost every packet in ip_output()

[Andrew Gallatin]

On 07/20/2016 15:09, Ryan Stone wrote:
> On Wed, Jul 20, 2016 at 3:00 PM, Drew Gallatin <gallatin at netflix.com
> <mailto:gallatin at netflix.com>> wrote:
>
>     I'd certainly prefer not to add any overhead.  Properly managing
>     this (and other similar) lists is a job for some lightweight
>     lifecycle based mechanism like concurrency kit or rcu.
>
>     Unless you have a solid  reason to fix it, I'd suggest just adding
>     the locking commented out (the way rwatson did with IN_IFADDR_RLOCK
>     in ip_input(), so that the next person to trip over it will know
>     what's going on.
>
>     Drew
>
>
> I'm not clear on the details, but we have some internal tests that were
> provoking this exact crash after a couple of days of stress testing.
>
> I believe that the actual fix would involve caching the result in the
> pcb.  When the endpoint is already known, there's no reason to check for
> a broadcast packet on every single packet.  On a system that uses a
> large number of IP addresses on an interface (e.g. for jails) the
> overhead of just iterating over the list is going to add up no matter
> what synchronization mechanism you use.

Caching sounds reasonable, but.. won't the lock still add overhead
to paths like unconnected udp and packet forwarding?