netdev - Re: Broken TX checksumming offloads

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]

Message-ID: <1291058003.20703.16.camel@bwh-desktop>
Date:	Mon, 29 Nov 2010 19:13:23 +0000
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Michał Mirosław <mirq-linux@...e.qmqm.pl>
Cc:	netdev@...r.kernel.org,
	Toshiharu Okada <toshiharu-linux@....okisemi.com>,
	e1000-devel@...ts.sourceforge.net
Subject: Re: Broken TX checksumming offloads

On Mon, 2010-11-29 at 19:17 +0100, Michał Mirosław wrote:
> Hi!
> 
> Unless I'm horribly mistaken, generic HW checksumming works as follows:
>  - driver sets netdev->features & NETIF_F_HW_CSUM to indicate support
>    for generic checksumming; if the flag is not set, networking core
>    will checksum skb before calling ndo_start_xmit (let's ignore
>    other checksumming options for now) and not pass skb with
>    skb->ip_summed == CHECKSUM_PARTIAL
>  - ndo_start_xmit() should use skb->csum_start and skb->csum_offset
>    (or skb->csum) to update checksum in software or instruct HW to do so
> 
> Looking at pch_gbe_xmit_frame() and its callee - pch_gbe_tx_queue() it
> looks like the driver should set NETIF_F_IP_CSUM instead of NETIF_F_HW_CSUM
> feature.
>
> Similar thing happens in ixgbe driver: it sets NETIF_F_HW_CSUM and checks
> for skb->ip_summed == CHECKSUM_PARTIAL, but then just warns on protocols
> other that TCP and SCTP (see: ixgbe_psum()).

AFAIK only {TCP,UDP}/IPv{4,6} use the simple 16-bit checksum algorithm
that NETIF_F_HW_CSUM implies, so in practice it is equivalent to
NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM even though it doesn't mean the same
thing.

Older kernel versions lacked a definititon of NETIF_F_IPV6_CSUM so
out-of-tree drivers and in-tree drivers that started out-of-tree are
likely to use NETIF_F_HW_CSUM as a workaround for that.

Since the minimum size of a {TCP,UDP}/IPv6/Ethernet frame is 62 bytes +
CRC, the workaround in pch_gbe_tx_queue() may not be needed for IPv6.
(I'm assuming that the critical length of 64 bytes actually includes the
CRC, although the workaround code currently assumes otherwise.)

> This means that these drivers might send packets with broken checksums
> when TX checksumming offload is enabled. I haven't checked other drivers, yet.

They might or they might not; it's hard to tell without access to the
hardware.  But if the driver never references csum_{start,offset} then
it is probably valid to replace NETIF_F_HW_CSUM with NETIF_F_IP_CSUM |
NETIF_F_IPV6_CSUM (and similarly for the ethtool operations).

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html