Page MenuHomeFreeBSD

lro: move packet rejection checks out of lro_rx_common to avoid queueing non-LRO'able packets
AcceptedPublic

Authored by gallatin on Thu, Apr 9, 11:12 PM.

Details

Summary

When lro mbuf queuing is enabled, we should not queue easily reject-able packets. Queuing them does a bit of extra work (sorting, timestamps) and can potentially delay urgent packets such as LACP PDUs. This change moves simple rejection tests from lro_rx_common() into lro_rx and (more importantly) into tcp_lro_queue_mbuf().

Note this change only moves the easy checks on forwarding and packet metadata, where the rejection criteria is already hot in cache. It does not move parsing and looking inside the packet to verify the ether protocol, ip protocol, etc. This could be done, but we risk essentially doubling the cache misses per-packet by doing so.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I am wondering if you can see the V_ipforwarding/V_ip6_forwarding hits?
Does it make any differences if you do the mbuf pkthdr checks first?

Silly idea I don't like much as inet and inet6 should be distinct but would it help if we would make sure they are in the same cache line?

In D56337#1289068, @bz wrote:

I am wondering if you can see the V_ipforwarding/V_ip6_forwarding hits?
Does it make any differences if you do the mbuf pkthdr checks first?

Silly idea I don't like much as inet and inet6 should be distinct but would it help if we would make sure they are in the same cache line?

Do you mean can I see cache misses from the forwarding checks? No.. they are amortized over thousands of packets.

In terms of putting them in the same line, yes, a single read-mostly bitmap could do that. Eg, uint8_t pkt_forwarding_prohibited with one bit for IPv4, one bit for IPv6 and one bit for administratively prohibited would do that. Good idea.
Its one of those things that would help reduce the "death by many cuts" by a single cut. I don't think it would move the needle in any profiling all by itself. But enough such changes could be impactful.

This revision is now accepted and ready to land.Fri, Apr 10, 8:48 AM