linux-kernel - Re: [PATCH 1/8] ns: Add common refcount into ns_common add use it as counter for net

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: <6f6f9989-9cde-f93a-ad0a-311f264398f0@virtuozzo.com>
Date:   Tue, 4 Aug 2020 17:36:52 +0300
From:   Kirill Tkhai <ktkhai@...tuozzo.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     christian.brauner@...ntu.com, akpm@...ux-foundation.org,
        viro@...iv.linux.org.uk, adobriyan@...il.com, davem@...emloft.net,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/8] ns: Add common refcount into ns_common add use it as
 counter for net_ns

On 04.08.2020 16:52, Eric W. Biederman wrote:
> Kirill Tkhai <ktkhai@...tuozzo.com> writes:
> 
>> On 04.08.2020 15:21, Eric W. Biederman wrote:
>>> Kirill Tkhai <ktkhai@...tuozzo.com> writes:
>>>
>>>> Currently, every type of namespaces has its own counter,
>>>> which is stored in ns-specific part. Say, @net has
>>>> struct net::count, @pid has struct pid_namespace::kref, etc.
>>>>
>>>> This patchset introduces unified counter for all types
>>>> of namespaces, and converts net namespace to use it first.
>>>
>>> And the other refcounts on struct net?
>>>
>>> How do they play into what you are trying to do?
>>
>> I just don't understand you. Different refcounters are related to different
>> problems, they are introduced to solve. This patchset changes only one refcounter,
>> and it does not touch other of them. What do you want to know about others?
>>
> 
> Why net::count not net::passive?  What problem are you trying to solve?

net::count is a counter, which indicates whether net is alive and still
initilized. net::passive does not guarantees that net has not been
deinitialized yet.

The same with another namespaces. All of merged counters indicate
that a namespace is alive and initialized. So, we merge them by this property.

> They both are reference counts on the network namespace.
> 
> I don't understand what you are trying to do, other than take a bunch of
> things that look similar and squash them all together.
> 
> What semantics does this magical common reference count have?
> Why is it better for the count to live in ns_common rather than it
> it's own dedicated field of struct net?

The only visible current effect is better readability and better
debugability with, say, crash on kernel dump.

> Given that decrementing still requires code per namespace to handle
> the count going to zero.  How does this benefit anyone?

Since namespaces are different by type, they require different destructors.
Generic counter can't solve any problem, the namespaces can address.

> Has the effect of cache line placement of the move of the reference
> count been considered?

I don't see any performance impact during namespace creation/destruction
test.

> All I see in the patch in question is switching a location that the
> count lives.  Which does not seem useful to me.
> 
> I am not fundamentally oppossed but I don't see the point.  At this
> point it looks like you are making things harder to maintain by making
> things common that are merely similar