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

From: Stephen Hemminger
Date: Sat Feb 23 2019 - 11:46:33 EST


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 ||