netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Pirko <jiri@resnulli.us>
To: David Ahern <dsahern@gmail.com>
Cc: David Ahern <dsahern@kernel.org>,
	davem@davemloft.net, netdev@vger.kernel.org
Subject: Re: [PATCH net] netdevsim: Restore per-network namespace accounting for fib entries
Date: Thu, 29 Aug 2019 08:28:50 +0200	[thread overview]
Message-ID: <20190829062850.GG2312@nanopsycho> (raw)
In-Reply-To: <2c561928-1052-4c33-848d-ed7b81e920cf@gmail.com>

Wed, Aug 28, 2019 at 11:26:03PM CEST, dsahern@gmail.com wrote:
>On 8/28/19 4:37 AM, Jiri Pirko wrote:
>> Tue, Aug 06, 2019 at 09:15:17PM CEST, dsahern@kernel.org wrote:
>>> From: David Ahern <dsahern@gmail.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.
>> 
>> David, please help me understand. If the counters are per-device, not
>> per-netns, they are both the same. If we have device (devlink instance)
>> is in a netns and take only things happening in this netns into account,
>> it should count exactly the same amount of fib entries, doesn't it?
>
>if you are only changing where the counters are stored - net_generic vs
>devlink private - then yes, they should be equivalent.

Okay.

>
>> 
>> I re-thinked the devlink netns patchset and currently I'm going in
>> slightly different direction. I'm having netns as an attribute of
>> devlink reload. So all the port netdevices and everything gets
>> re-instantiated into new netns. Works fine with mlxsw. There we also
>> re-register the fib notifier.
>> 
>> I think that this can work for your usecase in netdevsim too:
>> 1) devlink instance is registering a fib notifier to track all fib
>>    entries in a namespace it belongs to. The counters are per-device -
>>    counting fib entries in a namespace the device is in.
>> 2) another devlink instance can do the same tracking in the same
>>    namespace. No problem, it's a separate counter, but the numbers are
>>    the same. One can set different limits to different devlink
>>    instances, but you can have only one. That is the bahaviour you have
>>    now.
>> 3) on devlink reload, netdevsim re-instantiates ports and re-registers
>>    fib notifier
>> 4) on devlink reload with netns change, all should be fine as the
>>    re-registered fib nofitier replays the entries. The ports are
>>    re-instatiated in new netns.
>> 
>> This way, we would get consistent behaviour between netdevsim and real
>> devices (mlxsw), correct devlink-netns implementation (you also
>> suggested to move ports to the namespace). Everyone should be happy.
>> 
>> What do you think?
>> 
>
>Right now, registering the fib notifier walks all namespaces. That is
>not a scalable solution. Are you changing that to replay only a given
>netns? Are you changing the notifiers to be per-namespace?

Eventually, that seems like good idea. Currently I want to do
if (net==nsim_dev->mynet)
	done
check at the beginning of the notifier.


>
>Also, you are still allowing devlink instances to be created within a
>namespace?

Yes, netdevsim is planned to be created directly in namespace.

  reply	other threads:[~2019-08-29  6:28 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-06 19:15 [PATCH net] netdevsim: Restore per-network namespace accounting for fib entries David Ahern
2019-08-06 22:32 ` Jakub Kicinski
2019-08-07  6:27   ` Jiri Pirko
2019-08-07 12:39     ` David Ahern
2019-08-07 13:07       ` Jiri Pirko
2019-08-07 19:31         ` David Ahern
2019-08-12  4:02 ` David Miller
2019-08-12  8:36   ` Jiri Pirko
2019-08-12 15:28     ` David Miller
2019-08-13  7:14       ` Jiri Pirko
2019-08-13 14:41         ` David Ahern
2019-08-13 15:34           ` Jiri Pirko
2019-08-13 17:40         ` David Miller
2019-08-13 20:37           ` Jiri Pirko
2019-08-28 10:37 ` Jiri Pirko
2019-08-28 21:26   ` David Ahern
2019-08-29  6:28     ` Jiri Pirko [this message]
2019-08-29 12:54       ` David Ahern
2019-08-29 14:02         ` Jiri Pirko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190829062850.GG2312@nanopsycho \
    --to=jiri@resnulli.us \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=dsahern@kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).