netdev - Re: packet stuck in qdisc : patch proposal

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, 24 May 2022 10:13:53 +0200 (CEST)
From:   Vincent Ray <vray@...rayinc.com>
To:     linyunsheng <linyunsheng@...wei.com>
Cc:     davem <davem@...emloft.net>,
        方国炬 <guoju.fgj@...baba-inc.com>,
        kuba <kuba@...nel.org>, netdev <netdev@...r.kernel.org>,
        Samuel Jones <sjones@...rayinc.com>,
        vladimir oltean <vladimir.oltean@....com>,
        Guoju Fang <gjfang@...ux.alibaba.com>,
        Remy Gauguey <rgauguey@...rayinc.com>,
        Eric Dumazet <eric.dumazet@...il.com>, will@...nel.org
Subject: Re: packet stuck in qdisc : patch proposal

 
> Hi, thanks for the testing and debugging. So the main problem is that
> test_bit() is not an atomic operation, so using smp_mb __*_atomic() is
> not really helping, right?

Yes

> In that case we might only need to change smp_mb__before_atomic() to
> smp_rmb() in qdisc_run_begin(), as we only need to order test_bit()
> after set_bit() and clear_bit(), which is a read/write ordering?

I don't think so : we need to order test_bit() with respect to an earlier enqueue().
So we need a store/load barrier => smp_mb()

> By the way,  Guoju need to ensure ordering between spin_unlock() and
> test_bit() in qdisc_run_end(), which is a write/read ordering, so
> smp_mb() is used.

Agreed

But, for qdisc_run_begin(), I think Eric is right anyway, his fix, using test_and_set_bit() seems much better.
I'm going to try it and will tell you what it gives.

Thanks,

V