diff mbox

[net-next] netlink: Use random autobind rover

Message ID 20150517024534.GA5570@gondor.apana.org.au
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Herbert Xu May 17, 2015, 2:45 a.m. UTC
On Sat, May 16, 2015 at 05:08:19PM -0400, David Miller wrote:
>
> As far as I can tell, this ought to be fine as-is.

OK what about dropping the global rover altogether? Let's just
start randomly like UDP.

---8<---
Currently we use a global rover to select a port ID that is unique.
This used to work consistently when it was protected with a global
lock.  However as we're now lockless, the global rover can exhibit
pathological behaviour should multiple threads all stomp on it at
the same time.

Granted this will eventually resolve itself but the process is
suboptimal.

This patch replaces the global rover with a pseudorandom starting
point to avoid this issue.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Comments

David Miller May 18, 2015, 3:44 a.m. UTC | #1
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sun, 17 May 2015 10:45:34 +0800

> On Sat, May 16, 2015 at 05:08:19PM -0400, David Miller wrote:
>>
>> As far as I can tell, this ought to be fine as-is.
> 
> OK what about dropping the global rover altogether? Let's just
> start randomly like UDP.
> 
> ---8<---
> Currently we use a global rover to select a port ID that is unique.
> This used to work consistently when it was protected with a global
> lock.  However as we're now lockless, the global rover can exhibit
> pathological behaviour should multiple threads all stomp on it at
> the same time.
> 
> Granted this will eventually resolve itself but the process is
> suboptimal.
> 
> This patch replaces the global rover with a pseudorandom starting
> point to avoid this issue.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Yep, this is fine, applied.

Thanks!
--
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
diff mbox

Patch

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index ec4adbd..716c6d2 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1296,20 +1296,24 @@  static int netlink_autobind(struct socket *sock)
 	struct netlink_table *table = &nl_table[sk->sk_protocol];
 	s32 portid = task_tgid_vnr(current);
 	int err;
-	static s32 rover = -4097;
+	s32 rover = -4096;
+	bool ok;
 
 retry:
 	cond_resched();
 	rcu_read_lock();
-	if (__netlink_lookup(table, portid, net)) {
+	ok = !__netlink_lookup(table, portid, net);
+	rcu_read_unlock();
+	if (!ok) {
 		/* Bind collision, search negative portid values. */
-		portid = rover--;
-		if (rover > -4097)
+		if (rover == -4096)
+			/* rover will be in range [S32_MIN, -4097] */
+			rover = S32_MIN + prandom_u32_max(-4096 - S32_MIN);
+		else if (rover >= -4096)
 			rover = -4097;
-		rcu_read_unlock();
+		portid = rover--;
 		goto retry;
 	}
-	rcu_read_unlock();
 
 	err = netlink_insert(sk, portid);
 	if (err == -EADDRINUSE)