Re: [patch net-next 03/15] net: fib_notifier: propagate possible error during fib notifier registration

From: Jiri Pirko <jiri@resnulli.us>
To: Ido Schimmel <idosch@idosch.org>
Cc: netdev@vger.kernel.org, davem@davemloft.net, idosch@mellanox.com,
	dsahern@gmail.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 03/15] net: fib_notifier: propagate possible error during fib notifier registration
Date: Sun, 15 Sep 2019 11:41:05 +0200	[thread overview]
Message-ID: <20190915094105.GC2286@nanopsycho.orion> (raw)
In-Reply-To: <20190915081746.GB11194@splinter>

Sun, Sep 15, 2019 at 10:17:46AM CEST, idosch@idosch.org wrote:
>On Sat, Sep 14, 2019 at 08:45:56AM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Unlike events for registered notifier, during the registration, the
>> errors that happened for the block being registered are not propagated
>> up to the caller. For fib rules, this is already present, but not for
>
>What do you mean by "already present" ? You added it below for rules as
>well...

Right, will fix.

>
>> fib entries. So make sure the error is propagated for those as well.
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>>  include/net/ip_fib.h    |  2 +-
>>  net/core/fib_notifier.c |  2 --
>>  net/core/fib_rules.c    | 11 ++++++++---
>>  net/ipv4/fib_notifier.c |  4 +---
>>  net/ipv4/fib_trie.c     | 31 ++++++++++++++++++++++---------
>>  net/ipv4/ipmr_base.c    | 22 +++++++++++++++-------
>>  net/ipv6/ip6_fib.c      | 36 ++++++++++++++++++++++++------------
>>  7 files changed, 71 insertions(+), 37 deletions(-)
>> 
>> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
>> index 4cec9ecaa95e..caae0fa610aa 100644
>> --- a/include/net/ip_fib.h
>> +++ b/include/net/ip_fib.h
>> @@ -229,7 +229,7 @@ int __net_init fib4_notifier_init(struct net *net);
>>  void __net_exit fib4_notifier_exit(struct net *net);
>>  
>>  void fib_info_notify_update(struct net *net, struct nl_info *info);
>> -void fib_notify(struct net *net, struct notifier_block *nb);
>> +int fib_notify(struct net *net, struct notifier_block *nb);
>>  
>>  struct fib_table {
>>  	struct hlist_node	tb_hlist;
>> diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c
>> index b965f3c0ec9a..fbd029425638 100644
>> --- a/net/core/fib_notifier.c
>> +++ b/net/core/fib_notifier.c
>> @@ -65,8 +65,6 @@ static int fib_net_dump(struct net *net, struct notifier_block *nb)
>>  
>>  	rcu_read_lock();
>>  	list_for_each_entry_rcu(ops, &fn_net->fib_notifier_ops, list) {
>> -		int err;
>
>Looks like this should have been removed in previous patch

Correct, will move.

>
>> -
>>  		if (!try_module_get(ops->owner))
>>  			continue;
>>  		err = ops->fib_dump(net, nb);
>> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
>> index 28cbf07102bc..592d8aef90e3 100644
>> --- a/net/core/fib_rules.c
>> +++ b/net/core/fib_rules.c
>> @@ -354,15 +354,20 @@ int fib_rules_dump(struct net *net, struct notifier_block *nb, int family)
>>  {
>>  	struct fib_rules_ops *ops;
>>  	struct fib_rule *rule;
>> +	int err = 0;
>>  
>>  	ops = lookup_rules_ops(net, family);
>>  	if (!ops)
>>  		return -EAFNOSUPPORT;
>> -	list_for_each_entry_rcu(rule, &ops->rules_list, list)
>> -		call_fib_rule_notifier(nb, FIB_EVENT_RULE_ADD, rule, family);
>> +	list_for_each_entry_rcu(rule, &ops->rules_list, list) {
>> +		err = call_fib_rule_notifier(nb, FIB_EVENT_RULE_ADD,
>> +					     rule, family);
>
>Here you add it for rules
>
>> +		if (err)
>> +			break;
>> +	}
>>  	rules_ops_put(ops);
>>  
>> -	return 0;
>> +	return err;
>>  }
>>  EXPORT_SYMBOL_GPL(fib_rules_dump);