Re: [PATCH] netfilter: xtables: introduce xt_length revision 2½

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Saturday 2010-07-24 13:42, Eric Dumazet wrote:
>> +static bool
>> +length2_mt(const struct sk_buff *skb, struct xt_action_param *par)
>> +{
>> +	const struct xt_length_mtinfo2 *info = par->matchinfo;
>> +	const struct iphdr *iph = ip_hdr(skb);
>> +	unsigned int len = 0;
>> +	bool hit = true;
>> +
>> +	if (info->flags & XT_LENGTH_LAYER3)
>> +		len = ntohs(iph->tot_len);
>> +	else if (info->flags & XT_LENGTH_LAYER4)
>> +		len = ntohs(iph->tot_len) - par->thoff;
>> +	else if (info->flags & XT_LENGTH_LAYER5)
>> +		hit = xtlength_layer5(&len, skb, iph->protocol, par->thoff);
>> +	else if (info->flags & XT_LENGTH_LAYER7)
>> +		hit = xtlength_layer7(&len, skb, iph->protocol, par->thoff);
>> +	if (!hit)
>> +		return false;
>> +
>> +	return (len >= info->min && len <= info->max) ^
>> +	       !!(info->flags & XT_LENGTH_INVERT);
>> +}
>
>
>This serie of tests is expensive and useless.
>
>- A switch() would be faster,
>  - if you dont use a bit mask, but continuous values to get the layer.
>- Also, using a u16 is more expensive than a u32.
>- On x86, compiler is forced to use prefixes or conversions instructions
>  (movzwl), this makes code bigger.

I might agree with u16/u32 in that some CPUs need to run an extra
"AND" when they don't have (movw/cmpw), but... the other things
cast my doubts.

C does not specify a "speed" for switch or if. I agree that some
constructs may desire to be reworked to work around limitations of
the compiler's smartness, but in the case that is before us, it looks
like some of your arguments have no base - at least on x86_64.

length2_mt as it stands (bitmask, u16, if): 800 bytes
length2_mt with u32 flag: 800 bytes
length2_mt with switch: 800 bytes
length2_mt with continuous values and u32: 816 bytes
length2_mt with cont.v, u32, and switch: 816 bytes

The compiler is smart enough to see that a run of if tests against
the same variable with different values is transformable into a
switch statement.

>BTW, you mention "revision 2" in your patch title, while it is revision1

The description was copied from earlier; the code correctly contains
.revision = 2. Actually, since Xt-a uses 2, I should be using 3.


parent 7df0884ce144396fc151f2af7a73d5fb305f9b03 (v2.6.35-rc1-956-g7df0884)
commit 5be892abc049930994dd681be0f441727279aa8a
Author: Jan Engelhardt <jengelh@xxxxxxxxxx>
Date:   Sat Jul 24 10:47:08 2010 +0200

netfilter: xtables: introduce xt_length revision 3

Revision 3 adds support for layer-4, layer-5 and layer-7 length
matching. Using xt_u32 would be cumbersome to type and would not work
very well to skip an arbitrary number of IPv6 headers.

(Rev 1 and 2 were skipped to avoid confusion with other code, such as
from Xt-a.)

This can be used for packet scheduling; specific example are online
games where all data is transferred over the same port, but the regular
gameplay has a characteristically lower packet size than bulk downloads
of game maps. (Tested with Unreal Tournament 99.)

Signed-off-by: Jan Engelhardt <jengelh@xxxxxxxxxx>
---
 include/linux/netfilter/xt_length.h |   10 ++
 net/netfilter/xt_length.c           |  231 ++++++++++++++++++++++++++-
 2 files changed, 238 insertions(+), 3 deletions(-)

diff --git a/include/linux/netfilter/xt_length.h b/include/linux/netfilter/xt_length.h
index b82ed7c..457e3cc 100644
--- a/include/linux/netfilter/xt_length.h
+++ b/include/linux/netfilter/xt_length.h
@@ -8,4 +8,14 @@ struct xt_length_info {
     __u8	invert;
 };
 
+enum {
+	XT_LENGTH_LAYERMASK = 0x1F,
+	XT_LENGTH_INVERT    = 1 << 5,
+};
+
+struct xt_length_mtinfo3 {
+	__u32 min, max;
+	__u32 flags;
+};
+
 #endif /*_XT_LENGTH_H*/
diff --git a/net/netfilter/xt_length.c b/net/netfilter/xt_length.c
index 176e557..851d04d 100644
--- a/net/netfilter/xt_length.c
+++ b/net/netfilter/xt_length.c
@@ -1,20 +1,31 @@
 /* Kernel module to match packet length. */
 /* (C) 1999-2001 James Morris <jmorros@xxxxxxxxxxxxxxxx>
+ * Copyright © Jan Engelhardt <jengelh@xxxxxxxxxx>, 2007 - 2010
  *
  * 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/dccp.h>
 #include <linux/module.h>
 #include <linux/skbuff.h>
+#include <linux/icmp.h>
+#include <linux/ip.h>
 #include <linux/ipv6.h>
+#include <linux/sctp.h>
+#include <linux/tcp.h>
+#include <linux/udp.h>
 #include <net/ip.h>
-
-#include <linux/netfilter/xt_length.h>
+#include <net/ipv6.h>
 #include <linux/netfilter/x_tables.h>
+#include <linux/netfilter/xt_length.h>
+#include <linux/netfilter_ipv6/ip6_tables.h>
+#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
+#	define WITH_IPV6 1
+#endif
 
 MODULE_AUTHOR("James Morris <jmorris@xxxxxxxxxxxxxxxx>");
+MODULE_AUTHOR("Jan Engelhardt <jengelh@xxxxxxxxxx>");
 MODULE_DESCRIPTION("Xtables: Packet length (Layer3,4,5) match");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("ipt_length");
@@ -39,6 +50,202 @@ length_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 	return (pktlen >= info->min && pktlen <= info->max) ^ info->invert;
 }
 
+static bool
+xtlength_layer5_tcp(unsigned int *length, const struct sk_buff *skb,
+		    unsigned int offset)
+{
+	const struct tcphdr *tcph;
+	struct tcphdr buf;
+
+	tcph = skb_header_pointer(skb, offset, sizeof(buf), &buf);
+	if (tcph == NULL)
+		return false;
+
+	*length = skb->len - offset;
+	if (*length >= 4 * tcph->doff)
+		*length -= 4 * tcph->doff;
+	return true;
+}
+
+static bool
+xtlength_layer5_dccp(unsigned int *length, const struct sk_buff *skb,
+		     unsigned int offset)
+{
+	const struct dccp_hdr *dh;
+	struct dccp_hdr dhbuf;
+
+	dh = skb_header_pointer(skb, offset, sizeof(dhbuf), &dhbuf);
+	if (dh == NULL)
+		return false;
+
+	*length = skb->len - offset;
+	if (*length >= 4 * dh->dccph_doff)
+		*length -= 4 * dh->dccph_doff;
+	return true;
+}
+
+static inline bool
+xtlength_layer5(unsigned int *length, const struct sk_buff *skb,
+		unsigned int prot, unsigned int offset)
+{
+	switch (prot) {
+	case IPPROTO_TCP:
+		return xtlength_layer5_tcp(length, skb, offset);
+	case IPPROTO_UDP:
+	case IPPROTO_UDPLITE:
+		*length = skb->len - offset - sizeof(struct udphdr);
+		return true;
+	case IPPROTO_SCTP:
+		*length = skb->len - offset - sizeof(struct sctphdr);
+		return true;
+	case IPPROTO_DCCP:
+		return xtlength_layer5_dccp(length, skb, offset);
+	case IPPROTO_ICMP:
+		*length = skb->len - offset - sizeof(struct icmphdr);
+		return true;
+	case IPPROTO_ICMPV6:
+		*length = skb->len - offset -
+		          offsetof(struct icmp6hdr, icmp6_dataun);
+		return true;
+	case IPPROTO_AH:
+		*length = skb->len - offset - sizeof(struct ip_auth_hdr);
+		return true;
+	case IPPROTO_ESP:
+		*length = skb->len - offset - sizeof(struct ip_esp_hdr);
+		return true;
+	}
+	return false;
+}
+
+static bool
+xtlength_layer7_sctp(unsigned int *length, const struct sk_buff *skb,
+		     unsigned int offset)
+{
+	const struct sctp_chunkhdr *ch;
+	struct sctp_chunkhdr chbuf;
+	unsigned int pos;
+
+	*length = 0;
+	for (pos = sizeof(struct sctphdr); pos < skb->len;
+	     pos += ntohs(ch->length)) {
+		ch = skb_header_pointer(skb, offset + pos,
+		     sizeof(chbuf), &chbuf);
+		if (ch == NULL)
+			return false;
+		if (ch->type != SCTP_CID_DATA)
+			continue;
+		*length += ntohs(ch->length);
+	}
+	return true;
+}
+
+static bool xtlength_layer7(unsigned int *length, const struct sk_buff *skb,
+			    unsigned int proto, unsigned int offset)
+{
+	switch (proto) {
+	case IPPROTO_SCTP:
+		return xtlength_layer7_sctp(length, skb, offset);
+	default:
+		return xtlength_layer5(length, skb, proto, offset);
+	}
+}
+
+static bool
+length3_mt(const struct sk_buff *skb, struct xt_action_param *par)
+{
+	const struct xt_length_mtinfo3 *info = par->matchinfo;
+	const unsigned int layer = info->flags & XT_LENGTH_LAYERMASK;
+	const struct iphdr *iph = ip_hdr(skb);
+	unsigned int len = 0;
+	bool hit = true;
+
+	if (layer == 3)
+		len = ntohs(iph->tot_len);
+	else if (layer == 4)
+		len = ntohs(iph->tot_len) - par->thoff;
+	else if (layer == 5)
+		hit = xtlength_layer5(&len, skb, iph->protocol, par->thoff);
+	else if (layer == 7)
+		hit = xtlength_layer7(&len, skb, iph->protocol, par->thoff);
+	if (!hit)
+		return false;
+
+	return (len >= info->min && len <= info->max) ^
+	       !!(info->flags & XT_LENGTH_INVERT);
+}
+
+#ifdef WITH_IPV6
+/**
+ * llayer4_proto - figure out the L4 protocol in an IPv6 packet
+ * @skb:	skb pointer
+ * @offset:	position at which L4 starts (equal to 'protoff' in IPv4 code)
+ * @hotdrop:	hotdrop pointer
+ *
+ * Searches for a recognized L4 header. On success, fills in @offset and
+ * returns the protocol number. If not found, %NEXTHDR_MAX is returned.
+ * On error, @hotdrop is set.
+ */
+static unsigned int
+llayer4_proto(const struct sk_buff *skb, unsigned int *offset, bool *hotdrop)
+{
+	/*
+	 * Do encapsulation first so that %NEXTHDR_TCP does not hit the TCP
+	 * part in an IPv6-in-IPv6 encapsulation.
+	 */
+	static const unsigned int types[] =
+		{IPPROTO_IPV6, IPPROTO_IPIP, IPPROTO_ESP, IPPROTO_AH,
+		IPPROTO_ICMP, IPPROTO_TCP, IPPROTO_UDP, IPPROTO_UDPLITE,
+		IPPROTO_SCTP, IPPROTO_DCCP};
+	unsigned int i;
+	int err;
+
+	for (i = 0; i < ARRAY_SIZE(types); ++i) {
+		err = ipv6_find_hdr(skb, offset, types[i], NULL);
+		if (err >= 0)
+			return types[i];
+		if (err != -ENOENT) {
+			*hotdrop = true;
+			break;
+		}
+	}
+	return NEXTHDR_MAX;
+}
+
+static bool
+length3_mt6(const struct sk_buff *skb, struct xt_action_param *par)
+{
+	const struct xt_length_mtinfo3 *info = par->matchinfo;
+	const unsigned int layer = info->flags & XT_LENGTH_LAYERMASK;
+	const struct ipv6hdr *iph = ipv6_hdr(skb);
+	unsigned int len = 0, l4proto;
+	unsigned int thoff = par->thoff;
+	bool hit = true;
+
+	if (layer == 3) {
+		if (iph->payload_len == 0)
+			/* Jumbograms */
+			len = skb->len;
+		else
+			len = sizeof(struct ipv6hdr) + ntohs(iph->payload_len);
+	} else {
+		l4proto = llayer4_proto(skb, &thoff, &par->hotdrop);
+		if (l4proto == NEXTHDR_MAX)
+			return false;
+		if (layer == 4)
+			len = skb->len - thoff;
+		else if (layer == 5)
+			hit = xtlength_layer5(&len, skb, l4proto, thoff);
+		else if (layer == 7)
+			hit = xtlength_layer7(&len, skb, l4proto, thoff);
+	}
+	if (!hit)
+		return false;
+
+	return (len >= info->min && len <= info->max) ^
+	       !!(info->flags & XT_LENGTH_INVERT);
+}
+#endif
+
 static struct xt_match length_mt_reg[] __read_mostly = {
 	{
 		.name		= "length",
@@ -54,6 +261,24 @@ static struct xt_match length_mt_reg[] __read_mostly = {
 		.matchsize	= sizeof(struct xt_length_info),
 		.me		= THIS_MODULE,
 	},
+	{
+		.name           = "length",
+		.revision       = 3,
+		.family         = NFPROTO_IPV4,
+		.match          = length3_mt,
+		.matchsize      = sizeof(struct xt_length_mtinfo3),
+		.me             = THIS_MODULE,
+	},
+#ifdef WITH_IPV6
+	{
+		.name           = "length",
+		.revision       = 3,
+		.family         = NFPROTO_IPV6,
+		.match          = length3_mt6,
+		.matchsize      = sizeof(struct xt_length_mtinfo3),
+		.me             = THIS_MODULE,
+	},
+#endif
 };
 
 static int __init length_mt_init(void)
-- 
# Created with git-export-patch
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux