Re: TSO and netfilter (Re: Extremely slow network with e1000 & ip_conntrack)

[Harald Welte]

On Thu, Dec 11, 2003 at 05:41:36PM -0800, David S. Miller wrote:
> On Thu, 11 Dec 2003 12:03:15 +0100
> Harald Welte <laforge@xxxxxxxxxxxxx> wrote:
> 
> > The only interesting case is in ip_output.c:ip_queue_xmit(), where
> > tso_size and tso_segs are calculated, before NF_IP_LOCAL_OUT is run.
> > 
> > But changing the content or the size of the tcp payload should not
> > affect those calculations. 
> 
> It changes at least tso_segs, since if you decrease of increase the
> size of the payload the number of real TCP/IP packets the TSO engine
> will end up spitting out could be different.

I see.  So what about the networking core exporting an [inline] function
that recalculates tso_segs and tso_size (like the 'Hack zone' code
fragment in ip_queue_xmit() right now), called skb_tso_recalc() or
whatever name you prefer.

Or even better (since I assume TSO can only happen with
locally-originated datagrams), why don't we move the tso_size/tso_segs
calculation to happen after the LOCAL_OUT netfilter hook?  This way we
also get the ip_select_ident_more() right, which we couldn't easily
update from the proposed skb_tso_recalc() function.

yes, in that case we would need to have some fake code like
	if (skb->len > mtu && (sk->sk_route_caps&NETIF_F_TSO))
		skb_shinfo(skb)->tso_segs = 1;
in order to make the newly-created check for refragmentation in
conntrack still work.  Alternatively, create some inline function that 
gives a yes/no return if the skb would later become TSO or not.

> The one netfilter module I'm most concerned about is the one that
> handles non-passive FTP, I remember that one did strange things with
> the data stream, removed TCP options, and stuff like that.

There are no NAT helper modules as of now that touch the size of the TCP
header.  We have some experimental stuff in patch-o-matic (like
IPV3OPTSSTRIP target), but nothing in the stock kernel.  I will put a
review of those on our TODO list - but for the vanilla kernel there
shouldn't be a problem.

> > Even in the past, when we used to remove SACKPERM from the tcp
> > header, we just NOP'ed it out instead of resizing the header.
> 
> This may be what I was thinking about.

We now don't do that anymore and mangle the SACK options accordingly.
In any way, there is nothing that changes the size of the tcp or udp
header.  

> Currently all the TSO supporting drivers set the ip and tcp header
> checksum values themselves as appropriate, so there are no worries in
> this area.

good news.

Please get back to me with any comments you might have, thanks.

-- 
- Harald Welte <laforge@xxxxxxxxxxxxx>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

Attachment: pgp00001.pgp
Description: PGP signature