netdev - Re: [PATCH net-next 1/3] net: provide macros for commonly copied lockless queue stop/wake code

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]

Date:   Tue, 4 Apr 2023 15:36:55 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Herbert Xu <herbert@...dor.apana.org.au>
Cc:     Alexander Duyck <alexander.duyck@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>, davem@...emloft.net,
        netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com
Subject: Re: [PATCH net-next 1/3] net: provide macros for commonly copied
 lockless queue stop/wake code

On Tue, 4 Apr 2023 14:39:11 +0800 Herbert Xu wrote:
> Thanks for adding me to this thread as otherwise I would've surely
> missed it.
> 
> I see where the confusion is coming from.  The key is that we weren't
> trying to stop every single race, because not all of them are fatal.
> 
> In particular, we tolerate the race where a wake is done when it
> shouldn't be because the network stack copes with that by requeueing
> the skb onto the qdisc.
> 
> So it's a trade-off.  We could make our code water-tight, but then
> we would be incurring a penalty for every skb.  With our current
> approach, the penalty is only incurred in the unlikely event of a
> race which results in the unlucky skb being requeued.
> 
> The race that we do want to stop is a queue being stuck in a stopped
> state when it shouldn't because that indeed is fatal.
> 
> Going back to the proposed helpers, we only need one mb because
> that's all we need to fix the stuck/stopped queue race.

Thanks, I'm impressed you still remember the details :)

I'll leave it racy in the next version. Re-using the BQL barrier
is a bit more tricky on the xmit path than I thought. I'll just
document that false-positive wake ups are possible.