Skip to contentSkip to conversation

net-udp: deprioritize cpu match for udp socket lookup

PATCH In review Maciej Żenczykowski 3 participants 6 messages Dec 5, 2018 – Dec 10, 2018

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

PATCH 1/1 net-udp: deprioritize cpu match for udp socket lookup

From: Maciej Żenczykowski <[email protected]>

During udp socket lookup cpu match should be lowest priority, hence it should increase score by only 1.

The next priority is delivering v4 to v4 sockets, and v6 to v6 sockets. The v6 code path doesn't have to deal with this so it always gets a score of '4'. The v4 code path uses '4' or '2' depending on whether we're delivering to a v4 socket or a dualstack v6 socket.

This is more important than cpu match, so has to be greater than the '1' bump in score from cpu match.

All other matches (src/dst ip, src port) are even *more* important, so need to bump score by 4 for ipv4.

For ipv6 we could simply bump by 2, but let's keep the two code paths as similar as possible.

(also, while at it, remove two unnecessary unconditional score bumps)

---
 net/ipv4/udp.c | 3 +--
 net/ipv6/udp.c | 9 ++++-----
 2 files changed, 5 insertions(+), 7 deletions(-)
net/ipv4/udp.c +1 -2
index aff2a8e99e01..0c0ab0383cec 100644
@@ -380,7 +380,7 @@ static int compute_score(struct sock *sk, struct net *net,
 	    ipv6_only_sock(sk))
 		return -1;
 
-	score = (sk->sk_family == PF_INET) ? 2 : 1;
+	score = (sk->sk_family == PF_INET) ? 4 : 2;
 	inet = inet_sk(sk);
 
 	if (inet->inet_rcv_saddr) {
@@ -405,7 +405,6 @@ static int compute_score(struct sock *sk, struct net *net,
 					dif, sdif);
 	if (!dev_match)
 		return -1;
-	score += 4;
 
 	if (sk->sk_incoming_cpu == raw_smp_processor_id())
 		score++;
net/ipv6/udp.c +4 -5
index 09cba4cfe31f..5441062d7d5e 100644
@@ -125,31 +125,30 @@ static int compute_score(struct sock *sk, struct net *net,
 	    sk->sk_family != PF_INET6)
 		return -1;
 
-	score = 0;
+	score = 4;
 	inet = inet_sk(sk);
 
 	if (inet->inet_dport) {
 		if (inet->inet_dport != sport)
 			return -1;
-		score++;
+		score += 4;
 	}
 
 	if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr)) {
 		if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
 			return -1;
-		score++;
+		score += 4;
 	}
 
 	if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
 		if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
 			return -1;
-		score++;
+		score += 4;
 	}
 
 	dev_match = udp_sk_bound_dev_eq(net, sk->sk_bound_dev_if, dif, sdif);
 	if (!dev_match)
 		return -1;
-	score++;
 
 	if (sk->sk_incoming_cpu == raw_smp_processor_id())
 		score++;
Signature · 1 line
2.20.0.rc1.387.gf8505762e3-goog
maintainer
#1 lore raw

From: Maciej Żenczykowski <[email protected]>
Date: Wed, 5 Dec 2018 12:59:17 -0800

Quoted 16 lines

From: Maciej Żenczykowski <[email protected]>

During udp socket lookup cpu match should be lowest priority, hence it should increase score by only 1.

The next priority is delivering v4 to v4 sockets, and v6 to v6 sockets. The v6 code path doesn't have to deal with this so it always gets a score of '4'. The v4 code path uses '4' or '2' depending on whether we're delivering to a v4 socket or a dualstack v6 socket.

This is more important than cpu match, so has to be greater than the '1' bump in score from cpu match.

All other matches (src/dst ip, src port) are even *more* important, so need to bump score by 4 for ipv4.

Quoted 6 lines

For ipv6 we could simply bump by 2, but let's keep the two code paths as similar as possible.

(also, while at it, remove two unnecessary unconditional score bumps)

This doesn't apply to the current net tree.

Also "net-udp: " is a weird subsystem prefix, just use "udp: ".

Thank you.

#2 lore raw
Quoted 5 lines

This doesn't apply to the current net tree.

Also "net-udp: " is a weird subsystem prefix, just use "udp: ".

Thank you.

Interesting... this patch was on top of net-next/master, and it still rebases cleanly on current net-next/master.

Would you like it on net/master instead? It indeed doesn't apply cleanly there...

maintainer
#3 lore raw

From: Maciej Żenczykowski <[email protected]>
Date: Fri, 7 Dec 2018 16:46:36 -0800

Quoted 11 lines
Quoted 5 lines

This doesn't apply to the current net tree.

Also "net-udp: " is a weird subsystem prefix, just use "udp: ".

Thank you.

Interesting... this patch was on top of net-next/master, and it still rebases cleanly on current net-next/master.

Would you like it on net/master instead? It indeed doesn't apply cleanly there...

Well, it is a bug fix isn't it? Or is this more like a behavioral feature?

#4 lore raw
Quoted 9 lines · David Miller

From: Maciej Żenczykowski <[email protected]>
Date: Fri, 7 Dec 2018 16:46:36 -0800

Quoted 6 lines
Quoted 5 lines

This doesn't apply to the current net tree.

Also "net-udp: " is a weird subsystem prefix, just use "udp: ".

Thank you.

Quoted 8 lines
Quoted 5 lines

Interesting... this patch was on top of net-next/master, and it still rebases cleanly on current net-next/master.

Would you like it on net/master instead? It indeed doesn't apply cleanly there...

Well, it is a bug fix isn't it? Or is this more like a behavioral feature?

If this goes in can it target net-next first? Some soak time there will help show if there are any side effects before propagating to stable releases.

maintainer
#5 lore raw

From: David Ahern <[email protected]>
Date: Sun, 9 Dec 2018 20:29:04 -0700

Quoted 3 lines

If this goes in can it target net-next first? Some soak time there will help show if there are any side effects before propagating to stable releases.

Ok.

Maciej please resubmit this specifically targetting net-next.

Thank you.