netdev - Re: [PATCH net] netdevsim: Restore per-network namespace accounting for fib entries

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190807130739.GA2201@nanopsycho>
Date:   Wed, 7 Aug 2019 15:07:39 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     David Ahern <dsahern@...il.com>
Cc:     Jakub Kicinski <jakub.kicinski@...ronome.com>,
        David Ahern <dsahern@...nel.org>, davem@...emloft.net,
        netdev@...r.kernel.org
Subject: Re: [PATCH net] netdevsim: Restore per-network namespace accounting
 for fib entries

Wed, Aug 07, 2019 at 02:39:56PM CEST, dsahern@...il.com wrote:
>On 8/7/19 12:27 AM, Jiri Pirko wrote:
>> Wed, Aug 07, 2019 at 12:32:14AM CEST, jakub.kicinski@...ronome.com wrote:
>>> On Tue,  6 Aug 2019 12:15:17 -0700, David Ahern wrote:
>>>> From: David Ahern <dsahern@...il.com>
>>>>
>>>> Prior to the commit in the fixes tag, the resource controller in netdevsim
>>>> tracked fib entries and rules per network namespace. Restore that behavior.
>>>>
>>>> Fixes: 5fc494225c1e ("netdevsim: create devlink instance per netdevsim instance")
>>>> Signed-off-by: David Ahern <dsahern@...il.com>
>>>
>>> Thanks.
>>>
>>> Let's see what Jiri says, but to me this patch seems to indeed restore
>>> the original per-namespace accounting when the more natural way forward
>>> may perhaps be to make nsim only count the fib entries where
>> 
>> I think that:
>> 1) netdevsim is a glorified dummy device for testing kernel api, not for
>>    configuring per-namespace resource limitation.
>> 2) If the conclusion os to use devlink instead of cgroups for resourse
>>    limitations, it should be done in a separate code, not in netdevsim.
>> 
>> I would definitelly want to wait what is the result of discussion around 2)
>> first. But one way or another netdevsim code should not do this, I would
>> like to cutout the fib limitations from it instead, just to expose the
>> setup resource limits through debugfs like the rest of the configuration
>> of netdevsim.
>> 
>> 
>
>This is the most incredulous response. You break code you don't
>understand and argue that it should be left broken in older releases and
>ripped out in later ones rather than fixed back to its intent.

Yeah. I believe it was a mistake to add it in the first place. Abuses
netdevsim for something it is not. I'm fine to use devlink the way you
want to after we conclude 2), but outside netdevsim.

Again, netdevsim is there for config api testing purposes. If things
got broken, it is not that bit deal. I broke the way it is
instantiated significantly for example (iplink->sysfs).


>
>Again, the devlink resource controller was added by me specifically to
>test the ability to fail in-kernel fib notifiers. When I add the nexthop
>in-kernel notifier, netdevsim again provides a simple means of testing it.
>
>It satisfies all of the conditions and intents of netdevsim - test
>kernel APIs without hardware.

Well, what you want to do is limit kernel sw resources per-namespace.