IPV4: route use jhash3

PATCH Applied Stephen Hemminger 4 participants 19 messages Apr 1, 2008 – Apr 10, 2008 1 message not in the archive

Subsystems (7)

Networking Network drivers BPF IPv4/IPv6 +3 more subsystems

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Stephen Hemminger

author

2008-04-01 02:31 UTC #1 lore raw

PATCH 1/6 socket: sk_filter minor cleanups

Some minor style cleanups:
* Move __KERNEL__ definitions to one place in filter.h
* Use const for sk_filter_len
* Line wrapping
* Put EXPORT_SYMBOL next to function definition

Signed-off-by Stephen Hemminger <[email protected]>

include/linux/filter.h +15 ~~-16~~

@@ -37,21 +37,6 @@ struct sock_fprog	/* Required for SO_ATT
 	struct sock_filter __user *filter;
 };
 
-#ifdef __KERNEL__
-struct sk_filter
-{
-	atomic_t		refcnt;
-	unsigned int         	len;	/* Number of filter blocks */
-	struct rcu_head		rcu;
-	struct sock_filter     	insns[0];
-};
-
-static inline unsigned int sk_filter_len(struct sk_filter *fp)
-{
-	return fp->len*sizeof(struct sock_filter) + sizeof(*fp);
-}
-#endif
-
 /*
  * Instruction classes
  */

@@ -141,10 +126,24 @@ static inline unsigned int sk_filter_len
 #define SKF_LL_OFF    (-0x200000)
 
 #ifdef __KERNEL__
+struct sk_filter
+{
+	atomic_t		refcnt;
+	unsigned int         	len;	/* Number of filter blocks */
+	struct rcu_head		rcu;
+	struct sock_filter     	insns[0];
+};
+
+static inline unsigned int sk_filter_len(const struct sk_filter *fp)
+{
+	return fp->len * sizeof(struct sock_filter) + sizeof(*fp);
+}
+
 struct sk_buff;
 struct sock;
 
-extern unsigned int sk_run_filter(struct sk_buff *skb, struct sock_filter *filter, int flen);
+extern unsigned int sk_run_filter(struct sk_buff *skb,
+				  struct sock_filter *filter, int flen);
 extern int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk);
 extern int sk_detach_filter(struct sock *sk);
 extern int sk_chk_filter(struct sock_filter *filter, int flen);

net/core/filter.c +2 -3

@@ -275,6 +275,7 @@ load_b:
 
 	return 0;
 }
+EXPORT_SYMBOL(sk_run_filter);
 
 /**
  *	sk_chk_filter - verify socket filter code

@@ -385,6 +386,7 @@ int sk_chk_filter(struct sock_filter *fi
 
 	return (BPF_CLASS(filter[flen - 1].code) == BPF_RET) ? 0 : -EINVAL;
 }
+EXPORT_SYMBOL(sk_chk_filter);
 
 /**
  * 	sk_filter_rcu_release: Release a socket filter by rcu_head

@@ -467,6 +469,3 @@ int sk_detach_filter(struct sock *sk)
 	rcu_read_unlock_bh();
 	return ret;
 }
-
-EXPORT_SYMBOL(sk_chk_filter);
-EXPORT_SYMBOL(sk_run_filter);

David Miller

maintainer Applied

2008-04-10 08:39 UTC #2 lore raw

Re: [PATCH 1/6] socket: sk_filter minor cleanups

From: Stephen Hemminger <[email protected]>
Date: Mon, 31 Mar 2008 17:47:09 -0700

Quoted 7 lines

Some minor style cleanups:
* Move __KERNEL__ definitions to one place in filter.h
* Use const for sk_filter_len
* Line wrapping
* Put EXPORT_SYMBOL next to function definition
Signed-off-by Stephen Hemminger <[email protected]>

Applied to net-2.6.26, thanks!

Stephen Hemminger

author

2008-04-01 02:31 UTC #3 lore raw

PATCH 2/6 socket: sk_filter deinline

In reply to a message not in the archive

The sk_filter function is too big to be inlined. This saves 2296 bytes of text on allyesconfig.

Signed-off-by Stephen Hemminger <[email protected]>

include/linux/filter.h +1 -0

@@ -142,6 +142,7 @@ static inline unsigned int sk_filter_len
 struct sk_buff;
 struct sock;
 
+extern int sk_filter(struct sock *sk, struct sk_buff *skb);
 extern unsigned int sk_run_filter(struct sk_buff *skb,
 				  struct sock_filter *filter, int flen);
 extern int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk);

include/net/sock.h +0 ~~-35~~

@@ -927,41 +927,6 @@ extern void sk_common_release(struct soc
 extern void sock_init_data(struct socket *sock, struct sock *sk);
 
 /**
- *	sk_filter - run a packet through a socket filter
- *	@sk: sock associated with &sk_buff
- *	@skb: buffer to filter
- *	@needlock: set to 1 if the sock is not locked by caller.
- *
- * Run the filter code and then cut skb->data to correct size returned by
- * sk_run_filter. If pkt_len is 0 we toss packet. If skb->len is smaller
- * than pkt_len we keep whole skb->data. This is the socket level
- * wrapper to sk_run_filter. It returns 0 if the packet should
- * be accepted or -EPERM if the packet should be tossed.
- *
- */
-
-static inline int sk_filter(struct sock *sk, struct sk_buff *skb)
-{
-	int err;
-	struct sk_filter *filter;
-	
-	err = security_sock_rcv_skb(sk, skb);
-	if (err)
-		return err;
-	
-	rcu_read_lock_bh();
-	filter = rcu_dereference(sk->sk_filter);
-	if (filter) {
-		unsigned int pkt_len = sk_run_filter(skb, filter->insns,
-				filter->len);
-		err = pkt_len ? pskb_trim(skb, pkt_len) : -EPERM;
-	}
- 	rcu_read_unlock_bh();
-
-	return err;
-}
-
-/**
  *	sk_filter_release: Release a socket filter
  *	@sk: socket
  *	@fp: filter to remove

net/core/filter.c +35 -0

@@ -64,6 +64,41 @@ static inline void *load_pointer(struct 
 }
 
 /**
+ *	sk_filter - run a packet through a socket filter
+ *	@sk: sock associated with &sk_buff
+ *	@skb: buffer to filter
+ *	@needlock: set to 1 if the sock is not locked by caller.
+ *
+ * Run the filter code and then cut skb->data to correct size returned by
+ * sk_run_filter. If pkt_len is 0 we toss packet. If skb->len is smaller
+ * than pkt_len we keep whole skb->data. This is the socket level
+ * wrapper to sk_run_filter. It returns 0 if the packet should
+ * be accepted or -EPERM if the packet should be tossed.
+ *
+ */
+int sk_filter(struct sock *sk, struct sk_buff *skb)
+{
+	int err;
+	struct sk_filter *filter;
+
+	err = security_sock_rcv_skb(sk, skb);
+	if (err)
+		return err;
+
+	rcu_read_lock_bh();
+	filter = rcu_dereference(sk->sk_filter);
+	if (filter) {
+		unsigned int pkt_len = sk_run_filter(skb, filter->insns,
+				filter->len);
+		err = pkt_len ? pskb_trim(skb, pkt_len) : -EPERM;
+	}
+ 	rcu_read_unlock_bh();
+
+	return err;
+}
+EXPORT_SYMBOL(sk_filter);
+
+/**
  *	sk_run_filter - run a filter on a socket
  *	@skb: buffer to run the filter on
  *	@filter: filter to apply

David Miller

maintainer

2008-04-10 08:49 UTC #4 lore raw

Re: [PATCH 2/6] socket: sk_filter deinline

From: Stephen Hemminger <[email protected]>
Date: Mon, 31 Mar 2008 17:47:10 -0700

Quoted 4 lines

The sk_filter function is too big to be inlined. This saves 2296 bytes of text on allyesconfig.
Signed-off-by Stephen Hemminger <[email protected]>

Because of the bloat I'll apply this to net-2.6.26

But I think we can do better in many of the callsites.

Most of them are in the packet receive path, have BH's disabled, and a lot of the variables already laid out in registers.

I would like to see a new inline created that caters to these cases, which does something like:

	if (rcu_dereference(sk->sk_filter))
		return sk_filter(sk, skb);
	else
		return 0;

The security check muddles things here, but that should be inlined in such situations too I think.

It's all of that "pkt_len = this; pskb_trim(that);" that makes for most of the bloat. And that can stay in the un-inlined sk_filter() we have now.

Having a socket filter enabled on a TCP socket is so rare, but now everyone pays for the function call unconditionally.

Stephen Hemminger

author

2008-04-01 02:31 UTC #5 lore raw

PATCH 3/6 IPV4 : use xor rather than multiple ands for route compare

In reply to a message not in the archive

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 <[email protected]>

net/ipv4/route.c +8 -8

@@ -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();

Eric Dumazet

2008-04-01 05:52 UTC #6 lore raw

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

Quoted 23 lines · Stephen Hemminger

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 <[email protected]>
net/ipv4/route.c +1 -8
@@ -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) |

net/ipv4/route.c int ip_route_input(struct sk_buff *skb, · line 2092

Quoted 10 lines

net/ipv4/route.c +7 -0

+		     (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 ?

I prefer having some conditional branches instead of cache misses, given that the first two branches are usually discriminant.

Maybe we could let one test on (daddr,saddr) to do a fast segregation (with one cache line at most) of candidates, then one remaining compare on other keys ?

Stephen Hemminger

author

2008-04-01 20:08 UTC #7 lore raw

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

Quoted 28 lines · On Tue, 01 Apr 2008 07:52:03 +0200 Eric Dumazet <[email protected]> wrote:

Quoted 27 lines · Stephen Hemminger

Signed-off-by Stephen Hemminger <[email protected]>

net/ipv4/route.c +6 -8

@@ -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) |

Quoted 8 lines

net/ipv4/route.c int ip_route_input(struct sk_buff *skb, · line 2092
Quoted 6 lines
net/ipv4/route.c +2 -0
+		     (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.

net/ipv4/route.c +5 -5

@@ -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);

David Miller

maintainer

2008-04-10 08:51 UTC #8 lore raw

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

From: Stephen Hemminger <[email protected]>
Date: Tue, 1 Apr 2008 13:08:42 -0700

Quoted 13 lines

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

net/ipv4/route.c +0 -4

@@ -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 &&

net/ipv4/route.c int ip_route_input(struct sk_buff *skb, · line 2090

Quoted 10 lines

net/ipv4/route.c +5 -1

+		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);

Eric, any objections to this version?

YOSHIFUJI Hideaki / 吉藤英明

2008-04-10 09:00 UTC #9 lore raw

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

In article <[email protected]> (at Thu, 10 Apr 2008 01:51:18 -0700 (PDT)), David Miller <[email protected]> says:

Quoted 20 lines

From: Stephen Hemminger <[email protected]>
Date: Tue, 1 Apr 2008 13:08:42 -0700
Quoted 17 lines
The flow fields are all together, and the other parameters are local variables in registers so that compare should be in one cache line.
net/ipv4/route.c +4 -4
@@ -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 |

Quoted 8 lines

net/ipv4/route.c int ip_route_input(struct sk_buff *skb, · line 2090
Quoted 6 lines
net/ipv4/route.c +1 -1
+		     (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);
Eric, any objections to this version?

I'm not Eric, but well, I'm now doubting if this is really good. If the comparision chain is long and it is unlikely to pass all the tests, it would be better to cut the line.
If we use "or", we need to run through the test, in ayn case.

--yoshfuji

David Miller

maintainer

2008-04-10 10:56 UTC #10 lore raw

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

From: YOSHIFUJI Hideaki / 吉藤英明 <[email protected]>
Date: Thu, 10 Apr 2008 18:01:48 +0900 (JST)

Quoted 31 lines

In article <[email protected]> (at Thu, 10 Apr 2008 01:51:18 -0700 (PDT)), David Miller <[email protected]> says:
Quoted 20 lines
From: Stephen Hemminger <[email protected]>
Date: Tue, 1 Apr 2008 13:08:42 -0700
Quoted 17 lines
The flow fields are all together, and the other parameters are local variables in registers so that compare should be in one cache line.
net/ipv4/route.c +4 -4
@@ -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 |
Quoted 8 lines
net/ipv4/route.c int ip_route_input(struct sk_buff *skb, · line 2090
Quoted 6 lines
net/ipv4/route.c +1 -1
+		     (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);
Eric, any objections to this version?

Quoted 4 lines

I'm not Eric, but well, I'm now doubting if this is really good. If the comparision chain is long and it is unlikely to pass all the tests, it would be better to cut the line.
If we use "or", we need to run through the test, in ayn case.

Actually the case you mention it is part of the incentive for this change.

Branch prediction fares very poorly in such cases, and therefore it is better to mispredict one branch over all the data items in the same cache line than any one of several such branches. The above new sequence gets emitted by the compiler as several integer operations and one branch. As long as all the data items are in the same cacheline, this is optimal.

We made such a change for ethernet address comparisons a few years ago. At the time Eric showed that it mattered a lot for Athlon processors.

YOSHIFUJI Hideaki / 吉藤英明

2008-04-10 12:16 UTC #11 lore raw

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

In article <[email protected]> (at Thu, 10 Apr 2008 03:56:18 -0700 (PDT)), David Miller <[email protected]> says:

Quoted 7 lines

Branch prediction fares very poorly in such cases, and therefore it is better to mispredict one branch over all the data items in the same cache line than any one of several such branches. The above new sequence gets emitted by the compiler as several integer operations and one branch. As long as all the data items are in the same cacheline, this is optimal.

Okay, thanks.

--yoshfuji

Eric Dumazet

Acked-by

2008-04-10 09:27 UTC #12 lore raw

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

In reply to David Miller

Quoted 22 lines · David Miller

From: Stephen Hemminger <[email protected]>
Date: Tue, 1 Apr 2008 13:08:42 -0700
Quoted 18 lines
The flow fields are all together, and the other parameters are local variables in registers so that compare should be in one cache line.
net/ipv4/route.c +5 -4
@@ -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 &&

Quoted 9 lines

net/ipv4/route.c int ip_route_input(struct sk_buff *skb, · line 2090
Quoted 6 lines
net/ipv4/route.c +0 -1
 		    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);
Eric, any objections to this version?

No objections from me, thank you.

Acked-by Eric Dumazet <[email protected]>

David Miller

maintainer Applied

2008-04-10 11:00 UTC #13 lore raw

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

From: Eric Dumazet <[email protected]>
Date: Thu, 10 Apr 2008 11:26:57 +0200

Quoted 1 line

Acked-by Eric Dumazet <[email protected]>

Applied to net-2.6.26, thanks everyone.

Stephen Hemminger

author

2008-04-01 02:31 UTC #14 lore raw

PATCH 4/6 IPV4: route inline changes

In reply to a message not in the archive

Don't mark functions that are large as inline, let compiler decide. Also, use inline rather than __inline__.

Signed-off-by Stephen Hemminger <[email protected]>

net/ipv4/route.c +27 ~~-27~~

@@ -600,18 +600,18 @@ static inline int ip_rt_proc_init(void)
 }
 #endif /* CONFIG_PROC_FS */
 
-static __inline__ void rt_free(struct rtable *rt)
+static inline void rt_free(struct rtable *rt)
 {
 	call_rcu_bh(&rt->u.dst.rcu_head, dst_rcu_free);
 }
 
-static __inline__ void rt_drop(struct rtable *rt)
+static inline void rt_drop(struct rtable *rt)
 {
 	ip_rt_put(rt);
 	call_rcu_bh(&rt->u.dst.rcu_head, dst_rcu_free);
 }
 
-static __inline__ int rt_fast_clean(struct rtable *rth)
+static inline int rt_fast_clean(struct rtable *rth)
 {
 	/* Kill broadcast/multicast entries very aggresively, if they
 	   collide in hash table with more useful entries */

@@ -619,7 +619,7 @@ static __inline__ int rt_fast_clean(stru
 		rth->fl.iif && rth->u.dst.rt_next;
 }
 
-static __inline__ int rt_valuable(struct rtable *rth)
+static inline int rt_valuable(struct rtable *rth)
 {
 	return (rth->rt_flags & (RTCF_REDIRECTED | RTCF_NOTIFY)) ||
 		rth->u.dst.expires;

@@ -1420,7 +1420,7 @@ out:	kfree_skb(skb);
 static const unsigned short mtu_plateau[] =
 {32000, 17914, 8166, 4352, 2002, 1492, 576, 296, 216, 128 };
 
-static __inline__ unsigned short guess_mtu(unsigned short old_mtu)
+static inline unsigned short guess_mtu(unsigned short old_mtu)
 {
 	int i;

@@ -1750,11 +1750,11 @@ static void ip_handle_martian_source(str
 #endif
 }
 
-static inline int __mkroute_input(struct sk_buff *skb,
-				  struct fib_result* res,
-				  struct in_device *in_dev,
-				  __be32 daddr, __be32 saddr, u32 tos,
-				  struct rtable **result)
+static int __mkroute_input(struct sk_buff *skb,
+			   struct fib_result *res,
+			   struct in_device *in_dev,
+			   __be32 daddr, __be32 saddr, u32 tos,
+			   struct rtable **result)
 {
 
 	struct rtable *rth;

@@ -1846,11 +1846,11 @@ static inline int __mkroute_input(struct
 	return err;
 }
 
-static inline int ip_mkroute_input(struct sk_buff *skb,
-				   struct fib_result* res,
-				   const struct flowi *fl,
-				   struct in_device *in_dev,
-				   __be32 daddr, __be32 saddr, u32 tos)
+static int ip_mkroute_input(struct sk_buff *skb,
+			    struct fib_result *res,
+			    const struct flowi *fl,
+			    struct in_device *in_dev,
+			    __be32 daddr, __be32 saddr, u32 tos)
 {
 	struct rtable* rth = NULL;
 	int err;

@@ -2132,12 +2132,12 @@ int ip_route_input(struct sk_buff *skb, 
 	return ip_route_input_slow(skb, daddr, saddr, tos, dev);
 }
 
-static inline int __mkroute_output(struct rtable **result,
-				   struct fib_result* res,
-				   const struct flowi *fl,
-				   const struct flowi *oldflp,
-				   struct net_device *dev_out,
-				   unsigned flags)
+static int __mkroute_output(struct rtable **result,
+			    struct fib_result *res,
+			    const struct flowi *fl,
+			    const struct flowi *oldflp,
+			    struct net_device *dev_out,
+			    unsigned flags)
 {
 	struct rtable *rth;
 	struct in_device *in_dev;

@@ -2252,12 +2252,12 @@ static inline int __mkroute_output(struc
 	return err;
 }
 
-static inline int ip_mkroute_output(struct rtable **rp,
-				    struct fib_result* res,
-				    const struct flowi *fl,
-				    const struct flowi *oldflp,
-				    struct net_device *dev_out,
-				    unsigned flags)
+static int ip_mkroute_output(struct rtable **rp,
+			     struct fib_result *res,
+			     const struct flowi *fl,
+			     const struct flowi *oldflp,
+			     struct net_device *dev_out,
+			     unsigned flags)
 {
 	struct rtable *rth = NULL;
 	int err = __mkroute_output(&rth, res, fl, oldflp, dev_out, flags);

David Miller

maintainer Applied

2008-04-10 08:53 UTC #15 lore raw

Re: [PATCH 4/6] IPV4: route inline changes

From: Stephen Hemminger <[email protected]>
Date: Mon, 31 Mar 2008 17:47:12 -0700

Quoted 4 lines

Don't mark functions that are large as inline, let compiler decide. Also, use inline rather than __inline__.
Signed-off-by Stephen Hemminger <[email protected]>

Applied to net-2.6.26, thanks!

Stephen Hemminger

author

2008-04-01 02:31 UTC #16 lore raw

PATCH 5/6 IPV4: route use jhash3

In reply to a message not in the archive

Since route hash is a triple, use jhash_3words rather doing the mixing directly. This should be as fast and give better distribution.

Signed-off-by Stephen Hemminger <[email protected]>

net/ipv4/route.c +5 -7

@@ -259,16 +259,14 @@ static DEFINE_PER_CPU(struct rt_cache_st
 #define RT_CACHE_STAT_INC(field) \
 	(__raw_get_cpu_var(rt_cache_stat).field++)
 
-static unsigned int rt_hash_code(u32 daddr, u32 saddr)
+static inline unsigned int rt_hash(__be32 daddr, __be32 saddr, int idx)
 {
-	return jhash_2words(daddr, saddr, atomic_read(&rt_genid))
-		& rt_hash_mask;
+	return jhash_3words((__force u32)(__be32)(daddr),
+			    (__force u32)(__be32)(saddr),
+			    idx, atomic_read(&rt_genid))
+  		& rt_hash_mask;
 }
 
-#define rt_hash(daddr, saddr, idx) \
-	rt_hash_code((__force u32)(__be32)(daddr),\
-		     (__force u32)(__be32)(saddr) ^ ((idx) << 5))
-
 #ifdef CONFIG_PROC_FS
 struct rt_cache_iter_state {
 	struct seq_net_private p;

David Miller

maintainer Applied

2008-04-10 08:54 UTC #17 lore raw

From: Stephen Hemminger <[email protected]>
Date: Mon, 31 Mar 2008 17:47:13 -0700

Quoted 4 lines

Since route hash is a triple, use jhash_3words rather doing the mixing directly. This should be as fast and give better distribution.
Signed-off-by Stephen Hemminger <[email protected]>

Applied to net-2.6.26, but:

net/ipv4/route.c static DEFINE_PER_CPU(struct rt_cache_st · line 262

Quoted 2 lines

net/ipv4/route.c +1 -1

-static unsigned int rt_hash_code(u32 daddr, u32 saddr)
+static inline unsigned int rt_hash(__be32 daddr, __be32 saddr, int idx)

Are you sure you want to inline that? jhash expands to a lot of instructions

Stephen Hemminger

author

2008-04-01 02:31 UTC #18 lore raw

PATCH 6/6 IPV4: route rekey timer can be deferrable

In reply to a message not in the archive

No urgency on the rehash interval timer, so mark it as deferrable.

Signed-off-by Stephen Hemminger <[email protected]>

net/ipv4/route.c +3 -1

@@ -3058,7 +3058,9 @@ int __init ip_rt_init(void)
 	devinet_init();
 	ip_fib_init();
 
-	setup_timer(&rt_secret_timer, rt_secret_rebuild, 0);
+	rt_secret_timer.function = rt_secret_rebuild;
+	rt_secret_timer.data = 0;
+	init_timer_deferrable(&rt_secret_timer);
 
 	/* All the timers, started at system startup tend
 	   to synchronize. Perturb it a bit.

David Miller

maintainer Applied

2008-04-10 08:55 UTC #19 lore raw

Re: [PATCH 6/6] IPV4: route rekey timer can be deferrable

From: Stephen Hemminger <[email protected]>
Date: Mon, 31 Mar 2008 17:47:14 -0700

Quoted 3 lines

No urgency on the rehash interval timer, so mark it as deferrable.
Signed-off-by Stephen Hemminger <[email protected]>

Applied to net-2.6.26, thanks.

net/ipv4/route.c int __init ip_rt_init(void) · line 3063

Quoted 4 lines

net/ipv4/route.c +3 -1

-	setup_timer(&rt_secret_timer, rt_secret_rebuild, 0);
+	rt_secret_timer.function = rt_secret_rebuild;
+	rt_secret_timer.data = 0;
+	init_timer_deferrable(&rt_secret_timer);

I've seen enough of these that a setup_timer_deferrable() might be appropriate.