Re: [PATCH hyperv-fixes] hv_netvsc: Fix IP header checksum for coalesced packets

[Stephen Hemminger]

On Fri, 22 Feb 2019 18:25:03 +0000
Haiyang Zhang <haiyangz@xxxxxxxxxxxxxxxxx> wrote:

> From: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> 
> Incoming packets may have IP header checksum verified by the host.
> They may not have IP header checksum computed after coalescing.
> This patch re-compute the checksum when necessary, otherwise the
> packets may be dropped, because Linux network stack always checks it.
> 
> Signed-off-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> ---
>  drivers/net/hyperv/netvsc_drv.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 256adbd044f5..cf4897043e83 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -744,6 +744,14 @@ void netvsc_linkstatus_callback(struct net_device *net,
>  	schedule_delayed_work(&ndev_ctx->dwork, 0);
>  }
>  
> +static void netvsc_comp_ipcsum(struct sk_buff *skb)
> +{
> +	struct iphdr *iph = (struct iphdr *)skb->data;

Can you use iphdr(skb) here?

> +
> +	iph->check = 0;
> +	iph->check = ip_fast_csum(iph, iph->ihl);
> +}
> +
>  static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
>  					     struct netvsc_channel *nvchan)
>  {
> @@ -770,9 +778,17 @@ static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
>  	/* skb is already created with CHECKSUM_NONE */
>  	skb_checksum_none_assert(skb);
>  
> -	/*
> -	 * In Linux, the IP checksum is always checked.
> -	 * Do L4 checksum offload if enabled and present.
> +	/* Incoming packets may have IP header checksum verified by the host.
> +	 * They may not have IP header checksum computed after coalescing.
> +	 * We compute it here if the flags are set, because on Linux, the IP
> +	 * checksum is always checked.
> +	 */
> +	if (csum_info && csum_info->receive.ip_checksum_value_invalid &&
> +	    csum_info->receive.ip_checksum_succeeded &&
> +	    skb->protocol == htons(ETH_P_IP))
> +		netvsc_comp_ipcsum(skb);

Does this still handle for coalesced and non-coalesced packets
which are received with bad IP checksum?  My concern is that you are
potentially correcting the checksum for a packet whose received checksum was bad.


> +	/* Do L4 checksum offload if enabled and present.
>  	 */
>  	if (csum_info && (net->features & NETIF_F_RXCSUM)) {
>  		if (csum_info->receive.tcp_checksum_succeeded ||