[PATCH/RFC] [NETFILTER]: Fix invalid module autoloading by splitting iptable_nat

[PATCH/RFC] [NETFILTER]: Fix invalid module autoloading by splitting iptable_nat

[Harald Welte]

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

Hi Patrick, Dave,

I think we really need a solution for the last (known) remaining
dependency problem with 2.6.14.  Please see the description
below.  I _think_ the patch is fine, at least I couldn't find any case
where we could leak anything by splitting the code in two modules.

There's a slight semantic change, though.  If the user unloads
iptable_nat, all existing connections (including their configured NAT
mappings) will continue to work.  Only when ip_nat.ko is unloaded, the
NAT mappings are evicted from the conntrack table.  I like it that way,
since it's logical.

If Patrick has no objections, I vote for inclusion in 2.6.14.

Thanks!


[NETFILTER]: Fix invalid module autoloading by splitting iptable_nat

When you've enabled conntrack and NAT as a module (standard case in all
distributions), and you've also enabled the new conntrack netlink
interface, loading ip_conntrack_netlink.ko will auto-load iptable_nat.ko.
This causes a huge performance penalty, since for every packet you iterate
the nat code, even if you don't want it.

This patch splits iptable_nat.ko into the NAT core (ip_nat.ko) and the
iptables frontend (iptable_nat.ko).  Threfore, ip_conntrack_netlink.ko will
only pull ip_nat.ko, but not the frontend.  ip_nat.ko will "only" allocate
some resources, but not affect runtime performance.

This separation is also a nice step in anticipation of new packet filters
(nf-hipac, ipset, pkttables) being able to use the NAT core.

---
commit 5ad0f056192213b2f6ac9910f754be9e6abea574
tree b06756b046853df61b39139ae58d49d0ca84274c
parent 1699521ab4e23ba4dd9f8bdd894393b8e109fe43
author Harald Welte <laforge@hanuman.de.gnumonks.org> Sun, 25 Sep 2005 17:01:38 +0200
committer Harald Welte <laforge@hanuman.de.gnumonks.org> Sun, 25 Sep 2005 17:01:38 +0200

 include/linux/netfilter_ipv4/ip_nat_core.h |   12 ++++------
 net/ipv4/netfilter/Makefile                |    5 ++--
 net/ipv4/netfilter/ip_nat_core.c           |   35 +++++++++++++++++++---------
 net/ipv4/netfilter/ip_nat_helper.c         |    4 +++
 net/ipv4/netfilter/ip_nat_standalone.c     |   25 +++-----------------
 5 files changed, 40 insertions(+), 41 deletions(-)

diff --git a/include/linux/netfilter_ipv4/ip_nat_core.h b/include/linux/netfilter_ipv4/ip_nat_core.h
--- a/include/linux/netfilter_ipv4/ip_nat_core.h
+++ b/include/linux/netfilter_ipv4/ip_nat_core.h
@@ -5,16 +5,14 @@
 
 /* This header used to share core functionality between the standalone
    NAT module, and the compatibility layer's use of NAT for masquerading. */
-extern int ip_nat_init(void);
-extern void ip_nat_cleanup(void);
 
-extern unsigned int nat_packet(struct ip_conntrack *ct,
+extern unsigned int ip_nat_packet(struct ip_conntrack *ct,
 			       enum ip_conntrack_info conntrackinfo,
 			       unsigned int hooknum,
 			       struct sk_buff **pskb);
 
-extern int icmp_reply_translation(struct sk_buff **pskb,
-				  struct ip_conntrack *ct,
-				  enum ip_nat_manip_type manip,
-				  enum ip_conntrack_dir dir);
+extern int ip_nat_icmp_reply_translation(struct sk_buff **pskb,
+					 struct ip_conntrack *ct,
+					 enum ip_nat_manip_type manip,
+					 enum ip_conntrack_dir dir);
 #endif /* _IP_NAT_CORE_H */
diff --git a/net/ipv4/netfilter/Makefile b/net/ipv4/netfilter/Makefile
--- a/net/ipv4/netfilter/Makefile
+++ b/net/ipv4/netfilter/Makefile
@@ -4,7 +4,8 @@
 
 # objects for the standalone - connection tracking / NAT
 ip_conntrack-objs	:= ip_conntrack_standalone.o ip_conntrack_core.o ip_conntrack_proto_generic.o ip_conntrack_proto_tcp.o ip_conntrack_proto_udp.o ip_conntrack_proto_icmp.o
-iptable_nat-objs	:= ip_nat_standalone.o ip_nat_rule.o ip_nat_core.o ip_nat_helper.o ip_nat_proto_unknown.o ip_nat_proto_tcp.o ip_nat_proto_udp.o ip_nat_proto_icmp.o
+ip_nat-objs	:= ip_nat_core.o ip_nat_helper.o ip_nat_proto_unknown.o ip_nat_proto_tcp.o ip_nat_proto_udp.o ip_nat_proto_icmp.o
+iptable_nat-objs	:= ip_nat_rule.o ip_nat_standalone.o
 
 ip_conntrack_pptp-objs	:= ip_conntrack_helper_pptp.o ip_conntrack_proto_gre.o
 ip_nat_pptp-objs	:= ip_nat_helper_pptp.o ip_nat_proto_gre.o
@@ -40,7 +41,7 @@ obj-$(CONFIG_IP_NF_IPTABLES) += ip_table
 # the three instances of ip_tables
 obj-$(CONFIG_IP_NF_FILTER) += iptable_filter.o
 obj-$(CONFIG_IP_NF_MANGLE) += iptable_mangle.o
-obj-$(CONFIG_IP_NF_NAT) += iptable_nat.o
+obj-$(CONFIG_IP_NF_NAT) += iptable_nat.o ip_nat.o
 obj-$(CONFIG_IP_NF_RAW) += iptable_raw.o
 
 # matches
diff --git a/net/ipv4/netfilter/ip_nat_core.c b/net/ipv4/netfilter/ip_nat_core.c
--- a/net/ipv4/netfilter/ip_nat_core.c
+++ b/net/ipv4/netfilter/ip_nat_core.c
@@ -74,12 +74,14 @@ ip_nat_proto_find_get(u_int8_t protonum)
 
 	return p;
 }
+EXPORT_SYMBOL_GPL(ip_nat_proto_find_get);
 
 void
 ip_nat_proto_put(struct ip_nat_protocol *p)
 {
 	module_put(p->me);
 }
+EXPORT_SYMBOL_GPL(ip_nat_proto_put);
 
 /* We keep an extra hash for each conntrack, for fast searching. */
 static inline unsigned int
@@ -111,6 +113,7 @@ ip_nat_cheat_check(u_int32_t oldvalinv, 
 	return csum_fold(csum_partial((char *)diffs, sizeof(diffs),
 				      oldcheck^0xFFFF));
 }
+EXPORT_SYMBOL(ip_nat_cheat_check);
 
 /* Is this tuple already taken? (not by us) */
 int
@@ -127,6 +130,7 @@ ip_nat_used_tuple(const struct ip_conntr
 	invert_tuplepr(&reply, tuple);
 	return ip_conntrack_tuple_taken(&reply, ignored_conntrack);
 }
+EXPORT_SYMBOL(ip_nat_used_tuple);
 
 /* If we source map this tuple so reply looks like reply_tuple, will
  * that meet the constraints of range. */
@@ -347,6 +351,7 @@ ip_nat_setup_info(struct ip_conntrack *c
 
 	return NF_ACCEPT;
 }
+EXPORT_SYMBOL(ip_nat_setup_info);
 
 /* Returns true if succeeded. */
 static int
@@ -387,10 +392,10 @@ manip_pkt(u_int16_t proto,
 }
 
 /* Do packet manipulations according to ip_nat_setup_info. */
-unsigned int nat_packet(struct ip_conntrack *ct,
-			enum ip_conntrack_info ctinfo,
-			unsigned int hooknum,
-			struct sk_buff **pskb)
+unsigned int ip_nat_packet(struct ip_conntrack *ct,
+			   enum ip_conntrack_info ctinfo,
+			   unsigned int hooknum,
+			   struct sk_buff **pskb)
 {
 	enum ip_conntrack_dir dir = CTINFO2DIR(ctinfo);
 	unsigned long statusbit;
@@ -417,12 +422,13 @@ unsigned int nat_packet(struct ip_conntr
 	}
 	return NF_ACCEPT;
 }
+EXPORT_SYMBOL_GPL(ip_nat_packet);
 
 /* Dir is direction ICMP is coming from (opposite to packet it contains) */
-int icmp_reply_translation(struct sk_buff **pskb,
-			   struct ip_conntrack *ct,
-			   enum ip_nat_manip_type manip,
-			   enum ip_conntrack_dir dir)
+int ip_nat_icmp_reply_translation(struct sk_buff **pskb,
+				  struct ip_conntrack *ct,
+				  enum ip_nat_manip_type manip,
+				  enum ip_conntrack_dir dir)
 {
 	struct {
 		struct icmphdr icmp;
@@ -509,6 +515,7 @@ int icmp_reply_translation(struct sk_buf
 
 	return 1;
 }
+EXPORT_SYMBOL_GPL(ip_nat_icmp_reply_translation);
 
 /* Protocol registration. */
 int ip_nat_protocol_register(struct ip_nat_protocol *proto)
@@ -525,6 +532,7 @@ int ip_nat_protocol_register(struct ip_n
 	write_unlock_bh(&ip_nat_lock);
 	return ret;
 }
+EXPORT_SYMBOL(ip_nat_protocol_register);
 
 /* Noone stores the protocol anywhere; simply delete it. */
 void ip_nat_protocol_unregister(struct ip_nat_protocol *proto)
@@ -536,6 +544,7 @@ void ip_nat_protocol_unregister(struct i
 	/* Someone could be still looking at the proto in a bh. */
 	synchronize_net();
 }
+EXPORT_SYMBOL(ip_nat_protocol_unregister);
 
 #if defined(CONFIG_IP_NF_CONNTRACK_NETLINK) || \
     defined(CONFIG_IP_NF_CONNTRACK_NETLINK_MODULE)
@@ -582,7 +591,7 @@ EXPORT_SYMBOL_GPL(ip_nat_port_nfattr_to_
 EXPORT_SYMBOL_GPL(ip_nat_port_range_to_nfattr);
 #endif
 
-int __init ip_nat_init(void)
+static int __init ip_nat_init(void)
 {
 	size_t i;
 
@@ -624,10 +633,14 @@ static int clean_nat(struct ip_conntrack
 	return 0;
 }
 
-/* Not __exit: called from ip_nat_standalone.c:init_or_cleanup() --RR */
-void ip_nat_cleanup(void)
+static void __exit ip_nat_cleanup(void)
 {
 	ip_ct_iterate_cleanup(&clean_nat, NULL);
 	ip_conntrack_destroyed = NULL;
 	vfree(bysource);
 }
+
+MODULE_LICENSE("GPL");
+
+module_init(ip_nat_init);
+module_exit(ip_nat_cleanup);
diff --git a/net/ipv4/netfilter/ip_nat_helper.c b/net/ipv4/netfilter/ip_nat_helper.c
--- a/net/ipv4/netfilter/ip_nat_helper.c
+++ b/net/ipv4/netfilter/ip_nat_helper.c
@@ -199,6 +199,7 @@ ip_nat_mangle_tcp_packet(struct sk_buff 
 	}
 	return 1;
 }
+EXPORT_SYMBOL(ip_nat_mangle_tcp_packet);
 			
 /* Generic function for mangling variable-length address changes inside
  * NATed UDP connections (like the CONNECT DATA XXXXX MESG XXXXX INDEX XXXXX
@@ -256,6 +257,7 @@ ip_nat_mangle_udp_packet(struct sk_buff 
 
 	return 1;
 }
+EXPORT_SYMBOL(ip_nat_mangle_udp_packet);
 
 /* Adjust one found SACK option including checksum correction */
 static void
@@ -399,6 +401,7 @@ ip_nat_seq_adjust(struct sk_buff **pskb,
 
 	return 1;
 }
+EXPORT_SYMBOL(ip_nat_seq_adjust);
 
 /* Setup NAT on this expected conntrack so it follows master. */
 /* If we fail to get a free NAT slot, we'll get dropped on confirm */
@@ -425,3 +428,4 @@ void ip_nat_follow_master(struct ip_conn
 	/* hook doesn't matter, but it has to do destination manip */
 	ip_nat_setup_info(ct, &range, NF_IP_PRE_ROUTING);
 }
+EXPORT_SYMBOL(ip_nat_follow_master);
diff --git a/net/ipv4/netfilter/ip_nat_standalone.c b/net/ipv4/netfilter/ip_nat_standalone.c
--- a/net/ipv4/netfilter/ip_nat_standalone.c
+++ b/net/ipv4/netfilter/ip_nat_standalone.c
@@ -108,8 +108,8 @@ ip_nat_fn(unsigned int hooknum,
 	case IP_CT_RELATED:
 	case IP_CT_RELATED+IP_CT_IS_REPLY:
 		if ((*pskb)->nh.iph->protocol == IPPROTO_ICMP) {
-			if (!icmp_reply_translation(pskb, ct, maniptype,
-						    CTINFO2DIR(ctinfo)))
+			if (!ip_nat_icmp_reply_translation(pskb, ct, maniptype,
+							   CTINFO2DIR(ctinfo)))
 				return NF_DROP;
 			else
 				return NF_ACCEPT;
@@ -152,7 +152,7 @@ ip_nat_fn(unsigned int hooknum,
 	}
 
 	IP_NF_ASSERT(info);
-	return nat_packet(ct, ctinfo, hooknum, pskb);
+	return ip_nat_packet(ct, ctinfo, hooknum, pskb);
 }
 
 static unsigned int
@@ -325,15 +325,10 @@ static int init_or_cleanup(int init)
 		printk("ip_nat_init: can't setup rules.\n");
 		goto cleanup_nothing;
 	}
-	ret = ip_nat_init();
-	if (ret < 0) {
-		printk("ip_nat_init: can't setup rules.\n");
-		goto cleanup_rule_init;
-	}
 	ret = nf_register_hook(&ip_nat_in_ops);
 	if (ret < 0) {
 		printk("ip_nat_init: can't register in hook.\n");
-		goto cleanup_nat;
+		goto cleanup_rule_init;
 	}
 	ret = nf_register_hook(&ip_nat_out_ops);
 	if (ret < 0) {
@@ -374,8 +369,6 @@ static int init_or_cleanup(int init)
 	nf_unregister_hook(&ip_nat_out_ops);
  cleanup_inops:
 	nf_unregister_hook(&ip_nat_in_ops);
- cleanup_nat:
-	ip_nat_cleanup();
  cleanup_rule_init:
 	ip_nat_rule_cleanup();
  cleanup_nothing:
@@ -395,14 +388,4 @@ static void __exit fini(void)
 module_init(init);
 module_exit(fini);
 
-EXPORT_SYMBOL(ip_nat_setup_info);
-EXPORT_SYMBOL(ip_nat_protocol_register);
-EXPORT_SYMBOL(ip_nat_protocol_unregister);
-EXPORT_SYMBOL_GPL(ip_nat_proto_find_get);
-EXPORT_SYMBOL_GPL(ip_nat_proto_put);
-EXPORT_SYMBOL(ip_nat_cheat_check);
-EXPORT_SYMBOL(ip_nat_mangle_tcp_packet);
-EXPORT_SYMBOL(ip_nat_mangle_udp_packet);
-EXPORT_SYMBOL(ip_nat_used_tuple);
-EXPORT_SYMBOL(ip_nat_follow_master);
 MODULE_LICENSE("GPL");
-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH/RFC] [NETFILTER]: Fix invalid module autoloading by splitting iptable_nat

[Patrick McHardy]

Harald Welte wrote:
> Hi Patrick, Dave,
> 
> I think we really need a solution for the last (known) remaining
> dependency problem with 2.6.14.  Please see the description
> below.  I _think_ the patch is fine, at least I couldn't find any case
> where we could leak anything by splitting the code in two modules.
> 
> There's a slight semantic change, though.  If the user unloads
> iptable_nat, all existing connections (including their configured NAT
> mappings) will continue to work.  Only when ip_nat.ko is unloaded, the
> NAT mappings are evicted from the conntrack table.  I like it that way,
> since it's logical.

I agree, its more logical than having the table and the conntrack
part in one module.

Re: [PATCH/RFC] [NETFILTER]: Fix invalid module autoloading by splitting iptable_nat

[Harald Welte]

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

On Sun, Sep 25, 2005 at 06:26:28PM +0200, Patrick McHardy wrote:
> Harald Welte wrote:
> >Hi Patrick, Dave,
> >I think we really need a solution for the last (known) remaining
> >dependency problem with 2.6.14.  Please see the description
> >below.  I _think_ the patch is fine, at least I couldn't find any case
> >where we could leak anything by splitting the code in two modules.
> >There's a slight semantic change, though.  If the user unloads
> >iptable_nat, all existing connections (including their configured NAT
> >mappings) will continue to work.  Only when ip_nat.ko is unloaded, the
> >NAT mappings are evicted from the conntrack table.  I like it that way,
> >since it's logical.
> 
> I agree, its more logical than having the table and the conntrack
> part in one module.

Ok. Gandalf has reviewed the code, too.   This makes three to blame if
something goes wrong ;)

Dave, please apply 

(sorry for the missing Signed-off-by: Harald Welte <laforge@netfilter.org>)

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH/RFC] [NETFILTER]: Fix invalid module autoloading by splitting iptable_nat

[David S. Miller]

From: Harald Welte <laforge@netfilter.org>
Date: Sun, 25 Sep 2005 18:44:23 +0200

> Dave, please apply 
> 
> (sorry for the missing Signed-off-by: Harald Welte <laforge@netfilter.org>)

Done, thanks a lot.