All of lore.kernel.org
 help / color / mirror / Atom feed
From: Timo Juhani Lindfors <timo.lindfors@iki.fi>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH] ip: reuse ip_summed of first fragment for all subsequent fragments
Date: Tue, 11 Jan 2011 15:49:19 +0200	[thread overview]
Message-ID: <84mxn7czz4.fsf@sauna.l.org> (raw)
In-Reply-To: <20101228.134905.183040321.davem@davemloft.net> (David Miller's message of "Tue\, 28 Dec 2010 13\:49\:05 -0800 \(PST\)")

David Miller <davem@davemloft.net> writes:
> > -			skb->ip_summed = CHECKSUM_NONE;
> > +			skb->ip_summed = skb_prev->ip_summed;
> You can't just assign skb_prev->ip_summed here, if it's CHECKSUM_PARTIAL
> then things will go completely wrong.  This is especially true since
> we're about to zero out skb->csum.

Thanks, I clearly didn't understand the semantics of ip_summed. For
received packets CHECKSUM_PARTIAL apparently means that the checksum
is calculated by hardware and CHECKSUM_NONE that it is calculated by
software or it is not calculated at all. I guess the ip_summed of a
new fragment is set to CHECKSUM_NONE because hardware can not handle
checksums of fragments?

Anyways, socket option SO_NO_CHECK sets sk->sk_no_check. Could this be
checked before calculating checksums of each fragment? Currently
udp_push_pending_frames checks this but checksums have already been
calculated at that point (and the only job left is to sum the
checksums together). Here's a patch that works for me (=according to
perf time is no longer spent calculating checksums) but probably
should be reviewed carefully:

>From 67fe193db5a2e1a2efaa0fa8e1c1c2554e108b78 Mon Sep 17 00:00:00 2001
From: Timo Juhani Lindfors <timo.lindfors@iki.fi>
Date: Tue, 11 Jan 2011 14:36:23 +0200
Subject: [PATCH] ip: make sure disabling checksums really works

---
 net/ipv4/ip_output.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 439d2a3..9bc163a 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -80,6 +80,7 @@
 #include <linux/mroute.h>
 #include <linux/netlink.h>
 #include <linux/tcp.h>
+#include <net/udp.h>

 int sysctl_ip_default_ttl __read_mostly = IPDEFTTL;

@@ -1208,7 +1209,7 @@ ssize_t   ip_append_page(struct sock *sk, struct page *page,
                        goto error;
                }

-               if (skb->ip_summed == CHECKSUM_NONE) {
+               if (skb->ip_summed == CHECKSUM_NONE && sk->sk_no_check != UDP_CSUM_NOXMIT) {
                        __wsum csum;
                        csum = csum_page(page, offset, len);
                        skb->csum = csum_block_add(skb->csum, csum, skb->len);
--
1.7.2.3


  reply	other threads:[~2011-01-11 14:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-14  7:51 [PATCH] ip: use checksum options of first fragment also for subsequent fragments Timo Juhani Lindfors
2010-12-17 19:58 ` David Miller
2010-12-20 10:38   ` [PATCH] ip: reuse ip_summed of first fragment for all " Timo Juhani Lindfors
2010-12-28 21:49     ` David Miller
2011-01-11 13:49       ` Timo Juhani Lindfors [this message]
2011-01-13  2:42         ` David Miller
2011-02-16  9:10           ` Timo Juhani Lindfors

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=84mxn7czz4.fsf@sauna.l.org \
    --to=timo.lindfors@iki.fi \
    --cc=davem@davemloft.net \
    --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.