netdev - Re: [PATCH net] netdevice: Prefer NETIF_F_HW

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: <20170420061312.GF13789@unicorn.suse.cz>
Date:   Thu, 20 Apr 2017 08:13:12 +0200
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Vladislav Yasevich <vyasevich@...il.com>
Cc:     netdev@...r.kernel.org, tom@...bertland.com,
        Vladislav Yasevich <vyasevic@...hat.com>
Subject: Re: [PATCH net] netdevice: Prefer NETIF_F_HW_CSUM when intersecting
 features

On Wed, Apr 19, 2017 at 09:12:23PM -0400, Vladislav Yasevich wrote:
> While hardware device use either NETIF_F_(IP|IPV6)_CSUM or
> NETIF_F_HW_CSUM, all of the software devices use HW_CSUM.
> This results in an interesting situation when the software
> device is configured on top of hw device using (IP|IPV6)_CSUM.
> In this situation, the user can't turn off checksum offloading
> features on the software device.
> 
> This patch resolves that by prefering the NETIF_F_HW_CSUM setting
> when computing a feature intersect.

The reasoning makes sense to me but perhaps we should rename the helper
then to make it obvious that it doesn't work like intersection anymore
(not even in the logical sense).

> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 97456b25..3d811c1 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4019,9 +4019,9 @@ static inline netdev_features_t netdev_intersect_features(netdev_features_t f1,
>  {
>  	if ((f1 ^ f2) & NETIF_F_HW_CSUM) {
>  		if (f1 & NETIF_F_HW_CSUM)
> -			f1 |= (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
> +			f2 |= NETIF_F_HW_CSUM;
>  		else
> -			f2 |= (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
> +			f1 |= NETIF_F_HW_CSUM;
>  	}
>  
>  	return f1 & f2;

I hope I didn't miss something but it seems we can avoid nested ifs now:

 	if ((f1 ^ f2) & NETIF_F_HW_CSUM) {
		f1 |= NETIF_F_HW_CSUM;
		f2 |= NETIF_F_HW_CSUM;
 	}

Michal Kubecek