On 10/2/19 2:40 AM, Ido Schimmel wrote: > @@ -1269,14 +1269,19 @@ int fib_table_insert(struct net *net, struct > fib_table *tb, > new_fa->tb_id = tb->tb_id; > new_fa->fa_default = -1; > > - err = call_fib_entry_notifiers(net, event, key, plen, new_fa, extack); > + /* Insert new entry to the list. */ > + err = fib_insert_alias(t, tp, l, new_fa, fa, key); > if (err) > goto out_free_new_fa; > > - /* Insert new entry to the list. */ > - err = fib_insert_alias(t, tp, l, new_fa, fa, key); > + /* The alias was already inserted, so the node must exist. */ > + l = fib_find_node(t, &tp, key); > + if (WARN_ON_ONCE(!l)) > + goto out_free_new_fa;
Maybe I am missing something but, the 'l' is only needed for the error path, so optimize for the success case and only lookup the node if the notifier fails. > + > + err = call_fib_entry_notifiers(net, event, key, plen, new_fa, extack); > if (err) > - goto out_fib_notif; > + goto out_remove_new_fa; > > if (!plen) > tb->tb_num_default++; > @@ -1287,14 +1292,8 @@ int fib_table_insert(struct net *net, struct fib_table > *tb, > succeeded: > return 0; > > -out_fib_notif: > - /* notifier was sent that entry would be added to trie, but > - * the add failed and need to recover. Only failure for > - * fib_insert_alias is ENOMEM. > - */ > - NL_SET_ERR_MSG(extack, "Failed to insert route into trie"); > - call_fib_entry_notifiers(net, FIB_EVENT_ENTRY_DEL, key, > - plen, new_fa, NULL); > +out_remove_new_fa: > + fib_remove_alias(t, tp, l, new_fa); > out_free_new_fa: > kmem_cache_free(fn_alias_kmem, new_fa); > out: >