All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.