Re: [PATCH net-next v20 15/25] ovpn: implement multi-peer support
flat view
From: Sabrina Dubroca <sd@queasysnail.net>
Date: 2025-03-03 15:38:38
Also in:
linux-kselftest, lkml
2025-03-03, 15:45:23 +0100, Antonio Quartulli wrote:
On 03/03/2025 14:08, Sabrina Dubroca wrote:quoted
quoted
+ if (ovpn_sock && ovpn_sock->sock->sk == sk) + skip = false; + rcu_read_unlock(); + + if (skip) + continue;The skip/continue logic looks a tiny bit strange to me, maybe this:Hehe, it's like a double negation. I agree it can be improved.quoted
hash_for_each_safe(ovpn->peers->by_id, bkt, tmp, peer, hash_entry_id) { bool remove = true;does the netdev coding style allow to use locally scoped variables? Or should I declare everything at the beginning of the function? I had this rule in mind, but it may have been eliminated by now.
Based on a few samples from net/core/dev.c, I'd say it's allowed: https://web.git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/net/core/dev.c?id=357660d7596b#n4634 https://web.git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/net/core/dev.c?id=357660d7596b#n11404 https://web.git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/net/core/dev.c?id=357660d7596b#n12319 https://web.git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/net/core/dev.c?id=357660d7596b#n12531
quoted
/* if a socket was passed as argument, skip all peers except * those using it */ if (sk) { rcu_read_lock(); ovpn_sock = rcu_dereference(peer->sock); remove = ovpn_sock && ovpn_sock->sock->sk == sk; rcu_read_unlock(); } if (remove) ovpn_peer_remove(peer, reason, &release_list); } (only if you agree it looks better - if it's my opinion against yours, ignore me since it's really just coding style/taste)Yours look simpler/cleaner. I'll go with it.
ok :) -- Sabrina