netdev - Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

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: <20120703222235.GA14926@electric-eye.fr.zoreil.com>
Date:	Wed, 4 Jul 2012 00:22:35 +0200
From:	Francois Romieu <romieu@...zoreil.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Jitendra Kalsaria <jitendra.kalsaria@...gic.com>,
	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Ron Mercer <ron.mercer@...gic.com>,
	Dept-NX Linux NIC Driver 
	<Dept_NX_Linux_NIC_Driver@...gic.com>
Subject: Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

Eric Dumazet <eric.dumazet@...il.com> :
> On Tue, 2012-07-03 at 12:38 -0700, Jitendra Kalsaria wrote:
> 
> > I think my patch description might have been misleading. We are not
> > fixing a logical problem but rather a statistics reporting problem.
> > Our transmit function is not getting called when queue is full but
> > when we stop the queue it increment tx_error statistic and one of our
> > customers is running a test that deliberately floods the queue causing
> > it to periodically be stopped. The customer has not reported logical
> > problem with the test were driver perform very well but they merely
> > pointed out that we were incorrectly reporting the queue full
> > condition as a tx_error.
> > 
> > This patch was intended to remove the line that increments the
> > tx_error statistic when the queue is correctly stopped.
> 
> I believe everybody kindly ask you to fix the driver logic instead
> of trying to hide to your customers the problems.

:o/

Jitendra was speaking about qlge_ethtool_ops.self_test(). It will need
fixing as well.

> In fact, you could just BUG() at this point, and maybe David will accept
> such a patch.

Mildly. It would turn qlge_ethtool_ops.self_test() into a system killer.

[...]
> testing atomic_read(&tx_ring->tx_count) at the beginning of qlge_send()
> is too late. NETDEV_TX_BUSY is deprecated.

Yes.

Returning NETDEV_TX_BUSY when dma mapping fails in ql_map_send isn't nice
either.

-- 
Ueimor
--
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