[PATCH v1 net-next 00/10] net: fib_rules: RTNL-less RTM_NEWRULE and RTM_DELRULE.

[PATCH v1 net-next 00/10] net: fib_rules: RTNL-less RTM_NEWRULE and RTM_DELRULE.

[Kuniyuki Iwashima]

RTM_NEWRULE and RTM_DELRULE acquire rtnl_net_lock(), but this is
only for fib_unmerge() in IPv4.

Since commit d954a67a7dfa ("ipv4: fib_rule: Move fib4_rules_exit()
to ->exit()."), RTM_DELRULE no longer needs RTNL.

fib_unmerge() is one-time event for each netns, so we only need
RTNL for the first IPv4 rule.

This series introduces per-fib_rules_ops mutex and drops RTNL
from fib_rules code except for the first IPv4 RTM_NEWRULE.

The script below creates 1K rules in parallel in 4K netns, and
it got 20x/30x faster for IPv4/IPv6.

  #!/bin/bash
  N=4096
  F=rules.txt

  for i in $(seq $N); do ip netns add ns-$i; done
  printf 'rule add from all table %d\n' {1..1024} > $F

  for v in 4 6; do
        echo "=== IPv${v} ==="
        time { for i in $(seq $N); do nsenter \
        --net=/var/run/netns/ns-$i ip -$v -batch $F & done; wait; }
  done

  for i in $(seq $N); do ip netns del ns-$i; done
  rm -f $F

Without this series:

  # ./test.sh
  === IPv4 ===

  real  0m22.752s
  user  0m7.834s
  sys   92m46.721s
  === IPv6 ===

  real  0m35.181s
  user  0m8.635s
  sys   142m30.479s

With this series:

  # ./test.sh
  === IPv4 ===

  real  0m0.918s
  user  0m5.675s
  sys   2m7.024s
  === IPv6 ===

  real  0m1.214s
  user  0m7.917s
  sys   4m19.489s


Kuniyuki Iwashima (10):
  net: fib_rules: Make fib_rules_ops.delete() return void.
  ipv4: fib_rules: Make the need for fib_unmerge() explicit.
  ipv4: fib: Protect fib_new_table() with spinlock.
  ipv4: fib: Drop RTNL annotation for net->ipv4.fib_table_hash[].
  net: fib_rules: Add fib_rules_ops.lock.
  net: fib_rules: Remove unnecessary EXPORT_SYMBOL.
  net: fib_rules: Drop RTNL assertions.
  net: fib_rules: Use dev_get_by_name_rcu().
  net: fib_rules: Only hold RTNL for the first IPv4 RTM_NEWRULE.
  ipv6: fib_rules: Convert fib6_rules_net_exit_rtnl() to ->exit().

 include/net/fib_rules.h  |  4 +-
 include/net/ip_fib.h     |  3 +-
 include/net/netns/ipv4.h |  1 +
 net/core/fib_rules.c     | 82 +++++++++++++++++++++-------------------
 net/ipv4/fib_frontend.c  | 48 ++++++++++++++++-------
 net/ipv4/fib_rules.c     | 20 ++++++----
 net/ipv4/fib_trie.c      |  3 +-
 net/ipv6/fib6_rules.c    | 17 ++-------
 8 files changed, 101 insertions(+), 77 deletions(-)

-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v1 net-next 01/10] net: fib_rules: Make fib_rules_ops.delete() return void.

[Kuniyuki Iwashima]

Since commit d954a67a7dfa ("ipv4: fib_rule: Move fib4_rules_exit()
to ->exit()."), both fib4_rule_delete() and fib6_rule_delete() always
return 0.

Let's change the return type to void.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/fib_rules.h | 2 +-
 net/core/fib_rules.c    | 7 ++-----
 net/ipv4/fib_rules.c    | 4 +---
 net/ipv6/fib6_rules.c   | 4 +---
 4 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
index 7dee0ae616e3..f9a4bca51eda 100644
--- a/include/net/fib_rules.h
+++ b/include/net/fib_rules.h
@@ -82,7 +82,7 @@ struct fib_rules_ops {
 					     struct fib_rule_hdr *,
 					     struct nlattr **,
 					     struct netlink_ext_ack *);
-	int			(*delete)(struct fib_rule *);
+	void			(*delete)(struct fib_rule *);
 	int			(*compare)(struct fib_rule *,
 					   struct fib_rule_hdr *,
 					   struct nlattr **);
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index cf374c208732..961eb709f256 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -1055,11 +1055,8 @@ int fib_delrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto errout_free;
 	}
 
-	if (ops->delete) {
-		err = ops->delete(rule);
-		if (err)
-			goto errout_free;
-	}
+	if (ops->delete)
+		ops->delete(rule);
 
 	if (rule->tun_id)
 		ip_tunnel_unneed_metadata();
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index e068a5bace73..51d0ab423ed4 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -349,7 +349,7 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
 	return err;
 }
 
-static int fib4_rule_delete(struct fib_rule *rule)
+static void fib4_rule_delete(struct fib_rule *rule)
 {
 	struct net *net = rule->fr_net;
 
@@ -361,8 +361,6 @@ static int fib4_rule_delete(struct fib_rule *rule)
 	if (net->ipv4.fib_rules_require_fldissect &&
 	    fib_rule_requires_fldissect(rule))
 		net->ipv4.fib_rules_require_fldissect--;
-
-	return 0;
 }
 
 static int fib4_rule_compare(struct fib_rule *rule, struct fib_rule_hdr *frh,
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index e1b2b4fa6e18..5ab4dde07225 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -480,15 +480,13 @@ static int fib6_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
 	return err;
 }
 
-static int fib6_rule_delete(struct fib_rule *rule)
+static void fib6_rule_delete(struct fib_rule *rule)
 {
 	struct net *net = rule->fr_net;
 
 	if (net->ipv6.fib6_rules_require_fldissect &&
 	    fib_rule_requires_fldissect(rule))
 		net->ipv6.fib6_rules_require_fldissect--;
-
-	return 0;
 }
 
 static int fib6_rule_compare(struct fib_rule *rule, struct fib_rule_hdr *frh,
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v1 net-next 02/10] ipv4: fib_rules: Make the need for fib_unmerge() explicit.

[Kuniyuki Iwashima]

IPv4 local and main route tables are merged by default to avoid
unnecessary rule lookups.

When the first IPv4 rule is created, fib_unmerge() splits the
two tables.

However, fib4_rule_configure() currently always calls fib_unmerge(),
and even fetching a table via fib_get_table() requires RTNL (or RCU).

We will drop RTNL from fib_newrule() if not needed.

Let's call fib_unmerge() only once for the first rule.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/fib_rules.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index 51d0ab423ed4..16d202246a36 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -301,10 +301,12 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
 	    fib4_nl2rule_dscp_mask(tb[FRA_DSCP_MASK], rule4, extack) < 0)
 		goto errout;
 
-	/* split local/main if they are not already split */
-	err = fib_unmerge(net);
-	if (err)
-		goto errout;
+	if (!net->ipv4.fib_has_custom_rules) {
+		/* split local/main if they are not already split */
+		err = fib_unmerge(net);
+		if (err)
+			goto errout;
+	}
 
 	if (rule->table == RT_TABLE_UNSPEC && !rule->l3mdev) {
 		if (rule->action == FR_ACT_TO_TBL) {
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v1 net-next 03/10] ipv4: fib: Protect fib_new_table() with spinlock.

[Kuniyuki Iwashima]

fib_newrule() will drop RTNL except for the first IPv4 rule.

Then, fib4_rule_configure() could call fib_empty_table() and create
a new IPv4 fib_table without RTNL.

Currently, net->ipv4.fib_table_hash[] is only protected by RTNL.

As a prep, let's protect net->ipv4.fib_table_hash[] with a dedicated
spinlock.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/netns/ipv4.h |  1 +
 net/ipv4/fib_frontend.c  | 25 +++++++++++++++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 6e27c56514df..59506320558a 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -127,6 +127,7 @@ struct netns_ipv4 {
 	atomic_t		fib_num_tclassid_users;
 #endif
 	struct hlist_head	*fib_table_hash;
+	spinlock_t		fib_table_hash_lock;
 	struct sock		*fibnl;
 	struct hlist_head	*fib_info_hash;
 	unsigned int		fib_info_hash_bits;
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 42212970d735..336d70649eb9 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -76,7 +76,7 @@ static int __net_init fib4_rules_init(struct net *net)
 
 struct fib_table *fib_new_table(struct net *net, u32 id)
 {
-	struct fib_table *tb, *alias = NULL;
+	struct fib_table *tb, *new_tb, *alias = NULL;
 	unsigned int h;
 
 	if (id == 0)
@@ -85,14 +85,27 @@ struct fib_table *fib_new_table(struct net *net, u32 id)
 	if (tb)
 		return tb;
 
+	if (!check_net(net))
+		return NULL;
+
 	if (id == RT_TABLE_LOCAL && !net->ipv4.fib_has_custom_rules)
 		alias = fib_new_table(net, RT_TABLE_MAIN);
 
-	if (check_net(net))
-		tb = fib_trie_table(id, alias);
-	if (!tb)
+	new_tb = fib_trie_table(id, alias);
+	if (!new_tb)
 		return NULL;
 
+	spin_lock(&net->ipv4.fib_table_hash_lock);
+
+	tb = fib_get_table(net, id);
+	if (tb) {
+		spin_unlock(&net->ipv4.fib_table_hash_lock);
+		fib_free_table(new_tb);
+		return tb;
+	}
+
+	tb = new_tb;
+
 	switch (id) {
 	case RT_TABLE_MAIN:
 		rcu_assign_pointer(net->ipv4.fib_main, tb);
@@ -106,6 +119,9 @@ struct fib_table *fib_new_table(struct net *net, u32 id)
 
 	h = id & (FIB_TABLE_HASHSZ - 1);
 	hlist_add_head_rcu(&tb->tb_hlist, &net->ipv4.fib_table_hash[h]);
+
+	spin_unlock(&net->ipv4.fib_table_hash_lock);
+
 	return tb;
 }
 EXPORT_SYMBOL_GPL(fib_new_table);
@@ -1565,6 +1581,7 @@ static int __net_init ip_fib_net_init(struct net *net)
 	net->ipv4.sysctl_fib_multipath_hash_fields =
 		FIB_MULTIPATH_HASH_FIELD_DEFAULT_MASK;
 #endif
+	spin_lock_init(&net->ipv4.fib_table_hash_lock);
 
 	/* Avoid false sharing : Use at least a full cache line */
 	size = max_t(size_t, size, L1_CACHE_BYTES);
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v1 net-next 04/10] ipv4: fib: Drop RTNL annotation for net->ipv4.fib_table_hash[].

[Kuniyuki Iwashima]

fib_newrule() will drop RTNL except for the first IPv4 rule.

net->ipv4.fib_table_hash[] will be read with no protection,
but this is fine because fib_table is not destroyed until
netns dismantle except for the merged main/local table.

fib_unmerge() will continue to be called under RTNL, so other
readers (fib_flush() and fib_info_notify_update()) just have
to care about the concurrent hlist_add().

IPv6 and IPMR/IP6MR also take this strategy and use RCU helpers
to avoid data race against concurrent hlist_add().

Let's not use lockdep_rtnl_is_held() and rcu_dereference_rtnl()
for net->ipv4.fib_table_hash[].

Note that commit a7e53531234d ("fib_trie: Make fib_table rcu
safe") started to use the _safe version in fib_flush(), but it
is not needed thanks to RTNL.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/ip_fib.h    |  3 ++-
 net/ipv4/fib_frontend.c | 23 +++++++++++++----------
 net/ipv4/fib_trie.c     |  3 +--
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index c63a3c4967ae..0a35355fb0f3 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -302,7 +302,8 @@ static inline struct fib_table *fib_get_table(struct net *net, u32 id)
 		&net->ipv4.fib_table_hash[TABLE_LOCAL_INDEX] :
 		&net->ipv4.fib_table_hash[TABLE_MAIN_INDEX];
 
-	tb_hlist = rcu_dereference_rtnl(hlist_first_rcu(ptr));
+	/* Only fib4_rules_init() adds fib_table. */
+	tb_hlist = rcu_dereference_protected(hlist_first_rcu(ptr), true);
 
 	return hlist_entry(tb_hlist, struct fib_table, tb_hlist);
 }
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 336d70649eb9..54eb72695093 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -126,24 +126,28 @@ struct fib_table *fib_new_table(struct net *net, u32 id)
 }
 EXPORT_SYMBOL_GPL(fib_new_table);
 
-/* caller must hold either rtnl or rcu read lock */
 struct fib_table *fib_get_table(struct net *net, u32 id)
 {
-	struct fib_table *tb;
+	struct fib_table *tb = NULL;
 	struct hlist_head *head;
 	unsigned int h;
 
 	if (id == 0)
 		id = RT_TABLE_MAIN;
 	h = id & (FIB_TABLE_HASHSZ - 1);
-
 	head = &net->ipv4.fib_table_hash[h];
-	hlist_for_each_entry_rcu(tb, head, tb_hlist,
-				 lockdep_rtnl_is_held()) {
+
+	/* fib_table is not destroyed until ip_fib_net_exit()
+	 * except for the merged main/local table.
+	 * fib_unmerge() is called under RTNL, so other readers
+	 * under RTNL (e.g. fib_flush(), fib_info_notify_update())
+	 * can safely traverse the list with rcu_dereference_raw().
+	 */
+	hlist_for_each_entry_rcu(tb, head, tb_hlist, true)
 		if (tb->tb_id == id)
-			return tb;
-	}
-	return NULL;
+			break;
+
+	return tb;
 }
 #endif /* CONFIG_IP_MULTIPLE_TABLES */
 
@@ -206,10 +210,9 @@ void fib_flush(struct net *net)
 
 	for (h = 0; h < FIB_TABLE_HASHSZ; h++) {
 		struct hlist_head *head = &net->ipv4.fib_table_hash[h];
-		struct hlist_node *tmp;
 		struct fib_table *tb;
 
-		hlist_for_each_entry_safe(tb, tmp, head, tb_hlist)
+		hlist_for_each_entry_rcu(tb, head, tb_hlist, true)
 			flushed += fib_table_flush(net, tb, false);
 	}
 
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index e11dc86ceda0..d1d342d7148e 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -2137,8 +2137,7 @@ void fib_info_notify_update(struct net *net, struct nl_info *info)
 		struct hlist_head *head = &net->ipv4.fib_table_hash[h];
 		struct fib_table *tb;
 
-		hlist_for_each_entry_rcu(tb, head, tb_hlist,
-					 lockdep_rtnl_is_held())
+		hlist_for_each_entry_rcu(tb, head, tb_hlist, true)
 			__fib_info_notify_update(net, tb, info);
 	}
 }
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v1 net-next 05/10] net: fib_rules: Add fib_rules_ops.lock.

[Kuniyuki Iwashima]

We will no longer hold RTNL for RTM_NEWRULE and RMT_DELRULE
except for the first IPv4 RTM_NEWRULE.

Let's add per-fib_rules_ops mutex inside RTNL.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/fib_rules.h |  1 +
 net/core/fib_rules.c    | 20 ++++++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
index f9a4bca51eda..7636ef4da5ad 100644
--- a/include/net/fib_rules.h
+++ b/include/net/fib_rules.h
@@ -98,6 +98,7 @@ struct fib_rules_ops {
 	struct list_head	rules_list;
 	struct module		*owner;
 	struct net		*fro_net;
+	struct mutex		lock;
 	struct rcu_head		rcu;
 };
 
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 961eb709f256..8b9dac1bd4a7 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -172,6 +172,7 @@ fib_rules_register(const struct fib_rules_ops *tmpl, struct net *net)
 		return ERR_PTR(-ENOMEM);
 
 	INIT_LIST_HEAD(&ops->rules_list);
+	mutex_init(&ops->lock);
 	ops->fro_net = net;
 
 	err = __fib_rules_register(ops);
@@ -392,6 +393,7 @@ static int call_fib_rule_notifiers(struct net *net,
 	};
 
 	ASSERT_RTNL_NET(net);
+	lockdep_assert_held(&ops->lock);
 
 	/* Paired with READ_ONCE() in fib_rules_seq() */
 	WRITE_ONCE(ops->fib_rules_seq, ops->fib_rules_seq + 1);
@@ -910,6 +912,7 @@ int fib_newrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	if (!rtnl_held)
 		rtnl_net_lock(net);
+	mutex_lock(&ops->lock);
 
 	err = fib_nl2rule_rtnl(rule, ops, tb, extack);
 	if (err)
@@ -978,6 +981,7 @@ int fib_newrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	fib_rule_get(rule);
 
+	mutex_unlock(&ops->lock);
 	if (!rtnl_held)
 		rtnl_net_unlock(net);
 
@@ -988,6 +992,7 @@ int fib_newrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 	return 0;
 
 errout_free:
+	mutex_unlock(&ops->lock);
 	if (!rtnl_held)
 		rtnl_net_unlock(net);
 	kfree(rule);
@@ -1039,6 +1044,7 @@ int fib_delrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	if (!rtnl_held)
 		rtnl_net_lock(net);
+	mutex_lock(&ops->lock);
 
 	err = fib_nl2rule_rtnl(nlrule, ops, tb, extack);
 	if (err)
@@ -1093,6 +1099,7 @@ int fib_delrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	call_fib_rule_notifiers(net, FIB_EVENT_RULE_DEL, rule, ops, NULL);
 
+	mutex_unlock(&ops->lock);
 	if (!rtnl_held)
 		rtnl_net_unlock(net);
 
@@ -1104,6 +1111,7 @@ int fib_delrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 	return 0;
 
 errout_free:
+	mutex_unlock(&ops->lock);
 	if (!rtnl_held)
 		rtnl_net_unlock(net);
 	kfree(nlrule);
@@ -1403,20 +1411,28 @@ static int fib_rules_event(struct notifier_block *this, unsigned long event,
 
 	switch (event) {
 	case NETDEV_REGISTER:
-		list_for_each_entry(ops, &net->rules_ops, list)
+		list_for_each_entry(ops, &net->rules_ops, list) {
+			mutex_lock(&ops->lock);
 			attach_rules(&ops->rules_list, dev);
+			mutex_unlock(&ops->lock);
+		}
 		break;
 
 	case NETDEV_CHANGENAME:
 		list_for_each_entry(ops, &net->rules_ops, list) {
+			mutex_lock(&ops->lock);
 			detach_rules(&ops->rules_list, dev);
 			attach_rules(&ops->rules_list, dev);
+			mutex_unlock(&ops->lock);
 		}
 		break;
 
 	case NETDEV_UNREGISTER:
-		list_for_each_entry(ops, &net->rules_ops, list)
+		list_for_each_entry(ops, &net->rules_ops, list) {
+			mutex_lock(&ops->lock);
 			detach_rules(&ops->rules_list, dev);
+			mutex_unlock(&ops->lock);
+		}
 		break;
 	}
 
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v1 net-next 06/10] net: fib_rules: Remove unnecessary EXPORT_SYMBOL.

[Kuniyuki Iwashima]

All fib_rule users cannot be compiled as module.

  $ grep -E "config (INET|IPV6|IP_MROUTE|IPV6_MROUTE)\b" -A1 \
    net/{Kconfig,{ipv4,ipv6}/Kconfig}
  net/Kconfig:config INET
  net/Kconfig-	bool "TCP/IP networking"
  --
  net/ipv4/Kconfig:config IP_MROUTE
  net/ipv4/Kconfig-	bool "IP: multicast routing"
  --
  net/ipv6/Kconfig:menuconfig IPV6
  net/ipv6/Kconfig-	bool "The IPv6 protocol"
  --
  net/ipv6/Kconfig:config IPV6_MROUTE
  net/ipv6/Kconfig-	bool "IPv6: multicast routing"

Let's remove EXPORT_SYMBOL and friends for fib_rule.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/core/fib_rules.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 8b9dac1bd4a7..25a3fd997782 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -51,7 +51,6 @@ bool fib_rule_matchall(const struct fib_rule *rule)
 		return false;
 	return true;
 }
-EXPORT_SYMBOL_GPL(fib_rule_matchall);
 
 int fib_default_rule_add(struct fib_rules_ops *ops,
 			 u32 pref, u32 table)
@@ -78,7 +77,6 @@ int fib_default_rule_add(struct fib_rules_ops *ops,
 	list_add_tail(&r->list, &ops->rules_list);
 	return 0;
 }
-EXPORT_SYMBOL(fib_default_rule_add);
 
 static u32 fib_default_rule_pref(struct fib_rules_ops *ops)
 {
@@ -183,7 +181,6 @@ fib_rules_register(const struct fib_rules_ops *tmpl, struct net *net)
 
 	return ops;
 }
-EXPORT_SYMBOL_GPL(fib_rules_register);
 
 static void fib_rules_cleanup_ops(struct fib_rules_ops *ops)
 {
@@ -208,7 +205,6 @@ void fib_rules_unregister(struct fib_rules_ops *ops)
 	fib_rules_cleanup_ops(ops);
 	kfree_rcu(ops, rcu);
 }
-EXPORT_SYMBOL_GPL(fib_rules_unregister);
 
 static int uid_range_set(struct fib_kuid_range *range)
 {
@@ -364,7 +360,6 @@ int fib_rules_lookup(struct fib_rules_ops *ops, struct flowi *fl,
 
 	return err;
 }
-EXPORT_SYMBOL_GPL(fib_rules_lookup);
 
 static int call_fib_rule_notifier(struct notifier_block *nb,
 				  enum fib_event_type event_type,
@@ -425,7 +420,6 @@ int fib_rules_dump(struct net *net, struct notifier_block *nb, int family,
 
 	return err;
 }
-EXPORT_SYMBOL_GPL(fib_rules_dump);
 
 unsigned int fib_rules_seq_read(const struct net *net, int family)
 {
@@ -441,7 +435,6 @@ unsigned int fib_rules_seq_read(const struct net *net, int family)
 
 	return fib_rules_seq;
 }
-EXPORT_SYMBOL_GPL(fib_rules_seq_read);
 
 static struct fib_rule *rule_find(struct fib_rules_ops *ops,
 				  struct fib_rule_hdr *frh,
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v1 net-next 07/10] net: fib_rules: Drop RTNL assertions.

[Kuniyuki Iwashima]

Now, fib_rule structs are protected by per-fib_rules_ops mutex.

Let's drop ASSERT_RTNL_NET() and rtnl_dereference().

Note that fib_rules_event() iterates over net->rules_ops without
net->rules_mod_lock, but this is fine because all fib_rule users
are built-in and concurrent fib_rules_unregister() does not happen.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/core/fib_rules.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 25a3fd997782..5eef5d6ace82 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -387,7 +387,6 @@ static int call_fib_rule_notifiers(struct net *net,
 		.rule = rule,
 	};
 
-	ASSERT_RTNL_NET(net);
 	lockdep_assert_held(&ops->lock);
 
 	/* Paired with READ_ONCE() in fib_rules_seq() */
@@ -955,7 +954,7 @@ int fib_newrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 		list_for_each_entry(r, &ops->rules_list, list) {
 			if (r->action == FR_ACT_GOTO &&
 			    r->target == rule->pref &&
-			    rtnl_dereference(r->ctarget) == NULL) {
+			    !rcu_access_pointer(r->ctarget)) {
 				rcu_assign_pointer(r->ctarget, rule);
 				if (--ops->unresolved_rules == 0)
 					break;
@@ -1064,7 +1063,7 @@ int fib_delrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	if (rule->action == FR_ACT_GOTO) {
 		ops->nr_goto_rules--;
-		if (rtnl_dereference(rule->ctarget) == NULL)
+		if (!rcu_access_pointer(rule->ctarget))
 			ops->unresolved_rules--;
 	}
 
@@ -1082,7 +1081,7 @@ int fib_delrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 		if (&n->list == &ops->rules_list || n->pref != rule->pref)
 			n = NULL;
 		list_for_each_entry(r, &ops->rules_list, list) {
-			if (rtnl_dereference(r->ctarget) != rule)
+			if (rcu_access_pointer(r->ctarget) != rule)
 				continue;
 			rcu_assign_pointer(r->ctarget, n);
 			if (!n)
@@ -1400,8 +1399,6 @@ static int fib_rules_event(struct notifier_block *this, unsigned long event,
 	struct net *net = dev_net(dev);
 	struct fib_rules_ops *ops;
 
-	ASSERT_RTNL();
-
 	switch (event) {
 	case NETDEV_REGISTER:
 		list_for_each_entry(ops, &net->rules_ops, list) {
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v1 net-next 08/10] net: fib_rules: Use dev_get_by_name_rcu().

[Kuniyuki Iwashima]

We will no longer hold RTNL for RTM_NEWRULE and RMT_DELRULE
except for the first IPv4 RTM_NEWRULE.

Let's covnert __dev_get_by_name() in fib_nl2rule_rtnl() to
dev_get_by_name_rcu() and rename it to fib_nl2rule_locked().

Note that dev_get_by_name_rcu() must be called inside ops->lock
to serialise fib_rules_event() by __dev_change_net_namespace().

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/core/fib_rules.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 5eef5d6ace82..2b652dd83241 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -734,10 +734,10 @@ static int fib_nl2rule(struct net *net, struct nlmsghdr *nlh,
 	return err;
 }
 
-static int fib_nl2rule_rtnl(struct fib_rule *nlrule,
-			    struct fib_rules_ops *ops,
-			    struct nlattr *tb[],
-			    struct netlink_ext_ack *extack)
+static int fib_nl2rule_locked(struct fib_rule *nlrule,
+			      struct fib_rules_ops *ops,
+			      struct nlattr *tb[],
+			      struct netlink_ext_ack *extack)
 {
 	if (!tb[FRA_PRIORITY])
 		nlrule->pref = fib_default_rule_pref(ops);
@@ -748,12 +748,14 @@ static int fib_nl2rule_rtnl(struct fib_rule *nlrule,
 		return -EINVAL;
 	}
 
+	rcu_read_lock();
+
 	if (tb[FRA_IIFNAME]) {
 		struct net_device *dev;
 
-		dev = __dev_get_by_name(nlrule->fr_net, nlrule->iifname);
+		dev = dev_get_by_name_rcu(nlrule->fr_net, nlrule->iifname);
 		if (dev) {
-			nlrule->iifindex = dev->ifindex;
+			nlrule->iifindex = READ_ONCE(dev->ifindex);
 			nlrule->iif_is_l3_master = netif_is_l3_master(dev);
 		}
 	}
@@ -761,13 +763,15 @@ static int fib_nl2rule_rtnl(struct fib_rule *nlrule,
 	if (tb[FRA_OIFNAME]) {
 		struct net_device *dev;
 
-		dev = __dev_get_by_name(nlrule->fr_net, nlrule->oifname);
+		dev = dev_get_by_name_rcu(nlrule->fr_net, nlrule->oifname);
 		if (dev) {
-			nlrule->oifindex = dev->ifindex;
+			nlrule->oifindex = READ_ONCE(dev->ifindex);
 			nlrule->oif_is_l3_master = netif_is_l3_master(dev);
 		}
 	}
 
+	rcu_read_unlock();
+
 	return 0;
 }
 
@@ -906,7 +910,7 @@ int fib_newrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 		rtnl_net_lock(net);
 	mutex_lock(&ops->lock);
 
-	err = fib_nl2rule_rtnl(rule, ops, tb, extack);
+	err = fib_nl2rule_locked(rule, ops, tb, extack);
 	if (err)
 		goto errout_free;
 
@@ -1038,7 +1042,7 @@ int fib_delrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 		rtnl_net_lock(net);
 	mutex_lock(&ops->lock);
 
-	err = fib_nl2rule_rtnl(nlrule, ops, tb, extack);
+	err = fib_nl2rule_locked(nlrule, ops, tb, extack);
 	if (err)
 		goto errout_free;
 
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v1 net-next 09/10] net: fib_rules: Only hold RTNL for the first IPv4 RTM_NEWRULE.

[Kuniyuki Iwashima]

Now, RTM_DELRULE no longer needs RTNL, and the only RTNL dependant
in RTM_NEWRULE is fib_unmerge(), which is called for the first
IPv4 rule.

Let's add fib_rules_ops.need_rtnl() and hold RTNL only for the
first IPv4 rule.

Tested:
The script below creates 1K rules in parallel in 4K netns, and
it got 20x/30x faster for IPv4/IPv6.

  #!/bin/bash
  N=4096
  F=rules.txt

  for i in $(seq $N); do ip netns add ns-$i; done
  printf 'rule add from all table %d\n' {1..1024} > $F

  for v in 4 6; do
  	echo "=== IPv${v} ==="
  	time { for i in $(seq $N); do nsenter \
  	--net=/var/run/netns/ns-$i ip -$v -batch $F & done; wait; }
  done

  for i in $(seq $N); do ip netns del ns-$i; done
  rm -f $F

Without this series:

  # ./test.sh
  === IPv4 ===

  real	0m22.752s
  user	0m7.834s
  sys	92m46.721s
  === IPv6 ===

  real	0m35.181s
  user	0m8.635s
  sys	142m30.479s

With this series:

  # ./test.sh
  === IPv4 ===

  real	0m0.918s
  user	0m5.675s
  sys	2m7.024s
  === IPv6 ===

  real	0m1.214s
  user	0m7.917s
  sys	4m19.489s

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/fib_rules.h |  1 +
 net/core/fib_rules.c    | 15 ++++++---------
 net/ipv4/fib_rules.c    |  6 ++++++
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
index 7636ef4da5ad..c6b94790fa81 100644
--- a/include/net/fib_rules.h
+++ b/include/net/fib_rules.h
@@ -93,6 +93,7 @@ struct fib_rules_ops {
 	/* Called after modifications to the rules set, must flush
 	 * the route cache if one exists. */
 	void			(*flush_cache)(struct fib_rules_ops *ops);
+	bool			(*need_rtnl)(struct net *net);
 
 	int			nlgroup;
 	struct list_head	rules_list;
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 2b652dd83241..22e5e5e1a9c4 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -881,6 +881,7 @@ int fib_newrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct nlattr *tb[FRA_MAX + 1];
 	bool user_priority = false;
 	struct fib_rule_hdr *frh;
+	bool unlock_rtnl = false;
 
 	frh = nlmsg_payload(nlh, sizeof(*frh));
 	if (!frh) {
@@ -906,8 +907,10 @@ int fib_newrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (err)
 		goto errout;
 
-	if (!rtnl_held)
+	if (!rtnl_held && ops->need_rtnl && ops->need_rtnl(net)) {
+		unlock_rtnl = true;
 		rtnl_net_lock(net);
+	}
 	mutex_lock(&ops->lock);
 
 	err = fib_nl2rule_locked(rule, ops, tb, extack);
@@ -978,7 +981,7 @@ int fib_newrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 	fib_rule_get(rule);
 
 	mutex_unlock(&ops->lock);
-	if (!rtnl_held)
+	if (unlock_rtnl)
 		rtnl_net_unlock(net);
 
 	notify_rule_change(RTM_NEWRULE, rule, ops, nlh, NETLINK_CB(skb).portid);
@@ -989,7 +992,7 @@ int fib_newrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 
 errout_free:
 	mutex_unlock(&ops->lock);
-	if (!rtnl_held)
+	if (unlock_rtnl)
 		rtnl_net_unlock(net);
 	kfree(rule);
 errout:
@@ -1038,8 +1041,6 @@ int fib_delrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (err)
 		goto errout;
 
-	if (!rtnl_held)
-		rtnl_net_lock(net);
 	mutex_lock(&ops->lock);
 
 	err = fib_nl2rule_locked(nlrule, ops, tb, extack);
@@ -1096,8 +1097,6 @@ int fib_delrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 	call_fib_rule_notifiers(net, FIB_EVENT_RULE_DEL, rule, ops, NULL);
 
 	mutex_unlock(&ops->lock);
-	if (!rtnl_held)
-		rtnl_net_unlock(net);
 
 	notify_rule_change(RTM_DELRULE, rule, ops, nlh, NETLINK_CB(skb).portid);
 	fib_rule_put(rule);
@@ -1108,8 +1107,6 @@ int fib_delrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
 
 errout_free:
 	mutex_unlock(&ops->lock);
-	if (!rtnl_held)
-		rtnl_net_unlock(net);
 	kfree(nlrule);
 errout:
 	rules_ops_put(ops);
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index 16d202246a36..4edb0dca7be8 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -460,6 +460,11 @@ static void fib4_rule_flush_cache(struct fib_rules_ops *ops)
 	rt_cache_flush(ops->fro_net);
 }
 
+static bool fib4_rule_need_rtnl(struct net *net)
+{
+	return !net->ipv4.fib_has_custom_rules;
+}
+
 static const struct fib_rules_ops __net_initconst fib4_rules_ops_template = {
 	.family		= AF_INET,
 	.rule_size	= sizeof(struct fib4_rule),
@@ -473,6 +478,7 @@ static const struct fib_rules_ops __net_initconst fib4_rules_ops_template = {
 	.fill		= fib4_rule_fill,
 	.nlmsg_payload	= fib4_rule_nlmsg_payload,
 	.flush_cache	= fib4_rule_flush_cache,
+	.need_rtnl	= fib4_rule_need_rtnl,
 	.nlgroup	= RTNLGRP_IPV4_RULE,
 	.owner		= THIS_MODULE,
 };
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v1 net-next 10/10] ipv6: fib_rules: Convert fib6_rules_net_exit_rtnl() to ->exit().

[Kuniyuki Iwashima]

Now fib_rule is protected by per-ops mutex.

fib6_rules_net_exit_batch() no longer needs RTNL.

Let's convert it to ->exit() and drop RTNL.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv6/fib6_rules.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index 5ab4dde07225..04dab9329d0c 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -635,21 +635,14 @@ static int __net_init fib6_rules_net_init(struct net *net)
 	goto out;
 }
 
-static void __net_exit fib6_rules_net_exit_batch(struct list_head *net_list)
+static void __net_exit fib6_rules_net_exit(struct net *net)
 {
-	struct net *net;
-
-	rtnl_lock();
-	list_for_each_entry(net, net_list, exit_list) {
-		fib_rules_unregister(net->ipv6.fib6_rules_ops);
-		cond_resched();
-	}
-	rtnl_unlock();
+	fib_rules_unregister(net->ipv6.fib6_rules_ops);
 }
 
 static struct pernet_operations fib6_rules_net_ops = {
 	.init = fib6_rules_net_init,
-	.exit_batch = fib6_rules_net_exit_batch,
+	.exit = fib6_rules_net_exit,
 };
 
 int __init fib6_rules_init(void)
-- 
2.55.0.rc0.799.gd6f94ed593-goog