All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tom Parkin <tparkin@katalix.com>
To: Ben Hutchings <bhutchings@solarflare.com>, davem@davemloft.net
Cc: netdev@vger.kernel.org, jchapman@katalix.com
Subject: Re: [PATCH] l2tp: avoid checksum offload for fragmented packets
Date: Tue, 11 Jun 2013 09:00:38 +0100	[thread overview]
Message-ID: <20130611080038.GA2496@raven> (raw)
In-Reply-To: <1370437077.4021.15.camel@deadeye.wl.decadent.org.uk>

[-- Attachment #1: Type: text/plain, Size: 2125 bytes --]

On Wed, Jun 05, 2013 at 01:57:57PM +0100, Ben Hutchings wrote:
> On Wed, 2013-06-05 at 10:41 +0100, Tom Parkin wrote:
> > On Mon, Jun 03, 2013 at 03:44:12PM +0100, Ben Hutchings wrote:
> > > On Mon, 2013-06-03 at 08:49 +0100, Tom Parkin wrote:
> > > > Hardware offload for UDP datagram checksum calculation doesn't work with
> > > > fragmented IP packets -- the device will note the fragmentation and leave the
> > > > UDP checksum well alone.
> > > > 
> > > > As such, if we expect the L2TP packet to be fragmented by the IP layer we need
> > > > to perform the UDP checksum ourselves in software (ref: net/ipv4/udp.c).
> > > >
> > > > This change modifies the L2TP xmit path to fallback to software checksum
> > > > calculation if the L2TP packet + IP header exceeds the tunnel device MTU.
> > > [...]
> > > 
> > > Surely this should be done in the IP stack when fragmenting, not in any
> > > particular client?
> > > 
> > 
> > Hmm, that's a good question.
> > 
> > I'm not sure it makes sense to push this down into the IP layer, though.  Since 
> > it's the UDP checksum we're calculating, it seems reasonable to handle it at 
> > the UDP layer (which is where L2TP sits when using UDP encapsulation).
> 
> TCP, UDP and similar checksums can be handled generically, e.g. if
> dev_hard_start_xmit() finds the device doesn't actually do checksum
> offload then it calls skb_checksum_help() to fill it in.  I was thinking
> that since the IP layer makes the decision to fragment then it should
> also be responsible for filling in the checksum before doing so.  Why
> should the transport layer protocol have to guess?

Fair point!

I suppose an argument can be made either way, so really it comes down to a
question of taste and a feel for how the net tree "should" handle
this.

Dave -- could you give me a steer?  Are you happy to keep this kind of
calculation in the transport layer, or should I look to push something
generic into the IP code?

Thanks,
Tom
-- 
Tom Parkin
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

  reply	other threads:[~2013-06-11  8:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-03  7:49 [PATCH] l2tp: avoid checksum offload for fragmented packets Tom Parkin
2013-06-03  7:49 ` Tom Parkin
2013-06-03 14:44   ` Ben Hutchings
2013-06-05  9:41     ` Tom Parkin
2013-06-05 12:57       ` Ben Hutchings
2013-06-11  8:00         ` Tom Parkin [this message]
2013-06-11  8:16           ` David Miller

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=20130611080038.GA2496@raven \
    --to=tparkin@katalix.com \
    --cc=bhutchings@solarflare.com \
    --cc=davem@davemloft.net \
    --cc=jchapman@katalix.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.