Subject: Re: [patch net-next 1/7] ipv4: fib: Only flush FIB aliases belonging to currently flushed table

From: "Duyck, Alexander H" <[email-masked]>
Date: Thu, 9 Feb 2017 19:00:51 +0000

On Thu, 2017-02-09 at 10:28 +0100, Jiri Pirko wrote:
> From: Ido Schimmel <[email-masked]>
> 
> In case the MAIN table is flushed and its trie is shared with the LOCAL
> table, then we might be flushing FIB aliases belonging to the latter.
> This can lead to FIB_ENTRY_DEL notifications sent with the wrong table
> ID.
> 
> The above doesn't affect current listeners, as the table ID is ignored
> during entry deletion, but this will change later in the patchset.
> 
> When flushing a particular table, skip any aliases belonging to a
> different one.
> 
> Signed-off-by: Ido Schimmel <[email-masked]>
> Signed-off-by: Jiri Pirko <[email-masked]>
> CC: Alexander Duyck <[email-masked]>
> CC: Patrick McHardy <[email-masked]>
> ---
>  net/ipv4/fib_trie.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> index 2919d1a..5ef4596 100644
> --- a/net/ipv4/fib_trie.c
> +++ b/net/ipv4/fib_trie.c
> @@ -1963,7 +1963,8 @@ int fib_table_flush(struct net *net, struct fib_table *tb)
>  		hlist_for_each_entry_safe(fa, tmp, &n->leaf, fa_list) {
>  			struct fib_info *fi = fa->fa_info;
>  
> -			if (!fi || !(fi->fib_flags & RTNH_F_DEAD)) {
> +			if (!fi || !(fi->fib_flags & RTNH_F_DEAD) ||
> +			    tb->tb_id != fa->tb_id) {
>  				slen = fa->fa_slen;
>  				continue;
>  			}

One change I might make if you end up having to do a v2 would be to
test for the table ID first.  It can end up saving a few cycles in the
whole flushing process since the table ID is in the fib alias instead
of having to dereference the fib info.

That being said, I am just being a bit nit-picky so the code itself is
functionally correct and there is nothing here that should cause any
issues.

Reviewed-by: Alexander Duyck <[email-masked]>