[PATCH net] xen-netfront: reset skb network header before checksum

[PATCH net] xen-netfront: reset skb network header before checksum

[Wei Liu]

In ed1f50c3a ("net: add skb_checksum_setup") we introduced some checksum
functions in core driver. Subsequent change b5cf66cd1 ("xen-netfront:
use new skb_checksum_setup function") made use of those functions to
replace its own implementation.

However with that change netfront is broken. It sees a lot of checksum
error. That's because its own implementation of checksum function was a
bit hacky (dereferencing skb->data directly) while the new function was
implemented using ip_hdr(). The network header is not reset before skb
is passed to the new function. When the new function tries to do its
job, it's confused and reports error.

The fix is simple, we need to reset network header before passing skb to
checksum function. Netback is not affected as it already does the right
thing.

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Paul Durrant <paul.durrant@citrix.com>
---
 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 ff04d4f..95041b6 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -907,6 +907,7 @@ static int handle_incoming_queue(struct net_device *dev,
 
 		/* Ethernet work: Delayed to here as it peeks the header. */
 		skb->protocol = eth_type_trans(skb, dev);
+		skb_reset_network_header(skb);
 
 		if (checksum_setup(dev, skb)) {
 			kfree_skb(skb);
-- 
1.7.10.4

Re: [PATCH net] xen-netfront: reset skb network header before checksum

[Sander Eikelenboom]

Wednesday, February 19, 2014, 7:48:34 PM, you wrote:

> In ed1f50c3a ("net: add skb_checksum_setup") we introduced some checksum
> functions in core driver. Subsequent change b5cf66cd1 ("xen-netfront:
> use new skb_checksum_setup function") made use of those functions to
> replace its own implementation.

> However with that change netfront is broken. It sees a lot of checksum
> error. That's because its own implementation of checksum function was a
> bit hacky (dereferencing skb->data directly) while the new function was
> implemented using ip_hdr(). The network header is not reset before skb
> is passed to the new function. When the new function tries to do its
> job, it's confused and reports error.

> The fix is simple, we need to reset network header before passing skb to
> checksum function. Netback is not affected as it already does the right
> thing.

> Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Paul Durrant <paul.durrant@citrix.com>
> ---
>  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 ff04d4f..95041b6 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -907,6 +907,7 @@ static int handle_incoming_queue(struct net_device *dev,
>  
>                 /* Ethernet work: Delayed to here as it peeks the header. */
>                 skb->protocol = eth_type_trans(skb, dev);
> +               skb_reset_network_header(skb);
>  
>                 if (checksum_setup(dev, skb)) {
>                         kfree_skb(skb);

Ah that also explains why ICMP was not affected :-)
Just tested and this patch indeed fixes my problem, so:

Tested-By: Sander Eikelenboom <linux@eikelenboom.it>

Thanks Wei !

--
Sander

Re: [PATCH net] xen-netfront: reset skb network header before checksum

[David Miller]

From: Wei Liu <wei.liu2@citrix.com>
Date: Wed, 19 Feb 2014 18:48:34 +0000

> In ed1f50c3a ("net: add skb_checksum_setup") we introduced some checksum
> functions in core driver. Subsequent change b5cf66cd1 ("xen-netfront:
> use new skb_checksum_setup function") made use of those functions to
> replace its own implementation.
> 
> However with that change netfront is broken. It sees a lot of checksum
> error. That's because its own implementation of checksum function was a
> bit hacky (dereferencing skb->data directly) while the new function was
> implemented using ip_hdr(). The network header is not reset before skb
> is passed to the new function. When the new function tries to do its
> job, it's confused and reports error.
> 
> The fix is simple, we need to reset network header before passing skb to
> checksum function. Netback is not affected as it already does the right
> thing.
> 
> Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Applied, thanks.