diff mbox series

[net-next] net: fib_notifier: move fib_notifier_ops from struct net into per-net struct

Message ID 20190905180656.4756-1-jiri@resnulli.us
State Accepted
Delegated to: David Miller
Headers show
Series [net-next] net: fib_notifier: move fib_notifier_ops from struct net into per-net struct | expand

Commit Message

Jiri Pirko Sept. 5, 2019, 6:06 p.m. UTC 
From: Jiri Pirko <jiri@mellanox.com>

No need for fib_notifier_ops to be in struct net. It is used only by
fib_notifier as a private data. Use net_generic to introduce per-net
fib_notifier struct and move fib_notifier_ops there.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/net_namespace.h |  3 ---
 net/core/fib_notifier.c     | 29 +++++++++++++++++++++++------
 2 files changed, 23 insertions(+), 9 deletions(-)

Comments

Eric Dumazet Sept. 6, 2019, 5:54 a.m. UTC | #1 
On 9/5/19 8:06 PM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> No need for fib_notifier_ops to be in struct net. It is used only by
> fib_notifier as a private data. Use net_generic to introduce per-net
> fib_notifier struct and move fib_notifier_ops there.
> 
>

...

>  static struct pernet_operations fib_notifier_net_ops = {
>  	.init = fib_notifier_net_init,
>  	.exit = fib_notifier_net_exit,
> +	.id = &fib_notifier_net_id,
> +	.size = sizeof(struct fib_notifier_net),
>  };
>  
>  static int __init fib_notifier_init(void)
> 

Note that this will allocate a block of memory (in ops_init()) to hold this,
plus a second one to hold the pointer to this block.

Due to kmalloc() constraints, this block will use more memory.

Not sure your patch is a win, since it makes things a bit more complex.

Is it a preparation patch so that you can add later other fields in struct fib_notifier_net ?
Jiri Pirko Sept. 6, 2019, 6:05 a.m. UTC | #2 
Fri, Sep 06, 2019 at 07:54:39AM CEST, eric.dumazet@gmail.com wrote:
>
>
>On 9/5/19 8:06 PM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> No need for fib_notifier_ops to be in struct net. It is used only by
>> fib_notifier as a private data. Use net_generic to introduce per-net
>> fib_notifier struct and move fib_notifier_ops there.
>> 
>>
>
>...
>
>>  static struct pernet_operations fib_notifier_net_ops = {
>>  	.init = fib_notifier_net_init,
>>  	.exit = fib_notifier_net_exit,
>> +	.id = &fib_notifier_net_id,
>> +	.size = sizeof(struct fib_notifier_net),
>>  };
>>  
>>  static int __init fib_notifier_init(void)
>> 
>
>Note that this will allocate a block of memory (in ops_init()) to hold this,
>plus a second one to hold the pointer to this block.
>
>Due to kmalloc() constraints, this block will use more memory.

I'm aware. But we have net_generic for exactly this purpose not to
pullute struct net.


>
>Not sure your patch is a win, since it makes things a bit more complex.
>
>Is it a preparation patch so that you can add later other fields in struct fib_notifier_net ?

Yes.


>
David Miller Sept. 7, 2019, 3:28 p.m. UTC | #3 
From: Jiri Pirko <jiri@resnulli.us>
Date: Thu,  5 Sep 2019 20:06:56 +0200

> From: Jiri Pirko <jiri@mellanox.com>
> 
> No need for fib_notifier_ops to be in struct net. It is used only by
> fib_notifier as a private data. Use net_generic to introduce per-net
> fib_notifier struct and move fib_notifier_ops there.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

Applied.
diff mbox series

Patch

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index ab40d7afdc54..64bcb589a610 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -103,9 +103,6 @@  struct net {
 	/* core fib_rules */
 	struct list_head	rules_ops;
 
-	struct list_head	fib_notifier_ops;  /* Populated by
-						    * register_pernet_subsys()
-						    */
 	struct net_device       *loopback_dev;          /* The loopback */
 	struct netns_core	core;
 	struct netns_mib	mib;
diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c
index 13a40b831d6d..470a606d5e8d 100644
--- a/net/core/fib_notifier.c
+++ b/net/core/fib_notifier.c
@@ -5,8 +5,15 @@ 
 #include <linux/module.h>
 #include <linux/init.h>
 #include <net/net_namespace.h>
+#include <net/netns/generic.h>
 #include <net/fib_notifier.h>
 
+static unsigned int fib_notifier_net_id;
+
+struct fib_notifier_net {
+	struct list_head fib_notifier_ops;
+};
+
 static ATOMIC_NOTIFIER_HEAD(fib_chain);
 
 int call_fib_notifier(struct notifier_block *nb, struct net *net,
@@ -34,6 +41,7 @@  EXPORT_SYMBOL(call_fib_notifiers);
 
 static unsigned int fib_seq_sum(void)
 {
+	struct fib_notifier_net *fn_net;
 	struct fib_notifier_ops *ops;
 	unsigned int fib_seq = 0;
 	struct net *net;
@@ -41,8 +49,9 @@  static unsigned int fib_seq_sum(void)
 	rtnl_lock();
 	down_read(&net_rwsem);
 	for_each_net(net) {
+		fn_net = net_generic(net, fib_notifier_net_id);
 		rcu_read_lock();
-		list_for_each_entry_rcu(ops, &net->fib_notifier_ops, list) {
+		list_for_each_entry_rcu(ops, &fn_net->fib_notifier_ops, list) {
 			if (!try_module_get(ops->owner))
 				continue;
 			fib_seq += ops->fib_seq_read(net);
@@ -58,9 +67,10 @@  static unsigned int fib_seq_sum(void)
 
 static int fib_net_dump(struct net *net, struct notifier_block *nb)
 {
+	struct fib_notifier_net *fn_net = net_generic(net, fib_notifier_net_id);
 	struct fib_notifier_ops *ops;
 
-	list_for_each_entry_rcu(ops, &net->fib_notifier_ops, list) {
+	list_for_each_entry_rcu(ops, &fn_net->fib_notifier_ops, list) {
 		int err;
 
 		if (!try_module_get(ops->owner))
@@ -127,12 +137,13 @@  EXPORT_SYMBOL(unregister_fib_notifier);
 static int __fib_notifier_ops_register(struct fib_notifier_ops *ops,
 				       struct net *net)
 {
+	struct fib_notifier_net *fn_net = net_generic(net, fib_notifier_net_id);
 	struct fib_notifier_ops *o;
 
-	list_for_each_entry(o, &net->fib_notifier_ops, list)
+	list_for_each_entry(o, &fn_net->fib_notifier_ops, list)
 		if (ops->family == o->family)
 			return -EEXIST;
-	list_add_tail_rcu(&ops->list, &net->fib_notifier_ops);
+	list_add_tail_rcu(&ops->list, &fn_net->fib_notifier_ops);
 	return 0;
 }
 
@@ -167,18 +178,24 @@  EXPORT_SYMBOL(fib_notifier_ops_unregister);
 
 static int __net_init fib_notifier_net_init(struct net *net)
 {
-	INIT_LIST_HEAD(&net->fib_notifier_ops);
+	struct fib_notifier_net *fn_net = net_generic(net, fib_notifier_net_id);
+
+	INIT_LIST_HEAD(&fn_net->fib_notifier_ops);
 	return 0;
 }
 
 static void __net_exit fib_notifier_net_exit(struct net *net)
 {
-	WARN_ON_ONCE(!list_empty(&net->fib_notifier_ops));
+	struct fib_notifier_net *fn_net = net_generic(net, fib_notifier_net_id);
+
+	WARN_ON_ONCE(!list_empty(&fn_net->fib_notifier_ops));
 }
 
 static struct pernet_operations fib_notifier_net_ops = {
 	.init = fib_notifier_net_init,
 	.exit = fib_notifier_net_exit,
+	.id = &fib_notifier_net_id,
+	.size = sizeof(struct fib_notifier_net),
 };
 
 static int __init fib_notifier_init(void)