[PATCH] new 'tcpack' match

[PATCH] new 'tcpack' match

[Jonas Berlin]

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Matches TCP packets that has no payload, i.e. contains only ACKs.

~  iptables -A INPUT -p tcp -m tcpack --tcpack -j CLASSIFY 1:10

Or match non-ack packets:

~  iptables -A INPUT -p tcp -m tcpack ! --tcpack -j CLASSIFY 1:12

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCRkWWxyF48ZTvn+4RAoPhAJwLrvINY2VnZ5yqwvovQ8Ao1hv+7wCgzBLG
s8lz2LADN8KTr+r38X/oL4s=
=Y+Gd
-----END PGP SIGNATURE-----

[-- Attachment #2: tcpack-1.patch --]
[-- Type: text/x-patch, Size: 14684 bytes --]

Index: pom-all/tcpack/linux-2.6/include/linux/netfilter_ipv6/ip6t_tcpack.h
===================================================================
--- pom-all/tcpack/linux-2.6/include/linux/netfilter_ipv6/ip6t_tcpack.h	(revision 0)
+++ pom-all/tcpack/linux-2.6/include/linux/netfilter_ipv6/ip6t_tcpack.h	(revision 0)
@@ -0,0 +1,8 @@
+#ifndef _IP6T_TCPACK_H
+#define _IP6T_TCPACK_H
+
+struct ip6t_tcpack_info {
+      u_int8_t invert;
+};
+
+#endif /*_IP6T_TCPACK_H*/
Index: pom-all/tcpack/linux-2.6/include/linux/netfilter_ipv4/ipt_tcpack.h
===================================================================
--- pom-all/tcpack/linux-2.6/include/linux/netfilter_ipv4/ipt_tcpack.h	(revision 0)
+++ pom-all/tcpack/linux-2.6/include/linux/netfilter_ipv4/ipt_tcpack.h	(revision 0)
@@ -0,0 +1,8 @@
+#ifndef _IPT_TCPACK_H
+#define _IPT_TCPACK_H
+
+struct ipt_tcpack_info {
+      u_int8_t invert;
+};
+
+#endif /*_IPT_TCPACK_H*/
Index: pom-all/tcpack/linux-2.6/net/ipv4/netfilter/Makefile.ladd
===================================================================
--- pom-all/tcpack/linux-2.6/net/ipv4/netfilter/Makefile.ladd	(revision 0)
+++ pom-all/tcpack/linux-2.6/net/ipv4/netfilter/Makefile.ladd	(revision 0)
@@ -0,0 +1,2 @@
+obj-$(CONFIG_IP_NF_MATCH_TOS) += ipt_tos.o
+obj-$(CONFIG_IP_NF_MATCH_TCPACK) += ipt_tcpack.o
Index: pom-all/tcpack/linux-2.6/net/ipv4/netfilter/ipt_tcpack.c
===================================================================
--- pom-all/tcpack/linux-2.6/net/ipv4/netfilter/ipt_tcpack.c	(revision 0)
+++ pom-all/tcpack/linux-2.6/net/ipv4/netfilter/ipt_tcpack.c	(revision 0)
@@ -0,0 +1,72 @@
+/* Kernel module to match TCP ACK packets. */
+/* (C) 2005 Jonas Berlin <xkr47@outerspace.dyndns.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/tcp.h>
+
+#include <linux/netfilter_ipv4/ipt_tcpack.h>
+#include <linux/netfilter_ipv4/ip_tables.h>
+
+MODULE_AUTHOR("Jonas Berlin <xkr47@outerspace.dyndns.org>");
+MODULE_DESCRIPTION("iptables TCP ACK matching module");
+MODULE_LICENSE("GPL");
+
+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_tcpack_info *info = (const struct ipt_tcpack_info *)matchinfo;
+	u_int16_t ihl = skb->nh.iph->ihl * 4;
+	const struct tcphdr *tcph = (const struct tcphdr *)(skb->nh.raw + ihl);
+	return info->invert ^
+		(tcph->doff * 4 == ntohs(skb->nh.iph->tot_len) - ihl);
+}
+
+static int checkentry(const char *tablename,
+		   const struct ipt_ip *ip,
+		   void *matchinfo,
+		   unsigned int matchsize,
+		   unsigned int hook_mask)
+{
+	if (matchsize != IPT_ALIGN(sizeof(struct ipt_tcpack_info)))
+		return 0;
+
+	if (ip->proto != IPPROTO_TCP || (ip->invflags & IPT_INV_PROTO)) {
+		printk("tcpack: not valid for non-tcp\n");
+		return 0;
+	}
+
+	return 1;
+}
+
+static struct ipt_match tcpack_match = {
+	.name		= "tcpack",
+	.match		= &match,
+	.checkentry	= &checkentry,
+	.me		= THIS_MODULE,
+};
+
+static int __init init(void)
+{
+	return ipt_register_match(&tcpack_match);
+}
+
+static void __exit fini(void)
+{
+	ipt_unregister_match(&tcpack_match);
+}
+
+module_init(init);
+module_exit(fini);
Index: pom-all/tcpack/linux-2.6/net/ipv4/netfilter/Kconfig.ladd
===================================================================
--- pom-all/tcpack/linux-2.6/net/ipv4/netfilter/Kconfig.ladd	(revision 0)
+++ pom-all/tcpack/linux-2.6/net/ipv4/netfilter/Kconfig.ladd	(revision 0)
@@ -0,0 +1,9 @@
+config IP_NF_MATCH_TCPACK
+	tristate  'TCP ACK match support'
+	depends on IP_NF_IPTABLES
+	help
+	  This option adds a `tcpack' match, which allow you to match
+	  TCP packets containg no actual data, just ACKs.
+
+	  If you want to compile it as a module, say M here and read
+	  Documentation/modules.txt.  If unsure, say `N'.
Index: pom-all/tcpack/linux-2.6/net/ipv6/netfilter/Makefile.ladd
===================================================================
--- pom-all/tcpack/linux-2.6/net/ipv6/netfilter/Makefile.ladd	(revision 0)
+++ pom-all/tcpack/linux-2.6/net/ipv6/netfilter/Makefile.ladd	(revision 0)
@@ -0,0 +1,2 @@
+obj-$(CONFIG_IP6_NF_TARGET_LOG) += ip6t_LOG.o
+obj-$(CONFIG_IP6_NF_MATCH_TCPACK) += ip6t_tcpack.o
Index: pom-all/tcpack/linux-2.6/net/ipv6/netfilter/Kconfig.ladd
===================================================================
--- pom-all/tcpack/linux-2.6/net/ipv6/netfilter/Kconfig.ladd	(revision 0)
+++ pom-all/tcpack/linux-2.6/net/ipv6/netfilter/Kconfig.ladd	(revision 0)
@@ -0,0 +1,9 @@
+config IP6_NF_MATCH_TCPACK
+	tristate  'TCP ACK match support'
+	depends on IP6_NF_IPTABLES
+	help
+	  This option adds a `tcpack' match, which allow you to match
+	  TCP packets containg no actual data, just ACKs.
+
+	  If you want to compile it as a module, say M here and read
+	  Documentation/modules.txt.  If unsure, say `N'.
Index: pom-all/tcpack/linux-2.6/net/ipv6/netfilter/ip6t_tcpack.c
===================================================================
--- pom-all/tcpack/linux-2.6/net/ipv6/netfilter/ip6t_tcpack.c	(revision 0)
+++ pom-all/tcpack/linux-2.6/net/ipv6/netfilter/ip6t_tcpack.c	(revision 0)
@@ -0,0 +1,70 @@
+/* Kernel module to match TCP ACK packets. */
+/* (C) 2005 Jonas Berlin <xkr47@outerspace.dyndns.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/ipv6.h>
+
+#include <linux/netfilter_ipv6/ip6t_tcpack.h>
+#include <linux/netfilter_ipv6/ip6_tables.h>
+
+MODULE_AUTHOR("Jonas Berlin <xkr47@outerspace.dyndns.org>");
+MODULE_DESCRIPTION("ip6tables TCP ACK matching module");
+MODULE_LICENSE("GPL");
+
+static int
+match(const struct sk_buff *skb,
+      const struct net_device *in,
+      const struct net_device *out,
+      const void *matchinfo,
+      int offset,
+      unsigned int protoff,
+      int *hotdrop)
+{
+	const struct ip6t_tcpack_info *info = (const struct ip6t_tcpack_info *)matchinfo;
+	const struct tcphdr *tcph = (const struct tcphdr *)(skb->nh.raw + sizeof(struct ipv6hdr));
+	return info->invert ^
+		(tcph->doff * 4 == ntohs(skb->nh.ipv6h->payload_len));
+}
+
+static int checkentry(const char *tablename,
+		   const struct ip6t_ip6 *ip6,
+		   void *matchinfo,
+		   unsigned int matchsize,
+		   unsigned int hook_mask)
+{
+	if (matchsize != IP6T_ALIGN(sizeof(struct ip6t_tcpack_info)))
+		return 0;
+
+	if (ip6->proto != IPPROTO_TCP || (ip6->invflags & IP6T_INV_PROTO)) {
+		printk("tcpack: not valid for non-tcp\n");
+		return 0;
+	}
+
+	return 1;
+}
+
+static struct ip6t_match tcpack_match = {
+	.name		= "tcpack",
+	.match		= &match,
+	.checkentry	= &checkentry,
+	.me		= THIS_MODULE,
+};
+
+static int __init init(void)
+{
+	return ip6t_register_match(&tcpack_match);
+}
+
+static void __exit fini(void)
+{
+	ip6t_unregister_match(&tcpack_match);
+}
+
+module_init(init);
+module_exit(fini);
Index: pom-all/tcpack/iptables/extensions/libip6t_tcpack.c
===================================================================
--- pom-all/tcpack/iptables/extensions/libip6t_tcpack.c	(revision 0)
+++ pom-all/tcpack/iptables/extensions/libip6t_tcpack.c	(revision 0)
@@ -0,0 +1,88 @@
+/* Shared library add-on to ip6tables to add packet tcpack matching support. */
+#include <stdio.h>
+#include <getopt.h>
+
+#include <ip6tables.h>
+#include <linux/netfilter_ipv6/ip6t_tcpack.h>
+
+static void help(void) 
+{
+	printf(
+"tcpack v%s options:\n"
+"  [!] --tcpack  (match tcp ack packets)\n",
+IPTABLES_VERSION);
+}
+
+static struct option opts[] = {
+	{ "tcpack", 0, 0, '0'},
+	{ 0 }
+};
+
+static int
+parse(int c, char **argv, int invert, unsigned int *flags,
+      const struct ip6t_entry *entry,
+      unsigned int *nfcache,
+      struct ip6t_entry_match **match)
+{
+	struct ip6t_tcpack_info *info = (struct ip6t_tcpack_info *)(*match)->data;
+
+	switch (c)
+	{
+	case '0':
+		if(*flags)
+                        exit_error(PARAMETER_PROBLEM,
+                                   "Can't specify --ssrr twice");
+
+		info->invert = invert;
+		*flags = 1;
+		break;
+
+	default:
+		return 0;
+	}
+	return 1;
+}
+
+static void
+final_check(unsigned int flags)
+{
+	if (flags == 0)
+		exit_error(PARAMETER_PROBLEM,
+			   "tcpack match: you must specify the [!] --tcpack parameter.");
+}
+
+static void
+print(const struct ip6t_ip6 *ip6,
+      const struct ip6t_entry_match *match,
+      int numeric)
+{
+	const struct ip6t_tcpack_info *info = (const struct ip6t_tcpack_info *)match->data;
+	
+	printf("%stcpack", info->invert ? "!" : "");
+}
+
+static void
+save(const struct ip6t_ip6 *ip6, const struct ip6t_entry_match *match)
+{
+	const struct ip6t_tcpack_info *info = (const struct ip6t_tcpack_info *)match->data;
+
+	printf("%s--tcpack ", info->invert ? "! " : "");
+}
+
+static struct ip6tables_match tcpack = { 
+	.name		= "tcpack",
+	.version	= IPTABLES_VERSION,
+	.size		= IP6T_ALIGN(sizeof(struct ip6t_tcpack_info)),
+	.userspacesize	= IP6T_ALIGN(sizeof(struct ip6t_tcpack_info)),
+	.help		= &help,
+	.parse		= &parse,
+	.final_check	= &final_check,
+	.print		= &print,
+	.save		= &save,
+	.extra_opts	= opts
+};
+
+void _init(void)
+{
+	register_match6(&tcpack);
+}
Index: pom-all/tcpack/iptables/extensions/.tcpack-test
===================================================================
--- pom-all/tcpack/iptables/extensions/.tcpack-test	(revision 0)
+++ pom-all/tcpack/iptables/extensions/.tcpack-test	(revision 0)
@@ -0,0 +1,2 @@
+#! /bin/sh
+[ -f $KERNEL_DIR/include/linux/netfilter_ipv4/ipt_tcpack.h ] && echo tcpack

Property changes on: pom-all/tcpack/iptables/extensions/.tcpack-test
___________________________________________________________________
Name: svn:executable
   + *

Index: pom-all/tcpack/iptables/extensions/libipt_tcpack.man
===================================================================
--- pom-all/tcpack/iptables/extensions/libipt_tcpack.man	(revision 0)
+++ pom-all/tcpack/iptables/extensions/libipt_tcpack.man	(revision 0)
@@ -0,0 +1,7 @@
+Matches TCP packets that has no payload, i.e. contains only ACKs.
+.IP
+iptables -A INPUT -p tcp -m tcpack --tcpack -j CLASSIFY 1:10
+.P
+Or match non-ack packets:
+.IP
+iptables -A INPUT -p tcp -m tcpack ! --tcpack -j CLASSIFY 1:12
Index: pom-all/tcpack/iptables/extensions/.tcpack-test6
===================================================================
--- pom-all/tcpack/iptables/extensions/.tcpack-test6	(revision 0)
+++ pom-all/tcpack/iptables/extensions/.tcpack-test6	(revision 0)
@@ -0,0 +1,2 @@
+#! /bin/sh
+[ -f $KERNEL_DIR/include/linux/netfilter_ipv6/ip6t_tcpack.h ] && echo tcpack

Property changes on: pom-all/tcpack/iptables/extensions/.tcpack-test6
___________________________________________________________________
Name: svn:executable
   + *

Index: pom-all/tcpack/iptables/extensions/libip6t_tcpack.man
===================================================================
--- pom-all/tcpack/iptables/extensions/libip6t_tcpack.man	(revision 0)
+++ pom-all/tcpack/iptables/extensions/libip6t_tcpack.man	(revision 0)
@@ -0,0 +1,10 @@
+Matches TCP packets that has no payload, i.e. contains only ACKs.
+.IP
+ip6tables -A INPUT -p tcp -m tcpack --tcpack -j CLASSIFY 1:10
+.P
+Or match non-ack packets:
+.IP
+ip6tables -A INPUT -p tcp -m tcpack ! --tcpack -j CLASSIFY 1:12
+
+
+
Index: pom-all/tcpack/iptables/extensions/libipt_tcpack.c
===================================================================
--- pom-all/tcpack/iptables/extensions/libipt_tcpack.c	(revision 0)
+++ pom-all/tcpack/iptables/extensions/libipt_tcpack.c	(revision 0)
@@ -0,0 +1,88 @@
+/* Shared library add-on to iptables to add packet tcpack matching support. */
+#include <stdio.h>
+#include <getopt.h>
+
+#include <iptables.h>
+#include <linux/netfilter_ipv4/ipt_tcpack.h>
+
+static void help(void) 
+{
+	printf(
+"tcpack v%s options:\n"
+"  [!] --tcpack  (match tcp ack packets)\n",
+IPTABLES_VERSION);
+}
+
+static struct option opts[] = {
+	{ "tcpack", 0, 0, '0'},
+	{ 0 }
+};
+
+static int
+parse(int c, char **argv, int invert, unsigned int *flags,
+      const struct ipt_entry *entry,
+      unsigned int *nfcache,
+      struct ipt_entry_match **match)
+{
+	struct ipt_tcpack_info *info = (struct ipt_tcpack_info *)(*match)->data;
+
+	switch (c)
+	{
+	case '0':
+		if(*flags)
+                        exit_error(PARAMETER_PROBLEM,
+                                   "Can't specify --ssrr twice");
+
+		info->invert = invert;
+		*flags = 1;
+		break;
+
+	default:
+		return 0;
+	}
+	return 1;
+}
+
+static void
+final_check(unsigned int flags)
+{
+	if (flags == 0)
+		exit_error(PARAMETER_PROBLEM,
+			   "tcpack match: you must specify the [!] --tcpack parameter.");
+}
+
+static void
+print(const struct ipt_ip *ip,
+      const struct ipt_entry_match *match,
+      int numeric)
+{
+	const struct ipt_tcpack_info *info = (const struct ipt_tcpack_info *)match->data;
+	
+	printf("%stcpack", info->invert ? "!" : "");
+}
+
+static void
+save(const struct ipt_ip *ip, const struct ipt_entry_match *match)
+{
+	const struct ipt_tcpack_info *info = (const struct ipt_tcpack_info *)match->data;
+
+	printf("%s--tcpack ", info->invert ? "! " : "");
+}
+
+static struct iptables_match tcpack = { 
+	.name		= "tcpack",
+	.version	= IPTABLES_VERSION,
+	.size		= IPT_ALIGN(sizeof(struct ipt_tcpack_info)),
+	.userspacesize	= IPT_ALIGN(sizeof(struct ipt_tcpack_info)),
+	.help		= &help,
+	.parse		= &parse,
+	.final_check	= &final_check,
+	.print		= &print,
+	.save		= &save,
+	.extra_opts	= opts
+};
+
+void _init(void)
+{
+	register_match(&tcpack);
+}
Index: pom-all/tcpack/help
===================================================================
--- pom-all/tcpack/help	(revision 0)
+++ pom-all/tcpack/help	(revision 0)
@@ -0,0 +1,7 @@
+Matches TCP packets that has no payload, i.e. contains only ACKs.
+
+  iptables -A INPUT -p tcp -m tcpack --tcpack -j CLASSIFY 1:10
+
+Or match non-ack packets:
+
+  iptables -A INPUT -p tcp -m tcpack ! --tcpack -j CLASSIFY 1:12
Index: pom-all/tcpack/info
===================================================================
--- pom-all/tcpack/info	(revision 0)
+++ pom-all/tcpack/info	(revision 0)
@@ -0,0 +1,4 @@
+Title: Add support for matching TCP packets with only ACKs (no payload)
+Author: Jonas Berlin <xkr47@outerspace.dyndns.org>
+Status: testing
+Repository: extra

Re: [PATCH] new 'tcpack' match

[Jonas Berlin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jonas Berlin wrote:
| Matches TCP packets that has no payload, i.e. contains only ACKs.
|
|   iptables -A INPUT -p tcp -m tcpack --tcpack -j CLASSIFY 1:10
|
| Or match non-ack packets:
|
|   iptables -A INPUT -p tcp -m tcpack ! --tcpack -j CLASSIFY 1:12

Actually after thinking about it, the "ack" reference is quite bogus.. it
matches packets that simply has no data bytes in them.. regardless of the
possible flags SYN ACK FIN RST.

I think I'll rename it and rewrite the documentation a bit so it's clearer
and also add some more usage examples and ideas.

So if possible, please hold merging this patch into pom-ng :)

Sorry for my over-enthusiasm, this was my first self-written pom-ng module I
publicly announced :)

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCSA92xyF48ZTvn+4RAj45AJ9wjU6CsSq88PB4NxKI54h0fKzIHgCggZ7p
I5Uha/RhbnLdYi7Eu2Fk4ZU=
=WwNx
-----END PGP SIGNATURE-----

Re: [PATCH] new 'tcpack' match

[Patrick McHardy]

Jonas Berlin wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Matches TCP packets that has no payload, i.e. contains only ACKs.
> 
> ~  iptables -A INPUT -p tcp -m tcpack --tcpack -j CLASSIFY 1:10
> 
> Or match non-ack packets:
> 
> ~  iptables -A INPUT -p tcp -m tcpack ! --tcpack -j CLASSIFY 1:12

I can see that it would be useful, given all the half-working ACK
matching rules for shaping floating around, but we already a tcp
match, so it should be done in there, and, if possible, kept a little
bit more generic. Perhaps a data-len match option.

Regards
Patrick

Re[2]: [PATCH] new 'tcpack' match

[Maciej Soltysiak]

> I can see that it would be useful, given all the half-working ACK
> matching rules for shaping floating around, but we already a tcp
> match, so it should be done in there, and, if possible, kept a little
> bit more generic. Perhaps a data-len match option.
Hmm. I'm no guru, but isn't it possible to do that using the u32 filter
with tc? Which propably is faster than doing it in iptables anyway.

--
Maciej

Re: [PATCH] new 'tcpack' match

[Jonas Berlin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Patrick McHardy wrote:

|> Matches TCP packets that has no payload, i.e. contains only ACKs.
|
| I can see that it would be useful, given all the half-working ACK
| matching rules for shaping floating around, but we already a tcp
| match, so it should be done in there, and, if possible, kept a little

I was thinking of this, but my mind tricked me into believing that it was a
too bold thing to do..

Speaking of tcp match.. would you happen to know when the "h" union of
sk_buff is instantiated?
~   a) first time someone does -p something
~   b) somewhere before the filter table
~   c) in a seemingly unpredictable fashion
~   d) ?

I kindof recall sometimes using it and oopsing the kernel because it was not
yet initialized.. The patch I posted currently starts from nh and calculates
the tcp header start itself.. :)

| bit more generic. Perhaps a data-len match option.

I was thinking of this too but couldn't think of much use for more specific
length-matching, so I thought I'd save a few cycles.. but ok if you think
it's the way to go, I go. :)

Any parameter name suggestions? "[!] --has-payload" ?

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCUH32xyF48ZTvn+4RAk4DAJ9cnX1IXU6Qn0HuWoBoZvZPakpBAACeK/lY
PsmEkZ6Bk0G9cpixt0FaJic=
=j2oo
-----END PGP SIGNATURE-----

Re: [PATCH] new 'tcpack' match

[Phil Oester]

On Mon, Apr 04, 2005 at 02:36:24AM +0300, Jonas Berlin wrote:
> | bit more generic. Perhaps a data-len match option.
> 
> I was thinking of this too but couldn't think of much use for more specific
> length-matching, so I thought I'd save a few cycles.. but ok if you think
> it's the way to go, I go. :)
> 
> Any parameter name suggestions? "[!] --has-payload" ?

How bout just a '--datalen [!] X', where X can be any number, including 0?

Phil

Re: [PATCH] new 'tcpack' match

[Jonas Berlin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Maciej Soltysiak wrote:

|>I can see that it would be useful, given all the half-working ACK
|>matching rules for shaping floating around, but we already a tcp
|>match, so it should be done in there, and, if possible, kept a little
|>bit more generic. Perhaps a data-len match option.
|
| Hmm. I'm no guru, but isn't it possible to do that using the u32 filter
| with tc? Which propably is faster than doing it in iptables anyway.

Yeah it's possible, but no one seems to be doing it right..

What needs to be done is to compare the tcp header length with the total
packet length to ensure there are no data bytes. However, tc can only match
specific data bytes, and since there are at least 8 different tcp header
lengths that can occur during normal traffic in an established connection,
it means at least 8 filter rules to match the ack packets..

I've mostly seen solutions where the test is to see if the total length is
less than 64 bytes, and if, it means it's a packet that should be
prioritized. Unfortunately, since linux likes both the timestamp and the
sack options of tcp, the tcp header tends to expand beyond 63 bytes every
now and then. When this happens, the packets exceeding 63 bytes will
probably go off to another qdisc, which might keep them on a queue for a bit
too long, possibly resulting in the data transfer getting stalled
temporarily, and possibly unnecessary resends.

I aim to fix this all with one simple netfilter match which can give an
answer with simple arithmetics in one go instead of matching against a
preconfigured list of possible values one by one.

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCUISKxyF48ZTvn+4RAjSlAKDWorgIpKW8vDf0q+lk2hDsP/uhCwCdGrkl
Er4Rr5Hgw68jZEqeqeH2FtY=
=QPJY
-----END PGP SIGNATURE-----

Re: [PATCH] new 'tcpack' match

[Jonas Berlin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Phil Oester wrote:

|>Any parameter name suggestions? "[!] --has-payload" ?
|
| How bout just a '--datalen [!] X', where X can be any number, including 0?

Oops getting sleepy..

Yeah, datalen perhaps.. but should it maybe also match a range instead of
just one exact length?

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCUIUkxyF48ZTvn+4RAh72AJoCTo+gkpYRYcwcA4srpkKYTaal5QCfe1S/
/YN5r6+OjwrZDQrSmHOI6gI=
=XW4e
-----END PGP SIGNATURE-----

Re: [PATCH] new 'tcpack' match

[Phil Oester]

On Mon, Apr 04, 2005 at 03:07:07AM +0300, Jonas Berlin wrote:
> Yeah, datalen perhaps.. but should it maybe also match a range instead of
> just one exact length?

Sure, and maybe both ranges and multiple lengths, similar to how revision 1
of the multiport match handles ports.  So:

	--datalen [!] A[,B:C[,D]]

Phil

Re: [PATCH] new 'tcpack' match

[Patrick McHardy]

On Mon, 4 Apr 2005, Jonas Berlin wrote:
> Speaking of tcp match.. would you happen to know when the "h" union of
> sk_buff is instantiated?
> ~   a) first time someone does -p something
> ~   b) somewhere before the filter table
> ~   c) in a seemingly unpredictable fashion
> ~   d) ?

It is used by the upper layer protocols, you can't use it in netfilter
because it is unset for forwarded packets.

> | bit more generic. Perhaps a data-len match option.
>
> I was thinking of this too but couldn't think of much use for more specific
> length-matching, so I thought I'd save a few cycles.. but ok if you think
> it's the way to go, I go. :)
>
> Any parameter name suggestions? "[!] --has-payload" ?

--datalen?

Regards
Patrick

Re[2]: [PATCH] new 'tcpack' match

[Maciej Soltysiak]

> | Hmm. I'm no guru, but isn't it possible to do that using the u32 filter
> | with tc? Which propably is faster than doing it in iptables anyway.

> Yeah it's possible, but no one seems to be doing it right..
Ah, okay, I get it.

> I aim to fix this all with one simple netfilter match which can give an
> answer with simple arithmetics in one go instead of matching against a
> preconfigured list of possible values one by one.
Right.

I agree the best way to go would be to extend the tcp match with
the proposed semantics:
    --datalen [!] A[,B:C[,D]]


It also might be useful to match the sole header length.
   --headerlen [!] A[,B:C[,D]]

And the lenght of tcp+data

   --len

Possibly the similar for udp:
   --datalen, --headerlen, --len

And ICMP:
   --datalen, --len (icmp headers have fixed size)

-- 
Regards,
Maciej Soltysiak

Re: [PATCH] new 'tcpack' match

[Carl-Daniel Hailfinger]

Maciej Soltysiak schrieb:
>>I aim to fix this all with one simple netfilter match which can give an
>>answer with simple arithmetics in one go instead of matching against a
>>preconfigured list of possible values one by one.
> 
> Right.
> 
> I agree the best way to go would be to extend the tcp match with
> the proposed semantics:
>     --datalen [!] A[,B:C[,D]]
> 
> 
> It also might be useful to match the sole header length.
>    --headerlen [!] A[,B:C[,D]]
> 
> And the lenght of tcp+data
> 
>    --len
> 
> Possibly the similar for udp:
>    --datalen, --headerlen, --len
> 
> And ICMP:
>    --datalen, --len (icmp headers have fixed size)

Could we do this in a generic way? I'm currently rewriting the iptables
ACCOUNT target and it could benefit from that as well.

I'd need:
- length of whole IP packet
- length of layer 2 frame

Perhaps a generic prefix for all "length" options is appropriate?

--len-header-ip
--len-data-ip
--len-full-ip
--len-header-tcp
--len-data-tcp
--len-full-tcp
--len-header-ether
--len-data-ether
--len-full-ether
etc.

I realize that some of the options above don't make that much sense,
but you get the idea.

Regards,
Carl-Daniel
-- 
http://www.hailfinger.org/

Re: [PATCH] new 'tcpack' match

[Jonas Berlin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoting Maciej Soltysiak on 2005-04-04 13:30 UTC:

~ > I agree the best way to go would be to extend the tcp match with
| the proposed semantics:
|     --datalen [!] A[,B:C[,D]]

I think [!] A[:B] should suffice :)

| It also might be useful to match the sole header length.
|    --headerlen [!] A[,B:C[,D]]

Maybe [!] A[:B] here also..

Patric McHardy, what do you think, should I implement --headerlen also?

| And the lenght of tcp+data
|    --len

This should be done with the "length" match IMO. -m length
- --layer2-length maybe ? I could implement this too while at it..

| Possibly the similar for udp:
|    --datalen, --headerlen, --len

The udp header length is fixed (8 bytes), and thus --headerlen is
unneeded and --datalen N[:M] could maybe implemented as -m length
- --layer2-length N+8[:M+8] ? I do agree that it would be a bit 
unaesthetic :)

| And ICMP:
|    --datalen, --len (icmp headers have fixed size)

Same situation as udp, except maybe if the timestamps of the Timestamp
message would be considered a part of the header, in which case the
header size would no longer be fixed.. Anyway I guess --layer2-length
should suffice for icmp..

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCWl7dxyF48ZTvn+4RAiKlAJ9ngZw5WUOvn14AFl5sthU6tUHWHQCfUR/z
yxHXlrMHAI+eHryfG1RSuT4=
=nJX2
-----END PGP SIGNATURE-----

Re: [PATCH] new 'tcpack' match

[Jonas Berlin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoting Patrick McHardy on 2005-04-03 18:15 UTC:

|> ~  iptables -A INPUT -p tcp -m tcpack --tcpack -j CLASSIFY 1:10
|
| I can see that it would be useful, given all the half-working ACK
| matching rules for shaping floating around, but we already a tcp
| match, so it should be done in there, and, if possible, kept a little
| bit more generic. Perhaps a data-len match option.

Should I make a new revision of the tcp match in order not to break
binary compatibility?

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCWmmNxyF48ZTvn+4RAps7AKC2Bii7ZaWYecGwYgPQ8xJxmgLWqgCfRucE
PajDstxPkhhzN3q17+hXzfA=
=Gsrz
-----END PGP SIGNATURE-----

Re: [PATCH] new 'tcpack' match

[Jonas Berlin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoting Jonas Berlin on 2005-04-11 11:26 UTC:

> This should be done with the "length" match IMO. -m length
> --layer2-length maybe ? I could implement this too while at it..

Actually, that should probably have been layer 3 instead of 2..

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCWnbzxyF48ZTvn+4RAkS6AJ9QsGbuvsd0r4NS47tldVZIQtkTDwCgkEmu
b4+EZEqkUI4DlUwGIHviaWs=
=kotj
-----END PGP SIGNATURE-----

Re: [PATCH] new 'tcpack' match

[Patrick McHardy]

Jonas Berlin wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Quoting Patrick McHardy on 2005-04-03 18:15 UTC:
> 
> |> ~  iptables -A INPUT -p tcp -m tcpack --tcpack -j CLASSIFY 1:10
> |
> | I can see that it would be useful, given all the half-working ACK
> | matching rules for shaping floating around, but we already a tcp
> | match, so it should be done in there, and, if possible, kept a little
> | bit more generic. Perhaps a data-len match option.
> 
> Should I make a new revision of the tcp match in order not to break
> binary compatibility?

Yes.

Regards
Patrick

Re: [PATCH] new 'tcpack' match

[Carl-Daniel Hailfinger]

Jonas Berlin schrieb:
> Quoting Maciej Soltysiak on 2005-04-04 13:30 UTC:
> 
> ~ > I agree the best way to go would be to extend the tcp match with
> | the proposed semantics:
> |     --datalen [!] A[,B:C[,D]]
> 
> I think [!] A[:B] should suffice :)
> 
> | It also might be useful to match the sole header length.
> |    --headerlen [!] A[,B:C[,D]]
> 
> Maybe [!] A[:B] here also..
> 
> Patric McHardy, what do you think, should I implement --headerlen also?
> 
> | And the lenght of tcp+data
> |    --len
> 
> This should be done with the "length" match IMO. -m length
> --layer2-length maybe ? I could implement this too while at it..
> 
> | Possibly the similar for udp:
> |    --datalen, --headerlen, --len
> 
> The udp header length is fixed (8 bytes), and thus --headerlen is
> unneeded and --datalen N[:M] could maybe implemented as -m length
> --layer2-length N+8[:M+8] ? I do agree that it would be a bit
> unaesthetic :)
> 
> | And ICMP:
> |    --datalen, --len (icmp headers have fixed size)
> 
> Same situation as udp, except maybe if the timestamps of the Timestamp
> message would be considered a part of the header, in which case the
> header size would no longer be fixed.. Anyway I guess --layer2-length
> should suffice for icmp..

Perhaps a generic prefix for all "length" options is appropriate?

--len-header-ip
--len-data-ip
--len-full-ip
--len-header-tcp
--len-data-tcp
--len-full-tcp
--len-header-ether
--len-data-ether
--len-full-ether
etc.

Regards,
Carl-Daniel
-- 
http://www.hailfinger.org/