[NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[Benjamin Thery]

Hello Pavel,

I've found a problem with one of your patch related to netns:

* [NETNS] Move some code into __init section when CONFIG_NET_NS=n (v2)
   http://www.spinics.net/lists/netdev/msg43310.html

This patch introduces the __net_init/__net_exit/__net_initdata
defines to save some memory when CONFIG_NET_NS is not set.

Cedric Le Goater reported he had a *non-fatal* oops when booting
a 2.6.23-mm1-lxc1 kernel with CONFIG_NET_NS=n. (2.6.23-mm1-lxc1 
contains the NETNS49 patchset). The oops occured when modules 
related to iptables were loaded after the boot completes.

The problem is the following:

- Your patch adds the __net_initdata attribute to pernet_operations 
  structures.

- pernet_operations are registered via register_pernet_subsys() and 
  linked in the pernet_list during boot.

- At the end of boot, pernet_operations are freed (because of the
  __net_initdata attribute), and the pernet_list (or first_device list) 
  points to freed memory.

- After boot, network modules which are netns-aware try to register
  themselves with register_pernet_subsys() and ...KABOOM... page
  fault when accessing pernet_list (or first_device list).
  (I reproduce Cedric's oops with the command: iptables --list)

This is not a problem right now in 2.6.23-mm1 or net-2.6 because 
there are very few netns-aware network subsystems merged and they
are all initialized during boot. But it will be problematic when 
we will merge netns code for subsystems which can be built as 
modules (eg. iptables, ...). I'm not sure we can use 
__net_init_data for pernet_operations then. 
Maybe we can add some checks in register_pernet_operations
when CONFIG_NET_NS=n.

I haven't found a fix yet.

Regards,
Benjamin

-- 
B e n j a m i n   T h e r y  - BULL/DT/Open Software R&D

   http://www.bull.com

Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[Denis V. Lunev]

[-- Attachment #1: Type: text/plain, Size: 2021 bytes --]

The patch attached should help. The idea is simple. The "init" should be
called only once without NETNS. Period. No need for any lists.

I'll resend it to Dave after the ACK.

Regards,
	Den

Benjamin Thery wrote:
> Hello Pavel,
> 
> I've found a problem with one of your patch related to netns:
> 
> * [NETNS] Move some code into __init section when CONFIG_NET_NS=n (v2)
>    http://www.spinics.net/lists/netdev/msg43310.html
> 
> This patch introduces the __net_init/__net_exit/__net_initdata
> defines to save some memory when CONFIG_NET_NS is not set.
> 
> Cedric Le Goater reported he had a *non-fatal* oops when booting
> a 2.6.23-mm1-lxc1 kernel with CONFIG_NET_NS=n. (2.6.23-mm1-lxc1 
> contains the NETNS49 patchset). The oops occured when modules 
> related to iptables were loaded after the boot completes.
> 
> The problem is the following:
> 
> - Your patch adds the __net_initdata attribute to pernet_operations 
>   structures.
> 
> - pernet_operations are registered via register_pernet_subsys() and 
>   linked in the pernet_list during boot.
> 
> - At the end of boot, pernet_operations are freed (because of the
>   __net_initdata attribute), and the pernet_list (or first_device list) 
>   points to freed memory.
> 
> - After boot, network modules which are netns-aware try to register
>   themselves with register_pernet_subsys() and ...KABOOM... page
>   fault when accessing pernet_list (or first_device list).
>   (I reproduce Cedric's oops with the command: iptables --list)
> 
> This is not a problem right now in 2.6.23-mm1 or net-2.6 because 
> there are very few netns-aware network subsystems merged and they
> are all initialized during boot. But it will be problematic when 
> we will merge netns code for subsystems which can be built as 
> modules (eg. iptables, ...). I'm not sure we can use 
> __net_init_data for pernet_operations then. 
> Maybe we can add some checks in register_pernet_operations
> when CONFIG_NET_NS=n.
> 
> I haven't found a fix yet.
> 
> Regards,
> Benjamin
> 


[-- Attachment #2: diff-no-netns --]
[-- Type: text/plain, Size: 441 bytes --]

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 6f71db8..9ba4809 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -186,7 +186,11 @@ static int register_pernet_operations(struct list_head *list,
 	int error;
 
 	error = 0;
+#ifdef CONFIG_NET_NS
 	list_add_tail(&ops->list, list);
+#endif
+	INIT_LIST_HEAD(&ops->list);
+#endif
 	for_each_net(net) {
 		if (ops->init) {
 			error = ops->init(net);

Re: [Devel] Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n (resend, was wrong patch)

[Denis V. Lunev]

[-- Attachment #1: Type: text/plain, Size: 2091 bytes --]

Denis V. Lunev wrote:
> The patch attached should help. The idea is simple. The "init" should be
> called only once without NETNS. Period. No need for any lists.
> 
> I'll resend it to Dave after the ACK.
> 
> Regards,
> 	Den
> 
> Benjamin Thery wrote:
>> Hello Pavel,
>>
>> I've found a problem with one of your patch related to netns:
>>
>> * [NETNS] Move some code into __init section when CONFIG_NET_NS=n (v2)
>>    http://www.spinics.net/lists/netdev/msg43310.html
>>
>> This patch introduces the __net_init/__net_exit/__net_initdata
>> defines to save some memory when CONFIG_NET_NS is not set.
>>
>> Cedric Le Goater reported he had a *non-fatal* oops when booting
>> a 2.6.23-mm1-lxc1 kernel with CONFIG_NET_NS=n. (2.6.23-mm1-lxc1 
>> contains the NETNS49 patchset). The oops occured when modules 
>> related to iptables were loaded after the boot completes.
>>
>> The problem is the following:
>>
>> - Your patch adds the __net_initdata attribute to pernet_operations 
>>   structures.
>>
>> - pernet_operations are registered via register_pernet_subsys() and 
>>   linked in the pernet_list during boot.
>>
>> - At the end of boot, pernet_operations are freed (because of the
>>   __net_initdata attribute), and the pernet_list (or first_device list) 
>>   points to freed memory.
>>
>> - After boot, network modules which are netns-aware try to register
>>   themselves with register_pernet_subsys() and ...KABOOM... page
>>   fault when accessing pernet_list (or first_device list).
>>   (I reproduce Cedric's oops with the command: iptables --list)
>>
>> This is not a problem right now in 2.6.23-mm1 or net-2.6 because 
>> there are very few netns-aware network subsystems merged and they
>> are all initialized during boot. But it will be problematic when 
>> we will merge netns code for subsystems which can be built as 
>> modules (eg. iptables, ...). I'm not sure we can use 
>> __net_init_data for pernet_operations then. 
>> Maybe we can add some checks in register_pernet_operations
>> when CONFIG_NET_NS=n.
>>
>> I haven't found a fix yet.
>>
>> Regards,
>> Benjamin


[-- Attachment #2: diff-no-netns --]
[-- Type: text/plain, Size: 440 bytes --]

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 6f71db8..cc306dc 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -186,7 +186,11 @@ static int register_pernet_operations(struct list_head *list,
 	int error;
 
 	error = 0;
+#ifdef CONFIG_NET_NS
 	list_add_tail(&ops->list, list);
+#else
+	INIT_LIST_HEAD(&ops->list);
+#endif
 	for_each_net(net) {
 		if (ops->init) {
 			error = ops->init(net);

Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[Benjamin Thery]

Denis V. Lunev wrote:
> The patch attached should help. The idea is simple. The "init" should be
> called only once without NETNS. Period. No need for any lists.

This is the kind of idea I had but I didn't think it could be 
that simple. :) 
Thanks Denis.

> I'll resend it to Dave after the ACK.

Tested on x86_64 with CONFIG_NET_NS=n and y. 
It fixes the issue we observed.

Acked-by: Benjamin Thery <benjamin.thery@bull.net>


> Regards,
> 	Den
> 
> Benjamin Thery wrote:
>> Hello Pavel,
>>
>> I've found a problem with one of your patch related to netns:
>>
>> * [NETNS] Move some code into __init section when CONFIG_NET_NS=n (v2)
>>    http://www.spinics.net/lists/netdev/msg43310.html
>>
>> This patch introduces the __net_init/__net_exit/__net_initdata
>> defines to save some memory when CONFIG_NET_NS is not set.
>>
>> Cedric Le Goater reported he had a *non-fatal* oops when booting
>> a 2.6.23-mm1-lxc1 kernel with CONFIG_NET_NS=n. (2.6.23-mm1-lxc1 
>> contains the NETNS49 patchset). The oops occured when modules 
>> related to iptables were loaded after the boot completes.
>>
>> The problem is the following:
>>
>> - Your patch adds the __net_initdata attribute to pernet_operations 
>>   structures.
>>
>> - pernet_operations are registered via register_pernet_subsys() and 
>>   linked in the pernet_list during boot.
>>
>> - At the end of boot, pernet_operations are freed (because of the
>>   __net_initdata attribute), and the pernet_list (or first_device list) 
>>   points to freed memory.
>>
>> - After boot, network modules which are netns-aware try to register
>>   themselves with register_pernet_subsys() and ...KABOOM... page
>>   fault when accessing pernet_list (or first_device list).
>>   (I reproduce Cedric's oops with the command: iptables --list)
>>
>> This is not a problem right now in 2.6.23-mm1 or net-2.6 because 
>> there are very few netns-aware network subsystems merged and they
>> are all initialized during boot. But it will be problematic when 
>> we will merge netns code for subsystems which can be built as 
>> modules (eg. iptables, ...). I'm not sure we can use 
>> __net_init_data for pernet_operations then. 
>> Maybe we can add some checks in register_pernet_operations
>> when CONFIG_NET_NS=n.
>>
>> I haven't found a fix yet.
>>
>> Regards,
>> Benjamin
>>
> 


-- 
B e n j a m i n   T h e r y  - BULL/DT/Open Software R&D

   http://www.bull.com

Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[Eric W. Biederman]

Benjamin Thery <benjamin.thery@bull.net> writes:

> Denis V. Lunev wrote:
>> The patch attached should help. The idea is simple. The "init" should be
>> called only once without NETNS. Period. No need for any lists.
>
> This is the kind of idea I had but I didn't think it could be 
> that simple. :) 
> Thanks Denis.

It isn't.

>> I'll resend it to Dave after the ACK.
>
> Tested on x86_64 with CONFIG_NET_NS=n and y. 
> It fixes the issue we observed.
>
> Acked-by: Benjamin Thery <benjamin.thery@bull.net>

Try rmmod.

Eric

Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[Benjamin Thery]

Eric W. Biederman wrote:
> Benjamin Thery <benjamin.thery@bull.net> writes:
> 
>> Denis V. Lunev wrote:
>>> The patch attached should help. The idea is simple. The "init" should be
>>> called only once without NETNS. Period. No need for any lists.
>> This is the kind of idea I had but I didn't think it could be 
>> that simple. :) 
>> Thanks Denis.
> 
> It isn't.
> 
>>> I'll resend it to Dave after the ACK.
>> Tested on x86_64 with CONFIG_NET_NS=n and y. 
>> It fixes the issue we observed.
>>
>> Acked-by: Benjamin Thery <benjamin.thery@bull.net>
> 
> Try rmmod.

rmmod was part of my tests and it does work.
I did:

$ iptables --list

  modules x_tables, ip_tables & iptable_filter are loaded
  each calling register_pernet_subsys.

$ rmmod iptable_filter ip_tables x_tables

  No problem here

$ iptables --list

  To be sure I can load the modules again.


> 
> Eric
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
B e n j a m i n   T h e r y  - BULL/DT/Open Software R&D

   http://www.bull.com

Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[Eric W. Biederman]

Benjamin Thery <benjamin.thery@bull.net> writes:

> Eric W. Biederman wrote:
>> Benjamin Thery <benjamin.thery@bull.net> writes:
>> 
>>> Denis V. Lunev wrote:
>>>> The patch attached should help. The idea is simple. The "init" should be
>>>> called only once without NETNS. Period. No need for any lists.
>>> This is the kind of idea I had but I didn't think it could be 
>>> that simple. :) 
>>> Thanks Denis.
>> 
>> It isn't.
>> 
>>>> I'll resend it to Dave after the ACK.
>>> Tested on x86_64 with CONFIG_NET_NS=n and y. 
>>> It fixes the issue we observed.
>>>
>>> Acked-by: Benjamin Thery <benjamin.thery@bull.net>
>> 
>> Try rmmod.
>
> rmmod was part of my tests and it does work.
> I did:
>
> $ iptables --list
>
>   modules x_tables, ip_tables & iptable_filter are loaded
>   each calling register_pernet_subsys.
>
> $ rmmod iptable_filter ip_tables x_tables
>
>   No problem here
>
> $ iptables --list
>
>   To be sure I can load the modules again.

You haven't changed those modules to be mark struct
pernet_operations as __net_initdata have you?

If that is the case the symptoms you are seeing make sense.

Not doing the list walks helps when if it is only compiled in
kernel data structures that are removed.  However if it
is potentially modular data structures that are removed
the dereference of exit in unregister_pernet_subsys will also have
problems.

Eric

Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[Denis V. Lunev]

Eric W. Biederman wrote:
> Benjamin Thery <benjamin.thery@bull.net> writes:
> 
>> Eric W. Biederman wrote:
>>> Benjamin Thery <benjamin.thery@bull.net> writes:
>>>
>>>> Denis V. Lunev wrote:
>>>>> The patch attached should help. The idea is simple. The "init" should be
>>>>> called only once without NETNS. Period. No need for any lists.
>>>> This is the kind of idea I had but I didn't think it could be 
>>>> that simple. :) 
>>>> Thanks Denis.
>>> It isn't.

this will work due to INIT_LIST_HEAD with circles list to itself and a
del operation will work.

By the way, I think that we can in the case of undefined CONFIG_NET_NS
reduce register to calling ->init method and unregister to calling
->exit method.

This is a correct thing at least for now and will be welcomed by the all
embedded/etc people.

Regards,
	Den

Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[Eric W. Biederman]

"Denis V. Lunev" <dlunev@gmail.com> writes:

> Eric W. Biederman wrote:
>> Benjamin Thery <benjamin.thery@bull.net> writes:
>> 
>>> Eric W. Biederman wrote:
>>>> Benjamin Thery <benjamin.thery@bull.net> writes:
>>>>
>>>>> Denis V. Lunev wrote:
>>>>>> The patch attached should help. The idea is simple. The "init" should be
>>>>>> called only once without NETNS. Period. No need for any lists.
>>>>> This is the kind of idea I had but I didn't think it could be 
>>>>> that simple. :) 
>>>>> Thanks Denis.
>>>> It isn't.
>
> this will work due to INIT_LIST_HEAD with circles list to itself and a
> del operation will work.

Suppose I have this fragment of code in a module:

> static int __net_init xt_net_init(struct net *net)
> {
>         ...
> }
> 
> static void __net_exit xt_net_exit(struct net *net)
> {
>         ...
> }
> 
> static struct pernet_operations __net_initdata  xt_net_ops = {
> 	.init = xt_net_init,
> 	.exit = xt_net_exit,
> };
> 
> static int __init xt_init(void)
> {
> 	return register_pernet_subsys(&xt_net_ops);
> }
> 
> static void __exit xt_fini(void)
> {
> 	unregister_pernet_subsys(&xt_net_ops);
> }
> 
> module_init(xt_init);
> module_exit(xt_fini);

What happens during module removal when unregister_pernet_subys calls 
xt_net_ops.exit after xt_net_ops has been removed from the kernels
memory?


> By the way, I think that we can in the case of undefined CONFIG_NET_NS
> reduce register to calling ->init method and unregister to calling
> ->exit method.
>
> This is a correct thing at least for now and will be welcomed by the all
> embedded/etc people.

I'm not fundamentally opposed.  Earlier versions of my patchset
did that and more.   However I think the pain is greater then the
gain right now.  Especially since this concept seem to require
having quality inspected into it.

Eric

Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[David Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 25 Oct 2007 11:21:55 -0600

> > By the way, I think that we can in the case of undefined CONFIG_NET_NS
> > reduce register to calling ->init method and unregister to calling
> > ->exit method.
> >
> > This is a correct thing at least for now and will be welcomed by the all
> > embedded/etc people.
> 
> I'm not fundamentally opposed.  Earlier versions of my patchset
> did that and more.   However I think the pain is greater then the
> gain right now.  Especially since this concept seem to require
> having quality inspected into it.

I think the correct thing to do for now is to simply remove these
__net_* markers and their definitions.  There are so many tricky cases
that it is easier to just get rid of them.

Could someone send me a patch which does that?

Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[Benjamin Thery]

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

David Miller wrote:
> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Thu, 25 Oct 2007 11:21:55 -0600
> 
>>> By the way, I think that we can in the case of undefined CONFIG_NET_NS
>>> reduce register to calling ->init method and unregister to calling
>>> ->exit method.
>>>
>>> This is a correct thing at least for now and will be welcomed by the all
>>> embedded/etc people.
>> I'm not fundamentally opposed.  Earlier versions of my patchset
>> did that and more.   However I think the pain is greater then the
>> gain right now.  Especially since this concept seem to require
>> having quality inspected into it.
> 
> I think the correct thing to do for now is to simply remove these
> __net_* markers and their definitions.  There are so many tricky cases
> that it is easier to just get rid of them.
> 
> Could someone send me a patch which does that?

The attached patch revert Pavel's orginal patch from 2.6.23-mm1. 
It should work fine with net-2.6 too.

Benjamin

-- 
B e n j a m i n   T h e r y  - BULL/DT/Open Software R&D

   http://www.bull.com

[-- Attachment #2: NETNS-REVERT-Move-some-code-into-__init-section-when-CONFIG_NET_NS-is-not-set.patch --]
[-- Type: text/x-patch, Size: 8595 bytes --]

This patch reverts the patch sent by Pavel Emilyanov
that introduced __net_init/__net_exit/__net_initdata defines
to save some memory when CONFIG_NET_NS=n.

http://www.spinics.net/lists/netdev/msg43310.html

When CONFIG_NET_NS=n, this later patch causes an oops when a 
netns-aware module is loaded after boot. When initialized the 
module tries to register its pernet operations and add them in 
the pernet_list. Unfortunately this list is corrupted as its 
first entries have been freed at the end of the boot sequence.

Signed-off-by: Benjamin Thery <benjamin.thery@bull.net>
---
 drivers/net/loopback.c      |    6 +++---
 fs/proc/proc_net.c          |    8 ++++----
 include/linux/init.h        |    1 -
 include/net/net_namespace.h |    9 ---------
 net/core/dev.c              |   16 ++++++++--------
 net/core/dev_mcast.c        |    6 +++---
 net/netlink/af_netlink.c    |    6 +++---
 scripts/mod/modpost.c       |    1 -
 8 files changed, 21 insertions(+), 32 deletions(-)

Index: linux-2.6.23-mm1-lxc1/drivers/net/loopback.c
===================================================================
--- linux-2.6.23-mm1-lxc1.orig/drivers/net/loopback.c
+++ linux-2.6.23-mm1-lxc1/drivers/net/loopback.c
@@ -250,7 +250,7 @@ static void loopback_setup(struct net_de
 }
 
 /* Setup and register the loopback device. */
-static __net_init int loopback_net_init(struct net *net)
+static int loopback_net_init(struct net *net)
 {
 	struct net_device *dev;
 	int err;
@@ -278,14 +278,14 @@ out_free_netdev:
 	goto out;
 }
 
-static __net_exit void loopback_net_exit(struct net *net)
+static void loopback_net_exit(struct net *net)
 {
 	struct net_device *dev = net->loopback_dev;
 
 	unregister_netdev(dev);
 }
 
-static struct pernet_operations __net_initdata loopback_net_ops = {
+static struct pernet_operations loopback_net_ops = {
        .init = loopback_net_init,
        .exit = loopback_net_exit,
 };
Index: linux-2.6.23-mm1-lxc1/fs/proc/proc_net.c
===================================================================
--- linux-2.6.23-mm1-lxc1.orig/fs/proc/proc_net.c
+++ linux-2.6.23-mm1-lxc1/fs/proc/proc_net.c
@@ -140,7 +140,7 @@ static struct inode_operations proc_net_
 	.setattr	= proc_net_setattr,
 };
 
-static __net_init int proc_net_ns_init(struct net *net)
+static int proc_net_ns_init(struct net *net)
 {
 	struct proc_dir_entry *root, *netd, *net_statd;
 	int err;
@@ -178,19 +178,19 @@ free_root:
 	goto out;
 }
 
-static __net_exit void proc_net_ns_exit(struct net *net)
+static void proc_net_ns_exit(struct net *net)
 {
 	remove_proc_entry("stat", net->proc_net);
 	remove_proc_entry("net", net->proc_net_root);
 	kfree(net->proc_net_root);
 }
 
-struct pernet_operations __net_initdata proc_net_ns_ops = {
+struct pernet_operations proc_net_ns_ops = {
 	.init = proc_net_ns_init,
 	.exit = proc_net_ns_exit,
 };
 
-int __init proc_net_init(void)
+int proc_net_init(void)
 {
 	proc_net_shadow = proc_mkdir("net", NULL);
 	proc_net_shadow->proc_iops = &proc_net_dir_inode_operations;
Index: linux-2.6.23-mm1-lxc1/include/linux/init.h
===================================================================
--- linux-2.6.23-mm1-lxc1.orig/include/linux/init.h
+++ linux-2.6.23-mm1-lxc1/include/linux/init.h
@@ -57,7 +57,6 @@
  * The markers follow same syntax rules as __init / __initdata. */
 #define __init_refok     noinline __attribute__ ((__section__ (".text.init.refok")))
 #define __initdata_refok          __attribute__ ((__section__ (".data.init.refok")))
-#define __exit_refok     noinline __attribute__ ((__section__ (".exit.text.refok")))
 
 #ifdef MODULE
 #define __exit		__attribute__ ((__section__(".exit.text"))) __cold
Index: linux-2.6.23-mm1-lxc1/include/net/net_namespace.h
===================================================================
--- linux-2.6.23-mm1-lxc1.orig/include/net/net_namespace.h
+++ linux-2.6.23-mm1-lxc1/include/net/net_namespace.h
@@ -99,15 +99,6 @@ static inline void release_net(struct ne
 #define for_each_net(VAR)				\
 	list_for_each_entry(VAR, &net_namespace_list, list)
 
-#ifdef CONFIG_NET_NS
-#define __net_init
-#define __net_exit
-#define __net_initdata
-#else
-#define __net_init	__init
-#define __net_exit	__exit_refok
-#define __net_initdata	__initdata
-#endif
 
 struct pernet_operations {
 	struct list_head list;
Index: linux-2.6.23-mm1-lxc1/net/core/dev.c
===================================================================
--- linux-2.6.23-mm1-lxc1.orig/net/core/dev.c
+++ linux-2.6.23-mm1-lxc1/net/core/dev.c
@@ -2615,7 +2615,7 @@ static const struct file_operations ptyp
 };
 
 
-static int __net_init dev_proc_net_init(struct net *net)
+static int dev_proc_net_init(struct net *net)
 {
 	int rc = -ENOMEM;
 
@@ -2640,7 +2640,7 @@ out_dev:
 	goto out;
 }
 
-static void __net_exit dev_proc_net_exit(struct net *net)
+static void dev_proc_net_exit(struct net *net)
 {
 	wext_proc_exit(net);
 
@@ -2649,7 +2649,7 @@ static void __net_exit dev_proc_net_exit
 	proc_net_remove(net, "dev");
 }
 
-static struct pernet_operations __net_initdata dev_proc_ops = {
+static struct pernet_operations dev_proc_ops = {
 	.init = dev_proc_net_init,
 	.exit = dev_proc_net_exit,
 };
@@ -4280,7 +4280,7 @@ static struct hlist_head *netdev_create_
 }
 
 /* Initialize per network namespace state */
-static int __net_init netdev_init(struct net *net)
+static int netdev_init(struct net *net)
 {
 	INIT_LIST_HEAD(&net->dev_base_head);
 	rwlock_init(&dev_base_lock);
@@ -4301,18 +4301,18 @@ err_name:
 	return -ENOMEM;
 }
 
-static void __net_exit netdev_exit(struct net *net)
+static void netdev_exit(struct net *net)
 {
 	kfree(net->dev_name_head);
 	kfree(net->dev_index_head);
 }
 
-static struct pernet_operations __net_initdata netdev_net_ops = {
+static struct pernet_operations netdev_net_ops = {
 	.init = netdev_init,
 	.exit = netdev_exit,
 };
 
-static void __net_exit default_device_exit(struct net *net)
+static void default_device_exit(struct net *net)
 {
 	struct net_device *dev, *next;
 	/*
@@ -4338,7 +4338,7 @@ static void __net_exit default_device_ex
 	rtnl_unlock();
 }
 
-static struct pernet_operations __net_initdata default_device_ops = {
+static struct pernet_operations default_device_ops = {
 	.exit = default_device_exit,
 };
 
Index: linux-2.6.23-mm1-lxc1/net/core/dev_mcast.c
===================================================================
--- linux-2.6.23-mm1-lxc1.orig/net/core/dev_mcast.c
+++ linux-2.6.23-mm1-lxc1/net/core/dev_mcast.c
@@ -273,19 +273,19 @@ static const struct file_operations dev_
 
 #endif
 
-static int __net_init dev_mc_net_init(struct net *net)
+static int dev_mc_net_init(struct net *net)
 {
 	if (!proc_net_fops_create(net, "dev_mcast", 0, &dev_mc_seq_fops))
 		return -ENOMEM;
 	return 0;
 }
 
-static void __net_exit dev_mc_net_exit(struct net *net)
+static void dev_mc_net_exit(struct net *net)
 {
 	proc_net_remove(net, "dev_mcast");
 }
 
-static struct pernet_operations __net_initdata dev_mc_net_ops = {
+static struct pernet_operations dev_mc_net_ops = {
 	.init = dev_mc_net_init,
 	.exit = dev_mc_net_exit,
 };
Index: linux-2.6.23-mm1-lxc1/net/netlink/af_netlink.c
===================================================================
--- linux-2.6.23-mm1-lxc1.orig/net/netlink/af_netlink.c
+++ linux-2.6.23-mm1-lxc1/net/netlink/af_netlink.c
@@ -1915,7 +1915,7 @@ static struct net_proto_family netlink_f
 	.owner	= THIS_MODULE,	/* for consistency 8) */
 };
 
-static int __net_init netlink_net_init(struct net *net)
+static int netlink_net_init(struct net *net)
 {
 #ifdef CONFIG_PROC_FS
 	if (!proc_net_fops_create(net, "netlink", 0, &netlink_seq_fops))
@@ -1924,14 +1924,14 @@ static int __net_init netlink_net_init(s
 	return 0;
 }
 
-static void __net_exit netlink_net_exit(struct net *net)
+static void netlink_net_exit(struct net *net)
 {
 #ifdef CONFIG_PROC_FS
 	proc_net_remove(net, "netlink");
 #endif
 }
 
-static struct pernet_operations __net_initdata netlink_net_ops = {
+static struct pernet_operations netlink_net_ops = {
 	.init = netlink_net_init,
 	.exit = netlink_net_exit,
 };
Index: linux-2.6.23-mm1-lxc1/scripts/mod/modpost.c
===================================================================
--- linux-2.6.23-mm1-lxc1.orig/scripts/mod/modpost.c
+++ linux-2.6.23-mm1-lxc1/scripts/mod/modpost.c
@@ -715,7 +715,6 @@ static int secref_whitelist(const char *
 
 	/* Check for pattern 0 */
 	if ((strncmp(fromsec, ".text.init.refok", strlen(".text.init.refok")) == 0) ||
-	    (strncmp(fromsec, ".exit.text.refok", strlen(".exit.text.refok")) == 0) ||
 	    (strncmp(fromsec, ".data.init.refok", strlen(".data.init.refok")) == 0))
 		return 1;
 

Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[David Miller]

From: Benjamin Thery <benjamin.thery@bull.net>
Date: Fri, 26 Oct 2007 13:41:55 +0200

> David Miller wrote:
> > From: ebiederm@xmission.com (Eric W. Biederman)
> > Date: Thu, 25 Oct 2007 11:21:55 -0600
> > 
> >>> By the way, I think that we can in the case of undefined CONFIG_NET_NS
> >>> reduce register to calling ->init method and unregister to calling
> >>> ->exit method.
> >>>
> >>> This is a correct thing at least for now and will be welcomed by the all
> >>> embedded/etc people.
> >> I'm not fundamentally opposed.  Earlier versions of my patchset
> >> did that and more.   However I think the pain is greater then the
> >> gain right now.  Especially since this concept seem to require
> >> having quality inspected into it.
> > 
> > I think the correct thing to do for now is to simply remove these
> > __net_* markers and their definitions.  There are so many tricky cases
> > that it is easier to just get rid of them.
> > 
> > Could someone send me a patch which does that?
> 
> The attached patch revert Pavel's orginal patch from 2.6.23-mm1. 
> It should work fine with net-2.6 too.

Thanks for doing this.

But this appears to be still discussed, so I'll give
Denis and others another day to work out the fix they
want to include.

Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[Eric W. Biederman]

David Miller <davem@davemloft.net> writes:

> Thanks for doing this.
>
> But this appears to be still discussed, so I'll give
> Denis and others another day to work out the fix they
> want to include.

At this point I think all that really needs to happen is to remove
__net_initdata.  The function attributes seem sane.  Not that
I have any problem with the complete revert either.

I will send a patch to that effect in just a moment.

Eric

[PATCH] net: Marking struct pernet_operations __net_initdata was inappropriate

[Eric W. Biederman]

It is not safe to to place struct pernet_operations in a special section.
We need struct pernet_operations to last until we call unregister_pernet_subsys.
Which doesn't happen until module unload.

So marking struct pernet_operations is a disaster for modules in two ways.
- We discard it before we call the exit method it points to.
- Because I keep struct pernet_operations on a linked list discarding
  it for compiled in code removes elements in the middle of a linked
  list and does horrible things for linked insert.

So this looks safe assuming __exit_refok is not discarded
for modules.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 drivers/net/loopback.c      |    2 +-
 fs/proc/proc_net.c          |    2 +-
 include/net/net_namespace.h |    2 --
 net/core/dev.c              |    6 +++---
 net/core/dev_mcast.c        |    2 +-
 net/netlink/af_netlink.c    |    2 +-
 6 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 662b8d1..45f30a2 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -284,7 +284,7 @@ static __net_exit void loopback_net_exit(struct net *net)
 	unregister_netdev(dev);
 }
 
-static struct pernet_operations __net_initdata loopback_net_ops = {
+static struct pernet_operations loopback_net_ops = {
        .init = loopback_net_init,
        .exit = loopback_net_exit,
 };
diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
index 4edaad0..749def0 100644
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -185,7 +185,7 @@ static __net_exit void proc_net_ns_exit(struct net *net)
 	kfree(net->proc_net_root);
 }
 
-static struct pernet_operations __net_initdata proc_net_ns_ops = {
+static struct pernet_operations proc_net_ns_ops = {
 	.init = proc_net_ns_init,
 	.exit = proc_net_ns_exit,
 };
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 93aa87d..5279466 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -102,11 +102,9 @@ static inline void release_net(struct net *net)
 #ifdef CONFIG_NET_NS
 #define __net_init
 #define __net_exit
-#define __net_initdata
 #else
 #define __net_init	__init
 #define __net_exit	__exit_refok
-#define __net_initdata	__initdata
 #endif
 
 struct pernet_operations {
diff --git a/net/core/dev.c b/net/core/dev.c
index ddfef3b..853c8b5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2668,7 +2668,7 @@ static void __net_exit dev_proc_net_exit(struct net *net)
 	proc_net_remove(net, "dev");
 }
 
-static struct pernet_operations __net_initdata dev_proc_ops = {
+static struct pernet_operations dev_proc_ops = {
 	.init = dev_proc_net_init,
 	.exit = dev_proc_net_exit,
 };
@@ -4328,7 +4328,7 @@ static void __net_exit netdev_exit(struct net *net)
 	kfree(net->dev_index_head);
 }
 
-static struct pernet_operations __net_initdata netdev_net_ops = {
+static struct pernet_operations  netdev_net_ops = {
 	.init = netdev_init,
 	.exit = netdev_exit,
 };
@@ -4359,7 +4359,7 @@ static void __net_exit default_device_exit(struct net *net)
 	rtnl_unlock();
 }
 
-static struct pernet_operations __net_initdata default_device_ops = {
+static struct pernet_operations  default_device_ops = {
 	.exit = default_device_exit,
 };
 
diff --git a/net/core/dev_mcast.c b/net/core/dev_mcast.c
index 15241cf..ae35405 100644
--- a/net/core/dev_mcast.c
+++ b/net/core/dev_mcast.c
@@ -285,7 +285,7 @@ static void __net_exit dev_mc_net_exit(struct net *net)
 	proc_net_remove(net, "dev_mcast");
 }
 
-static struct pernet_operations __net_initdata dev_mc_net_ops = {
+static struct pernet_operations dev_mc_net_ops = {
 	.init = dev_mc_net_init,
 	.exit = dev_mc_net_exit,
 };
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 3252729..4f994c0 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1888,7 +1888,7 @@ static void __net_exit netlink_net_exit(struct net *net)
 #endif
 }
 
-static struct pernet_operations __net_initdata netlink_net_ops = {
+static struct pernet_operations netlink_net_ops = {
 	.init = netlink_net_init,
 	.exit = netlink_net_exit,
 };
-- 
1.5.3.rc6.17.g1911

Re: [PATCH] net: Marking struct pernet_operations __net_initdata was inappropriate

[David Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Fri, 26 Oct 2007 17:45:33 -0600

> 
> It is not safe to to place struct pernet_operations in a special section.
> We need struct pernet_operations to last until we call unregister_pernet_subsys.
> Which doesn't happen until module unload.
> 
> So marking struct pernet_operations is a disaster for modules in two ways.
> - We discard it before we call the exit method it points to.
> - Because I keep struct pernet_operations on a linked list discarding
>   it for compiled in code removes elements in the middle of a linked
>   list and does horrible things for linked insert.
> 
> So this looks safe assuming __exit_refok is not discarded
> for modules.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>

Applied, thanks Eric.

Although juding by his comments I though that Denis had different
plans in mind to fix this.

Re: [PATCH] net: Marking struct pernet_operations __net_initdata was inappropriate

[Eric W. Biederman]

David Miller <davem@davemloft.net> writes:
>
> Applied, thanks Eric.
>
> Although juding by his comments I though that Denis had different
> plans in mind to fix this.

He might.  Somehow I wasn't on that thread so I missed it until after
I sent this patch.  Reading through that thread  again it looks like
he had a thought for another attribute.  It all sounded very clever.

This patch is minimal stupid and should just work.  Doubtless the
clever patch can be applied on top, once the details are figured
out.

Eric

Re: [PATCH] net: Marking struct pernet_operations __net_initdata was inappropriate

[David Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Sat, 27 Oct 2007 00:07:12 -0600

> This patch is minimal stupid and should just work.  Doubtless the
> clever patch can be applied on top, once the details are figured
> out.

That is true and that's why I applied your patch.

Thanks!

Re: [NETNS] Oops in register_pernet_operations() with CONFIG_NET_NS=n

[Eric W. Biederman]

"Denis V. Lunev" <den@sw.ru> writes:

> The patch attached should help. The idea is simple. The "init" should be
> called only once without NETNS. Period. No need for any lists.
>
> I'll resend it to Dave after the ACK.

First in the case of the code that is currently merged none of 
the __net_init __net_exit or __net_initdata can be modular, so for
2.6.24 there is no fix needed. Yeah.

Second the whole concept of concept pernet_operations being __init
doesn't work when you have modular code that calls unregister_pernet_subsys().
Because unregister calls the exit method from the pernet_operations
structure.  So the patch doesn't even begin to address the real
issue.

Third from my perspective CONFIG_NET_NS is a temporary measure
designed to last only until we have enough implementation experience 
so that we can feel comfortable removing the experimental status of
the network namespace work.  It was not my intention for it to be a
space saving measure.  So I think it is silly to go marking up the
patches in development with __net_initdata etc.

At least so far I think __net_initdata is a totally bogus concept
and I'm not certain about the other two. 

Eric