netdev - Re: [PATCH net-next v6 00/11] Modify action API for implementing lockless actions

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: <CAM_iQpUtybeq1T5hKgTRhZwaRJf1g-YBfR_+G3jfWoifi2+8CQ@mail.gmail.com>
Date:   Thu, 12 Jul 2018 20:54:45 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     David Miller <davem@...emloft.net>
Cc:     Vlad Buslov <vladbu@...lanox.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Yevgeny Kliteynik <kliteyn@...lanox.com>
Subject: Re: [PATCH net-next v6 00/11] Modify action API for implementing
 lockless actions

On Sat, Jul 7, 2018 at 8:43 PM David Miller <davem@...emloft.net> wrote:
>
> From: Vlad Buslov <vladbu@...lanox.com>
> Date: Thu,  5 Jul 2018 17:24:22 +0300
>
> > Currently, all netlink protocol handlers for updating rules, actions and
> > qdiscs are protected with single global rtnl lock which removes any
> > possibility for parallelism. This patch set is a first step to remove
> > rtnl lock dependency from TC rules update path.
>  ...
>
> I'll apply this for now, I reviewed it a few more times and I see
> where you are going with this.

Dear David,

I don't understand why you even believe the claim of lockless
updaters here, it at least should raise a red flag when you see any
kinda of this claim.

I know you don't trust me, how about thinking it in this way:

Why does RCU still require a lock for RCU writers? (Or at least
RCU recommends a lock, if anyone really wants to point out some
lockless algorithm here.)

or:

If writers could really go lockless as easily as Vlad claims, how could
even Paul E. McKenney never bring it into RCU?

Maybe Vlad is much cleverer than any of us here, and maybe he really
discovers a very brilliant algorithm to allow TC actions to be updated
locklessly, why not wait until he shows a proof (either code or a paper)?
Is there a rush? I don't see it.

In fact, I discussed this with Vlad a little bit at netdev TC workshop.
I never see any brilliant algorithm from him from his slides, and I was
told by him he used "copy and replace" to archive parallel updaters, I
told him that is basically how RCU works and RCU writers have to be
sync'ed with a lock (or at least recommended).

Also, to confirm my judgement, I checked this with Paul privately too.
Paul said you have to be extremely careful to go lockless, it is very hard
to be bug free for lockless, although he _never_ says it is impossible.

My _personal_ bet is that, lockless updates for TC filters or actions
are impossible unless there are more things hiding behind "copy and
replace", for example, some brilliant lockless algorithm. If lockless is
really impossible in this circumstance, then many of your efforts in
this patchset are vain, by the way.

I _do_ believe you can break RTNL down to per device, per filter or per
action, but no matter how small the locking scope is, there is still a lock.
With a lock, there is no need to make things friendly to lockless, like
making an integer increment inside an action to be atomic (your patch
02/11).

Please _do_ prove my personal judgement is wrong, by showing your
final code or a formal paper/article. I am very *happy* to be proved
to be wrong here, I am very open to change my mind here.

Vlad, we need your proof. Please prove I am wrong, seriously!!! :)

Thanks to anyone for proving me I am wrong just in case!!! :)