netdev - Re: [patch net-next v2 1/3] net: devlink: allow to change namespaces

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: <05ec935f-2375-13fe-95b0-f80ca74ccca2@gmail.com>
Date:   Wed, 31 Jul 2019 14:20:18 -0600
From:   David Ahern <dsahern@...il.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     Jakub Kicinski <jakub.kicinski@...ronome.com>,
        netdev@...r.kernel.org, davem@...emloft.net,
        sthemmin@...rosoft.com, mlxsw@...lanox.com
Subject: Re: [patch net-next v2 1/3] net: devlink: allow to change namespaces

On 7/31/19 1:58 PM, David Ahern wrote:
> On 7/31/19 1:46 PM, David Ahern wrote:
>> On 7/31/19 1:45 PM, Jiri Pirko wrote:
>>>> check. e.g., what happens if a resource controller has been configured
>>>> for the devlink instance and it is moved to a namespace whose existing
>>>> config exceeds those limits?
>>>
>>> It's moved with all the values. The whole instance is moved.
>>>
>>
>> The values are moved, but the FIB in a namespace could already contain
>> more routes than the devlink instance allows.
>>
> 
> From a quick test your recent refactoring to netdevsim broke the
> resource controller. It was, and is intended to be, per network namespace.
> 

Specifically this commit:

commit 5fc494225c1eb81309cc4c91f183cd30e4edb674
Author: Jiri Pirko <jiri@...lanox.com>
Date:   Thu Apr 25 15:59:42 2019 +0200

    netdevsim: create devlink instance per netdevsim instance

    Currently there is one devlink instance created per network namespace.
    That is quite odd considering the fact that devlink instance should
    represent an ASIC. The following patches are going to move the devlink
    instance even more down to a bus device, but until then, have one
    devlink instance per netdevsim instance. Struct nsim_devlink is
    introduced to hold fib setting.

    The changes in the fib code are only related to holding the
    configuration per devlink instance instead of network namespace.

broke the intent of the resource controller clearly stated in the
original commit

commit 37923ed6b8cea94d7d76038e2f72c57a0b45daab
Author: David Ahern <dsa@...ulusnetworks.com>
Date:   Tue Mar 27 18:22:00 2018 -0700

    netdevsim: Add simple FIB resource controller via devlink

...
    Currently, devlink only supports initial namespace. Code is in place to
    adapt netdevsim to a per namespace controller once the network namespace
    issues are resolved.

And discussed here based on the RFC and v1 of the original patchset:

https://lore.kernel.org/netdev/03eade79-1727-3a31-8e31-a0a7f51b72cf@cumulusnetworks.com/

and

https://lore.kernel.org/netdev/a916f016-5d8b-3f56-0a84-95d1712bec4c@cumulusnetworks.com/

This is a regression that needs to be fixed.