LRO restructuring?

LRO restructuring?

[Andrew Gallatin]

Hi,

You mentioned in the recent "Re: [RFC][PATCH 1/1] cxgb3i: cxgb3 iSCSI
initiator" thread that you were planning to restructure LRO to
preserve headers so as to make forwarding possible without totally
disabling LRO.

For lro_receive_frags() based LRO, it would be ideal to locate the
header in place in the frag via the mac_hdr argument to the
get_frag_header() callback.  Eg, I'm hoping that neither the driver
nor the LRO module will need to allocate extra memory per frame and
copy the headers to it in the common case when forwarding is
not enabled.  That would add quite a bit of overhead.

With respect to hardware LRO and headers:  Would it be possible
to notify the driver via some sort of callback whether the headers
are required?  I think most hardware LRO implementations are going
to collapse the headers, and having the option to fallback to software
LRO for forwarding might be needed for those devices which will throw
away the intermediate headers.

Last, have you considered simply allowing "inexact" forwarding, where
the ingress NIC is doing LRO and the egress nic is doing TSO?  You
loose exact framing information (eg, what you emit might not be framed
exactly as you receive it), but you can still do filtering, and the
host overhead is very low.

Thanks,

Drew

Re: LRO restructuring?

[David Miller]

From: Andrew Gallatin <gallatin@myri.com>
Date: Mon, 11 Aug 2008 09:30:33 -0400

> Last, have you considered simply allowing "inexact" forwarding, where
> the ingress NIC is doing LRO and the egress nic is doing TSO?  You
> loose exact framing information (eg, what you emit might not be framed
> exactly as you receive it), but you can still do filtering, and the
> host overhead is very low.

Intermediate nodes are not supposed to change the transport layer
checksum if at all possible, especially on routers.

Otherwise it is much more difficult to diagnose checksum errors,
and figure out what caused such an error.

When the router doesn't modify the checksum, we know it's an end-node.
Even a firewall only "adjusts" checksums based upon packet
modifications for NAT and such, which will preserve end-node created
errors.

So no this isn't really an option.

This is why Herbert wants to preserve the original headers,
we're not supposed to change them.

Re: LRO restructuring?

[Herbert Xu]

On Mon, Aug 11, 2008 at 09:30:33AM -0400, Andrew Gallatin wrote:
>
> For lro_receive_frags() based LRO, it would be ideal to locate the
> header in place in the frag via the mac_hdr argument to the
> get_frag_header() callback.  Eg, I'm hoping that neither the driver
> nor the LRO module will need to allocate extra memory per frame and
> copy the headers to it in the common case when forwarding is
> not enabled.  That would add quite a bit of overhead.

You don't have to save the whole thing, just save enough so we
can easily/exactly reconstruct it on output, i.e., save the lengths.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: LRO restructuring?

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 12 Aug 2008 08:50:33 +0800

> On Mon, Aug 11, 2008 at 09:30:33AM -0400, Andrew Gallatin wrote:
> >
> > For lro_receive_frags() based LRO, it would be ideal to locate the
> > header in place in the frag via the mac_hdr argument to the
> > get_frag_header() callback.  Eg, I'm hoping that neither the driver
> > nor the LRO module will need to allocate extra memory per frame and
> > copy the headers to it in the common case when forwarding is
> > not enabled.  That would add quite a bit of overhead.
> 
> You don't have to save the whole thing, just save enough so we
> can easily/exactly reconstruct it on output, i.e., save the lengths.

And the checksums :-)  As an intermediate node we don't want
to touch the checksum.

The length and the checksum is two u16 values, which would be
able to fit in a single 32-bit descriptor or something like
that.

Re: LRO restructuring?

[Herbert Xu]

On Mon, Aug 11, 2008 at 05:54:34PM -0700, David Miller wrote:
>
> And the checksums :-)  As an intermediate node we don't want
> to touch the checksum.

Yeah if it wasn't verified then we must store this as well.

> The length and the checksum is two u16 values, which would be
> able to fit in a single 32-bit descriptor or something like
> that.

Yep.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: LRO restructuring?

[Rick Jones]

Herbert Xu wrote:
> On Mon, Aug 11, 2008 at 05:54:34PM -0700, David Miller wrote:
> 
>>And the checksums :-)  As an intermediate node we don't want
>>to touch the checksum.
> 
> 
> Yeah if it wasn't verified then we must store this as well.

Even if it was verified I think you want to keep the checksums from the 
header.   Since an intermediate device isn't supposed to be peeking at 
the TCP part anyway, it wouldn't do to drop the segment ourselves, pass 
it along to be dropped by the ultimate reciever.  And if there is 
something amis in the verification or the regeneration, we don't want to 
  introduce silent data corruption.

Likely that also goes for the IP header checksum...

rick jones

Re: LRO restructuring?

[David Miller]

From: Rick Jones <rick.jones2@hp.com>
Date: Mon, 11 Aug 2008 18:30:11 -0700

> Even if it was verified I think you want to keep the checksums from the 
> header.   Since an intermediate device isn't supposed to be peeking at 
> the TCP part anyway, it wouldn't do to drop the segment ourselves, pass 
> it along to be dropped by the ultimate reciever.  And if there is 
> something amis in the verification or the regeneration, we don't want to 
>   introduce silent data corruption.
> 
> Likely that also goes for the IP header checksum...

IP header is a little different, intermediate nodes should verify it
(and we do adjust it when  decrementing TTL).

Re: LRO restructuring?

[Herbert Xu]

On Mon, Aug 11, 2008 at 06:39:13PM -0700, David Miller wrote:
> From: Rick Jones <rick.jones2@hp.com>
> Date: Mon, 11 Aug 2008 18:30:11 -0700
> 
> > Even if it was verified I think you want to keep the checksums from the 
> > header.   Since an intermediate device isn't supposed to be peeking at 
> > the TCP part anyway, it wouldn't do to drop the segment ourselves, pass 
> > it along to be dropped by the ultimate reciever.  And if there is 
> > something amis in the verification or the regeneration, we don't want to 
> >   introduce silent data corruption.

Well I wasn't suggesting that it be dropped, but simply skip LRO
if the inbound packet fails the checksum check.

But yeah, it's only two bytes so we might as well always have it.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: LRO restructuring?

[Andrew Gallatin]

David Miller wrote:
> From: Andrew Gallatin <gallatin@myri.com>
> Date: Mon, 11 Aug 2008 09:30:33 -0400
> 
>> Last, have you considered simply allowing "inexact" forwarding, where
>> the ingress NIC is doing LRO and the egress nic is doing TSO?  You
>> loose exact framing information (eg, what you emit might not be framed
>> exactly as you receive it), but you can still do filtering, and the
>> host overhead is very low.
> 
> Intermediate nodes are not supposed to change the transport layer
> checksum if at all possible, especially on routers.

Indeed.  Nor should they change lengths, or anything else.
Everything about this "inexact" forwarding is illegal as
hell.  However, you have to admit that it is an interesting hack :)

Drew

Re: LRO restructuring?

[Herbert Xu]

Andrew Gallatin <gallatin@myri.com> wrote:
>
> Indeed.  Nor should they change lengths, or anything else.
> Everything about this "inexact" forwarding is illegal as
> hell.  However, you have to admit that it is an interesting hack :)

Solutions like this have been deployed.  For instance, many satellite
networks use transparent TCP proxies to mitigate the effect of large
latencies on older TCP stacks that don't have modern congestion
control algorithms.

Surprisingly there are actually very few problems.  The biggest
one (apart from scalability) is with non-TCP traffic masquerading
as TCP such as Cisco's VPN solution.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: LRO restructuring?

[James Huang]

Hi Herbert,

    Any idea when this LRO restructuring work will be done? 
Making LRO available even when ip forwarding is enabled will significantly 
improve performace of network appliances in the data path.  

I have some questions on this: 
(1) Based on the emails in this thread, I suppose you are going to keep the 
original length of each segment you coalesced into the big packet and use that 
info to segment the big packet on the output path.  In case the packet was 
modified by an appliance in the path and the total length is changed (e.g. NAT 
on ftp control packets), should the corresponding segment length info also get 
updated?  This same question also applies to the checksums.

(2) Do you make sure all of the segments to be coalesced have the same DF bit?

(3) I think bridged packets should not be LROed. Whether a packet is bridged 
or not can be based on the L2 MAC destination address.  Is this how it is done?

(4) Does LRO work only for IPv4?  Any plan to extend it to support IPv6?

Thanks,
James Huang

Re: LRO restructuring?

[Ben Hutchings]

On Wed, 2009-02-18 at 19:25 +0000, James Huang wrote:
> Hi Herbert,
> 
>     Any idea when this LRO restructuring work will be done? 
> Making LRO available even when ip forwarding is enabled will significantly 
> improve performace of network appliances in the data path.  

Herbert has added "GRO" rather than immediately replacing the inet_lro
code.  An early version of this is in 2.6.29 and there is more in
net-next-2.6 destined for 2.6.30.

> I have some questions on this: 
[...]
> (3) I think bridged packets should not be LROed. Whether a packet is bridged 
> or not can be based on the L2 MAC destination address.  Is this how it is done?

GRO preserves enough information to reconstruct the original frames on
output, so there is no specific check for bridging.  Presumably it would
be cheaper not to do use GRO if the frames are not going to hit the
TCP/IP stack though.

> (4) Does LRO work only for IPv4?  Any plan to extend it to support IPv6?

IPv6 is covered.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: LRO restructuring?

[Stephen Hemminger]

On Wed, 18 Feb 2009 19:25:49 +0000 (UTC)
James Huang <jamesclhuang@gmail.com> wrote:

> Hi Herbert,
> 
>     Any idea when this LRO restructuring work will be done? 
> Making LRO available even when ip forwarding is enabled will significantly 
> improve performace of network appliances in the data path.  

LRO with ip forwarding is a bad idea since it violates the end-to-end
principle. It might even be a DO NOT in host requirements RFC.

Re: LRO restructuring?

[Herbert Xu]

James Huang <jamesclhuang@gmail.com> wrote:
> Hi Herbert,

Please cc me if you're going to address the email to me :)

>    Any idea when this LRO restructuring work will be done? 

The target is to get GRO into 2.6.30, and have all LRO drivers
converted by 2.6.31.

> Making LRO available even when ip forwarding is enabled will significantly 
> improve performace of network appliances in the data path.  

I'd like to see more performance data on the forwarding side.
The act of aggregation isn't exactly free so it very much depends
on what you're forwarding.

The driver for all this work is bridging used for virtualisation
where the data is very likely to be mergeable.

> I have some questions on this: 
> (1) Based on the emails in this thread, I suppose you are going to keep the 
> original length of each segment you coalesced into the big packet and use that 
> info to segment the big packet on the output path.  In case the packet was 
> modified by an appliance in the path and the total length is changed (e.g. NAT 
> on ftp control packets), should the corresponding segment length info also get 
> updated?  This same question also applies to the checksums.

If packet length changes due to NAT then we're no loner bound
by the end-to-end restriction so we can do whatever we want.

For GRO we treat it in exactly the same way as a GSO packet that
undergoes NAT.  That is, NAT just sees a very large IP packet and
does what it has to do, and on final output we resegment the packet.

> (2) Do you make sure all of the segments to be coalesced have the same DF bit?

Yes.

> (3) I think bridged packets should not be LROed. Whether a packet is bridged 
> or not can be based on the L2 MAC destination address.  Is this how it is done?

We currently require the Ethernet header to be identical.

> (4) Does LRO work only for IPv4?  Any plan to extend it to support IPv6?

IPv6 is supported.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: LRO restructuring?

[James Huang]

Hi all,
 
    One question about inet_lro.c (linux 2.6.28.6)
In lro_proc_segment(), it does not check the number of available frags entries 
in lro_desc->parent against the number of new fragments to be added before 
calling lro_add_frags().  Is that a bug?  I see that lro_proc_segment() does 
some type of checking at the end, but could that be too late (i.e. overflow 
the frags[])??
 
    Also I think some obvious optimization in lro_get_desc() can be done by 
combining the two for loops into one.
 
-- James Huang

Re: LRO restructuring?

[Herbert Xu]

On Thu, Feb 19, 2009 at 02:17:11PM -0800, James Huang wrote:
> Hi all,
> 
>     One question about inet_lro.c (linux 2.6.28.6)
> In lro_proc_segment(), it does not check the number of available frags
> entries in lro_desc->parent against the number of new fragments to be added
> before calling lro_add_frags().  Is that a bug?  I see that
> lro_proc_segment() does some type of checking at the end, but could that be
> too late (i.e. overflow the frags[])??
> 
>     Also I think some obvious optimization in lro_get_desc() can be done by
> combining the two for loops into one.

That file is obsolete.  The GRO code that will replace it lives
in various places:

	net/core/dev.c
	net/core/skbuff.c
	net/ipv4/af_inet.c
	net/ipv6/af_inet6.c
	net/ipv4/tcp_ipv4.c
	net/ipv6/tcp_ipv6.c
	net/ipv4/tcp.c
	net/8021q/vlan_core.c

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt