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.
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);
>
