[Xen-devel] xen-netback: maybe_pull_tail questions

[Zoltan Kiss <zoltan.kiss@xxxxxxxxxx>]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel