Very first try: ipt_connrate patch.

Okay, this is my very first patch concerning anything netfilter related. I have no idea which versions I should build it against, how to deal with the patch-o-matic, let alone patch-o-matic-ng.

So, I'd appreciate some hand-holding in getting this thing into a form where it can be easily used by other people.

Attached are two patches.

One is against the 2.6.0 kernel, but will probably apply against any 2.6 kernel. This one should be rather clean - just needs to be improved a bit on some cases and the addition of a few more descriptions.

And the other one is against iptables 1.2.9. This is a rather hackish patch, 95% of it is copy-pasted from libipt_connbytes.c, which doesn't seem to be that perfect itself either. It is also built against the debian patched version of iptables 1.2.9, so I'm not sure how cleanly the Makefile part applies to others.

I will continue to clean up and tweak these patches in the following days, but I would be glad to get as many comments about it as early as possible, also on the usefulness of what I'm doing here.

Thanks in advance,
-- Naked

diff -uprN kernel-source-2.6.0.old/include/linux/netfilter_ipv4/ip_conntrack.h kernel-source-2.6.0/include/linux/netfilter_ipv4/ip_conntrack.h
@@ -98,6 +98,10 @@ union ip_conntrack_nat_help {
 };
 #endif
 
+#ifdef CONFIG_IP_NF_CONNTRACK_RATE
+#include <linux/netfilter_ipv4/ip_conntrack_rate.h>
+#endif
+
 #ifdef __KERNEL__
 
 #include <linux/types.h>

@@ -206,6 +210,10 @@ struct ip_conntrack
 	} nat;
 #endif /* CONFIG_IP_NF_NAT_NEEDED */
 
+#ifdef CONFIG_IP_NF_CONNTRACK_RATE
+        struct ip_conntrack_rate rate;
+#endif
+
 };
 
 /* get master conntrack via master expectation */

diff -uprN kernel-source-2.6.0.old/include/linux/netfilter_ipv4/ip_conntrack_rate.h kernel-source-2.6.0/include/linux/netfilter_ipv4/ip_conntrack_rate.h
@@ -0,0 +1,33 @@
+#ifndef _IP_CONNTRACK_RATE_H
+#define _IP_CONNTRACK_RATE_H
+
+/* estimation interval, in jiffies */
+#define IP_CONNTRACK_RATE_INTERVAL (3 * HZ)
+
+/* scale on how many tokens per byte to generate */
+#define IP_CONNTRACK_RATE_SCALE 100
+
+/* per conntrack: transfer rate in connection */
+struct ip_conntrack_rate {
+	/* jiffies of previous received packet */
+	unsigned long prev;
+	/* average rate of tokens per jiffy */
+	u_int32_t avgrate;
+};
+
+#ifdef __KERNEL__
+
+/* Count a packet of len into given rate structure. */
+extern void
+ip_conntrack_rate_count(struct ip_conntrack_rate *ctr,
+			unsigned int len);
+
+/* Return current rate as bytes per second. Note that the returned
+   rate is the rate at last received packet, not counting time has
+   that passed after it. */
+extern u_int32_t
+ip_conntrack_rate_get(struct ip_conntrack_rate *ctr);
+
+#endif /* __KERNEL__ */
+
+#endif /* _IP_CONNTRACK_RATE_H */

diff -uprN kernel-source-2.6.0.old/include/linux/netfilter_ipv4/ipt_connrate.h kernel-source-2.6.0/include/linux/netfilter_ipv4/ipt_connrate.h
@@ -0,0 +1,12 @@
+#ifndef _IPT_CONNRATE_H
+#define _IPT_CONNRATE_H
+
+struct ipt_connrate_info
+{
+	/* Per connection transfer rate, in bytes per second. If
+	   'from' is smaller or equal to 'to', rate is matched to be
+	   inside the inclusive range [from,to], otherwise rate is
+	   matched to be outside the inclusive range [to,from]. */
+	unsigned long from, to;
+};
+#endif

diff -uprN kernel-source-2.6.0.old/net/ipv4/netfilter/Kconfig kernel-source-2.6.0/net/ipv4/netfilter/Kconfig
@@ -19,6 +19,22 @@ config IP_NF_CONNTRACK
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config IP_NF_CONNTRACK_RATE
+	bool "Connection rate estimation"
+	depends on IP_NF_CONNTRACK
+	help
+
+	  This enables per connection transfer rate estimation in connection
+	  tracking code. This enlarges the amount of memory required by each
+	  connection tracked a bit and adds the overhead of calculating the
+	  transmission rate on every received packet.
+	
+	  This is required to be able to match on the per connection transfer
+	  rate, and can be a nice statistic to see in the connection tracking
+	  table, but is useless otherwise.
+	
+	  If unsure, say N.
+
 config IP_NF_FTP
 	tristate "FTP protocol support"
 	depends on IP_NF_CONNTRACK

@@ -256,6 +272,19 @@ config IP_NF_MATCH_CONNTRACK
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config IP_NF_MATCH_CONNRATE
+	tristate "Connection rate match support"
+	depends on IP_NF_CONNTRACK_RATE && IP_NF_CONNTRACK && IP_NF_IPTABLES
+	help
+	  This allows matching on the transfer rate on a per connection basis.
+
+	  Connection transfer rate estimation is performed separately by the
+	  connection tracking code and is unaffected by the presence of matches
+	  on it. Several connection rate matches may match a single packet and
+	  every match will see the same rate.
+
+	  To compile it as a module, choose M here.  If unsure, say N.
+
 config IP_NF_MATCH_OWNER
 	tristate "Owner match support"
 	depends on IP_NF_IPTABLES

diff -uprN kernel-source-2.6.0.old/net/ipv4/netfilter/Makefile kernel-source-2.6.0/net/ipv4/netfilter/Makefile
@@ -18,6 +18,7 @@ ipchains-objs		:= $(ip_nf_compat-objs) i
 
 # connection tracking
 obj-$(CONFIG_IP_NF_CONNTRACK) += ip_conntrack.o
+obj-$(CONFIG_IP_NF_CONNTRACK_RATE) += ip_conntrack_rate.o
 
 # connection tracking helpers
 obj-$(CONFIG_IP_NF_AMANDA) += ip_conntrack_amanda.o

@@ -62,6 +63,7 @@ obj-$(CONFIG_IP_NF_MATCH_LENGTH) += ipt_
 obj-$(CONFIG_IP_NF_MATCH_TTL) += ipt_ttl.o
 obj-$(CONFIG_IP_NF_MATCH_STATE) += ipt_state.o
 obj-$(CONFIG_IP_NF_MATCH_CONNTRACK) += ipt_conntrack.o
+obj-$(CONFIG_IP_NF_MATCH_CONNRATE) += ipt_connrate.o
 obj-$(CONFIG_IP_NF_MATCH_TCPMSS) += ipt_tcpmss.o
 
 obj-$(CONFIG_IP_NF_MATCH_PHYSDEV) += ipt_physdev.o

diff -uprN kernel-source-2.6.0.old/net/ipv4/netfilter/ip_conntrack_core.c kernel-source-2.6.0/net/ipv4/netfilter/ip_conntrack_core.c
@@ -774,6 +774,11 @@ resolve_normal_ct(struct sk_buff *skb,
 		*set_reply = 0;
 	}
 	skb->nfct = &h->ctrack->infos[*ctinfo];
+
+#ifdef CONFIG_IP_NF_CONNTRACK_RATE
+	ip_conntrack_rate_count(&h->ctrack->rate, skb->len);
+#endif
+
 	return h->ctrack;
 }
 

diff -uprN kernel-source-2.6.0.old/net/ipv4/netfilter/ip_conntrack_standalone.c kernel-source-2.6.0/net/ipv4/netfilter/ip_conntrack_standalone.c
@@ -105,6 +105,10 @@ print_conntrack(char *buffer, struct ip_
 		len += sprintf(buffer + len, "[ASSURED] ");
 	len += sprintf(buffer + len, "use=%u ",
 		       atomic_read(&conntrack->ct_general.use));
+#ifdef CONFIG_IP_NF_CONNTRACK_RATE
+	len += sprintf(buffer + len, "rate=%u ",
+		       ip_conntrack_rate_get(&conntrack->rate));
+#endif
 	len += sprintf(buffer + len, "\n");
 
 	return len;

diff -uprN ../../debian.old/build/iptables-1.2.9/extensions/.connrate-test iptables-1.2.9/extensions/.connrate-test
@@ -0,0 +1,2 @@
+#! /bin/sh
+[ -f $KERNEL_DIR/net/ipv4/netfilter/ipt_connrate.c ] && echo connrate

diff -uprN ../../debian.old/build/iptables-1.2.9/extensions/Makefile iptables-1.2.9/extensions/Makefile
@@ -5,7 +5,7 @@
 # header files are present in the include/linux directory of this iptables
 # package (HW)
 #
-PF_EXT_SLIB:=ah connlimit connmark conntrack dscp ecn esp helper icmp iprange length limit mac mark multiport owner physdev pkttype realm rpc standard state tcp tcpmss tos ttl udp unclean CLASSIFY CONNMARK DNAT DSCP ECN LOG MARK MASQUERADE MIRROR NETMAP NOTRACK REDIRECT REJECT SAME SNAT TARPIT TCPMSS TOS TRACE TTL ULOG
+PF_EXT_SLIB:=ah connlimit connmark connrate conntrack dscp ecn esp helper icmp iprange length limit mac mark multiport owner physdev pkttype realm rpc standard state tcp tcpmss tos ttl udp unclean CLASSIFY CONNMARK DNAT DSCP ECN LOG MARK MASQUERADE MIRROR NETMAP NOTRACK REDIRECT REJECT SAME SNAT TARPIT TCPMSS TOS TRACE TTL ULOG
 PF6_EXT_SLIB:=eui64 hl icmpv6 length limit mac mark multiport owner standard tcp udp HL LOG MARK TRACE
 
 # Optionals

since it is 2.6.x, please put it in patch-o-matic-ng.

all the wholefiles go in as files themselves. i.e. patch-o-matic-ng/connrate/linux/include/linux/netfilter_ipv4/ip_conntrack_rate.h

The rest is a bit more tricky, and I have to admit that there is no way but to look at the dozens of examples in order to figure it out.

As long as you only add a libipt_connrate.c file (together with a .libipt_connrate-test), this will usually be applied without any problems.

instead of adding it here, please include a .connrate-test file

- Harald Welte <[email protected]>             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

[...]

Okay.

Right. I think I can manage. So I guess the next version of the patch that I send should be a diff against the patch-o-matic-ng directory.

Yup.

This is intentional. 'prev' is used to store a jiffies value, as by the comment - which is an unsigned long on all platforms I believe. 'avgrate' is again something calculated internally in the algorithm, and it should be 32 bits to keep the assumptions I make valid.

However, I haven't tested the code on 64bit architectures, so there may be some other mistakes in there - and also, I am not sure what to do about jiffies wrapping, I need to revisit that.

[...]

Actually I did - but at some testing phase and due to the contorted way debian builds the iptables package, I couldn't get it to automatically detect connrate - so I went the easy way on the first time. I also wasn't sure if just adding the -test file would be enough, but I guess it is.

Thank you for the review - I'll get a second version of the patch with the changes done when I find the time.

-- Naked

the issue is sparc64, which has 32bit userspace and 64bit kernelspace. So libipt_XX.c will be compiled with a 32bit unsigned long, and the kernel with a 64bit unsigned long :(

yes, it is. The .test file checks against your currently installed kernel source tree. if the kernel source doesn't have the connrate kernel patch applied, then it won't build the libipt_connrate plugin.

- Harald Welte <[email protected]>             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

Ah, right! There's a mixup between two things here.

First of all, you are correct, struct ipt_connrate_info is at fault:
,----

| struct ipt_connrate_info
| {
|        /* Per connection transfer rate, in bytes per second. If
|           'from' is smaller or equal to 'to', rate is matched to be
|           inside the inclusive range [from,to], otherwise rate is
|           matched to be outside the inclusive range [to,from]. */
|        unsigned long from, to;
| };

`----

And that was just a mistake from my side - I will change them to be u_int32_t. The value they compare against is already u_int32_t.

But, the other structure, namely ip_conntrack_rate:
,----

| struct ip_conntrack_rate {
|        /* jiffies of previous received packet */
|        unsigned long prev;
|        /* average rate of tokens per jiffy */
|        u_int32_t avgrate;
| };

`----

This is a part of the ip_conntrack struct. This structure seems to be inside #ifdef __KERNEL__ to begin with, and there are no need for userspace software to access it. So I believe it is fine.

But if it would be needed from userspace, what is the correct datatype that will hold a kernel jiffies value both inside the kernel and in userspace?

Thanks,
-- Naked

[...]

Right. I didn't see any locking in resolve_normal_ct, so I assumed no locking was needed. Then I peeked in the functions it calls and every one of them gets the ip_conntrack_lock.

But, I am a bit confused on the best way to handle locking. For example, ip_conntrack_proto_tcp has only one RWLOCK which it uses. I can undestand this for the actual connection tracking table, but for the subentries in it... well, why would writing to one conntrack element block reading from another conntrack element - or simultaneous writing to two different conntrack elements and so on. Shouldn't there be some locking per connection tracking entry, so other entries could simultaneously be processed on other processors?

SMP performance and correctness is an important issue for me, as I am specifically running this on an SMP machine. Luckily I haven't ran into any problems yet - thanks for spotting this. I shall implement things as proto_tcp does it for now.

-- Naked

There are patches for per-bucket locking in pom, look at extra/conntrack_locking.patch if you are interested.

That should do it for now. I wouldn't expect any performance problems as the critical section is really short.

Regards,
Patrick

Ah. Okay, I'll settle for "it's in the works".

Agreed.

-- Naked