netdev - Re: [PATCH net 5/8] qlcnic: Fix reset recovery after transmit timeout

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: <20130508.120658.318501645917412400.davem@davemloft.net>
Date:	Wed, 08 May 2013 12:06:58 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	shahed.shaikh@...gic.com
Cc:	netdev@...r.kernel.org, Dept_NX_Linux_NIC_Driver@...gic.com,
	sony.chacko@...gic.com
Subject: Re: [PATCH net 5/8] qlcnic: Fix reset recovery after transmit
 timeout

From: Shahed Shaikh <shahed.shaikh@...gic.com>
Date: Tue, 7 May 2013 15:55:03 -0400

> @@ -435,10 +435,7 @@ static void qlcnic_83xx_idc_attach_driver(struct qlcnic_adapter *adapter)
>  	}
>  done:
>  	netif_device_attach(netdev);
> -	if (netif_running(netdev)) {
> -		netif_carrier_on(netdev);
> -		netif_wake_queue(netdev);
> -	}
> +	adapter->netdev->trans_start = jiffies;
>  }
>  
>  static int qlcnic_83xx_idc_enter_failed_state(struct qlcnic_adapter *adapter,

This is not right.

Multiqueue aware drivers should never access netdev->trans_start
directly, and I see several such writes in this driver.

Anything you write here will be totally ignored by the rest of the
kernel, because this value is overwritten by every call to dev_trans_start()
which is the only valid method by which to determine this value.

dev_trans_start() walks all of the transmit queues, recording the most
recent txq->trans_start timestamp in netdev->trans_start.

So if you write netdev->trans_start in your driver, it does nothing,
because even the dev_watchdog() time is going to inspect the per-queue
txq->trans_start values and ignore the netdev->trans_start value for
devices whose driver suppports multiqueue as qlcnic does.
--
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