netdev - RE: [net v2 2/3] i40e: Fix Rx checksum error counter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 7 Jan 2015 20:14:02 +0000
From:	"Singhai, Anjali" <anjali.singhai@...el.com>
To:	Tom Herbert <therbert@...gle.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:	David Miller <davem@...emloft.net>,
	Linux Netdev List <netdev@...r.kernel.org>,
	"nhorman@...hat.com" <nhorman@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>,
	"jogreene@...hat.com" <jogreene@...hat.com>,
	"Rose, Gregory V" <gregory.v.rose@...el.com>
Subject: RE: [net v2 2/3] i40e: Fix Rx checksum error counter

On Tue, 6 Jan 2015 21:43:57 -0800
Tom Herbert <therbert@...gle.com> wrote:
> > @@ -1337,15 +1335,19 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
> >                                           skb->protocol == htons(ETH_P_8021AD))
> >                                           ? VLAN_HLEN : 0;
> >
> > -               rx_udp_csum = udp_csum(skb);
> > -               iph = ip_hdr(skb);
> > -               csum = csum_tcpudp_magic(
> > -                               iph->saddr, iph->daddr,
> > -                               (skb->len - skb_transport_offset(skb)),
> > -                               IPPROTO_UDP, rx_udp_csum);
> > +               if ((ip_hdr(skb)->protocol == IPPROTO_UDP) &&
> > +                   (udp_hdr(skb)->check != 0)) {
> > +                       rx_udp_csum = udp_csum(skb);
> 
> Doesn't this compute the whole checksum of the packet making the fact 
> that device verified inner checksum pretty much irrelevant? It would 
> probably be just as well to return CHECKSUM_NONE and let the stack 
> deal with it and remove all this complexity.

This is only calculating outer UDP csum, inner csums are offloaded and so is the outer IP csum. Overall this is less work than asking the stack to do all of those by marking it as CHECKSUM_UNNECESSARY. We do have a patch in line to use csum_level but I believe even with that we would be asking the stack to do more work than necessary if we indicate that only inner checksums are offloaded. With our HW we are able to offload 3 out of 4 csums.