Patchwork udp: break from the lookup when hitting the maximum score value

login
register
mail settings
Submitter Lucian Adrian Grijincu
Date Oct. 23, 2009, 4:36 p.m.
Message ID <200910231936.16022.lgrijincu@ixiacom.com>
Download mbox | patch
Permalink /patch/36793/
State Rejected
Delegated to: David Miller
Headers show

Comments

Lucian Adrian Grijincu - Oct. 23, 2009, 4:36 p.m.
Before udp hashes were converted to rcu in
	udp: introduce struct udp_table and multiple spinlocks
	645ca708f936b2fbeb79e52d7823e3eb2c0905f8
we stopped searching in list upon hitting the maximum score value (which is 
9).

This got removed in the conversion to rcu.
I'm not sure whether this was intentional or it just slipped by.

As far as I understand it this does not interfere with the lockless rcu: there 
is another score check the result will have to pass and if it doesn't have a 
score of 9 (which will be the value of badness) we'll just restart the lookup.

Even if the node was deleted from the chain and reclaimed at a later time, if 
at the second score test we have value 9 again, we can still return with this 
result.

Am I missing something?
Eric Dumazet - Oct. 23, 2009, 4:57 p.m.
Lucian Adrian Grijincu a écrit :
> Before udp hashes were converted to rcu in
> 	udp: introduce struct udp_table and multiple spinlocks
> 	645ca708f936b2fbeb79e52d7823e3eb2c0905f8
> we stopped searching in list upon hitting the maximum score value (which is 
> 9).
> 
> This got removed in the conversion to rcu.
> I'm not sure whether this was intentional or it just slipped by.
> 
> As far as I understand it this does not interfere with the lockless rcu: there 
> is another score check the result will have to pass and if it doesn't have a 
> score of 9 (which will be the value of badness) we'll just restart the lookup.
> 
> Even if the node was deleted from the chain and reclaimed at a later time, if 
> at the second score test we have value 9 again, we can still return with this 
> result.
> 
> Am I missing something?
> 
> 

This was intentional.

This never happens in practice and slowdown lookups.

(To reach score 9, your UDP socket must be connected, and bound to a device)
Most developpers dont even know UDP socket can be connected...

We added large hashtables in commit f86dcc5aa8c7908f2c287e7a211228df599e3e71
(udp: dynamically size hash tables at boot time), so average chain length should
be small anyway...



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index d0d436d..69464b9 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -294,6 +294,11 @@  begin:
 	sk_nulls_for_each_rcu(sk, node, &hslot->head) {
 		score = compute_score(sk, net, saddr, hnum, sport,
 				      daddr, dport, dif);
+		if (score == 9) {
+			result = sk;
+			badness = score;
+			goto skip_end_of_nulls_check;
+		}
 		if (score > badness) {
 			result = sk;
 			badness = score;
@@ -307,6 +312,7 @@  begin:
 	if (get_nulls_value(node) != hash)
 		goto begin;
 
+skip_end_of_nulls_check:
 	if (result) {
 		if (unlikely(!atomic_inc_not_zero(&result->sk_refcnt)))
 			result = NULL;