netdev - Re: [net PATCH 1/2] ipv4: Restore fib_trie_flush_external function and fix call ordering

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: <20161115195107.GB1783@nanopsycho.orion>
Date:   Tue, 15 Nov 2016 20:51:07 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Alexander Duyck <alexander.h.duyck@...el.com>
Cc:     netdev@...r.kernel.org, Eric Dumazet <edumazet@...gle.com>,
        Jiri Pirko <jiri@...lanox.com>, davem@...emloft.net
Subject: Re: [net PATCH 1/2] ipv4: Restore fib_trie_flush_external function
 and fix call ordering

Tue, Nov 15, 2016 at 11:46:06AM CET, alexander.h.duyck@...el.com wrote:
>The patch that removed the FIB offload infrastructure was a bit too
>aggressive and also removed code needed to clean up us splitting the table
>if additional rules were added.  Specifically the function
>fib_trie_flush_external was called at the end of a new rule being added to
>flush the foreign trie entries from the main trie.
>
>I updated the code so that we only call fib_trie_flush_external on the main
>table so that we flush the entries for local from main.  This way we don't
>call it for every rule change which is what was happening previously.

Well, the function was introduced by:

commit 104616e74e0b464d449fdd2ee2f547d2fad71610
Author: Scott Feldman <sfeldma@...il.com>
Date:   Thu Mar 5 21:21:16 2015 -0800

    switchdev: don't support custom ip rules, for now
    
    Keep switchdev FIB offload model simple for now and don't allow custom ip
    rules.

Why this was not needed before? What changed in between:
104616e74e0b464d449fdd2ee2f547d2fad71610 ("switchdev: don't support custom ip rules, for now")
and
347e3b28c1ba2 ("switchdev: remove FIB offload infrastructure")


?


>
>Fixes: 347e3b28c1ba2 ("switchdev: remove FIB offload infrastructure")
>Reported-by: Eric Dumazet <edumazet@...gle.com>
>Cc: Jiri Pirko <jiri@...lanox.com>
>Signed-off-by: Alexander Duyck <alexander.h.duyck@...el.com>
>---
> include/net/ip_fib.h    |    1 +
> net/ipv4/fib_frontend.c |   20 +++++++++++---
> net/ipv4/fib_trie.c     |   65 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 81 insertions(+), 5 deletions(-)
>
>diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
>index b9314b4..f390c3b 100644
>--- a/include/net/ip_fib.h
>+++ b/include/net/ip_fib.h
>@@ -243,6 +243,7 @@ int fib_table_dump(struct fib_table *table, struct sk_buff *skb,
> 		   struct netlink_callback *cb);
> int fib_table_flush(struct net *net, struct fib_table *table);
> struct fib_table *fib_trie_unmerge(struct fib_table *main_tb);
>+void fib_table_flush_external(struct fib_table *table);
> void fib_free_table(struct fib_table *tb);
> 
> #ifndef CONFIG_IP_MULTIPLE_TABLES
>diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
>index c3b8047..161fc0f 100644
>--- a/net/ipv4/fib_frontend.c
>+++ b/net/ipv4/fib_frontend.c
>@@ -151,7 +151,7 @@ static void fib_replace_table(struct net *net, struct fib_table *old,
> 
> int fib_unmerge(struct net *net)
> {
>-	struct fib_table *old, *new;
>+	struct fib_table *old, *new, *main_table;
> 
> 	/* attempt to fetch local table if it has been allocated */
> 	old = fib_get_table(net, RT_TABLE_LOCAL);
>@@ -162,11 +162,21 @@ int fib_unmerge(struct net *net)
> 	if (!new)
> 		return -ENOMEM;
> 
>+	/* table is already unmerged */
>+	if (new == old)
>+		return 0;
>+
> 	/* replace merged table with clean table */
>-	if (new != old) {
>-		fib_replace_table(net, old, new);
>-		fib_free_table(old);
>-	}
>+	fib_replace_table(net, old, new);
>+	fib_free_table(old);
>+
>+	/* attempt to fetch main table if it has been allocated */
>+	main_table = fib_get_table(net, RT_TABLE_MAIN);
>+	if (!main_table)
>+		return 0;
>+
>+	/* flush local entries from main table */
>+	fib_table_flush_external(main_table);
> 
> 	return 0;
> }
>diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
>index 4cff74d..735edc9 100644
>--- a/net/ipv4/fib_trie.c
>+++ b/net/ipv4/fib_trie.c
>@@ -1760,6 +1760,71 @@ struct fib_table *fib_trie_unmerge(struct fib_table *oldtb)
> 	return NULL;
> }
> 
>+/* Caller must hold RTNL */
>+void fib_table_flush_external(struct fib_table *tb)
>+{
>+	struct trie *t = (struct trie *)tb->tb_data;
>+	struct key_vector *pn = t->kv;
>+	unsigned long cindex = 1;
>+	struct hlist_node *tmp;
>+	struct fib_alias *fa;
>+
>+	/* walk trie in reverse order */
>+	for (;;) {
>+		unsigned char slen = 0;
>+		struct key_vector *n;
>+
>+		if (!(cindex--)) {
>+			t_key pkey = pn->key;
>+
>+			/* cannot resize the trie vector */
>+			if (IS_TRIE(pn))
>+				break;
>+
>+			/* resize completed node */
>+			pn = resize(t, pn);
>+			cindex = get_index(pkey, pn);
>+
>+			continue;
>+		}
>+
>+		/* grab the next available node */
>+		n = get_child(pn, cindex);
>+		if (!n)
>+			continue;
>+
>+		if (IS_TNODE(n)) {
>+			/* record pn and cindex for leaf walking */
>+			pn = n;
>+			cindex = 1ul << n->bits;
>+
>+			continue;
>+		}
>+
>+		hlist_for_each_entry_safe(fa, tmp, &n->leaf, fa_list) {
>+			/* if alias was cloned to local then we just
>+			 * need to remove the local copy from main
>+			 */
>+			if (tb->tb_id != fa->tb_id) {
>+				hlist_del_rcu(&fa->fa_list);
>+				alias_free_mem_rcu(fa);
>+				continue;
>+			}
>+
>+			/* record local slen */
>+			slen = fa->fa_slen;
>+		}
>+
>+		/* update leaf slen */
>+		n->slen = slen;
>+
>+		if (hlist_empty(&n->leaf)) {
>+			put_child_root(pn, n->key, NULL);
>+			node_free(n);
>+		}
>+	}
>+}
>+
> /* Caller must hold RTNL. */
> int fib_table_flush(struct net *net, struct fib_table *tb)
> {
>