Skip to contentSkip to conversation

net/ipv6/udp.c: remove duplicate udp_get_port code

PATCH Applied [email protected] 4 participants 4 messages Aug 17, 2006 – Aug 27, 2006

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 net/ipv6/udp.c: remove duplicate udp_get_port code

[NET]: UDPv4 and UDPv6 use an almost identical version of the get_port function, which is unnecessary since the (long) code differs in only one if-statement.

This patch creates one common function which is called by udp_v4_get_port() and udp_v6_get_port(). As a result,
* duplicated code is removed
* udp_port_rover and local port lookup can now be removed from udp.h
* further savings follow since the same function will be used by UDP-Litev4 and UDP-Litev6

In contrast to the patch sent in response to Yoshifujis comments (fixed by this variant), the code below also removes the EXPORT_SYMBOL(udp_port_rover), since udp_port_rover can now remain local to net/ipv4/udp.c.

---

 include/net/udp.h |   18 +---------
 net/ipv4/udp.c    |   96 +++++++++++++++++++++++++++++++++---------------------
 net/ipv6/udp.c    |   76 +-----------------------------------------
 3 files changed, 64 insertions(+), 126 deletions(-)
include/net/udp.h +2 -16
index 766fba1..c490a0f 100644
@@ -30,25 +30,9 @@ #include <linux/seq_file.h>
 
 #define UDP_HTABLE_SIZE		128
 
-/* udp.c: This needs to be shared by v4 and v6 because the lookup
- *        and hashing code needs to work with different AF's yet
- *        the port space is shared.
- */
 extern struct hlist_head udp_hash[UDP_HTABLE_SIZE];
 extern rwlock_t udp_hash_lock;
 
-extern int udp_port_rover;
-
-static inline int udp_lport_inuse(u16 num)
-{
-	struct sock *sk;
-	struct hlist_node *node;
-
-	sk_for_each(sk, node, &udp_hash[num & (UDP_HTABLE_SIZE - 1)])
-		if (inet_sk(sk)->num == num)
-			return 1;
-	return 0;
-}
 
 /* Note: this must match 'valbool' in sock_setsockopt */
 #define UDP_CSUM_NOXMIT		1
@@ -63,6 +47,8 @@ extern struct proto udp_prot;
 
 struct sk_buff;
 
+extern int	udp_get_port(struct sock *sk, unsigned short snum,
+			     int (*saddr_cmp)(struct sock *, struct sock *));
 extern void	udp_err(struct sk_buff *, u32);
 
 extern int	udp_sendmsg(struct kiocb *iocb, struct sock *sk,
net/ipv4/udp.c +60 -36

142 lines not on this page · raw message

net/ipv6/udp.c +2 -74

83 lines not on this page · raw message

#1 lore raw
Quoted 4 lines · On Thu, 17 Aug 2006, [email protected] wrote:
net/ipv4/udp.c +0 -4
-			if (inet2->num == snum &&
-			    sk2 != sk &&
-			    !ipv6_only_sock(sk2) &&
-			    (!sk2->sk_bound_dev_if ||

net/ipv4/udp.c static int udp_v4_get_port(struct sock * · line 199

Quoted 10 lines
net/ipv4/udp.c +6 -2
-			     !sk->sk_bound_dev_if ||
-			     sk2->sk_bound_dev_if == sk->sk_bound_dev_if) &&
 
 
+		sk_for_each(sk2, node, head)
+			if (inet_sk(sk2)->num == snum                        &&
+			    sk2 != sk                                        &&
+			    (!sk2->sk_reuse        || !sk->sk_reuse)         &&
+			    (!sk2->sk_bound_dev_if || !sk->sk_bound_dev_if
+			     || sk2->sk_bound_dev_if == sk->sk_bound_dev_if) &&

Doesn't this change the behavior for IPV6_V6ONLY sockets ?

- James

Signature · 2 lines
James Morris
<[email protected]>
#2 lore raw

Hi,

In article <[email protected]> (at Thu, 17 Aug 2006 10:56:40 -0400 (EDT)), James Morris <[email protected]> says:

Quoted 10 lines · On Thu, 17 Aug 2006, [email protected] wrote:
Quoted 7 lines
net/ipv4/udp.c +0 -6
-			if (inet2->num == snum &&
-			    sk2 != sk &&
-			    !ipv6_only_sock(sk2) &&
-			    (!sk2->sk_bound_dev_if ||
-			     !sk->sk_bound_dev_if ||
-			     sk2->sk_bound_dev_if == sk->sk_bound_dev_if) &&
Quoted 9 lines

net/ipv4/udp.c static int udp_v4_get_port(struct sock * · line 199

Quoted 6 lines
net/ipv4/udp.c +6 -0
+		sk_for_each(sk2, node, head)
+			if (inet_sk(sk2)->num == snum                        &&
+			    sk2 != sk                                        &&
+			    (!sk2->sk_reuse        || !sk->sk_reuse)         &&
+			    (!sk2->sk_bound_dev_if || !sk->sk_bound_dev_if
+			     || sk2->sk_bound_dev_if == sk->sk_bound_dev_if) &&

Doesn't this change the behavior for IPV6_V6ONLY sockets ?

It is tested in ipv4_rcv_saddr_equal() (called vi saddr_cmp), isn't it?

--yoshfuji

maintainer Applied
#3 lore raw
In reply to [email protected]

From: [email protected]
Date: Thu, 17 Aug 2006 13:25:46 +0100

Quoted 10 lines

[NET]: UDPv4 and UDPv6 use an almost identical version of the get_port function, which is unnecessary since the (long) code differs in only one if-statement.

This patch creates one common function which is called by udp_v4_get_port() and udp_v6_get_port(). As a result,
* duplicated code is removed
* udp_port_rover and local port lookup can now be removed from udp.h
* further savings follow since the same function will be used by UDP-Litev4 and UDP-Litev6

Quoted 5 lines

In contrast to the patch sent in response to Yoshifujis comments (fixed by this variant), the code below also removes the EXPORT_SYMBOL(udp_port_rover), since udp_port_rover can now remain local to net/ipv4/udp.c.

Applied, and I marked udp_port_rover "static" for
good measure.

Thanks.