Sun, Sep 15, 2019 at 10:05:47PM CEST, dsah...@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);
>>
>
>
>
>
>
