netdev - Re: [Patch net-next v2 3/4] net_sched: remove tc class reference counting

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: <20170825093645.GD2023@nanopsycho>
Date:   Fri, 25 Aug 2017 11:36:45 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Florian Westphal <fw@...len.de>
Cc:     Cong Wang <xiyou.wangcong@...il.com>, netdev@...r.kernel.org,
        jhs@...atatu.com
Subject: Re: [Patch net-next v2 3/4] net_sched: remove tc class reference
 counting

Fri, Aug 25, 2017 at 11:18:50AM CEST, fw@...len.de wrote:
>Jiri Pirko <jiri@...nulli.us> wrote:
>> Fri, Aug 25, 2017 at 01:51:29AM CEST, xiyou.wangcong@...il.com wrote:
>> >For TC classes, their ->get() and ->put() are always paired, and the
>> >reference counting is completely useless, because:
>> >
>> >1) For class modification and dumping paths, we already hold RTNL lock,
>> >   so all of these ->get(),->change(),->put() are atomic.
>> 
>> There is ongoing initiative by Florian to avoid taking RTNL for some
>> rtnetlink calls. I think that for dumping it could be done in tc as well.
>> Don't we need the refcnt then?
>
>Dumping is a problem at this time because several places depend on RTNL
>to ensure we get a consistent state, even "simple" functions like
>rtnl_fill_ifinfo, see e.g.  2907c35ff64708065e5a7fd54e8ded8263eb3074
>(net: hold rtnl again in dump callbacks).
>
>So for these places we already need some other way (e.g. seqlock)
>to ensure we don't put garbage in netlink skb.
>
>At this time I think that it is better if Congs patches go in
>(Unless there are other problems of course) as they simplify
>things quite a bit, and I am not sure that we need refcount.
>
>It might be enough to use rcu and detect when the class we just read
>from might have been in inconsistent state (so we can retry).
>
>Does that make sense to you?

It does. Thanks!