fix kbuild warning in iptable_nat.o

As iptable_nat.o seems to be compiled into the kernel (from ip_nat_rule.c and ip_nat_standalone.c) by default, I don't think we need the __exit function (correct me if I am wrong).

G'day list,

Sorry but I just noticed that the last patch removed a static from the init function. It really should have one, so here is an updated patch.

WARNING: net/ipv4/netfilter/iptable_nat.o - Section mismatch: reference to .init.text:ip_nat_rule_init from .text between 'init_or_cleanup' (at offset 0x455) and 'nat_decode_session'

This seems to be caused by init_or_cleanup() in net/ip4/netfilter/ip_nat_standalone.c calling ip_nat_rule_init() in net/ip4/netfilter/ip_nat_rule.c.

The problem is that init_or_cleanup() is called from both __init and __exit functions, so is not marked as __init.

As iptable_nat.o seems to be compiled into the kernel (from ip_nat_rule.c and ip_nat_standalone.c) by default, I don't think we need the __exit function (correct me if I am wrong).

So the patch below removes the unneeded module unload stuff and renames init_or_cleanup() to init() and marks it __init.

Signed-off-by Darren Jenkins <[email protected]>

ipv4/netfilter/ip_nat_standalone.c renamed from ipv4/netfilter/ip_nat_standalone.c.orig +2 -10

@@ -354,14 +354,12 @@ static struct nf_hook_ops ip_nat_adjust_
 };
 
 
-static int init_or_cleanup(int init)
+static int __init init(void)
 {
-	int ret = 0;
+	int ret;
 
 	need_conntrack();
 
-	if (!init) goto cleanup;
-
 #ifdef CONFIG_XFRM
 	BUG_ON(ip_nat_decode_session != NULL);
 	ip_nat_decode_session = nat_decode_session;

@@ -403,8 +401,6 @@ static int init_or_cleanup(int init)
 	}
 	return ret;
 
- cleanup:
-	nf_unregister_hook(&ip_nat_local_in_ops);
  cleanup_localoutops:
 	nf_unregister_hook(&ip_nat_local_out_ops);
  cleanup_adjustout_ops:

@@ -425,17 +421,7 @@ static int init_or_cleanup(int init)
 	return ret;
 }
 
-static int __init init(void)
-{
-	return init_or_cleanup(1);
-}

Quoted 5 lines · On Tue, 28 Mar 2006 23:19:33 +1100 Darren Jenkins\\ wrote:

G'day list,

Sorry but I just noticed that the last patch removed a static from the init function. It really should have one, so here is an updated patch.

Quoted 7 lines

WARNING: net/ipv4/netfilter/iptable_nat.o - Section mismatch: reference to .init.text:ip_nat_rule_init from .text between 'init_or_cleanup' (at offset 0x455) and 'nat_decode_session'

This seems to be caused by init_or_cleanup() in net/ip4/netfilter/ip_nat_standalone.c calling ip_nat_rule_init() in net/ip4/netfilter/ip_nat_rule.c.

Yes.

Quoted 2 lines

The problem is that init_or_cleanup() is called from both __init and __exit functions, so is not marked as __init.

Right, it cannot be marked __init.

Quoted 3 lines

As iptable_nat.o seems to be compiled into the kernel (from ip_nat_rule.c and ip_nat_standalone.c) by default, I don't think we need the __exit function (correct me if I am wrong).

That may be safe in this case but not in general. And I don't know enough about this case to say if it is or not.

What would happen if you simply removed /__init/ from ip_nat_rule_init()?

Quoted 47 lines

So the patch below removes the unneeded module unload stuff and renames init_or_cleanup() to init() and marks it __init.

Signed-off-by Darren Jenkins <[email protected]>

ipv4/netfilter/ip_nat_standalone.c renamed from ipv4/netfilter/ip_nat_standalone.c.orig +2 -15

@@ -354,14 +354,12 @@ static struct nf_hook_ops ip_nat_adjust_
 };
 
 
-static int init_or_cleanup(int init)
+static int __init init(void)
 {
-	int ret = 0;
+	int ret;
 
 	need_conntrack();
 
-	if (!init) goto cleanup;
-
 #ifdef CONFIG_XFRM
 	BUG_ON(ip_nat_decode_session != NULL);
 	ip_nat_decode_session = nat_decode_session;

@@ -403,8 +401,6 @@ static int init_or_cleanup(int init)
 	}
 	return ret;
 
- cleanup:
-	nf_unregister_hook(&ip_nat_local_in_ops);
  cleanup_localoutops:
 	nf_unregister_hook(&ip_nat_local_out_ops);
  cleanup_adjustout_ops:

@@ -425,17 +421,7 @@ static int init_or_cleanup(int init)
 	return ret;
 }
 
-static int __init init(void)
-{
-	return init_or_cleanup(1);
-}
-
-static void __exit fini(void)
-{
-	init_or_cleanup(0);
-}
 

ipv4/netfilter/ip_nat_standalone.c static int init_or_cleanup(int init) · line 427

Quoted 4 lines

ipv4/netfilter/ip_nat_standalone.c renamed from ipv4/netfilter/ip_nat_standalone.c.orig +0 -1

 module_init(init);
-module_exit(fini);
 
 MODULE_LICENSE("GPL");

Quoted 22 lines · On Tue, 28 Mar 2006 09:18:56 -0800 Randy.Dunlap wrote:

Quoted 5 lines · On Tue, 28 Mar 2006 23:19:33 +1100 Darren Jenkins\\ wrote:

G'day list,

Sorry but I just noticed that the last patch removed a static from the init function. It really should have one, so here is an updated patch.

Quoted 7 lines

WARNING: net/ipv4/netfilter/iptable_nat.o - Section mismatch: reference to .init.text:ip_nat_rule_init from .text between 'init_or_cleanup' (at offset 0x455) and 'nat_decode_session'

This seems to be caused by init_or_cleanup() in net/ip4/netfilter/ip_nat_standalone.c calling ip_nat_rule_init() in net/ip4/netfilter/ip_nat_rule.c.

Yes.

Quoted 2 lines

The problem is that init_or_cleanup() is called from both __init and __exit functions, so is not marked as __init.

Right, it cannot be marked __init.

Quoted 8 lines

Quoted 3 lines

As iptable_nat.o seems to be compiled into the kernel (from ip_nat_rule.c and ip_nat_standalone.c) by default, I don't think we need the __exit function (correct me if I am wrong).

That may be safe in this case but not in general. And I don't know enough about this case to say if it is or not.

What would happen if you simply removed /__init/ from ip_nat_rule_init()?

Then there is an __initdata section problem. Hrm.
So the problem is that the error/exit ending code in init_or_cleanup() has a dual role. I think that some parts of your original patch make sense, but that you cannot just drop the exit function.

ISTM that you picked a messy one to start on. 8(
I'll look a bit more to see if I have any ideas.

Quoted 2 lines

Quoted 2 lines

So the patch below removes the unneeded module unload stuff and renames init_or_cleanup() to init() and marks it __init.

On 28.03.2006 [23:19:33 +1100], Darren Jenkins" wrote:

<snip>

Quoted 3 lines

As iptable_nat.o seems to be compiled into the kernel (from ip_nat_rule.c and ip_nat_standalone.c) by default, I don't think we need the __exit function (correct me if I am wrong).

<snip>

What happens when we do build NAT filtering support as a module? Just because the default is non-modular, does not mean we can ignore the option.

Thanks,
Nish

Quoted 8 lines · Nishanth Aravamudan

On 28.03.2006 [23:19:33 +1100], Darren Jenkins" wrote:

<snip>

Quoted 3 lines

As iptable_nat.o seems to be compiled into the kernel (from ip_nat_rule.c and ip_nat_standalone.c) by default, I don't think we need the __exit function (correct me if I am wrong).

Quoted 9 lines

<snip>

What happens when we do build NAT filtering support as a module? Just because the default is non-modular, does not mean we can ignore the option.

Thanks,
Nish

Actually I thought that as in the Makefile, it does not depend on any config options to be built. That it would only ever be compiled into the kernel.
Thinking about it, I guess it could be a module in the next higher level Makefile, and I should probably read up on how a Makefile works anyway. :)

Darren J.