netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Hemminger <shemminger@vyatta.com>
To: Kieran Mansley <kmansley@solarflare.com>
Cc: netdev@vger.kernel.org
Subject: Re: LRO/GSO interaction when packets are forwarded
Date: Fri, 7 Mar 2008 08:25:38 -0800	[thread overview]
Message-ID: <20080307082538.1a674ae1@extreme> (raw)
In-Reply-To: <1204898997.4220.41.camel@moonstone.uk.level5networks.com>

On Fri, 07 Mar 2008 14:09:57 +0000
Kieran Mansley <kmansley@solarflare.com> wrote:

> We've seen a couple of problems when using a bridge or IP forwarding
> combined with LRO packets generated by a network device driver.  As you
> know, LRO packets can be either be page based (and passed up with
> lro_receive_page()) or use the skb frag_list (and passed up with
> lro_receive_skb()).  In both cases it is likely that the device driver
> will have set CHECKSUM_UNNECESSARY to indicate that the packet has been
> checksummed by the device, and gso_size to mark it as an LRO packet and
> indicate the actual received MSS.

First off, no hardware should ever do LRO on non-local packets. If the
hardware isn't smart enough to do this, I guess the bridge code to have
an API to turn it off. IP should also turn it off if ip_forwarding
is enabled on that device.


> If this skb goes directly to the network stack everything is fine.  The
> problem comes when this packet instead goes into a bridge and is then
> retransmitted on another device.  The skb seems to pass through the
> bridge relatively unmodified and because it has gso_size set the
> transmit path will attempt to segment it.  If page-based allocation has
> been used, this is fine, but if the skb frag_list has been used the
> transmit path BUGs in skb_gso_segment():

You can't do LRO with bridging, it is that simple, it is a protocol
layering violation.

 
> http://lxr.linux.no/linux+v2.6.24.3/net/core/dev.c#L1410
> 
> Secondly, the same function hopes that a GSO packet will have
> CHECKSUM_PARTIAL set - if this packet had originated from a stack rather
> than from an LRO device this would be the case - but instead it will
> most likely have CHECKSUM_UNNECESSARY.
> 
> Both of these problems are essentially being caused by gso_size and the
> ip_summed field have slightly different meanings on the receive and
> transmit paths, and the bridge/IP forwarding stuff not translating from
> one to the other.  To be fair to the bridge, it would not be obvious to
> it that it will be passing the packet to a real device (that will invoke
> the transmit path) or to a stack.
> 
> This leads me to my questions:
> 
>  - any idea why other drivers aren't hitting this problem?  One
> possibility is that they're using lro_receive_page rather then
> lro_receive_skb, but I'd still expect to see the CHECKSUM_PARTIAL
> warning.  I'm wondering if having LRO and forwarding between devices is
> a relatively rare thing, and so it just hasn't been tested.


>  - any suggestion as to the best place to try and fix this up?  My
> preference is making the transmit path cope with a packet that has the
> frag_list in use.  Making it cope with CHECKSUM_UNNECESSARY should also
> be possible but to be honest I'm finding skb_gso_segment's handling of
> CHECKSUM_PARTIAL a bit hard to follow.  The alternative would be I
> suppose to get the bridge and IP forwarding code to fix the socket
> buffer up before transmitting it, or for the driver to somehow know that
> it this packet will be forwarded and so it shouldn't use LRO.

In br_add_if, it should have a way to tell the device to turn LRO off.
dev_change_flags?

  reply	other threads:[~2008-03-07 16:25 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-07 14:09 LRO/GSO interaction when packets are forwarded Kieran Mansley
2008-03-07 16:25 ` Stephen Hemminger [this message]
2008-03-07 17:06   ` Kieran Mansley
2008-03-07 21:43     ` [PATCH] ethtool: command line support for lro Stephen Hemminger
2008-03-10 18:07       ` Ben Hutchings
2008-03-10 18:29         ` Stephen Hemminger
2008-03-10 18:50           ` Ben Hutchings
2008-04-17 12:11         ` Ben Hutchings
2008-04-30 18:36           ` Kok, Auke
2008-05-02 14:34             ` Ben Hutchings
2008-09-14  2:09           ` Jeff Garzik
2008-03-11 16:50     ` LRO/GSO interaction when packets are forwarded Kieran Mansley
2008-04-22 21:15     ` Ben Hutchings
2008-04-22 23:01       ` Stephen Hemminger
2008-04-23  6:00         ` Jarek Poplawski
2008-04-23  6:15           ` Jarek Poplawski
2008-04-23 10:07             ` Ben Hutchings
2008-04-23 10:38               ` Jarek Poplawski
2008-04-23 10:42                 ` David Miller
2008-04-23 11:09                   ` Jarek Poplawski
2008-04-23 10:04         ` Ben Hutchings

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080307082538.1a674ae1@extreme \
    --to=shemminger@vyatta.com \
    --cc=kmansley@solarflare.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).