public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Eric Dumazet <dada1@cosmosbay•com>
To: Patrick McHardy <kaber@trash•net>
Cc: Andi Kleen <andi@firstfloor•org>,
	"David S. Miller" <davem@davemloft•net>,
	Netfilter Developers <netfilter-devel@vger•kernel.org>,
	Linux Network Development list <netdev@vger•kernel.org>
Subject: Re: [PATCH] netfilter: unfold two critical loops in ip_packet_match()
Date: Wed, 18 Feb 2009 17:33:53 +0100	[thread overview]
Message-ID: <499C3871.4030600@cosmosbay.com> (raw)
In-Reply-To: <499C2766.5090904@trash.net>

Patrick McHardy a écrit :
> Eric Dumazet wrote:
>> Patrick McHardy a écrit :
>>> The interface name matching has shown up in profiles forever
>>> though and we've actually already tried to optimize it IIRC.
>>>
>>> Eric, I'm trying to keep all the *tables files synchronized,
>>> could you send me a patch updating the other ones as well
>>> please?
>>
>> While doing this, I found arp_tables is still using loop using
>> byte operations.
>>
>> Also, I could not find how iniface_mask[], outiface_mask[], iniface[]
>> and outiface[] were forced to long word alignment ...
>> (in struct ipt_ip, struct ip6t_ip6, struct arpt_arp)
> 
> In case of IPv4 and IPv6 they are already suitable aligned, it
> simply performing the comparison in unsigned long quantities.
> struct arpt_arp unfortunately doesn't properly align the interface
> names, so we need to continue to do byte-wise comparisons.
> 
> 

I see, but #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS can help here ;)

ifname_compare() should be static in three files (ipv4_ip_tables, ipv6_ip_tables and arp_tables),
since only arp_tables variant has the alignement problem.

[PATCH] netfilter: unfold two critical loops in arp_packet_match()

x86 and powerpc can perform long word accesses in an efficient maner.
We can use this to unroll two loops in arp_packet_match(), to
perform arithmetic on long words instead of bytes. This is a win
on x86_64 for example.

Signed-off-by: Eric Dumazet <dada1@cosmosbay•com>
---
 net/ipv4/netfilter/arp_tables.c |   44 +++++++++++++++++++++++-------
 1 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 7ea88b6..b5db463 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -73,6 +73,36 @@ static inline int arp_devaddr_compare(const struct arpt_devaddr_info *ap,
 	return (ret != 0);
 }
 
+/*
+ * Unfortunatly, _b and _mask are not aligned to an int (or long int)
+ * Some arches dont care, unrolling the loop is a win on them.
+ */
+static unsigned long ifname_compare(const char *_a, const char *_b, const char *_mask)
+{
+#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
+	const unsigned long *a = (const unsigned long *)_a;
+	const unsigned long *b = (const unsigned long *)_b;
+	const unsigned long *mask = (const unsigned long *)_mask;
+	unsigned long ret;
+
+	ret = (a[0] ^ b[0]) & mask[0];
+	if (IFNAMSIZ > sizeof(unsigned long))
+		ret |= (a[1] ^ b[1]) & mask[1];
+	if (IFNAMSIZ > 2 * sizeof(unsigned long))
+		ret |= (a[2] ^ b[2]) & mask[2];
+	if (IFNAMSIZ > 3 * sizeof(unsigned long))
+		ret |= (a[3] ^ b[3]) & mask[3];
+	BUILD_BUG_ON(IFNAMSIZ > 4 * sizeof(unsigned long));
+#else
+	unsigned long ret = 0;
+	int i;
+
+	for (i = 0; i < IFNAMSIZ; i++)
+		ret |= (_a[i] ^ _b[i]) & _mask[i];
+#endif
+	return ret;
+}
+
 /* Returns whether packet matches rule or not. */
 static inline int arp_packet_match(const struct arphdr *arphdr,
 				   struct net_device *dev,
@@ -83,7 +113,7 @@ static inline int arp_packet_match(const struct arphdr *arphdr,
 	const char *arpptr = (char *)(arphdr + 1);
 	const char *src_devaddr, *tgt_devaddr;
 	__be32 src_ipaddr, tgt_ipaddr;
-	int i, ret;
+	long ret;
 
 #define FWINV(bool, invflg) ((bool) ^ !!(arpinfo->invflags & (invflg)))
 
@@ -156,10 +186,7 @@ static inline int arp_packet_match(const struct arphdr *arphdr,
 	}
 
 	/* Look for ifname matches.  */
-	for (i = 0, ret = 0; i < IFNAMSIZ; i++) {
-		ret |= (indev[i] ^ arpinfo->iniface[i])
-			& arpinfo->iniface_mask[i];
-	}
+	ret = ifname_compare(indev, arpinfo->iniface, arpinfo->iniface_mask);
 
 	if (FWINV(ret != 0, ARPT_INV_VIA_IN)) {
 		dprintf("VIA in mismatch (%s vs %s).%s\n",
@@ -168,10 +195,7 @@ static inline int arp_packet_match(const struct arphdr *arphdr,
 		return 0;
 	}
 
-	for (i = 0, ret = 0; i < IFNAMSIZ; i++) {
-		ret |= (outdev[i] ^ arpinfo->outiface[i])
-			& arpinfo->outiface_mask[i];
-	}
+	ret = ifname_compare(outdev, arpinfo->outiface, arpinfo->outiface_mask);
 
 	if (FWINV(ret != 0, ARPT_INV_VIA_OUT)) {
 		dprintf("VIA out mismatch (%s vs %s).%s\n",
@@ -221,7 +245,7 @@ unsigned int arpt_do_table(struct sk_buff *skb,
 			   const struct net_device *out,
 			   struct xt_table *table)
 {
-	static const char nulldevname[IFNAMSIZ];
+	static const char nulldevname[IFNAMSIZ] __attribute__((aligned(sizeof(long))));
 	unsigned int verdict = NF_DROP;
 	const struct arphdr *arp;
 	bool hotdrop = false;



  reply	other threads:[~2009-02-18 16:34 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-26 22:15 32 core net-next stack/netfilter "scaling" Rick Jones
2009-01-26 23:10 ` Eric Dumazet
2009-01-26 23:14   ` Stephen Hemminger
2009-01-26 23:19   ` Rick Jones
2009-01-27  9:10     ` Eric Dumazet
2009-01-27  9:15       ` Patrick McHardy
2009-01-27 11:29         ` Eric Dumazet
2009-01-27 11:37           ` Patrick McHardy
2009-01-27 16:23         ` Eric Dumazet
2009-01-27 17:33           ` Patrick McHardy
2009-01-27 18:02             ` Rick Jones
2009-01-27 19:09               ` Rick Jones
2009-01-27 19:24                 ` Rick Jones
2009-01-27 22:17                   ` Eric Dumazet
2009-01-27 22:29                     ` Rick Jones
2009-01-27 22:34                       ` Eric Dumazet
2009-01-27 22:43                         ` Rick Jones
2009-01-28 13:55                   ` Eric Dumazet
2009-01-28 16:25                     ` Patrick McHardy
2009-01-28 17:07                       ` Eric Dumazet
2009-01-28 17:34                         ` Eric Dumazet
2009-01-29 15:31                           ` [PATCH] netfilter: unfold two critical loops in ip_packet_match() Eric Dumazet
2009-01-30 15:47                             ` Andi Kleen
2009-01-30 16:54                               ` Eric Dumazet
2009-01-30 17:27                                 ` Andi Kleen
2009-01-30 17:27                                   ` Eric Dumazet
2009-01-30 17:50                                     ` Andi Kleen
2009-02-09 13:41                                   ` Patrick McHardy
2009-02-18 15:10                                     ` Eric Dumazet
2009-02-18 15:21                                       ` Patrick McHardy
2009-02-18 16:33                                         ` Eric Dumazet [this message]
2009-02-18 16:52                                           ` Patrick McHardy
2009-02-18 17:36                                           ` [PATCH] netfilter: xt_physdev fixes Eric Dumazet
2009-02-18 18:14                                             ` Patrick McHardy
2009-02-19  8:00                                               ` [PATCH] netfilter: unfold two loops in physdev_mt() Eric Dumazet
2009-02-19  8:14                                                 ` [PATCH] netfilter: unfold two loops in ip6_packet_match() Eric Dumazet
2009-02-19 10:19                                                   ` Patrick McHardy
2009-02-19 10:17                                                 ` [PATCH] netfilter: unfold two loops in physdev_mt() Patrick McHardy
2009-02-20 10:02                             ` [PATCH] netfilter: unfold two critical loops in ip_packet_match() Eric Dumazet
2009-02-20 10:04                               ` Patrick McHardy
2009-02-09 14:57                           ` 32 core net-next stack/netfilter "scaling" Patrick McHardy
2009-02-10 18:44   ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=499C3871.4030600@cosmosbay.com \
    --to=dada1@cosmosbay$(echo .)com \
    --cc=andi@firstfloor$(echo .)org \
    --cc=davem@davemloft$(echo .)net \
    --cc=kaber@trash$(echo .)net \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=netfilter-devel@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox