np added a subscriber: np.
np added a reviewer: lstewart.
np added a comment.

LRO affects the kernel TCP code in subtle (and almost always undesirable) ways 
by "compressing" multiple TCP headers into one.  Think TCP timestamps, bursty 
changes in sequence space as seen by the kernel, what happens to pure acks, 
etc.  Our LRO implementation is even willing to combine multiple received TCP 
PSHes into one.  All this is a decent tradeoff when a handful of segments are 
involved but the proposed LRO_PLEN_MAX (1MB) is 700+ MSS sized segments at 1500 
MTU.  I wonder how well the kernel TCP will deal with such big bubbles of data. 
 Please do get the big picture reviewed by one of our TCP protocol experts.

M_HASHTYPE_LRO_TCP isn't really a hash type and will likely confuse the RSS 
code.  There is some value in providing the hash type to the stack but with 
your proposed change the hash type will be clobbered by tcp_lro_flush.  Data 
for a single stream will show up in the stack with either the correct hash type 
or M_HASHTYPE_LRO_TCP.  Not pretty.  

I wonder what one of these gigantic LRO'd packet looks like to bpf consumers?  
If they go by the ip header then they will likely get confused.  A good test 
would be to see if wireshark is able to follow the TCP stream accurately or 
does it lose track when it encounters one of these VLRO (Very Large RO) packets?

At the very least, allow drivers to opt out of this VLRO by
a) making LRO_PLEN_MAX per lro_ctrl, to be set when the LRO structures are 
initialized by the driver.
b) never clobbering the hash type in tcp_lro.c if the total length accumulated 
is <= 65535.

REVISION DETAIL
  https://reviews.freebsd.org/D1761

To: hselasky, rmacklem, rrs, glebius, gnn, emaste, imp, adrian, bz, rwatson, 
lstewart
Cc: np, freebsd-net
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"

Reply via email to