[PATCH] Introducing the Change API

[PATCH] Introducing the Change API

[Pablo Neira]

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

Hi,

I've finished the change API patch which goes on top of the conntrack 
event API[1]. This patch provides a way to modify some parts of a 
conntrack such as protocol and helper private info.

I've defined three generic functions:

a) helper handlings
generic_change_help
generic_change_new_expect

b) proto handlings
generic_change_proto

Since there are mostly the same thing, if there's any weird protocol, we 
can still defined our own function. I've also added a new field in 
ip_conntrack_protocol and ip_conntrack_helper, to make their locks, if 
any, accessible. So it's flexible enough.

Comments and review welcome.

Next step, work on the ctnetlink-nfnetlink stuff. Is there any plan to 
push forward both patches?

Refs:
[1] 
https://lists.netfilter.org/pipermail/netfilter-devel/2004-November/017453.html

--
Pablo

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

diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/net/ipv4/netfilter/ip_conntrack_amanda.c change/net/ipv4/netfilter/ip_conntrack_amanda.c
--- linux-2.5/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-12-04 20:13:32.000000000 +0100
+++ change/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-12-04 19:35:16.000000000 +0100
@@ -136,6 +136,8 @@
 	.mask = { .src = { .u = { 0xFFFF } },
 		 .dst = { .protonum = 0xFFFF },
 	},
+	.change_help = generic_change_help,
+	.change_new_expect = generic_change_new_expect,
 };
 
 static void __exit fini(void)
diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/net/ipv4/netfilter/ip_conntrack_core.c change/net/ipv4/netfilter/ip_conntrack_core.c
--- linux-2.5/net/ipv4/netfilter/ip_conntrack_core.c	2004-12-04 20:13:32.000000000 +0100
+++ change/net/ipv4/netfilter/ip_conntrack_core.c	2004-12-04 20:14:15.000000000 +0100
@@ -1244,6 +1244,51 @@
 	}
 }
 
+void generic_change_proto(struct ip_conntrack *ct,
+			  union ip_conntrack_proto *p)
+{
+	struct ip_conntrack_tuple *t = &ct->tuplehash[IP_CT_DIR_REPLY].tuple;
+	struct ip_conntrack_protocol *proto;
+	
+	proto = ip_ct_find_proto(t->dst.protonum);
+	if (proto->lock != NULL) {
+		WRITE_LOCK(proto->lock);
+		memcpy(&ct->proto, p, sizeof(union ip_conntrack_proto));
+		WRITE_UNLOCK(proto->lock);
+	} else
+		memcpy(&ct->proto, p, sizeof(union ip_conntrack_proto));
+}
+
+void generic_change_help(struct ip_conntrack *ct, union ip_conntrack_help *h)
+{
+	struct ip_conntrack_helper *helper= ct->helper;
+
+	if (helper->lock != NULL) {
+		LOCK_BH(helper->lock);
+		memcpy(&ct->help, h, sizeof(ct->help));
+		UNLOCK_BH(helper->lock);
+	} else
+		memcpy(&ct->help, h, sizeof(ct->help));
+}
+
+int generic_change_new_expect(struct ip_conntrack_expect *exp,
+			      union ip_conntrack_expect_proto *p,
+			      union ip_conntrack_expect_help *h)
+{
+	struct ip_conntrack_helper *helper = exp->expectant->helper;
+
+	if (h == NULL)
+		return -1;
+	if (helper->lock != NULL) {
+		LOCK_BH(helper->lock);
+		memcpy(&exp->help, h, sizeof(exp->help));
+		UNLOCK_BH(helper->lock);
+	} else
+		memcpy(&exp->help, h, sizeof(exp->help));
+
+	return 0;
+}
+
 /* Fast function for those who don't want to parse /proc (and I don't
    blame them). */
 /* Reversing the socket's dst/src point of view gives us the reply
diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/net/ipv4/netfilter/ip_conntrack_ftp.c change/net/ipv4/netfilter/ip_conntrack_ftp.c
--- linux-2.5/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-12-04 20:13:32.000000000 +0100
+++ change/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-12-04 19:35:11.000000000 +0100
@@ -440,6 +440,9 @@
 		ftp[i].flags = IP_CT_HELPER_F_REUSE_EXPECT;
 		ftp[i].me = ip_conntrack_ftp;
 		ftp[i].help = help;
+		ftp[i].lock = &ip_ftp_lock;
+		ftp[i].change_help = generic_change_help;
+		ftp[i].change_new_expect = generic_change_new_expect;
 
 		tmpname = &ftp_names[i][0];
 		if (ports[i] == FTP_PORT)
diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/net/ipv4/netfilter/ip_conntrack_irc.c change/net/ipv4/netfilter/ip_conntrack_irc.c
--- linux-2.5/net/ipv4/netfilter/ip_conntrack_irc.c	2004-12-04 20:13:32.000000000 +0100
+++ change/net/ipv4/netfilter/ip_conntrack_irc.c	2004-12-04 19:34:57.000000000 +0100
@@ -271,6 +271,9 @@
 		hlpr->flags = IP_CT_HELPER_F_REUSE_EXPECT;
 		hlpr->me = ip_conntrack_irc;
 		hlpr->help = help;
+		hlpr->lock = &irc_buffer_lock;
+		hlpr->change_help = generic_change_help;
+		hlpr->change_new_expect = generic_change_new_expect;
 
 		tmpname = &irc_names[i][0];
 		if (ports[i] == IRC_PORT)
diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/net/ipv4/netfilter/ip_conntrack_proto_icmp.c change/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
--- linux-2.5/net/ipv4/netfilter/ip_conntrack_proto_icmp.c	2004-12-04 20:13:32.000000000 +0100
+++ change/net/ipv4/netfilter/ip_conntrack_proto_icmp.c	2004-12-04 19:09:42.000000000 +0100
@@ -109,16 +109,17 @@
 	return NF_ACCEPT;
 }
 
+static u_int8_t valid_new[] = {
+	[ICMP_ECHO] = 1,
+	[ICMP_TIMESTAMP] = 1,
+	[ICMP_INFO_REQUEST] = 1,
+	[ICMP_ADDRESS] = 1
+};
+
 /* Called when a new connection for this protocol found. */
 static int icmp_new(struct ip_conntrack *conntrack,
 		    const struct sk_buff *skb)
 {
-	static u_int8_t valid_new[]
-		= { [ICMP_ECHO] = 1,
-		    [ICMP_TIMESTAMP] = 1,
-		    [ICMP_INFO_REQUEST] = 1,
-		    [ICMP_ADDRESS] = 1 };
-
 	if (conntrack->tuplehash[0].tuple.dst.u.icmp.type >= sizeof(valid_new)
 	    || !valid_new[conntrack->tuplehash[0].tuple.dst.u.icmp.type]) {
 		/* Can't create a new ICMP `conn' with this. */
@@ -266,6 +267,17 @@
 	return icmp_error_message(skb, ctinfo, hooknum);
 }
 
+static int icmp_change_check_tuples(struct ip_conntrack_tuple *orig,
+				    struct ip_conntrack_tuple *reply)
+{
+	unsigned int type = orig->dst.u.icmp.type;
+
+	if (type >= sizeof(valid_new) || !valid_new[type])
+		return -EINVAL;
+
+	return 0;
+}
+
 struct ip_conntrack_protocol ip_conntrack_protocol_icmp =
 {
 	.proto 			= IPPROTO_ICMP,
@@ -277,4 +289,6 @@
 	.packet			= icmp_packet,
 	.new			= icmp_new,
 	.error			= icmp_error,
+	.change_check_tuples    = icmp_change_check_tuples,
+	.change_proto           = generic_change_proto,
 };
diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/net/ipv4/netfilter/ip_conntrack_proto_sctp.c change/net/ipv4/netfilter/ip_conntrack_proto_sctp.c
--- linux-2.5/net/ipv4/netfilter/ip_conntrack_proto_sctp.c	2004-12-04 20:13:32.000000000 +0100
+++ change/net/ipv4/netfilter/ip_conntrack_proto_sctp.c	2004-12-04 19:17:53.000000000 +0100
@@ -506,6 +506,7 @@
 struct ip_conntrack_protocol ip_conntrack_protocol_sctp = { 
 	.proto 		 = IPPROTO_SCTP, 
 	.name 		 = "sctp",
+	.lock		 = &sctp_lock,
 	.pkt_to_tuple 	 = sctp_pkt_to_tuple, 
 	.invert_tuple 	 = sctp_invert_tuple, 
 	.print_tuple 	 = sctp_print_tuple, 
@@ -514,6 +515,7 @@
 	.new 		 = sctp_new, 
 	.destroy 	 = NULL, 
 	.exp_matches_pkt = sctp_exp_matches_pkt, 
+	.change_proto	 = generic_change_proto,
 	.me 		 = THIS_MODULE 
 };
 
diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/net/ipv4/netfilter/ip_conntrack_proto_tcp.c change/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
--- linux-2.5/net/ipv4/netfilter/ip_conntrack_proto_tcp.c	2004-12-04 20:13:32.000000000 +0100
+++ change/net/ipv4/netfilter/ip_conntrack_proto_tcp.c	2004-12-04 19:09:06.000000000 +0100
@@ -1079,6 +1079,7 @@
 {
 	.proto 			= IPPROTO_TCP,
 	.name 			= "tcp",
+	.lock			= &tcp_lock,
 	.pkt_to_tuple 		= tcp_pkt_to_tuple,
 	.invert_tuple 		= tcp_invert_tuple,
 	.print_tuple 		= tcp_print_tuple,
@@ -1087,4 +1088,5 @@
 	.new 			= tcp_new,
 	.exp_matches_pkt	= tcp_exp_matches_pkt,
 	.error			= tcp_error,
+	.change_proto           = generic_change_proto,
 };
diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/net/ipv4/netfilter/ip_conntrack_proto_udp.c change/net/ipv4/netfilter/ip_conntrack_proto_udp.c
--- linux-2.5/net/ipv4/netfilter/ip_conntrack_proto_udp.c	2004-12-04 20:13:32.000000000 +0100
+++ change/net/ipv4/netfilter/ip_conntrack_proto_udp.c	2004-12-04 19:09:13.000000000 +0100
@@ -146,4 +146,5 @@
 	.packet			= udp_packet,
 	.new			= udp_new,
 	.error			= udp_error,
+	.change_proto           = generic_change_proto,
 };
diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/net/ipv4/netfilter/ip_conntrack_standalone.c change/net/ipv4/netfilter/ip_conntrack_standalone.c
--- linux-2.5/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-12-04 20:13:32.000000000 +0100
+++ change/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-12-04 14:58:09.000000000 +0100
@@ -880,6 +880,9 @@
 {
 }
 
+EXPORT_SYMBOL(generic_change_proto);
+EXPORT_SYMBOL(generic_change_help);
+EXPORT_SYMBOL(generic_change_new_expect);
 #ifdef CONFIG_IP_NF_CONNTRACK_EVENTS
 EXPORT_SYMBOL(ip_conntrack_chain);
 EXPORT_SYMBOL(ip_conntrack_register_notifier);
diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/net/ipv4/netfilter/ip_conntrack_tftp.c change/net/ipv4/netfilter/ip_conntrack_tftp.c
--- linux-2.5/net/ipv4/netfilter/ip_conntrack_tftp.c	2004-12-04 20:13:32.000000000 +0100
+++ change/net/ipv4/netfilter/ip_conntrack_tftp.c	2004-12-04 19:34:43.000000000 +0100
@@ -123,6 +123,8 @@
 		tftp[i].flags = IP_CT_HELPER_F_REUSE_EXPECT;
 		tftp[i].me = THIS_MODULE;
 		tftp[i].help = tftp_help;
+		tftp[i].change_help = generic_change_help;
+		tftp[i].change_new_expect = generic_change_new_expect;
 
 		tmpname = &tftp_names[i][0];
 		if (ports[i] == TFTP_PORT)
diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/include/linux/netfilter_ipv4/ip_conntrack_helper.h change/include/linux/netfilter_ipv4/ip_conntrack_helper.h
--- linux-2.5/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-12-04 20:13:32.000000000 +0100
+++ change/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-12-04 20:02:28.000000000 +0100
@@ -18,6 +18,7 @@
 	unsigned int max_expected;	/* Maximum number of concurrent 
 					 * expected connections */
 	unsigned int timeout;		/* timeout for expecteds */
+	spinlock_t *lock;		/* protect private info and buffer */
 
 	/* Mask of things we will help (compared against server response) */
 	struct ip_conntrack_tuple tuple;
@@ -28,6 +29,11 @@
 	int (*help)(struct sk_buff *skb,
 		    struct ip_conntrack *ct,
 		    enum ip_conntrack_info conntrackinfo);
+
+	void (*change_help)(struct ip_conntrack *, union ip_conntrack_help *);
+	int (*change_new_expect)(struct ip_conntrack_expect *,
+				 union ip_conntrack_expect_proto *,
+				 union ip_conntrack_expect_help *);
 };
 
 extern int ip_conntrack_helper_register(struct ip_conntrack_helper *);
@@ -45,5 +51,10 @@
 extern int ip_conntrack_change_expect(struct ip_conntrack_expect *expect,
 				      struct ip_conntrack_tuple *newtuple);
 extern void ip_conntrack_unexpect_related(struct ip_conntrack_expect *exp);
+extern void generic_change_help(struct ip_conntrack *ct,
+				union ip_conntrack_help *h);
+extern int generic_change_new_expect(struct ip_conntrack_expect *exp,
+				     union ip_conntrack_expect_proto *p,
+				     union ip_conntrack_expect_help *h);
 
 #endif /*_IP_CONNTRACK_HELPER_H*/
Los ficheros binarios linux-2.5/include/linux/netfilter_ipv4/.ip_conntrack.h.swp y change/include/linux/netfilter_ipv4/.ip_conntrack.h.swp son distintos
diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/include/linux/netfilter_ipv4/ip_conntrack_protocol.h change/include/linux/netfilter_ipv4/ip_conntrack_protocol.h
--- linux-2.5/include/linux/netfilter_ipv4/ip_conntrack_protocol.h	2004-12-04 20:13:32.000000000 +0100
+++ change/include/linux/netfilter_ipv4/ip_conntrack_protocol.h	2004-12-04 19:37:57.000000000 +0100
@@ -13,6 +13,9 @@
 	/* Protocol name */
 	const char *name;
 
+	/* Lock which protects private proto stuff */
+	rwlock_t *lock;
+
 	/* Try to fill in the third arg: dataoff is offset past IP
            hdr.  Return true if possible. */
 	int (*pkt_to_tuple)(const struct sk_buff *skb,
@@ -51,6 +54,17 @@
 	int (*error)(struct sk_buff *skb, enum ip_conntrack_info *ctinfo,
 		     unsigned int hooknum);
 
+	/* check if tuples are valid for a new connection */
+	int (*change_check_tuples)(struct ip_conntrack_tuple *orig,
+				   struct ip_conntrack_tuple *reply);
+
+	/* check protocol data is valid */
+	int (*change_check_proto)(union ip_conntrack_proto *p);
+
+	/* change protocol info on behalf of ctnetlink */
+	void (*change_proto)(struct ip_conntrack *ct,
+			     union ip_conntrack_proto *p);
+
 	/* Module (if any) which this is connected to. */
 	struct module *me;
 };
@@ -67,6 +81,10 @@
 	return ip_ct_protos[protocol];
 }
 
+/* Change API */
+extern void generic_change_proto(struct ip_conntrack *conntrack,
+				 union ip_conntrack_proto *p);
+
 /* Existing built-in protocols */
 extern struct ip_conntrack_protocol ip_conntrack_protocol_tcp;
 extern struct ip_conntrack_protocol ip_conntrack_protocol_udp;

Re: [PATCH] Introducing the Change API

[Patrick McHardy]

Pablo Neira wrote:

> Hi,
>
> I've finished the change API patch which goes on top of the conntrack 
> event API[1]. This patch provides a way to modify some parts of a 
> conntrack such as protocol and helper private info.
>
> I've defined three generic functions:
>
> a) helper handlings
> generic_change_help
> generic_change_new_expect
>
> b) proto handlings
> generic_change_proto
>
> Since there are mostly the same thing, if there's any weird protocol, 
> we can still defined our own function. I've also added a new field in 
> ip_conntrack_protocol and ip_conntrack_helper, to make their locks, if 
> any, accessible. So it's flexible enough.
>
> Comments and review welcome.

Please prefix the generic_ names with ip_ct_ or something similar.
Some more comments below.

>
> Next step, work on the ctnetlink-nfnetlink stuff. Is there any plan to 
> push forward both patches?

I think that depends on what we want to do with ip_conntrack once
nf_conntrack is merged. Since I really don't feel like keeping
three codebases in sync (ip_conntrack 2.4 + 2.6 and nf_conntrack),
I would prefer to remove it. If we do this, it doesn't make much
sense to add further features to it. But I'm not sure what Harald's
and Krisztian's plans are for ct_sync, so they might disagree with me.

Regards
Patrick

>
> Refs:
> [1] 
> https://lists.netfilter.org/pipermail/netfilter-devel/2004-November/017453.html 
>
>
> -- 
> Pablo
>
>
>diff -Nru --exclude='*cmd.c' --exclude='*.ko' --exclude='*.cmd' --exclude='*.o' --exclude=Makefile --exclude=SCCS --exclude='*.rej' --exclude='*.d' --exclude='*.mod.c' linux-2.5/net/ipv4/netfilter/ip_conntrack_core.c change/net/ipv4/netfilter/ip_conntrack_core.c
>--- linux-2.5/net/ipv4/netfilter/ip_conntrack_core.c	2004-12-04 20:13:32.000000000 +0100
>+++ change/net/ipv4/netfilter/ip_conntrack_core.c	2004-12-04 20:14:15.000000000 +0100
>@@ -1244,6 +1244,51 @@
> 	}
> }
> 
>+void generic_change_proto(struct ip_conntrack *ct,
>+			  union ip_conntrack_proto *p)
>+{
>+	struct ip_conntrack_tuple *t = &ct->tuplehash[IP_CT_DIR_REPLY].tuple;
>+	struct ip_conntrack_protocol *proto;
>+	
>+	proto = ip_ct_find_proto(t->dst.protonum);
>
^^^ Keep in mind for ctnetlink that you may not sleep here

>+	if (proto->lock != NULL) {
>+		WRITE_LOCK(proto->lock);
>
^^^ This won't compile with CONFIG_NETFILTER_DEBUG. I think
    using normal locks and locking functions is the easiest
    solution.

>+		memcpy(&ct->proto, p, sizeof(union ip_conntrack_proto));
>+		WRITE_UNLOCK(proto->lock);
>+	} else
>+		memcpy(&ct->proto, p, sizeof(union ip_conntrack_proto));
>+}
>+
>

Re: [PATCH] Introducing the Change API

[Harald Welte]

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

On Sun, Dec 05, 2004 at 09:23:53PM +0100, Patrick McHardy wrote:

> I think that depends on what we want to do with ip_conntrack once
> nf_conntrack is merged. Since I really don't feel like keeping
> three codebases in sync (ip_conntrack 2.4 + 2.6 and nf_conntrack),
> I would prefer to remove it. If we do this, it doesn't make much
> sense to add further features to it. But I'm not sure what Harald's
> and Krisztian's plans are for ct_sync, so they might disagree with me.

I think we should move to nf_conntrack ASAP.  All new
code/features/API's should be developed on top of nf_conntrack.  We
really should put ip_conntrack into a maintainance-only mode once
nf_conntrack becomes mainline.

Actually I only want to do some nf_conntrack vs. ip_conntrack
benchmarking, which I never got aroun doing up to now :(  With a little
luck, I can do this over the weekend.  If the results are acceptable,
nf_conntrack will be submitted maybe as soon as 2.6.11. 

nfnetlink-ctnetlink in it's existing form will not be submitted to
mainline.  I'm workin on a new (ctnetlink2) patch, that is
template-based and to a certain degree arch/alignment/endian
independent.   If this ctnetlink2 will be submitted upstream, it will at
least support nf_conntrack, maybe also ip_conntrack - but not the other
way around.

> Regards
> Patrick
-- 
- Harald Welte <laforge@netfilter.org>             http://www.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: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Introducing the Change API

[KOVACS Krisztian]

  Hi,

2004-12-16, cs keltezéssel 13:40-kor Harald Welte ezt írta:
> I think we should move to nf_conntrack ASAP.  All new
> code/features/API's should be developed on top of nf_conntrack.  We
> really should put ip_conntrack into a maintainance-only mode once
> nf_conntrack becomes mainline.
> 
> Actually I only want to do some nf_conntrack vs. ip_conntrack
> benchmarking, which I never got aroun doing up to now :(  With a little
> luck, I can do this over the weekend.  If the results are acceptable,
> nf_conntrack will be submitted maybe as soon as 2.6.11. 

  For me, there's a major problem with nf_conntrack: missing IPv4 NAT
support, probably this is the only reason I did not start porting
ct_sync to nf_conntrack yet.

  Harald, are there plans to implement IPv4 NAT support for
nf_conntrack? If there are, would it be built on top of a generalized,
layer 3 independent core, or will it be just a "quick hack" for IPv4?

-- 
 Regards,
   Krisztian KOVACS

Re: [PATCH] Introducing the Change API

[Pablo Neira]

Hi,

Harald Welte wrote:

>On Sun, Dec 05, 2004 at 09:23:53PM +0100, Patrick McHardy wrote:
>
>  
>
>>I think that depends on what we want to do with ip_conntrack once
>>nf_conntrack is merged. Since I really don't feel like keeping
>>three codebases in sync (ip_conntrack 2.4 + 2.6 and nf_conntrack),
>>I would prefer to remove it. If we do this, it doesn't make much
>>sense to add further features to it. But I'm not sure what Harald's
>>and Krisztian's plans are for ct_sync, so they might disagree with me.
>>    
>>
>
>I think we should move to nf_conntrack ASAP.  All new
>code/features/API's should be developed on top of nf_conntrack.  We
>really should put ip_conntrack into a maintainance-only mode once
>nf_conntrack becomes mainline.
>

I don't mind about porting both patches to nf_conntrack, as soon as it 
is pushed forward.

>Actually I only want to do some nf_conntrack vs. ip_conntrack
>benchmarking, which I never got aroun doing up to now :(  With a little
>luck, I can do this over the weekend.  If the results are acceptable,
>nf_conntrack will be submitted maybe as soon as 2.6.11. 
>

ok, looking forward seeing the result of your benchmark.

>nfnetlink-ctnetlink in it's existing form will not be submitted to
>mainline.  I'm workin on a new (ctnetlink2) patch, that is
>template-based and to a certain degree arch/alignment/endian
>independent.
>

I'm willing to contribute to ctnetlink, actually I wrote the event and 
change API patches thinking about contibuting to ctnetlink and ct_sync. 
Please, consider sending me such patch as soon as it gets some kind of 
shape.

>   If this ctnetlink2 will be submitted upstream, it will at
>least support nf_conntrack, maybe also ip_conntrack - but not the other
>way around.
>  
>

Ok, thanks Harald, this clarifies which are the plans for conntrack 
stuff in future.

--
Pablo

Re: [PATCH] Introducing the Change API

[Patrick McHardy]

Harald Welte wrote:

>On Sun, Dec 05, 2004 at 09:23:53PM +0100, Patrick McHardy wrote:
>
>  
>
>>I think that depends on what we want to do with ip_conntrack once
>>nf_conntrack is merged. Since I really don't feel like keeping
>>three codebases in sync (ip_conntrack 2.4 + 2.6 and nf_conntrack),
>>I would prefer to remove it. If we do this, it doesn't make much
>>sense to add further features to it. But I'm not sure what Harald's
>>and Krisztian's plans are for ct_sync, so they might disagree with me.
>>    
>>
>
>I think we should move to nf_conntrack ASAP.  All new
>code/features/API's should be developed on top of nf_conntrack.  We
>really should put ip_conntrack into a maintainance-only mode once
>nf_conntrack becomes mainline.
>  
>
Agreed.

>Actually I only want to do some nf_conntrack vs. ip_conntrack
>benchmarking, which I never got aroun doing up to now :(  With a little
>luck, I can do this over the weekend.  If the results are acceptable,
>nf_conntrack will be submitted maybe as soon as 2.6.11. 
>  
>
I would like to give it some cleanup first. Since it doesn't need to
deal with ipchains compatibility there is a lot possible cleanup.
I will probably do this in the next couple of weeks.

Regards
Patrick

Re: [PATCH] Introducing the Change API

[Harald Welte]

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

On Fri, Dec 17, 2004 at 06:28:55AM +0100, Patrick McHardy wrote:
> >Actually I only want to do some nf_conntrack vs. ip_conntrack
> >benchmarking, which I never got aroun doing up to now :(  With a little
> >luck, I can do this over the weekend.  If the results are acceptable,
> >nf_conntrack will be submitted maybe as soon as 2.6.11. 
> > 
> I would like to give it some cleanup first. Since it doesn't need to
> deal with ipchains compatibility there is a lot possible cleanup.
> I will probably do this in the next couple of weeks.

What kind of cleanup are you planning?  the provisions in conntrack are
not that big for the compat stuff... it's more the provisions in nat
(which we don't have yet for nf_conntrack).

I'm willing to put some effort into this.  The question is:  Do we
really need this cleanup before the merge, or can we do it afterwards?

I mean, it seems to be stable - and it's not any more unclean than the
current ip_conntrack code.  And we'll mark it as EXPERIMENTAL first
anyway, so we have more freedom to play with it ;)

> Regards
> Patrick

-- 
- Harald Welte <laforge@netfilter.org>             http://www.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: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Introducing the Change API

[Patrick McHardy]

Harald Welte wrote:

>On Fri, Dec 17, 2004 at 06:28:55AM +0100, Patrick McHardy wrote:
>  
>
>>I would like to give it some cleanup first. Since it doesn't need to
>>deal with ipchains compatibility there is a lot possible cleanup.
>>I will probably do this in the next couple of weeks.
>>    
>>
>
>What kind of cleanup are you planning?  the provisions in conntrack are
>not that big for the compat stuff... it's more the provisions in nat
>(which we don't have yet for nf_conntrack).
>
Hmm I haven't looked into it very deeply yet, but I would like to kill
the nf_conntrack_standalone file and remove unneeded EXPORT_SYMBOLs.
I would also like to make naming more consistent and get rid of these
horrible long function names (nf_conntrack_do_something) by using the
nf_ct_ prefix everywhere.

>I'm willing to put some effort into this.  The question is:  Do we
>really need this cleanup before the merge, or can we do it afterwards?
>
>I mean, it seems to be stable - and it's not any more unclean than the
>current ip_conntrack code.  And we'll mark it as EXPERIMENTAL first
>anyway, so we have more freedom to play with it ;)
>  
>
Yes, but it's just less overhead doing this in your own tree than
submitting patches for all cleanups.

Regards
Patrick