netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] add new iptables ipt_connbytes match
@ 2005-08-11 20:03 Harald Welte
  2005-08-11 22:42 ` David S. Miller
  2005-08-12  2:52 ` Patrick McHardy
  0 siblings, 2 replies; 26+ messages in thread
From: Harald Welte @ 2005-08-11 20:03 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, Netfilter Development Mailinglist


[-- Attachment #1.1: Type: text/plain, Size: 424 bytes --]

Hi Dave,

please apply to your net-2.6.14 tree:

-- 
- 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 #1.2: 46-ipt_connbytes.patch --]
[-- Type: text/plain, Size: 8278 bytes --]

[NETFILTER] Add new iptables "connbytes" match

This patch ads a new "connbytes" match that utilizes the CONFIG_NF_CT_ACCT
per-connection byte and packet counters.  Using it you can do things like
packet classification on average packet size within a connection.

Signed-off-by: Harald Welte <laforge@netfilter.org>

---
commit 98a0b5554e6e54a5a5a21b21b212186dc3c2118c
tree 43acbfa60366e1b111272851f1a1745a43a71d74
parent 5896088f554d8c2e7f3226973d1413df339e6b75
author Harald Welte <laforge@netfilter.org> Do, 11 Aug 2005 21:15:45 +0200
committer Harald Welte <laforge@netfilter.org> Do, 11 Aug 2005 21:15:45 +0200

 include/linux/netfilter_ipv4/ipt_connbytes.h |   25 ++++
 net/ipv4/netfilter/Kconfig                   |   11 ++
 net/ipv4/netfilter/Makefile                  |    1 
 net/ipv4/netfilter/ipt_connbytes.c           |  166 ++++++++++++++++++++++++++
 4 files changed, 202 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter_ipv4/ipt_connbytes.h b/include/linux/netfilter_ipv4/ipt_connbytes.h
new file mode 100644
--- /dev/null
+++ b/include/linux/netfilter_ipv4/ipt_connbytes.h
@@ -0,0 +1,25 @@
+#ifndef _IPT_CONNBYTES_H
+#define _IPT_CONNBYTES_H
+
+enum ipt_connbytes_what {
+	IPT_CONNBYTES_WHAT_PKTS,
+	IPT_CONNBYTES_WHAT_BYTES,
+	IPT_CONNBYTES_WHAT_AVGPKT,
+};
+
+enum ipt_connbytes_direction {
+	IPT_CONNBYTES_DIR_ORIGINAL,
+	IPT_CONNBYTES_DIR_REPLY,
+	IPT_CONNBYTES_DIR_BOTH,
+};
+
+struct ipt_connbytes_info
+{
+	struct {
+		u_int64_t from;	/* count to be matched */
+		u_int64_t to;	/* count to be matched */
+	} count;
+	u_int8_t what;		/* ipt_connbytes_what */
+	u_int8_t direction;	/* ipt_connbytes_direction */
+};
+#endif
diff --git a/net/ipv4/netfilter/Kconfig b/net/ipv4/netfilter/Kconfig
--- a/net/ipv4/netfilter/Kconfig
+++ b/net/ipv4/netfilter/Kconfig
@@ -386,6 +386,16 @@ config IP_NF_MATCH_CONNMARK
 	  <file:Documentation/modules.txt>.  The module will be called
 	  ipt_connmark.o.  If unsure, say `N'.
 
+config IP_NF_MATCH_CONNBYTES
+	tristate  'Connection byte/packet counter match support'
+	depends on IP_NF_CT_ACCT && IP_NF_IPTABLES
+	help
+	  This option adds a `connbytes' match, which allows you to match the
+	  number of bytes and/or packets for each direction within a connection.
+
+	  If you want to compile it as a module, say M here and read
+	  <file:Documentation/modules.txt>.  If unsure, say `N'.
+
 config IP_NF_MATCH_HASHLIMIT
 	tristate  'hashlimit match support'
 	depends on IP_NF_IPTABLES
@@ -723,6 +733,5 @@ config IP_NF_CONNTRACK_NETLINK
         help
           This option enables support for a netlink-based userspace interface
 
-
 endmenu
 
diff --git a/net/ipv4/netfilter/Makefile b/net/ipv4/netfilter/Makefile
--- a/net/ipv4/netfilter/Makefile
+++ b/net/ipv4/netfilter/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_IP_NF_MATCH_TTL) += ipt_ttl
 obj-$(CONFIG_IP_NF_MATCH_STATE) += ipt_state.o
 obj-$(CONFIG_IP_NF_MATCH_CONNMARK) += ipt_connmark.o
 obj-$(CONFIG_IP_NF_MATCH_CONNTRACK) += ipt_conntrack.o
+obj-$(CONFIG_IP_NF_MATCH_CONNBYTES) += ipt_connbytes.o
 obj-$(CONFIG_IP_NF_MATCH_TCPMSS) += ipt_tcpmss.o
 obj-$(CONFIG_IP_NF_MATCH_REALM) += ipt_realm.o
 obj-$(CONFIG_IP_NF_MATCH_ADDRTYPE) += ipt_addrtype.o
diff --git a/net/ipv4/netfilter/ipt_connbytes.c b/net/ipv4/netfilter/ipt_connbytes.c
new file mode 100644
--- /dev/null
+++ b/net/ipv4/netfilter/ipt_connbytes.c
@@ -0,0 +1,166 @@
+/* Kernel module to match connection tracking byte counter.
+ * GPL (C) 2002 Martin Devera (devik@cdi.cz).
+ *
+ * 2004-07-20 Harald Welte <laforge@netfilter.org>
+ * 	- reimplemented to use per-connection accounting counters
+ * 	- add functionality to match number of packets
+ * 	- add functionality to match average packet size
+ * 	- add support to match directions seperately
+ *
+ */
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/netfilter_ipv4/ip_conntrack.h>
+#include <linux/netfilter_ipv4/ip_tables.h>
+#include <linux/netfilter_ipv4/ipt_connbytes.h>
+
+#include <asm/div64.h>
+#include <asm/bitops.h>
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Harald Welte <laforge@netfilter.org>");
+MODULE_DESCRIPTION("iptables match for matching number of pkts/bytes per connection");
+
+/* 64bit divisor, dividend and result. dynamic precision */
+static u_int64_t div64_64(u_int64_t divisor, u_int64_t dividend)
+{
+	u_int64_t result = divisor;
+
+	if (dividend > 0xffffffff) {
+		int first_bit = find_first_bit((unsigned long *) &dividend, sizeof(dividend));
+		/* calculate number of bits to shift. shift exactly enough
+		 * bits to make dividend fit in 32bits. */
+		int num_shift = (64 - 32 - first_bit);
+		/* first bit has to be < 32, since dividend was > 0xffffffff */
+		result = result >> num_shift;
+		dividend = dividend >> num_shift;
+	}
+
+	do_div(divisor, dividend);
+
+	return divisor;
+}
+
+static int
+match(const struct sk_buff *skb,
+      const struct net_device *in,
+      const struct net_device *out,
+      const void *matchinfo,
+      int offset,
+      int *hotdrop)
+{
+	const struct ipt_connbytes_info *sinfo = matchinfo;
+	enum ip_conntrack_info ctinfo;
+	struct ip_conntrack *ct;
+	u_int64_t what = 0;	/* initialize to make gcc happy */
+
+	if (!(ct = ip_conntrack_get((struct sk_buff *)skb, &ctinfo)))
+		return 0; /* no match */
+
+	switch (sinfo->what) {
+	case IPT_CONNBYTES_WHAT_PKTS:
+		switch (sinfo->direction) {
+		case IPT_CONNBYTES_DIR_ORIGINAL:
+			what = ct->counters[IP_CT_DIR_ORIGINAL].packets;
+			break;
+		case IPT_CONNBYTES_DIR_REPLY:
+			what = ct->counters[IP_CT_DIR_REPLY].packets;
+			break;
+		case IPT_CONNBYTES_DIR_BOTH:
+			what = ct->counters[IP_CT_DIR_ORIGINAL].packets;
+			what += ct->counters[IP_CT_DIR_REPLY].packets;
+			break;
+		}
+		break;
+	case IPT_CONNBYTES_WHAT_BYTES:
+		switch (sinfo->direction) {
+		case IPT_CONNBYTES_DIR_ORIGINAL:
+			what = ct->counters[IP_CT_DIR_ORIGINAL].bytes;
+			break;
+		case IPT_CONNBYTES_DIR_REPLY:
+			what = ct->counters[IP_CT_DIR_REPLY].bytes;
+			break;
+		case IPT_CONNBYTES_DIR_BOTH:
+			what = ct->counters[IP_CT_DIR_ORIGINAL].bytes;
+			what += ct->counters[IP_CT_DIR_REPLY].bytes;
+			break;
+		}
+		break;
+	case IPT_CONNBYTES_WHAT_AVGPKT:
+		switch (sinfo->direction) {
+		case IPT_CONNBYTES_DIR_ORIGINAL:
+			what = div64_64(ct->counters[IP_CT_DIR_ORIGINAL].bytes,
+					ct->counters[IP_CT_DIR_ORIGINAL].packets);
+			break;
+		case IPT_CONNBYTES_DIR_REPLY:
+			what = div64_64(ct->counters[IP_CT_DIR_REPLY].bytes,
+					ct->counters[IP_CT_DIR_REPLY].packets);
+			break;
+		case IPT_CONNBYTES_DIR_BOTH:
+			{
+				u_int64_t bytes;
+				u_int64_t pkts;
+				bytes = ct->counters[IP_CT_DIR_ORIGINAL].bytes +
+					ct->counters[IP_CT_DIR_REPLY].bytes;
+				pkts = ct->counters[IP_CT_DIR_ORIGINAL].packets+
+					ct->counters[IP_CT_DIR_REPLY].packets;
+
+				/* FIXME_THEORETICAL: what to do if sum
+				 * overflows ? */
+
+				what = div64_64(bytes, pkts);
+			}
+			break;
+		}
+		break;
+	}
+
+	if (sinfo->count.to)
+		return (what <= sinfo->count.to && what >= sinfo->count.from);
+	else
+		return (what >= sinfo->count.from);
+}
+
+static int check(const char *tablename,
+		 const struct ipt_ip *ip,
+		 void *matchinfo,
+		 unsigned int matchsize,
+		 unsigned int hook_mask)
+{
+	const struct ipt_connbytes_info *sinfo = matchinfo;
+
+	if (matchsize != IPT_ALIGN(sizeof(struct ipt_connbytes_info)))
+		return 0;
+
+	if (sinfo->what != IPT_CONNBYTES_WHAT_PKTS &&
+	    sinfo->what != IPT_CONNBYTES_WHAT_BYTES &&
+	    sinfo->what != IPT_CONNBYTES_WHAT_AVGPKT)
+		return 0;
+
+	if (sinfo->direction != IPT_CONNBYTES_DIR_ORIGINAL &&
+	    sinfo->direction != IPT_CONNBYTES_DIR_REPLY &&
+	    sinfo->direction != IPT_CONNBYTES_DIR_BOTH)
+		return 0;
+
+	return 1;
+}
+
+static struct ipt_match state_match = {
+	.name		= "connbytes",
+	.match		= &match,
+	.checkentry	= &check,
+	.me		= THIS_MODULE
+};
+
+static int __init init(void)
+{
+	return ipt_register_match(&state_match);
+}
+
+static void __exit fini(void)
+{
+	ipt_unregister_match(&state_match);
+}
+
+module_init(init);
+module_exit(fini);

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-11 20:03 [PATCH] add new iptables ipt_connbytes match Harald Welte
@ 2005-08-11 22:42 ` David S. Miller
  2005-08-12 11:39   ` Andi Kleen
  2005-08-12 11:46   ` Harald Welte
  2005-08-12  2:52 ` Patrick McHardy
  1 sibling, 2 replies; 26+ messages in thread
From: David S. Miller @ 2005-08-11 22:42 UTC (permalink / raw)
  To: laforge; +Cc: netdev, netfilter-devel

From: Harald Welte <laforge@netfilter.org>
Date: Thu, 11 Aug 2005 22:03:49 +0200

> +struct ipt_connbytes_info
> +{
> +	struct {
> +		u_int64_t from;	/* count to be matched */
> +		u_int64_t to;	/* count to be matched */
> +	} count;
> +	u_int8_t what;		/* ipt_connbytes_what */
> +	u_int8_t direction;	/* ipt_connbytes_direction */
> +};

Won't work in x86 --> x86_64 compat environments.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-11 20:03 [PATCH] add new iptables ipt_connbytes match Harald Welte
  2005-08-11 22:42 ` David S. Miller
@ 2005-08-12  2:52 ` Patrick McHardy
  2005-08-12 11:56   ` Harald Welte
  1 sibling, 1 reply; 26+ messages in thread
From: Patrick McHardy @ 2005-08-12  2:52 UTC (permalink / raw)
  To: Harald Welte; +Cc: Linux Netdev List, Netfilter Development Mailinglist

Harald Welte wrote:
> +/* 64bit divisor, dividend and result. dynamic precision */
> +static u_int64_t div64_64(u_int64_t divisor, u_int64_t dividend)
> +{
> +	u_int64_t result = divisor;
> +
> +	if (dividend > 0xffffffff) {
> +		int first_bit = find_first_bit((unsigned long *) &dividend, sizeof(dividend));
> +		/* calculate number of bits to shift. shift exactly enough
> +		 * bits to make dividend fit in 32bits. */
> +		int num_shift = (64 - 32 - first_bit);
> +		/* first bit has to be < 32, since dividend was > 0xffffffff */
> +		result = result >> num_shift;
> +		dividend = dividend >> num_shift;
> +	}
> +
> +	do_div(divisor, dividend);
> +
> +	return divisor;
> +}

This functions looks broken. Divisor and divident are mixed up, the
shifted result variable is not used in the actual division, the
"first bit has to be < 32" assumption is wrong and num_shift is
calculated incorrectly. To find a 32-bit divisor consisting of the
most-significant 32 bits we need to find the highest bit set and
subtract 32 from this, then right-shift by that value if it is larger
than 0. I can send a fixed patch tomorrow but I'm too tired now.

> +	case IPT_CONNBYTES_WHAT_PKTS:

I would really prefer the name IPT_CONNBYTES_PKTS :)

Regards
Patrick

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-11 22:42 ` David S. Miller
@ 2005-08-12 11:39   ` Andi Kleen
  2005-08-12 11:57     ` Patrick McHardy
  2005-08-12 11:46   ` Harald Welte
  1 sibling, 1 reply; 26+ messages in thread
From: Andi Kleen @ 2005-08-12 11:39 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, netfilter-devel

"David S. Miller" <davem@davemloft.net> writes:

> From: Harald Welte <laforge@netfilter.org>
> Date: Thu, 11 Aug 2005 22:03:49 +0200
> 
> > +struct ipt_connbytes_info
> > +{
> > +	struct {
> > +		u_int64_t from;	/* count to be matched */
> > +		u_int64_t to;	/* count to be matched */
> > +	} count;
> > +	u_int8_t what;		/* ipt_connbytes_what */
> > +	u_int8_t direction;	/* ipt_connbytes_direction */
> > +};
> 
> Won't work in x86 --> x86_64 compat environments.

Thanks for catching it.

The aligned u64 trick probably will

#define aligned_u64 unsigned long long __attribute__((aligned(8)))

It just forces i386 to be aligned too.

Then use aligned_u64 instead of u64/__u64/u_int64_t in all user visible 
places. Similar for signed types.

-Andi

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-11 22:42 ` David S. Miller
  2005-08-12 11:39   ` Andi Kleen
@ 2005-08-12 11:46   ` Harald Welte
  1 sibling, 0 replies; 26+ messages in thread
From: Harald Welte @ 2005-08-12 11:46 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, netfilter-devel

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

On Thu, Aug 11, 2005 at 03:42:04PM -0700, David S. Miller wrote:
> From: Harald Welte <laforge@netfilter.org>
> Date: Thu, 11 Aug 2005 22:03:49 +0200
> 
> > +struct ipt_connbytes_info
> > +{
> > +	struct {
> > +		u_int64_t from;	/* count to be matched */
> > +		u_int64_t to;	/* count to be matched */
> > +	} count;
> > +	u_int8_t what;		/* ipt_connbytes_what */
> > +	u_int8_t direction;	/* ipt_connbytes_direction */
> > +};
> 
> Won't work in x86 --> x86_64 compat environments.

jeez, it bites me again (the connbytes match was written some time
ago..) .  I'll try (and actually verify) Andi Kleen's suggested version.

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-12  2:52 ` Patrick McHardy
@ 2005-08-12 11:56   ` Harald Welte
  2005-08-13  1:20     ` Patrick McHardy
  0 siblings, 1 reply; 26+ messages in thread
From: Harald Welte @ 2005-08-12 11:56 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Linux Netdev List, Netfilter Development Mailinglist

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

On Fri, Aug 12, 2005 at 04:52:49AM +0200, Patrick McHardy wrote:

> This functions looks broken. 

I feared it...

> Divisor and divident are mixed up, the
> shifted result variable is not used in the actual division, the
> "first bit has to be < 32" assumption is wrong and num_shift is
> calculated incorrectly. To find a 32-bit divisor consisting of the
> most-significant 32 bits we need to find the highest bit set and
> subtract 32 from this, then right-shift by that value if it is larger
> than 0. I can send a fixed patch tomorrow but I'm too tired now.

Thanks.

> >+	case IPT_CONNBYTES_WHAT_PKTS:
> 
> I would really prefer the name IPT_CONNBYTES_PKTS :)

I _think_ it's sure to change it, since we don't include ipt_connbytes.h
in the iptables package.

Just send two incremental patches to Dave.

Cheers,
	Harald
-- 
- 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 --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-12 11:39   ` Andi Kleen
@ 2005-08-12 11:57     ` Patrick McHardy
  2005-08-12 12:03       ` Andi Kleen
  0 siblings, 1 reply; 26+ messages in thread
From: Patrick McHardy @ 2005-08-12 11:57 UTC (permalink / raw)
  To: Andi Kleen; +Cc: netdev, netfilter-devel

Andi Kleen wrote:
> "David S. Miller" <davem@davemloft.net> writes:
>>
>>Won't work in x86 --> x86_64 compat environments.
> 
> Thanks for catching it.
> 
> The aligned u64 trick probably will
> 
> #define aligned_u64 unsigned long long __attribute__((aligned(8)))
> 
> It just forces i386 to be aligned too.
> 
> Then use aligned_u64 instead of u64/__u64/u_int64_t in all user visible 
> places. Similar for signed types.

Unfortunately one of the iptables structures which is needed to get the
ruleset in the kernel (ipt_replace) is differently sized when compiled
for 32/64 bit. IIRC it doesn't work at all currently.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-12 11:57     ` Patrick McHardy
@ 2005-08-12 12:03       ` Andi Kleen
  2005-08-12 15:37         ` Harald Welte
  0 siblings, 1 reply; 26+ messages in thread
From: Andi Kleen @ 2005-08-12 12:03 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netdev, netfilter-devel, Andi Kleen

> Unfortunately one of the iptables structures which is needed to get the
> ruleset in the kernel (ipt_replace) is differently sized when compiled
> for 32/64 bit. IIRC it doesn't work at all currently.

Yes that's the old bug and cannot be fixed without breaking compatibility. 

But we hope that ctnetlink will not repeat that mistake. That is why I'm suggesting
to use aligned_u64 in all new interfaces

-Andi

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-12 12:03       ` Andi Kleen
@ 2005-08-12 15:37         ` Harald Welte
  2005-08-12 18:12           ` David S. Miller
  2005-08-12 18:23           ` Andi Kleen
  0 siblings, 2 replies; 26+ messages in thread
From: Harald Welte @ 2005-08-12 15:37 UTC (permalink / raw)
  To: Andi Kleen; +Cc: netdev, netfilter-devel, Patrick McHardy

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

On Fri, Aug 12, 2005 at 02:03:20PM +0200, Andi Kleen wrote:
> > Unfortunately one of the iptables structures which is needed to get the
> > ruleset in the kernel (ipt_replace) is differently sized when compiled
> > for 32/64 bit. IIRC it doesn't work at all currently.
> 
> Yes that's the old bug and cannot be fixed without breaking compatibility. 
> 
> But we hope that ctnetlink will not repeat that mistake. That is why I'm suggesting
> to use aligned_u64 in all new interfaces

I'll soon push a patch for all nfnetlink_{conntrack,queue,log} stuff for
net-2.6.14.  Don't worry about that.

But getting back to the original connbytes issue.  Is it worth fixing
it, if the core iptables doesn't even work (the "old bug")?

I don't think that we're ever going to fix that bug in the old
{get,set}sockopt interface, but rather introduce a netlink interface
when pkt_tables matures.

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-12 15:37         ` Harald Welte
@ 2005-08-12 18:12           ` David S. Miller
  2005-08-12 18:23           ` Andi Kleen
  1 sibling, 0 replies; 26+ messages in thread
From: David S. Miller @ 2005-08-12 18:12 UTC (permalink / raw)
  To: laforge; +Cc: netdev, netfilter-devel, ak, kaber

From: Harald Welte <laforge@netfilter.org>
Date: Fri, 12 Aug 2005 17:37:30 +0200

> But getting back to the original connbytes issue.  Is it worth fixing
> it, if the core iptables doesn't even work (the "old bug")?

I think it is a good policy to not let in new code, regardless of
context, which uses __u64 in user visible structures a way we know
isn't compatible.

So please let's fix this up.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-12 15:37         ` Harald Welte
  2005-08-12 18:12           ` David S. Miller
@ 2005-08-12 18:23           ` Andi Kleen
  2005-08-12 19:03             ` Harald Welte
  1 sibling, 1 reply; 26+ messages in thread
From: Andi Kleen @ 2005-08-12 18:23 UTC (permalink / raw)
  To: Harald Welte, Andi Kleen, Patrick McHardy, David S. Miller,
	netdev, netfilter-devel

> I don't think that we're ever going to fix that bug in the old
> {get,set}sockopt interface, but rather introduce a netlink interface
> when pkt_tables matures.

All new interfaces should be emulation clean, so that if the old interface
is replaced later it should eventually work. The best way to do that
is to use aligned_u64. Should probably put that into linux/types.h

-Andi

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-12 18:23           ` Andi Kleen
@ 2005-08-12 19:03             ` Harald Welte
  2005-08-12 19:08               ` Andi Kleen
  2005-08-12 19:09               ` David S. Miller
  0 siblings, 2 replies; 26+ messages in thread
From: Harald Welte @ 2005-08-12 19:03 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, netfilter-devel, Patrick McHardy, Andi Kleen


[-- Attachment #1.1: Type: text/plain, Size: 878 bytes --]

On Fri, Aug 12, 2005 at 08:23:55PM +0200, Andi Kleen wrote:
> > I don't think that we're ever going to fix that bug in the old
> > {get,set}sockopt interface, but rather introduce a netlink interface
> > when pkt_tables matures.
> 
> All new interfaces should be emulation clean, so that if the old interface
> is replaced later it should eventually work. The best way to do that
> is to use aligned_u64. Should probably put that into linux/types.h

Ok, I hope everyone is fine with this patch:

-- 
- 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 #1.2: 48-aligned_u64.patch --]
[-- Type: text/plain, Size: 3326 bytes --]

[NETFILTER] introduce and use aligned_u64 data type

As proposed by Andi Kleen, this is required esp. for x86_64 architecture,
where 64bit code needs 8byte aligned 64bit data types, but 32bit userspace
apps will only align to 4bytes.

Signed-off-by: Harald Welte <laforge@netfilter.org>

---
commit 30da9a3da187af74b2e2d00becf2d9cab3624ddd
tree 7666f6ce67e96beedc8884f1aba18ea80a20e2b1
parent 7c249f391a3b9bc86ec07d734959c532a3c7a3f6
author Harald Welte <laforge@netfilter.org> Fr, 12 Aug 2005 21:00:28 +0200
committer Harald Welte <laforge@netfilter.org> Fr, 12 Aug 2005 21:00:28 +0200

 include/linux/netfilter/nfnetlink_log.h      |    5 +++--
 include/linux/netfilter/nfnetlink_queue.h    |    5 +++--
 include/linux/netfilter_ipv4/ipt_connbytes.h |    4 ++--
 include/linux/types.h                        |    3 +++
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_log.h b/include/linux/netfilter/nfnetlink_log.h
--- a/include/linux/netfilter/nfnetlink_log.h
+++ b/include/linux/netfilter/nfnetlink_log.h
@@ -5,6 +5,7 @@
  * and not any kind of function definitions.  It is shared between kernel and
  * userspace.  Don't put kernel specific stuff in here */
 
+#include <linux/types.h>
 #include <linux/netfilter/nfnetlink.h>
 
 enum nfulnl_msg_types {
@@ -27,8 +28,8 @@ struct nfulnl_msg_packet_hw {
 } __attribute__ ((packed));
 
 struct nfulnl_msg_packet_timestamp {
-	u_int64_t	sec;
-	u_int64_t	usec;
+	aligned_u64	sec;
+	aligned_u64	usec;
 } __attribute__ ((packed));
 
 #define NFULNL_PREFIXLEN	30	/* just like old log target */
diff --git a/include/linux/netfilter/nfnetlink_queue.h b/include/linux/netfilter/nfnetlink_queue.h
--- a/include/linux/netfilter/nfnetlink_queue.h
+++ b/include/linux/netfilter/nfnetlink_queue.h
@@ -1,6 +1,7 @@
 #ifndef _NFNETLINK_QUEUE_H
 #define _NFNETLINK_QUEUE_H
 
+#include <linux/types.h>
 #include <linux/netfilter/nfnetlink.h>
 
 enum nfqnl_msg_types {
@@ -24,8 +25,8 @@ struct nfqnl_msg_packet_hw {
 } __attribute__ ((packed));
 
 struct nfqnl_msg_packet_timestamp {
-	u_int64_t	sec;
-	u_int64_t	usec;
+	aligned_u64	sec;
+	aligned_u64	usec;
 } __attribute__ ((packed));
 
 enum nfqnl_attr_type {
diff --git a/include/linux/netfilter_ipv4/ipt_connbytes.h b/include/linux/netfilter_ipv4/ipt_connbytes.h
--- a/include/linux/netfilter_ipv4/ipt_connbytes.h
+++ b/include/linux/netfilter_ipv4/ipt_connbytes.h
@@ -16,8 +16,8 @@ enum ipt_connbytes_direction {
 struct ipt_connbytes_info
 {
 	struct {
-		u_int64_t from;	/* count to be matched */
-		u_int64_t to;	/* count to be matched */
+		aligned_u64 from;	/* count to be matched */
+		aligned_u64 to;		/* count to be matched */
 	} count;
 	u_int8_t what;		/* ipt_connbytes_what */
 	u_int8_t direction;	/* ipt_connbytes_direction */
diff --git a/include/linux/types.h b/include/linux/types.h
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -123,6 +123,9 @@ typedef		__u64		u_int64_t;
 typedef		__s64		int64_t;
 #endif
 
+/* this is a special 64bit data type that is 8-byte aligned */
+#define aligned_u64 unsigned long long __attribute__((aligned(8)))
+
 /*
  * The type used for indexing onto a disc or disc partition.
  * If required, asm/types.h can override it and define

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-12 19:03             ` Harald Welte
@ 2005-08-12 19:08               ` Andi Kleen
  2005-08-12 19:09               ` David S. Miller
  1 sibling, 0 replies; 26+ messages in thread
From: Andi Kleen @ 2005-08-12 19:08 UTC (permalink / raw)
  To: Harald Welte, David Miller, Patrick McHardy, netdev,
	netfilter-devel, Andi Kleen


Looks good. Thanks,
-Andi

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-12 19:03             ` Harald Welte
  2005-08-12 19:08               ` Andi Kleen
@ 2005-08-12 19:09               ` David S. Miller
  2005-08-13 14:50                 ` Harald Welte
                                   ` (2 more replies)
  1 sibling, 3 replies; 26+ messages in thread
From: David S. Miller @ 2005-08-12 19:09 UTC (permalink / raw)
  To: laforge; +Cc: netdev, netfilter-devel, kaber, ak

From: Harald Welte <laforge@netfilter.org>
Date: Fri, 12 Aug 2005 21:03:43 +0200

> Ok, I hope everyone is fine with this patch:

It is, but I did not add the connbytes patch into my tree so I can't
use this patch as-is.  That's why I replied "this is broken, fix u64
alignment" to the connbytes patch instead of "applied, thanks" :-)

Please untangle this stuff.  This is how we end up with a big mess of
noise changesets in the tree, due to how we have been putting
half-working changes in first then a bunch of "fixup" patches.  I'd
like to avoid that, because I then spend a lot of time redoing things
when I rebase the tree later.

So in this case, send me the aligned_u64 patch seperately which
doesn't assume connbytes is in the tree.  Then another patch which
adds connbytes with the proper usage of aligned_u64.

Thanks Harald.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-12 11:56   ` Harald Welte
@ 2005-08-13  1:20     ` Patrick McHardy
  2005-08-13 14:51       ` Harald Welte
                         ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Patrick McHardy @ 2005-08-13  1:20 UTC (permalink / raw)
  To: Harald Welte; +Cc: Linux Netdev List, Netfilter Development Mailinglist

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

Harald Welte wrote:
> Just send two incremental patches to Dave.

Here they are. The first patch fixes the div64_64 function, the second
one renames some constants.


[-- Attachment #2: 01.diff --]
[-- Type: text/x-patch, Size: 1723 bytes --]

[NETFILTER]: Fix div64_64 in ipt_connbytes

Signded-off-by: Patrick McHardy <kaber@trash.net>

---
commit 62084bc1a04e2fbc492566fa30997bd0a7aa2d0a
tree 083c8042609e0da81f0be9e15583d5d31b54e685
parent 68e734a5864ba568058c3b8ea63fac3d7d567542
author Patrick McHardy <kaber@trash.net> Sat, 13 Aug 2005 03:16:32 +0200
committer Patrick McHardy <kaber@trash.net> Sat, 13 Aug 2005 03:16:32 +0200

 net/ipv4/netfilter/ipt_connbytes.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_connbytes.c b/net/ipv4/netfilter/ipt_connbytes.c
--- a/net/ipv4/netfilter/ipt_connbytes.c
+++ b/net/ipv4/netfilter/ipt_connbytes.c
@@ -22,23 +22,19 @@ MODULE_AUTHOR("Harald Welte <laforge@net
 MODULE_DESCRIPTION("iptables match for matching number of pkts/bytes per connection");
 
 /* 64bit divisor, dividend and result. dynamic precision */
-static u_int64_t div64_64(u_int64_t divisor, u_int64_t dividend)
+static u_int64_t div64_64(u_int64_t dividend, u_int64_t divisor)
 {
-	u_int64_t result = divisor;
+	u_int32_t d = divisor;
 
-	if (dividend > 0xffffffff) {
-		int first_bit = find_first_bit((unsigned long *) &dividend, sizeof(dividend));
-		/* calculate number of bits to shift. shift exactly enough
-		 * bits to make dividend fit in 32bits. */
-		int num_shift = (64 - 32 - first_bit);
-		/* first bit has to be < 32, since dividend was > 0xffffffff */
-		result = result >> num_shift;
-		dividend = dividend >> num_shift;
-	}
+	if (divisor > 0xffffffffULL) {
+		unsigned int shift = fls(divisor >> 32);
 
-	do_div(divisor, dividend);
+		d = divisor >> shift;
+		dividend >>= shift;
+	}
 
-	return divisor;
+	do_div(dividend, d);
+	return dividend;
 }
 
 static int

[-- Attachment #3: 02.diff --]
[-- Type: text/x-patch, Size: 2507 bytes --]

[NETFILTER]: Nicer names for ipt_connbytes constants

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit e0d3e09ba22a139cbee328bc2622e984b65ba53e
tree 83efbc8d53045825333db5f4e321e28b331f7e30
parent 8f48c662a6f9cd2cdaec9d9866cebf8b40155f70
author Patrick McHardy <kaber@trash.net> Sat, 13 Aug 2005 03:18:30 +0200
committer Patrick McHardy <kaber@trash.net> Sat, 13 Aug 2005 03:18:30 +0200

 include/linux/netfilter_ipv4/ipt_connbytes.h |    6 +++---
 net/ipv4/netfilter/ipt_connbytes.c           |   12 ++++++------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/linux/netfilter_ipv4/ipt_connbytes.h b/include/linux/netfilter_ipv4/ipt_connbytes.h
--- a/include/linux/netfilter_ipv4/ipt_connbytes.h
+++ b/include/linux/netfilter_ipv4/ipt_connbytes.h
@@ -2,9 +2,9 @@
 #define _IPT_CONNBYTES_H
 
 enum ipt_connbytes_what {
-	IPT_CONNBYTES_WHAT_PKTS,
-	IPT_CONNBYTES_WHAT_BYTES,
-	IPT_CONNBYTES_WHAT_AVGPKT,
+	IPT_CONNBYTES_PKTS,
+	IPT_CONNBYTES_BYTES,
+	IPT_CONNBYTES_AVGPKT,
 };
 
 enum ipt_connbytes_direction {
diff --git a/net/ipv4/netfilter/ipt_connbytes.c b/net/ipv4/netfilter/ipt_connbytes.c
--- a/net/ipv4/netfilter/ipt_connbytes.c
+++ b/net/ipv4/netfilter/ipt_connbytes.c
@@ -54,7 +54,7 @@ match(const struct sk_buff *skb,
 		return 0; /* no match */
 
 	switch (sinfo->what) {
-	case IPT_CONNBYTES_WHAT_PKTS:
+	case IPT_CONNBYTES_PKTS:
 		switch (sinfo->direction) {
 		case IPT_CONNBYTES_DIR_ORIGINAL:
 			what = ct->counters[IP_CT_DIR_ORIGINAL].packets;
@@ -68,7 +68,7 @@ match(const struct sk_buff *skb,
 			break;
 		}
 		break;
-	case IPT_CONNBYTES_WHAT_BYTES:
+	case IPT_CONNBYTES_BYTES:
 		switch (sinfo->direction) {
 		case IPT_CONNBYTES_DIR_ORIGINAL:
 			what = ct->counters[IP_CT_DIR_ORIGINAL].bytes;
@@ -82,7 +82,7 @@ match(const struct sk_buff *skb,
 			break;
 		}
 		break;
-	case IPT_CONNBYTES_WHAT_AVGPKT:
+	case IPT_CONNBYTES_AVGPKT:
 		switch (sinfo->direction) {
 		case IPT_CONNBYTES_DIR_ORIGINAL:
 			what = div64_64(ct->counters[IP_CT_DIR_ORIGINAL].bytes,
@@ -128,9 +128,9 @@ static int check(const char *tablename,
 	if (matchsize != IPT_ALIGN(sizeof(struct ipt_connbytes_info)))
 		return 0;
 
-	if (sinfo->what != IPT_CONNBYTES_WHAT_PKTS &&
-	    sinfo->what != IPT_CONNBYTES_WHAT_BYTES &&
-	    sinfo->what != IPT_CONNBYTES_WHAT_AVGPKT)
+	if (sinfo->what != IPT_CONNBYTES_PKTS &&
+	    sinfo->what != IPT_CONNBYTES_BYTES &&
+	    sinfo->what != IPT_CONNBYTES_AVGPKT)
 		return 0;
 
 	if (sinfo->direction != IPT_CONNBYTES_DIR_ORIGINAL &&

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-12 19:09               ` David S. Miller
@ 2005-08-13 14:50                 ` Harald Welte
  2005-08-13 20:54                   ` David S. Miller
  2005-08-13 15:45                 ` [PATCH] introduce and use aligned_u64 in nfnetlink Harald Welte
  2005-08-13 15:46                 ` [PATCH] add new iptables ipt_connbytes match Harald Welte
  2 siblings, 1 reply; 26+ messages in thread
From: Harald Welte @ 2005-08-13 14:50 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, netfilter-devel, kaber, ak

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

On Fri, Aug 12, 2005 at 12:09:04PM -0700, David S. Miller wrote:
> From: Harald Welte <laforge@netfilter.org>
> Date: Fri, 12 Aug 2005 21:03:43 +0200
> 
> > Ok, I hope everyone is fine with this patch:
> 
> It is, but I did not add the connbytes patch into my tree so I can't
> use this patch as-is.  That's why I replied "this is broken, fix u64
> alignment" to the connbytes patch instead of "applied, thanks" :-)
> 
> Please untangle this stuff.  This is how we end up with a big mess of
> noise changesets in the tree, due to how we have been putting
> half-working changes in first then a bunch of "fixup" patches.  I'd
> like to avoid that, because I then spend a lot of time redoing things
> when I rebase the tree later.

Fine, I can understand that in the case of the connbytes/unaligned case.

Speaking more generally:
I know I've been the exact opposite with all the nfnetlink_* stuff.
But the problem is to a certain degree that bugs aren't discovered until
it's in some tree that a lot of people use...

I think our whole usual strategy 'put it in patch-o-matic and wait until
bugs have been shaken out' doesn't really work because there's too
little people actually using the code from there.

So for new development, I'm now more inclined to push things sooner to
you - even more for code that only adds new featurss. If you generally
dislike that, please let me know.

> So in this case, send me the aligned_u64 patch seperately which
> doesn't assume connbytes is in the tree.  Then another patch which
> adds connbytes with the proper usage of aligned_u64.

sure.  no problem. Will be sent soon.

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-13  1:20     ` Patrick McHardy
@ 2005-08-13 14:51       ` Harald Welte
  2005-08-13 20:59         ` David S. Miller
  2005-08-13 20:58       ` David S. Miller
  2005-08-16 11:18       ` Amin Azez
  2 siblings, 1 reply; 26+ messages in thread
From: Harald Welte @ 2005-08-13 14:51 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Linux Netdev List, Netfilter Development Mailinglist

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

On Sat, Aug 13, 2005 at 03:20:06AM +0200, Patrick McHardy wrote:
> Harald Welte wrote:
> >Just send two incremental patches to Dave.
> 
> Here they are. The first patch fixes the div64_64 function, the second
> one renames some constants.

Ok,  just in case Dave was waiting for my comments (which are usually
not required since Patricks patches tend to have a higher quality than
mine):

ACK-ed-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 --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH] introduce and use aligned_u64 in nfnetlink
  2005-08-12 19:09               ` David S. Miller
  2005-08-13 14:50                 ` Harald Welte
@ 2005-08-13 15:45                 ` Harald Welte
  2005-08-13 20:56                   ` David S. Miller
  2005-08-13 15:46                 ` [PATCH] add new iptables ipt_connbytes match Harald Welte
  2 siblings, 1 reply; 26+ messages in thread
From: Harald Welte @ 2005-08-13 15:45 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, netfilter-devel, kaber, ak


[-- Attachment #1.1: Type: text/plain, Size: 416 bytes --]

This time without the ipt_connbytes hunk:

-- 
- 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 #1.2: 48-aligned_u64.patch --]
[-- Type: text/plain, Size: 2669 bytes --]

[NETFILTER] introduce and use aligned_u64 data type

As proposed by Andi Kleen, this is required esp. for x86_64 architecture,
where 64bit code needs 8byte aligned 64bit data types, but 32bit userspace
apps will only align to 4bytes.

Signed-off-by: Harald Welte <laforge@netfilter.org>

---
commit 30da9a3da187af74b2e2d00becf2d9cab3624ddd
tree 7666f6ce67e96beedc8884f1aba18ea80a20e2b1
parent 7c249f391a3b9bc86ec07d734959c532a3c7a3f6
author Harald Welte <laforge@netfilter.org> Fr, 12 Aug 2005 21:00:28 +0200
committer Harald Welte <laforge@netfilter.org> Fr, 12 Aug 2005 21:00:28 +0200

 include/linux/netfilter/nfnetlink_log.h      |    5 +++--
 include/linux/netfilter/nfnetlink_queue.h    |    5 +++--
 include/linux/types.h                        |    3 +++
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_log.h b/include/linux/netfilter/nfnetlink_log.h
--- a/include/linux/netfilter/nfnetlink_log.h
+++ b/include/linux/netfilter/nfnetlink_log.h
@@ -5,6 +5,7 @@
  * and not any kind of function definitions.  It is shared between kernel and
  * userspace.  Don't put kernel specific stuff in here */
 
+#include <linux/types.h>
 #include <linux/netfilter/nfnetlink.h>
 
 enum nfulnl_msg_types {
@@ -27,8 +28,8 @@ struct nfulnl_msg_packet_hw {
 } __attribute__ ((packed));
 
 struct nfulnl_msg_packet_timestamp {
-	u_int64_t	sec;
-	u_int64_t	usec;
+	aligned_u64	sec;
+	aligned_u64	usec;
 } __attribute__ ((packed));
 
 #define NFULNL_PREFIXLEN	30	/* just like old log target */
diff --git a/include/linux/netfilter/nfnetlink_queue.h b/include/linux/netfilter/nfnetlink_queue.h
--- a/include/linux/netfilter/nfnetlink_queue.h
+++ b/include/linux/netfilter/nfnetlink_queue.h
@@ -1,6 +1,7 @@
 #ifndef _NFNETLINK_QUEUE_H
 #define _NFNETLINK_QUEUE_H
 
+#include <linux/types.h>
 #include <linux/netfilter/nfnetlink.h>
 
 enum nfqnl_msg_types {
@@ -24,8 +25,8 @@ struct nfqnl_msg_packet_hw {
 } __attribute__ ((packed));
 
 struct nfqnl_msg_packet_timestamp {
-	u_int64_t	sec;
-	u_int64_t	usec;
+	aligned_u64	sec;
+	aligned_u64	usec;
 } __attribute__ ((packed));
 
 enum nfqnl_attr_type {
diff --git a/include/linux/types.h b/include/linux/types.h
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -123,6 +123,9 @@ typedef		__u64		u_int64_t;
 typedef		__s64		int64_t;
 #endif
 
+/* this is a special 64bit data type that is 8-byte aligned */
+#define aligned_u64 unsigned long long __attribute__((aligned(8)))
+
 /*
  * The type used for indexing onto a disc or disc partition.
  * If required, asm/types.h can override it and define

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-12 19:09               ` David S. Miller
  2005-08-13 14:50                 ` Harald Welte
  2005-08-13 15:45                 ` [PATCH] introduce and use aligned_u64 in nfnetlink Harald Welte
@ 2005-08-13 15:46                 ` Harald Welte
  2005-08-13 20:56                   ` David S. Miller
  2 siblings, 1 reply; 26+ messages in thread
From: Harald Welte @ 2005-08-13 15:46 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, netfilter-devel, kaber, ak


[-- Attachment #1.1: Type: text/plain, Size: 444 bytes --]

The reworked version (already uses aligned_u64).  Pleas apply, thanks

-- 
- 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 #1.2: 46-ipt_connbytes.patch --]
[-- Type: text/plain, Size: 8283 bytes --]

[NETFILTER] Add new iptables "connbytes" match

This patch ads a new "connbytes" match that utilizes the CONFIG_NF_CT_ACCT
per-connection byte and packet counters.  Using it you can do things like
packet classification on average packet size within a connection.

Signed-off-by: Harald Welte <laforge@netfilter.org>

---
commit 98a0b5554e6e54a5a5a21b21b212186dc3c2118c
tree 43acbfa60366e1b111272851f1a1745a43a71d74
parent 5896088f554d8c2e7f3226973d1413df339e6b75
author Harald Welte <laforge@netfilter.org> Do, 11 Aug 2005 21:15:45 +0200
committer Harald Welte <laforge@netfilter.org> Do, 11 Aug 2005 21:15:45 +0200

 include/linux/netfilter_ipv4/ipt_connbytes.h |   25 ++++
 net/ipv4/netfilter/Kconfig                   |   11 ++
 net/ipv4/netfilter/Makefile                  |    1 
 net/ipv4/netfilter/ipt_connbytes.c           |  166 ++++++++++++++++++++++++++
 4 files changed, 202 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter_ipv4/ipt_connbytes.h b/include/linux/netfilter_ipv4/ipt_connbytes.h
new file mode 100644
--- /dev/null
+++ b/include/linux/netfilter_ipv4/ipt_connbytes.h
@@ -0,0 +1,25 @@
+#ifndef _IPT_CONNBYTES_H
+#define _IPT_CONNBYTES_H
+
+enum ipt_connbytes_what {
+	IPT_CONNBYTES_WHAT_PKTS,
+	IPT_CONNBYTES_WHAT_BYTES,
+	IPT_CONNBYTES_WHAT_AVGPKT,
+};
+
+enum ipt_connbytes_direction {
+	IPT_CONNBYTES_DIR_ORIGINAL,
+	IPT_CONNBYTES_DIR_REPLY,
+	IPT_CONNBYTES_DIR_BOTH,
+};
+
+struct ipt_connbytes_info
+{
+	struct {
+		aligned_u64 from;	/* count to be matched */
+		aligned_u64 to;		/* count to be matched */
+	} count;
+	u_int8_t what;		/* ipt_connbytes_what */
+	u_int8_t direction;	/* ipt_connbytes_direction */
+};
+#endif
diff --git a/net/ipv4/netfilter/Kconfig b/net/ipv4/netfilter/Kconfig
--- a/net/ipv4/netfilter/Kconfig
+++ b/net/ipv4/netfilter/Kconfig
@@ -386,6 +386,16 @@ config IP_NF_MATCH_CONNMARK
 	  <file:Documentation/modules.txt>.  The module will be called
 	  ipt_connmark.o.  If unsure, say `N'.
 
+config IP_NF_MATCH_CONNBYTES
+	tristate  'Connection byte/packet counter match support'
+	depends on IP_NF_CT_ACCT && IP_NF_IPTABLES
+	help
+	  This option adds a `connbytes' match, which allows you to match the
+	  number of bytes and/or packets for each direction within a connection.
+
+	  If you want to compile it as a module, say M here and read
+	  <file:Documentation/modules.txt>.  If unsure, say `N'.
+
 config IP_NF_MATCH_HASHLIMIT
 	tristate  'hashlimit match support'
 	depends on IP_NF_IPTABLES
@@ -723,6 +733,5 @@ config IP_NF_CONNTRACK_NETLINK
         help
           This option enables support for a netlink-based userspace interface
 
-
 endmenu
 
diff --git a/net/ipv4/netfilter/Makefile b/net/ipv4/netfilter/Makefile
--- a/net/ipv4/netfilter/Makefile
+++ b/net/ipv4/netfilter/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_IP_NF_MATCH_TTL) += ipt_ttl
 obj-$(CONFIG_IP_NF_MATCH_STATE) += ipt_state.o
 obj-$(CONFIG_IP_NF_MATCH_CONNMARK) += ipt_connmark.o
 obj-$(CONFIG_IP_NF_MATCH_CONNTRACK) += ipt_conntrack.o
+obj-$(CONFIG_IP_NF_MATCH_CONNBYTES) += ipt_connbytes.o
 obj-$(CONFIG_IP_NF_MATCH_TCPMSS) += ipt_tcpmss.o
 obj-$(CONFIG_IP_NF_MATCH_REALM) += ipt_realm.o
 obj-$(CONFIG_IP_NF_MATCH_ADDRTYPE) += ipt_addrtype.o
diff --git a/net/ipv4/netfilter/ipt_connbytes.c b/net/ipv4/netfilter/ipt_connbytes.c
new file mode 100644
--- /dev/null
+++ b/net/ipv4/netfilter/ipt_connbytes.c
@@ -0,0 +1,166 @@
+/* Kernel module to match connection tracking byte counter.
+ * GPL (C) 2002 Martin Devera (devik@cdi.cz).
+ *
+ * 2004-07-20 Harald Welte <laforge@netfilter.org>
+ * 	- reimplemented to use per-connection accounting counters
+ * 	- add functionality to match number of packets
+ * 	- add functionality to match average packet size
+ * 	- add support to match directions seperately
+ *
+ */
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/netfilter_ipv4/ip_conntrack.h>
+#include <linux/netfilter_ipv4/ip_tables.h>
+#include <linux/netfilter_ipv4/ipt_connbytes.h>
+
+#include <asm/div64.h>
+#include <asm/bitops.h>
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Harald Welte <laforge@netfilter.org>");
+MODULE_DESCRIPTION("iptables match for matching number of pkts/bytes per connection");
+
+/* 64bit divisor, dividend and result. dynamic precision */
+static u_int64_t div64_64(u_int64_t divisor, u_int64_t dividend)
+{
+	u_int64_t result = divisor;
+
+	if (dividend > 0xffffffff) {
+		int first_bit = find_first_bit((unsigned long *) &dividend, sizeof(dividend));
+		/* calculate number of bits to shift. shift exactly enough
+		 * bits to make dividend fit in 32bits. */
+		int num_shift = (64 - 32 - first_bit);
+		/* first bit has to be < 32, since dividend was > 0xffffffff */
+		result = result >> num_shift;
+		dividend = dividend >> num_shift;
+	}
+
+	do_div(divisor, dividend);
+
+	return divisor;
+}
+
+static int
+match(const struct sk_buff *skb,
+      const struct net_device *in,
+      const struct net_device *out,
+      const void *matchinfo,
+      int offset,
+      int *hotdrop)
+{
+	const struct ipt_connbytes_info *sinfo = matchinfo;
+	enum ip_conntrack_info ctinfo;
+	struct ip_conntrack *ct;
+	u_int64_t what = 0;	/* initialize to make gcc happy */
+
+	if (!(ct = ip_conntrack_get((struct sk_buff *)skb, &ctinfo)))
+		return 0; /* no match */
+
+	switch (sinfo->what) {
+	case IPT_CONNBYTES_WHAT_PKTS:
+		switch (sinfo->direction) {
+		case IPT_CONNBYTES_DIR_ORIGINAL:
+			what = ct->counters[IP_CT_DIR_ORIGINAL].packets;
+			break;
+		case IPT_CONNBYTES_DIR_REPLY:
+			what = ct->counters[IP_CT_DIR_REPLY].packets;
+			break;
+		case IPT_CONNBYTES_DIR_BOTH:
+			what = ct->counters[IP_CT_DIR_ORIGINAL].packets;
+			what += ct->counters[IP_CT_DIR_REPLY].packets;
+			break;
+		}
+		break;
+	case IPT_CONNBYTES_WHAT_BYTES:
+		switch (sinfo->direction) {
+		case IPT_CONNBYTES_DIR_ORIGINAL:
+			what = ct->counters[IP_CT_DIR_ORIGINAL].bytes;
+			break;
+		case IPT_CONNBYTES_DIR_REPLY:
+			what = ct->counters[IP_CT_DIR_REPLY].bytes;
+			break;
+		case IPT_CONNBYTES_DIR_BOTH:
+			what = ct->counters[IP_CT_DIR_ORIGINAL].bytes;
+			what += ct->counters[IP_CT_DIR_REPLY].bytes;
+			break;
+		}
+		break;
+	case IPT_CONNBYTES_WHAT_AVGPKT:
+		switch (sinfo->direction) {
+		case IPT_CONNBYTES_DIR_ORIGINAL:
+			what = div64_64(ct->counters[IP_CT_DIR_ORIGINAL].bytes,
+					ct->counters[IP_CT_DIR_ORIGINAL].packets);
+			break;
+		case IPT_CONNBYTES_DIR_REPLY:
+			what = div64_64(ct->counters[IP_CT_DIR_REPLY].bytes,
+					ct->counters[IP_CT_DIR_REPLY].packets);
+			break;
+		case IPT_CONNBYTES_DIR_BOTH:
+			{
+				u_int64_t bytes;
+				u_int64_t pkts;
+				bytes = ct->counters[IP_CT_DIR_ORIGINAL].bytes +
+					ct->counters[IP_CT_DIR_REPLY].bytes;
+				pkts = ct->counters[IP_CT_DIR_ORIGINAL].packets+
+					ct->counters[IP_CT_DIR_REPLY].packets;
+
+				/* FIXME_THEORETICAL: what to do if sum
+				 * overflows ? */
+
+				what = div64_64(bytes, pkts);
+			}
+			break;
+		}
+		break;
+	}
+
+	if (sinfo->count.to)
+		return (what <= sinfo->count.to && what >= sinfo->count.from);
+	else
+		return (what >= sinfo->count.from);
+}
+
+static int check(const char *tablename,
+		 const struct ipt_ip *ip,
+		 void *matchinfo,
+		 unsigned int matchsize,
+		 unsigned int hook_mask)
+{
+	const struct ipt_connbytes_info *sinfo = matchinfo;
+
+	if (matchsize != IPT_ALIGN(sizeof(struct ipt_connbytes_info)))
+		return 0;
+
+	if (sinfo->what != IPT_CONNBYTES_WHAT_PKTS &&
+	    sinfo->what != IPT_CONNBYTES_WHAT_BYTES &&
+	    sinfo->what != IPT_CONNBYTES_WHAT_AVGPKT)
+		return 0;
+
+	if (sinfo->direction != IPT_CONNBYTES_DIR_ORIGINAL &&
+	    sinfo->direction != IPT_CONNBYTES_DIR_REPLY &&
+	    sinfo->direction != IPT_CONNBYTES_DIR_BOTH)
+		return 0;
+
+	return 1;
+}
+
+static struct ipt_match state_match = {
+	.name		= "connbytes",
+	.match		= &match,
+	.checkentry	= &check,
+	.me		= THIS_MODULE
+};
+
+static int __init init(void)
+{
+	return ipt_register_match(&state_match);
+}
+
+static void __exit fini(void)
+{
+	ipt_unregister_match(&state_match);
+}
+
+module_init(init);
+module_exit(fini);

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-13 14:50                 ` Harald Welte
@ 2005-08-13 20:54                   ` David S. Miller
  0 siblings, 0 replies; 26+ messages in thread
From: David S. Miller @ 2005-08-13 20:54 UTC (permalink / raw)
  To: laforge; +Cc: netdev, netfilter-devel, kaber, ak

From: Harald Welte <laforge@netfilter.org>
Date: Sat, 13 Aug 2005 16:50:23 +0200

> So for new development, I'm now more inclined to push things sooner to
> you - even more for code that only adds new featurss. If you generally
> dislike that, please let me know.

I think this is the way to go.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] introduce and use aligned_u64 in nfnetlink
  2005-08-13 15:45                 ` [PATCH] introduce and use aligned_u64 in nfnetlink Harald Welte
@ 2005-08-13 20:56                   ` David S. Miller
  0 siblings, 0 replies; 26+ messages in thread
From: David S. Miller @ 2005-08-13 20:56 UTC (permalink / raw)
  To: laforge; +Cc: netdev, netfilter-devel, kaber, ak

From: Harald Welte <laforge@netfilter.org>
Date: Sat, 13 Aug 2005 17:45:34 +0200

> [NETFILTER] introduce and use aligned_u64 data type

Applied.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-13 15:46                 ` [PATCH] add new iptables ipt_connbytes match Harald Welte
@ 2005-08-13 20:56                   ` David S. Miller
  0 siblings, 0 replies; 26+ messages in thread
From: David S. Miller @ 2005-08-13 20:56 UTC (permalink / raw)
  To: laforge; +Cc: netdev, netfilter-devel, kaber, ak

From: Harald Welte <laforge@netfilter.org>
Date: Sat, 13 Aug 2005 17:46:19 +0200

> [NETFILTER] Add new iptables "connbytes" match

Applied.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-13  1:20     ` Patrick McHardy
  2005-08-13 14:51       ` Harald Welte
@ 2005-08-13 20:58       ` David S. Miller
  2005-08-16 11:18       ` Amin Azez
  2 siblings, 0 replies; 26+ messages in thread
From: David S. Miller @ 2005-08-13 20:58 UTC (permalink / raw)
  To: kaber; +Cc: laforge, netdev, netfilter-devel

From: Patrick McHardy <kaber@trash.net>
Subject: Re: [PATCH] add new iptables ipt_connbytes match
Date: Sat, 13 Aug 2005 03:20:06 +0200

> Harald Welte wrote:
> > Just send two incremental patches to Dave.
> 
> Here they are. The first patch fixes the div64_64 function, the second
> one renames some constants.

Both applied, thanks Patrick.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-13 14:51       ` Harald Welte
@ 2005-08-13 20:59         ` David S. Miller
  0 siblings, 0 replies; 26+ messages in thread
From: David S. Miller @ 2005-08-13 20:59 UTC (permalink / raw)
  To: laforge; +Cc: netdev, netfilter-devel, kaber

From: Harald Welte <laforge@netfilter.org>
Date: Sat, 13 Aug 2005 16:51:57 +0200

> Ok,  just in case Dave was waiting for my comments (which are usually
> not required since Patricks patches tend to have a higher quality than
> mine):
> 
> ACK-ed-by: Harald Welte <laforge@netfilter.org>

I like to see ACKs, because it shows explicit agreement amongst
the various developers involved.

Thanks.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-13  1:20     ` Patrick McHardy
  2005-08-13 14:51       ` Harald Welte
  2005-08-13 20:58       ` David S. Miller
@ 2005-08-16 11:18       ` Amin Azez
  2005-08-17 10:29         ` Patrick McHardy
  2 siblings, 1 reply; 26+ messages in thread
From: Amin Azez @ 2005-08-16 11:18 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Linux Netdev List, Netfilter Development Mailinglist

Work well done, so oughtn't div64_64 to go in
include/asm-generic/div64.h one day, to be available kernel wide, as
do_div64_64

I see that net/core/pktgen.c is full of 64 bit division and maybe could
benefit from this, and perhaps a version that does remainders too.

Amin

Patrick McHardy wrote:
> Harald Welte wrote:
> 
>> Just send two incremental patches to Dave.
> 
> 
> Here they are. The first patch fixes the div64_64 function, the second
> one renames some constants.
> 
> 
> ------------------------------------------------------------------------
> 
> [NETFILTER]: Fix div64_64 in ipt_connbytes
> 
> Signded-off-by: Patrick McHardy <kaber@trash.net>
> 
> ---
> commit 62084bc1a04e2fbc492566fa30997bd0a7aa2d0a
> tree 083c8042609e0da81f0be9e15583d5d31b54e685
> parent 68e734a5864ba568058c3b8ea63fac3d7d567542
> author Patrick McHardy <kaber@trash.net> Sat, 13 Aug 2005 03:16:32 +0200
> committer Patrick McHardy <kaber@trash.net> Sat, 13 Aug 2005 03:16:32 +0200
> 
>  net/ipv4/netfilter/ipt_connbytes.c |   22 +++++++++-------------
>  1 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/net/ipv4/netfilter/ipt_connbytes.c b/net/ipv4/netfilter/ipt_connbytes.c
> --- a/net/ipv4/netfilter/ipt_connbytes.c
> +++ b/net/ipv4/netfilter/ipt_connbytes.c
> @@ -22,23 +22,19 @@ MODULE_AUTHOR("Harald Welte <laforge@net
>  MODULE_DESCRIPTION("iptables match for matching number of pkts/bytes per connection");
>  
>  /* 64bit divisor, dividend and result. dynamic precision */
> -static u_int64_t div64_64(u_int64_t divisor, u_int64_t dividend)
> +static u_int64_t div64_64(u_int64_t dividend, u_int64_t divisor)

...

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] add new iptables ipt_connbytes match
  2005-08-16 11:18       ` Amin Azez
@ 2005-08-17 10:29         ` Patrick McHardy
  0 siblings, 0 replies; 26+ messages in thread
From: Patrick McHardy @ 2005-08-17 10:29 UTC (permalink / raw)
  To: Amin Azez; +Cc: Linux Netdev List, Netfilter Development Mailinglist

Amin Azez wrote:
> Work well done, so oughtn't div64_64 to go in
> include/asm-generic/div64.h one day, to be available kernel wide, as
> do_div64_64
> 
> I see that net/core/pktgen.c is full of 64 bit division and maybe could
> benefit from this, and perhaps a version that does remainders too.

So far nothing besides ipt_connbytes and pktgen seems to need
it, and they use different versions. I'd rather leave it where
it is to not encourage more use of it.

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2005-08-17 10:29 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-08-11 20:03 [PATCH] add new iptables ipt_connbytes match Harald Welte
2005-08-11 22:42 ` David S. Miller
2005-08-12 11:39   ` Andi Kleen
2005-08-12 11:57     ` Patrick McHardy
2005-08-12 12:03       ` Andi Kleen
2005-08-12 15:37         ` Harald Welte
2005-08-12 18:12           ` David S. Miller
2005-08-12 18:23           ` Andi Kleen
2005-08-12 19:03             ` Harald Welte
2005-08-12 19:08               ` Andi Kleen
2005-08-12 19:09               ` David S. Miller
2005-08-13 14:50                 ` Harald Welte
2005-08-13 20:54                   ` David S. Miller
2005-08-13 15:45                 ` [PATCH] introduce and use aligned_u64 in nfnetlink Harald Welte
2005-08-13 20:56                   ` David S. Miller
2005-08-13 15:46                 ` [PATCH] add new iptables ipt_connbytes match Harald Welte
2005-08-13 20:56                   ` David S. Miller
2005-08-12 11:46   ` Harald Welte
2005-08-12  2:52 ` Patrick McHardy
2005-08-12 11:56   ` Harald Welte
2005-08-13  1:20     ` Patrick McHardy
2005-08-13 14:51       ` Harald Welte
2005-08-13 20:59         ` David S. Miller
2005-08-13 20:58       ` David S. Miller
2005-08-16 11:18       ` Amin Azez
2005-08-17 10:29         ` Patrick McHardy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).