netdev - Re: [PATCH 3/6] IPV4 : use xor rather than multiple ands for route compare

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 1 Apr 2008 13:08:42 -0700
From:	Stephen Hemminger <shemminger@...tta.com>
To:	Eric Dumazet <dada1@...mosbay.com>
Cc:	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH 3/6] IPV4 : use xor rather than multiple ands for route
 compare

On Tue, 01 Apr 2008 07:52:03 +0200
Eric Dumazet <dada1@...mosbay.com> wrote:

> Stephen Hemminger a écrit :
> > The comparison in ip_route_input is a hot path, by recoding the C
> > "and" as bit operations, fewer conditional branches get generated
> > so the code should be faster. Maybe someday Gcc will be smart
> > enough to do this?
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@...tta.com>
> > 
> > --- a/net/ipv4/route.c	2008-03-31 10:57:30.000000000 -0700
> > +++ b/net/ipv4/route.c	2008-03-31 11:10:44.000000000 -0700
> > @@ -2079,14 +2079,14 @@ int ip_route_input(struct sk_buff *skb, 
> >  	rcu_read_lock();
> >  	for (rth = rcu_dereference(rt_hash_table[hash].chain); rth;
> >  	     rth = rcu_dereference(rth->u.dst.rt_next)) {
> > -		if (rth->fl.fl4_dst == daddr &&
> > -		    rth->fl.fl4_src == saddr &&
> > -		    rth->fl.iif == iif &&
> > -		    rth->fl.oif == 0 &&
> > -		    rth->fl.mark == skb->mark &&
> > -		    rth->fl.fl4_tos == tos &&
> > -		    net_eq(dev_net(rth->u.dst.dev), net) &&
> > -		    rth->rt_genid == atomic_read(&rt_genid)) {
> > +		if (((rth->fl.fl4_dst ^ daddr) |
> > +		     (rth->fl.fl4_src ^ saddr) |
> > +		     (rth->fl.iif ^ iif) |
> > +		     rth->fl.oif |
> > +		     (rth->fl.mark ^ skb->mark) |
> > +		     (rth->fl.fl4_tos ^ tos) |
> > +		     (rth->rt_genid ^ atomic_read(&rt_genid))) == 0 &&
> > +		    net_eq(dev_net(rth->u.dst.dev), net)) {
> >  			dst_use(&rth->u.dst, jiffies);
> >  			RT_CACHE_STAT_INC(in_hit);
> >  			rcu_read_unlock();
> > 
> 
> Are you sure all fields share same cache lines, on 32bit and 64bit arches ?

The flow fields are all together, and the other parameters are local variables
in registers so that compare should be in one cache line.

--- a/net/ipv4/route.c	2008-03-31 17:12:30.000000000 -0700
+++ b/net/ipv4/route.c	2008-04-01 13:05:46.000000000 -0700
@@ -2079,12 +2079,12 @@ int ip_route_input(struct sk_buff *skb, 
 	rcu_read_lock();
 	for (rth = rcu_dereference(rt_hash_table[hash].chain); rth;
 	     rth = rcu_dereference(rth->u.dst.rt_next)) {
-		if (rth->fl.fl4_dst == daddr &&
-		    rth->fl.fl4_src == saddr &&
-		    rth->fl.iif == iif &&
-		    rth->fl.oif == 0 &&
+		if (((rth->fl.fl4_dst ^ daddr) |
+		     (rth->fl.fl4_src ^ saddr) |
+		     (rth->fl.iif ^ iif) |
+		     rth->fl.oif |
+		     (rth->fl.fl4_tos ^ tos)) == 0 &&
 		    rth->fl.mark == skb->mark &&
-		    rth->fl.fl4_tos == tos &&
 		    net_eq(dev_net(rth->u.dst.dev), net) &&
 		    rth->rt_genid == atomic_read(&rt_genid)) {
 			dst_use(&rth->u.dst, jiffies);
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html