Re: [Xen-devel] [PATCH] xen-netfront: reset skb transport header before checksum

[Eric Dumazet <eric.dumazet@xxxxxxxxx>]

On Mon, 2015-05-11 at 18:34 -0700, Venkat Venkatsubra wrote:
> In ed1f50c3a ("net: add skb_checksum_setup") some checksum functions
> were introduced in core. Subsequent change b5cf66cd1 ("xen-netfront:
> use new skb_checksum_setup function") made use of those functions to
> replace its own implementation. With that change ip_hdr() and tcp_hdr()
> were not pointing at the right place. d554f73df ("xen-netfront: reset
> skb network header before checksum") fixed the ip_hdr().
> 
> This patch fixes tcp_hdr().
> 
> We saw this problem when LRO was enabled on the host and it results in
> netfront setting skb->ip_summed to CHECKSUM_UNNECESSARY.
> 
> Signed-off-by: Venkat Venkatsubra <venkat.x.venkatsubra@xxxxxxxxxx>
> ---
>  drivers/net/xen-netfront.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 3f45afd..ed5c242 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -889,6 +889,7 @@ static int handle_incoming_queue(struct netfront_queue 
> *queue,
>               /* Ethernet work: Delayed to here as it peeks the header. */
>               skb->protocol = eth_type_trans(skb, queue->info->netdev);
>               skb_reset_network_header(skb);
> +             skb_set_transport_header(skb, ip_hdrlen(skb));
>  
>               if (checksum_setup(queue->info->netdev, skb)) {
>                       kfree_skb(skb);

This looks bogus for IPv6.

I would rather fix skb_checksum_setup_ip() and
skb_checksum_setup_ipv6(), since you already use correctly
skb_maybe_pull_tail() there.

In general, setting network and transport header at this point seems
extra work.




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