[KJ][Patch][update] fix kbuild warning in iptable_nat.o

[KJ][Patch][update] fix kbuild warning in iptable_nat.o

[Darren Jenkins\]

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

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 <darrenrjenkins@gmail.com>

--- net/ipv4/netfilter/ip_nat_standalone.c.orig	2006-03-28 21:38:33.000000000 +1100
+++ net/ipv4/netfilter/ip_nat_standalone.c	2006-03-28 23:12:37.000000000 +1100
@@ -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);
-}
 
 module_init(init);
-module_exit(fini);
 
 MODULE_LICENSE("GPL");



[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ][Patch][update] fix kbuild warning in iptable_nat.o

[Nishanth Aravamudan]

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

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

<snip>

> 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

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ][Patch][update] fix kbuild warning in iptable_nat.o

[Randy.Dunlap]

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

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.
> 
> 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.

> 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.

> 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()?


> 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 <darrenrjenkins@gmail.com>
> 
> --- net/ipv4/netfilter/ip_nat_standalone.c.orig	2006-03-28 21:38:33.000000000 +1100
> +++ net/ipv4/netfilter/ip_nat_standalone.c	2006-03-28 23:12:37.000000000 +1100
> @@ -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);
> -}
>  
>  module_init(init);
> -module_exit(fini);
>  
>  MODULE_LICENSE("GPL");

---
~Randy

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ][Patch][update] fix kbuild warning in iptable_nat.o

[Randy.Dunlap]

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

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

> 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.
> > 
> > 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.
> 
> > 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.
> 
> > 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.


> > 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 <darrenrjenkins@gmail.com>
> > 
> > --- net/ipv4/netfilter/ip_nat_standalone.c.orig	2006-03-28 21:38:33.000000000 +1100
> > +++ net/ipv4/netfilter/ip_nat_standalone.c	2006-03-28 23:12:37.000000000 +1100
> > @@ -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);
> > -}
> >  
> >  module_init(init);
> > -module_exit(fini);
> >  
> >  MODULE_LICENSE("GPL");


---
~Randy

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ][Patch][update] fix kbuild warning in iptable_nat.o

[Randy.Dunlap]

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

On Tue, 28 Mar 2006 09:45:42 -0800 Randy.Dunlap wrote:

> On Tue, 28 Mar 2006 09:18:56 -0800 Randy.Dunlap wrote:
> 
> > 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.
> > > 
> > > 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.
> > 
> > > 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.
> > 
> > > 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.

The cleanest thing that I can see to do is to separate the init and exit
functions (like you have done somewhat) and duplicate the exit/cleanup code
in its own function (but without the labels:).
And put a comment on the exit function that it must (or should) be the same
as the init function's error/cleanup path.

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

---
~Randy

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ][Patch][update] fix kbuild warning in iptable_nat.o

[Darren Jenkins\]

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

On Tue, 2006-03-28 at 09:06 -0800, Nishanth Aravamudan wrote:
> On 28.03.2006 [23:19:33 +1100], Darren Jenkins" wrote:
> 
> <snip>
> 
> > 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
> 

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.


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ][Patch][update] fix kbuild warning in iptable_nat.o

[Adrian Bunk]

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

On Wed, Mar 29, 2006 at 08:48:14PM +1100, Darren Jenkins" wrote:
> On Tue, 2006-03-28 at 09:06 -0800, Nishanth Aravamudan wrote:
> > On 28.03.2006 [23:19:33 +1100], Darren Jenkins" wrote:
> > 
> > <snip>
> > 
> > > 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
> > 
> 
> 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. :)

It's actually in the same Makefile:

...
iptable_nat-objs        := ip_nat_rule.o ip_nat_standalone.o
...
obj-$(CONFIG_IP_NF_NAT) += iptable_nat.o
...


The first statement expresses that iptable_nat is built from two object 
files.

The second statement tells that iptable_nat is built depending on 
CONFIG_IP_NF_NAT.

CONFIG_IP_NF_NAT is a tristate.


> Darren J.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors