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

From: David Ahern <dsahern@gmail.com>
To: Jiri Pirko <jiri@resnulli.us>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>,
	netdev@vger.kernel.org, davem@davemloft.net,
	sthemmin@microsoft.com, mlxsw@mellanox.com
Subject: Re: [patch net-next v2 1/3] net: devlink: allow to change namespaces
Date: Mon, 5 Aug 2019 08:10:39 -0600	[thread overview]
Message-ID: <796ba97c-9915-9a44-e933-4a7e22aaef2e@gmail.com> (raw)
In-Reply-To: <20190805055422.GA2349@nanopsycho.orion>

On 8/4/19 11:54 PM, Jiri Pirko wrote:
> There was implicit devlink instance creation per-namespace. No relation
> any actual device. It was wrong and misuse of devlink.
> 
> Now you have 1 devlink instance per 1 device as it should be. Also, you
> have fib resource control for this device, also as it is done for real
> devices, like mlxsw.
> 
> Could you please describe your usecase? Perhaps we can handle
> it differently.

I have described this before, multiple times.

It is documented in the commit log for the initial fib.c in netdevsim
(37923ed6b8cea94d7d76038e2f72c57a0b45daab) and
https://lore.kernel.org/netdev/20180328012200.15175-7-dsa@cumulusnetworks.com/

And this comment in the discussion thread:

https://lore.kernel.org/netdev/e9c59b0c-328e-d343-6e8d-d19f643d2e9d@cumulusnetworks.com/:
"The intention is to treat the kernel's tables *per namespace* as a
standalone entity that can be managed very similar to ASIC resources."

So, to state this again, the fib.c in the RFC version
https://lore.kernel.org/netdev/20180322225757.10377-8-dsa@cumulusnetworks.com/

targeted this:

   namespace 1 |  namespace 2  | ... | namespace N
               |               |     |
               |               |     |
   devlink 1   |    devlink 2  | ... |  devlink N

and each devlink instance has resource limits for the number of fib
rules and fib entries *for that namespace* only.

You objected to how the devlink instances per namespace was implemented,
so the non-RFC version limited the devlink instance and resource
controller to init_net only. Fine. I accepted that limitation until
someone had time to work on devlink instances per network namespace
which you are doing now. So, the above goal will be achievable but first
you need to fix the breakage you put into v5.2 and forward.

Your commit 5fc494225c1eb81309cc4c91f183cd30e4edb674 changed that from a
per-namepace accounting to all namespaces managed by a single devlink
instance in init_net - which is completely wrong.

Move the fib accounting back to per namespace as the original code
intended. If you now want the devlink instance to be namespace based
then it should be trivial for you to fix it and will work going forward.