From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zoltan Kiss Subject: xen-netback: maybe_pull_tail questions Date: Thu, 28 Nov 2013 16:39:45 +0000 Message-ID: <529771D1.3060100@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Vm4dK-0008W8-4L for xen-devel@lists.xenproject.org; Thu, 28 Nov 2013 16:40:06 +0000 List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "xen-devel@lists.xenproject.org" , Wei Liu , Ian Campbell , Paul Durrant List-Id: xen-devel@lists.xenproject.org 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