Skip to contentSkip to conversation

Consolidate udp hash calculations

PATCH In review Pavel Emelianov 2 participants 4 messages May 4, 2007 – May 7, 2007

Subsystems (3)
Networking Network drivers UDP
lore
Share & tools
lore

Pavel Emelianov

author
#0 lore raw

PATCH 1/1 Consolidate udp hash calculations

Make access to udphash/udplitehash symmetrical to inet hashes.

This may also help network namespaces, since they tend to use one hash for different namespaces by selecting the hash chain depending on a hash value and the namespace.

Signed-off-by Pavel Emelianov <[email protected]>

---

include/linux/udp.h +5 -0
index 6de445c..a40ccd4 100644
@@ -49,6 +49,11 @@ static inline struct udphdr *udp_hdr(con
 #include <net/inet_sock.h>
 #define UDP_HTABLE_SIZE		128
 
+static inline unsigned int udp_hashfn(unsigned int num)
+{
+	return num & (UDP_HTABLE_SIZE - 1);
+}
+
 struct udp_sock {
 	/* inet_sock has to be the first member */
 	struct inet_sock inet;
net/ipv4/udp.c +9 -14
index 113e0c4..8ea1c81 100644
@@ -132,7 +132,7 @@ static inline int __udp_lib_port_inuse(u
 	struct hlist_node *node;
 	struct inet_sock *inet;
 
-	sk_for_each(sk, node, &udptable[hash & (UDP_HTABLE_SIZE - 1)]) {
+	sk_for_each(sk, node, &udptable[udp_hashfn(hash)]) {
 		if (sk->sk_hash != hash)
 			continue;
 		inet = inet_sk(sk);

@@ -159,7 +159,6 @@ int __udp_lib_get_port(struct sock *sk, 
 					 const struct sock *sk2 )    )
 {
 	struct hlist_node *node;
-	struct hlist_head *head;
 	struct sock *sk2;
 	unsigned int hash;
 	int    error = 1;

@@ -175,10 +174,11 @@ int __udp_lib_get_port(struct sock *sk, 
 		best = result = *port_rover;
 		for (i = 0; i < UDP_HTABLE_SIZE; i++, result++) {
 			int size;
+			struct hlist_head *head;
 
 			hash = hash_port_and_addr(result,
 					inet_sk(sk)->rcv_saddr);
-			head = &udptable[hash & (UDP_HTABLE_SIZE - 1)];
+			head = &udptable[udp_hashfn(hash)];
 			if (hlist_empty(head)) {
 				if (result > sysctl_local_port_range[1])
 					result = sysctl_local_port_range[0] +

@@ -222,9 +222,7 @@ gotit:
 		*port_rover = snum = result;
 	} else {
 		hash = hash_port_and_addr(snum, 0);
-		head = &udptable[hash & (UDP_HTABLE_SIZE - 1)];
-
-		sk_for_each(sk2, node, head)
+		sk_for_each(sk2, node, &udptable[udp_hashfn(hash)])
 			if (sk2->sk_hash == hash &&
 			    sk2 != sk &&
 			    inet_sk(sk2)->num == snum &&

@@ -237,9 +235,7 @@ gotit:
 		if (inet_sk(sk)->rcv_saddr) {
 			hash = hash_port_and_addr(snum,
 						  inet_sk(sk)->rcv_saddr);
-			head = &udptable[hash & (UDP_HTABLE_SIZE - 1)];
-
-			sk_for_each(sk2, node, head)
+			sk_for_each(sk2, node, &udptable[udp_hashfn(hash)])
 				if (sk2->sk_hash == hash &&
 				    sk2 != sk &&
 				    inet_sk(sk2)->num == snum &&

@@ -255,8 +251,7 @@ gotit:
 	inet_sk(sk)->num = snum;
 	sk->sk_hash = hash;
 	if (sk_unhashed(sk)) {
-		head = &udptable[hash & (UDP_HTABLE_SIZE - 1)];
-		sk_add_node(sk, head);
+		sk_add_node(sk, &udptable[udp_hashfn(hash)]);
 		sock_prot_inc_use(sk->sk_prot);
 	}
 	error = 0;

@@ -304,7 +299,7 @@ static struct sock *__udp4_lib_lookup(__
 
 lookup:
 
-	sk_for_each(sk, node, &udptable[hash & (UDP_HTABLE_SIZE - 1)]) {
+	sk_for_each(sk, node, &udptable[udp_hashfn(hash)]) {
 		struct inet_sock *inet = inet_sk(sk);
 
 		if (sk->sk_hash != hash || ipv6_only_sock(sk) ||

@@ -1205,8 +1200,8 @@ static int __udp4_lib_mcast_deliver(stru
 
 	read_lock(&udp_hash_lock);
 
-	sk = sk_head(&udptable[hash & (UDP_HTABLE_SIZE - 1)]);
-	skw = sk_head(&udptable[hashwild & (UDP_HTABLE_SIZE - 1)]);
+	sk = sk_head(&udptable[udp_hashfn(hash)]);
+	skw = sk_head(&udptable[udp_hashfn(hashwild)]);
 
 	sk = udp_v4_mcast_next(sk, hash, hport, daddr, uh->source, saddr, dif);
 	if (!sk) {
net/ipv6/udp.c +2 -2
index b083c09..a0a8915 100644
@@ -67,7 +67,7 @@ static struct sock *__udp6_lib_lookup(st
 	int badness = -1;
 
 	read_lock(&udp_hash_lock);
-	sk_for_each(sk, node, &udptable[hnum & (UDP_HTABLE_SIZE - 1)]) {
+	sk_for_each(sk, node, &udptable[udp_hashfn(hnum)]) {
 		struct inet_sock *inet = inet_sk(sk);
 
 		if (sk->sk_hash == hnum && sk->sk_family == PF_INET6) {

@@ -350,7 +350,7 @@ static int __udp6_lib_mcast_deliver(stru
 	int dif;
 
 	read_lock(&udp_hash_lock);
-	sk = sk_head(&udptable[ntohs(uh->dest) & (UDP_HTABLE_SIZE - 1)]);
+	sk = sk_head(&udptable[udp_hashfn(ntohs(uh->dest))]);
 	dif = inet6_iif(skb);
 	sk = udp_v6_mcast_next(sk, uh->dest, daddr, uh->source, saddr, dif);
 	if (!sk) {
maintainer
#1 lore raw

From: Pavel Emelianov <[email protected]>
Date: Fri, 04 May 2007 18:51:36 +0400

Quoted 7 lines

Make access to udphash/udplitehash symmetrical to inet hashes.

This may also help network namespaces, since they tend to use one hash for different namespaces by selecting the hash chain depending on a hash value and the namespace.

Signed-off-by Pavel Emelianov <[email protected]>

This is not the "UDP Hash Function", it is a mask to bring the hash value modulo of the hash table size.

So your function name is misleading and makes the code even worse.

I'm also not so sure this cleanup really adds any clarity, even with a proper function name.

I can only assume you wish to do something with the UDP hash table sizes, and therefore only want to have a need to touch one function.

author
#2 lore raw
Quoted 11 lines · David Miller

From: Pavel Emelianov <[email protected]>
Date: Fri, 04 May 2007 18:51:36 +0400

Quoted 7 lines

Make access to udphash/udplitehash symmetrical to inet hashes.

This may also help network namespaces, since they tend to use one hash for different namespaces by selecting the hash chain depending on a hash value and the namespace.

Signed-off-by Pavel Emelianov <[email protected]>
Quoted 8 lines

This is not the "UDP Hash Function", it is a mask to bring the hash value modulo of the hash table size.

So your function name is misleading and makes the code even worse.

I'm also not so sure this cleanup really adds any clarity, even with a proper function name.

The same is applicable to inet_lhashfn and inet_bhashfn, but they are called hashfn-s and do exist in kernel. This hashfn will make things look similar.

Quoted 2 lines

I can only assume you wish to do something with the UDP hash table sizes, and therefore only want to have a need to touch one function.

You are right, I have told that this must help network namespaces, by making the hash differ depending on the namespace passed.

maintainer
#3 lore raw

From: Pavel Emelianov <[email protected]>
Date: Mon, 07 May 2007 11:07:15 +0400

Quoted 6 lines
Quoted 2 lines · David Miller

I can only assume you wish to do something with the UDP hash table sizes, and therefore only want to have a need to touch one function.

You are right, I have told that this must help network namespaces, by making the hash differ depending on the namespace passed.

Then make your code rearrangements as part of your namespace patches so we can see what the consolidation is even for.