netdev - Re: [RFC PATCH net-next 02/15] ipv4: Notify route after insertion to the routing table

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <576d658d-6aab-558c-0a20-13133217d3b6@gmail.com>
Date:   Wed, 2 Oct 2019 19:34:35 -0600
From:   David Ahern <dsahern@...il.com>
To:     Ido Schimmel <idosch@...sch.org>, netdev@...r.kernel.org
Cc:     davem@...emloft.net, jiri@...lanox.com,
        jakub.kicinski@...ronome.com, saeedm@...lanox.com,
        mlxsw@...lanox.com, Ido Schimmel <idosch@...lanox.com>
Subject: Re: [RFC PATCH net-next 02/15] ipv4: Notify route after insertion to
 the routing table

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:
>