netdev - Re: [PATCH net 2/7] qlcnic: Fix bug in tuning offloads using QLogic application

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: <1367009665.4035.3.camel@bwh-desktop.uk.solarflarecom.com>
Date:	Fri, 26 Apr 2013 21:54:25 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Shahed Shaikh <shahed.shaikh@...gic.com>
CC:	<davem@...emloft.net>, <netdev@...r.kernel.org>,
	<Dept_NX_Linux_NIC_Driver@...gic.com>,
	Manish Chopra <manish.chopra@...gic.com>
Subject: Re: [PATCH net 2/7] qlcnic: Fix bug in tuning offloads using QLogic
 application

On Fri, 2013-04-26 at 08:09 -0400, Shahed Shaikh wrote:
> From: Manish Chopra <manish.chopra@...gic.com>
> 
> o LRO was not getting disabled when disabling checksum offloads
>   using QLogic application.
> o LRO was not getting re-enabled while enabling checksum offloads
>   back
[...]
> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> @@ -975,7 +975,7 @@ qlcnic_set_netdev_features(struct qlcnic_adapter *adapter,
>  		struct qlcnic_esw_func_cfg *esw_cfg)
>  {
>  	struct net_device *netdev = adapter->netdev;
> -	unsigned long features, vlan_features;
> +	netdev_features_t features, vlan_features;
>  
>  	if (qlcnic_83xx_check(adapter))
>  		return;
> @@ -994,8 +994,14 @@ qlcnic_set_netdev_features(struct qlcnic_adapter *adapter,
>  		features |= NETIF_F_LRO;
>  
>  	if (esw_cfg->offload_flags & BIT_0) {
> -		netdev->features |= features;
>  		adapter->rx_csum = 1;
> +		if (adapter->flags & QLCNIC_LRO_WAS_ENABLED) {
> +			if (qlcnic_config_hw_lro(adapter, QLCNIC_LRO_ENABLED))
> +				return;
> +			netdev_info(netdev, "Enabling LRO as Rx checksum is on\n");
> +			netdev->features |= NETIF_F_LRO;
> +		}
[...]

This is absolutely wrong for mainline Linux.  The networking core keeps
track of whether the user wanted a feature enabled in
netdev->wanted_features.  You must not try to manage this yourself.

The proper thing to do is to implement ndo_fix_features and mask out
NETIF_F_LRO there whenever it depends on another feature that has been
disabled.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
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