netdev - Re: [PATCH net] qede: Fix Tx timeout due to xmit

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] [day] [month] [year] [list]

Message-Id: <20160818.211337.397326041714337261.davem@davemloft.net>
Date:   Thu, 18 Aug 2016 21:13:37 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     Yuval.Mintz@...gic.com
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net] qede: Fix Tx timeout due to xmit_more

From: Yuval Mintz <Yuval.Mintz@...gic.com>
Date: Tue, 16 Aug 2016 18:40:18 +0300

> Driver uses netif_tx_queue_stopped() to make sure the xmit_more
> indication will be honored, but that only checks for DRV_XOFF.
> 
> At the same time, it's possible that during transmission the DQL will
> close the transmission queue with STACK_XOFF indication.
> In re-configuration flows, when the threshold is relatively low, it's
> possible that the device has no pending tranmissions, and during
> tranmission the driver would miss doorbelling the HW.
> Since there are no pending transmission, there will never be a Tx
> completion [and thus the DQL would not remove the STACK_XOFF indication],
> eventually causing the Tx queue to timeout.
> 
> While we're at it - also doorbell in case driver has to close the
> transmission queue on its own [although this one is less important -
> if the ring is full, we're bound to receive completion eventually,
> which means the doorbell would only be postponed and not indefinetly
> blocked].
> 
> Fixes: 312e06761c99 ("qede: Utilize xmit_more")
> Signed-off-by: Yuval Mintz <Yuval.Mintz@...gic.com>

Applied, thanks.

> Do notice that we have pending patch intended for 'net-next'
> ("qede: Add support for per-queue stats") that might cause a merge
> conflict with this one - although it should be trivial to resolve.

Ok.