Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 23 Feb 2019 08:46:24 -0800
From:   Stephen Hemminger <stephen@networkplumber.org>
To:     Haiyang Zhang <haiyangz@linuxonhyperv.com>
Cc:     haiyangz@microsoft.com, sashal@kernel.org,
        linux-hyperv@vger.kernel.org, kys@microsoft.com,
        sthemmin@microsoft.com, olaf@aepfle.de, vkuznets@redhat.com,
        davem@davemloft.net, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH hyperv-fixes] hv_netvsc: Fix IP header checksum for
 coalesced packets
Message-ID: <20190223084624.0d0c60a0@shemminger-XPS-13-9360>
In-Reply-To: <20190222182503.12160-1-haiyangz@linuxonhyperv.com>
References: <20190222182503.12160-1-haiyangz@linuxonhyperv.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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

> From: Haiyang Zhang <haiyangz@microsoft.com>
> 
> 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@microsoft.com>
> ---
>  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 ||