All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zoltan Kiss <zoltan.kiss@citrix.com>
To: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Ian Campbell <Ian.Campbell@citrix.com>,
	Paul Durrant <Paul.Durrant@citrix.com>
Subject: xen-netback: maybe_pull_tail questions
Date: Thu, 28 Nov 2013 16:39:45 +0000	[thread overview]
Message-ID: <529771D1.3060100@citrix.com> (raw)

Hi,

I've found some things in the newest checksum handling code which I 
don't fully get. Maybe it's my fault, or maybe it's a problem indeed.
More exactly, the header_size variable we pass to maybe_pull_tail() 
seems odd. In checksum_setup_ip() we start with:

	struct iphdr *iph = (void *)skb->data;
...
	off = sizeof(struct iphdr);

	header_size = skb->network_header + off + MAX_IPOPTLEN;
	maybe_pull_tail(skb, header_size);

	off = iph->ihl * 4;

First, why don't we set off to the real IP header length at the first 
place if we already know that?
Second, skb->network_header was just reset in xenvif_tx_submit() before 
we called this function, and it contains the size of the headroom (32 
bytes, if I'm correct, set by skb_reserve(skb, NET_SKB_PAD + 
NET_IP_ALIGN) in xenvif_tx_build_gops()). I think we need sizeof(struct 
ethhdr) here, or something like that, and no MAX_IPOPTLEN, as off should 
contain the right size already.
And this applies to other places where we set header_size based on 
skb->network_header.
I noticed this thing when I checked some ftrace outputs on 3.12, and 
I've found that maybe_pull_tail cause pulling despite in 
xenvif_tx_submit we already checked if the linear buffer need more data 
to reach PKT_PROT_LEN ( = 128). Here skb->network_header + off + 
MAX_IPOPTLEN should be 32 + 20 + 40 = 92, so we shouldn't do it. Or am I 
missing something?
As a workaround, I've commented out this maybe_pull_tail(), and works fine.

Regards,

Zoli

             reply	other threads:[~2013-11-28 16:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-28 16:39 Zoltan Kiss [this message]
2013-11-28 16:57 ` xen-netback: maybe_pull_tail questions Paul Durrant
2013-11-28 18:04   ` Zoltan Kiss
2013-11-29 12:54     ` Paul Durrant

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=529771D1.3060100@citrix.com \
    --to=zoltan.kiss@citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=Paul.Durrant@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.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.