Page MenuHomeFreeBSD

tcp lro: use flowid only when it has hash properties
ClosedPublic

Authored by tuexen on Thu, Oct 9, 7:48 AM.

Details

Summary

When a packet is provided to LRO using the tcp_lro_queue_mbuf(), a sequence number is computed based on the m_pkthdr .flowid provided by he driver. The implicit assumption is that the m_pkthdr .flowid has hash properties.
The recent use of tcp_lro_queue_mbuf() in iflib exposed a bug in at least one driver (igc) , which

  • reports always that is uses M_HASHTYPE_OPAQUE.
  • sets in some cases m_pkthdr .flowid not consistently for packets belonging to the same TCP connection.

This results in severe performance problems for the base TCP stack, since it handles the packets in the wrong sequence, although they were received in the correct sequence.
To protect against such misbehaving drivers, just take the m_pkthdr .flowid only into account, if it has hash properties.

The performance problems were observed by gallatin@ and analyzed together with rrs@.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tuexen requested review of this revision.Thu, Oct 9, 7:48 AM

To be clear, igc worked fine when configured with multiple queues. I had configured it with a single rx queue in order to debug something else, and it took the somewhat reasonable approach of returning garbage as the hash since it didn't need to hash packets to multiple queues. Thank you for coming up with this... it is much cleaner than my solution, and should ensure that any other non-iflib drivers avoid this behavior as well.

FWIW, recv perf with LRO enabled from a freebsd sender 1600km away goes from 64Kb/s to 500Mb/s.

Also, this needs to be MFC'ed to 15 for the release..

This revision is now accepted and ready to land.Thu, Oct 9, 12:53 PM

Also, this needs to be MFC'ed to 15 for the release..

Already done in 4774687d4087.