netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Pirko <jiri@resnulli.us>
To: David Ahern <dsahern@gmail.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net, idosch@mellanox.com,
	jakub.kicinski@netronome.com, tariqt@mellanox.com,
	saeedm@mellanox.com, kuznet@ms2.inr.ac.ru,
	yoshfuji@linux-ipv6.org, shuah@kernel.org, mlxsw@mellanox.com
Subject: Re: [patch net-next 02/15] net: fib_notifier: make FIB notifier per-netns
Date: Mon, 16 Sep 2019 07:38:01 +0200	[thread overview]
Message-ID: <20190916053801.GG2286@nanopsycho.orion> (raw)
In-Reply-To: <87139e84-4310-6632-c5d5-64610d4cc56e@gmail.com>

Sun, Sep 15, 2019 at 10:05:47PM CEST, dsahern@gmail.com wrote:
>On 9/14/19 12:45 AM, Jiri Pirko wrote:
>>  #define FIB_DUMP_MAX_RETRIES 5
>> -int register_fib_notifier(struct notifier_block *nb,
>> +int register_fib_notifier(struct net *net, struct notifier_block *nb,
>>  			  void (*cb)(struct notifier_block *nb))
>>  {
>>  	int retries = 0;
>>  	int err;
>>  
>>  	do {
>> -		unsigned int fib_seq = fib_seq_sum();
>> -		struct net *net;
>> -
>> -		rcu_read_lock();
>> -		for_each_net_rcu(net) {
>> -			err = fib_net_dump(net, nb);
>> -			if (err)
>> -				goto err_fib_net_dump;
>> -		}
>> -		rcu_read_unlock();
>> -
>> -		if (fib_dump_is_consistent(nb, cb, fib_seq))
>> +		unsigned int fib_seq = fib_seq_sum(net);
>> +
>> +		err = fib_net_dump(net, nb);
>> +		if (err)
>> +			return err;
>> +
>> +		if (fib_dump_is_consistent(net, nb, cb, fib_seq))
>>  			return 0;
>>  	} while (++retries < FIB_DUMP_MAX_RETRIES);
>
>This is still more complicated than it needs to be. Why lump all
>fib_notifier_ops into 1 dump when they are separate databases with
>separate seq numbers? Just dump them 1 at a time and retry that 1
>database as needed.

Well I think that what you describe is out of scope of this patch. It is
another optimization of fib_notifier. The aim of this patchset is not
optimization of fib_notifier, but devlink netns change. This patchset is
just a dependency.

Can't we do optimization in another patchset? I already struggled to
keep this one within 15-patch limit.

Thanks

>
>ie., This:
>    list_for_each_entry_rcu(ops, &net->fib_notifier_ops, list) {
>should be in register_fib_notifier and not fib_net_dump.
>
>as it stands you are potentially replaying way more than is needed when
>a dump is inconsistent.
>
>
>>  
>>  	return -EBUSY;
>> -
>> -err_fib_net_dump:
>> -	rcu_read_unlock();
>> -	return err;
>>  }
>>  EXPORT_SYMBOL(register_fib_notifier);
>>  
>> -int unregister_fib_notifier(struct notifier_block *nb)
>> +int unregister_fib_notifier(struct net *net, struct notifier_block *nb)
>>  {
>> -	return atomic_notifier_chain_unregister(&fib_chain, nb);
>> +	struct fib_notifier_net *fn_net = net_generic(net, fib_notifier_net_id);
>> +
>> +	return atomic_notifier_chain_unregister(&fn_net->fib_chain, nb);
>>  }
>>  EXPORT_SYMBOL(unregister_fib_notifier);
>>  
>
>
>
>
>

  reply	other threads:[~2019-09-16  5:38 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-14  6:45 [patch net-next 00/15] devlink: allow devlink instances to change network namespace Jiri Pirko
2019-09-14  6:45 ` [patch net-next 01/15] netdevsim: change fib accounting and limitations to be per-device Jiri Pirko
2019-09-14  6:45 ` [patch net-next 02/15] net: fib_notifier: make FIB notifier per-netns Jiri Pirko
2019-09-15  8:06   ` Ido Schimmel
2019-09-15  9:37     ` Jiri Pirko
2019-09-15 20:05   ` David Ahern
2019-09-16  5:38     ` Jiri Pirko [this message]
2019-09-16 14:54       ` David Ahern
2019-09-14  6:45 ` [patch net-next 03/15] net: fib_notifier: propagate possible error during fib notifier registration Jiri Pirko
2019-09-15  8:17   ` Ido Schimmel
2019-09-15  9:41     ` Jiri Pirko
2019-09-14  6:45 ` [patch net-next 04/15] mlxsw: spectrum_router: Don't rely on missing extack to symbolize dump Jiri Pirko
2019-09-14  6:45 ` [patch net-next 05/15] net: fib_notifier: propagate extack down to the notifier block callback Jiri Pirko
2019-09-14  6:45 ` [patch net-next 06/15] net: devlink: export devlink net getter Jiri Pirko
2019-09-14  6:46 ` [patch net-next 07/15] mlxsw: spectrum: Take devlink net instead of init_net Jiri Pirko
2019-09-14  6:46 ` [patch net-next 08/15] mlxsw: Register port netdevices into net of core Jiri Pirko
2019-09-15  8:37   ` Ido Schimmel
2019-09-14  6:46 ` [patch net-next 09/15] mlxsw: Propagate extack down to register_fib_notifier() Jiri Pirko
2019-09-15  8:39   ` Ido Schimmel
2019-09-14  6:46 ` [patch net-next 10/15] netdevsim: add all ports in nsim_dev_create() and del them in destroy() Jiri Pirko
2019-09-14  6:46 ` [patch net-next 11/15] netdevsim: implement proper devlink reload Jiri Pirko
2019-09-14  6:46 ` [patch net-next 12/15] netdevsim: register port netdevices into net of device Jiri Pirko
2019-09-14  6:46 ` [patch net-next 13/15] netdevsim: take devlink net instead of init_net Jiri Pirko
2019-09-14  6:46 ` [patch net-next 14/15] net: devlink: allow to change namespaces during reload Jiri Pirko
2019-09-15  8:58   ` Ido Schimmel
2019-09-15  9:43     ` Jiri Pirko
2019-09-14  6:46 ` [patch net-next 15/15] selftests: netdevsim: add tests for devlink reload with resources Jiri Pirko
2019-09-14  6:57 ` [patch iproute2-next 1/2] devlink: introduce cmdline option to switch to a different namespace Jiri Pirko
2019-09-15 18:01   ` David Ahern
2019-09-14  6:57 ` [patch iproute2-next 2/2] devlink: extend reload command to add support for network namespace change Jiri Pirko
2019-09-15  7:16   ` Ido Schimmel
2019-09-15  9:44     ` Jiri Pirko
2019-09-16  7:01 ` [patch net-next 00/15] devlink: allow devlink instances to change network namespace David Miller
2019-09-16  7:09   ` 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=20190916053801.GG2286@nanopsycho.orion \
    --to=jiri@resnulli.us \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=idosch@mellanox.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=mlxsw@mellanox.com \
    --cc=netdev@vger.kernel.org \
    --cc=saeedm@mellanox.com \
    --cc=shuah@kernel.org \
    --cc=tariqt@mellanox.com \
    --cc=yoshfuji@linux-ipv6.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).