Re: [PATCH net-next v18 20/25] ovpn: implement peer add/get/dump/delete via netlink
From: Antonio Quartulli <antonio@openvpn.net>
Date: 2025-01-20 14:51:17
Also in:
linux-kselftest, lkml
On 17/01/2025 12:48, Sabrina Dubroca wrote: [...]
quoted hunk ↗ jump to hunk
-------- 8< --------diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c index 72357bb5f30b..19aa4ee6d468 100644 --- a/drivers/net/ovpn/netlink.c +++ b/drivers/net/ovpn/netlink.c@@ -733,6 +733,9 @@ int ovpn_nl_peer_del_doit(struct sk_buff *skb, struct genl_info *info) netdev_dbg(ovpn->dev, "del peer %u\n", peer->id); ret = ovpn_peer_del(peer, OVPN_DEL_PEER_REASON_USERSPACE); + if (ret >= 0 && peer->sock) + wait_for_completion(&peer->sock_detach); + ovpn_peer_put(peer); return ret;diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c index b032390047fe..6120521d0c32 100644 --- a/drivers/net/ovpn/peer.c +++ b/drivers/net/ovpn/peer.c@@ -92,6 +92,7 @@ struct ovpn_peer *ovpn_peer_new(struct ovpn_priv *ovpn, u32 id) ovpn_peer_stats_init(&peer->vpn_stats); ovpn_peer_stats_init(&peer->link_stats); INIT_WORK(&peer->keepalive_work, ovpn_peer_keepalive_send); + init_completion(&peer->sock_detach); ret = dst_cache_init(&peer->dst_cache, GFP_KERNEL); if (ret < 0) {diff --git a/drivers/net/ovpn/peer.h b/drivers/net/ovpn/peer.h index 7a062cc5a5a4..8c54bf5709ef 100644 --- a/drivers/net/ovpn/peer.h +++ b/drivers/net/ovpn/peer.h@@ -112,6 +112,7 @@ struct ovpn_peer { struct rcu_head rcu; struct work_struct remove_work; struct work_struct keepalive_work; + struct completion sock_detach; }; /**diff --git a/drivers/net/ovpn/socket.c b/drivers/net/ovpn/socket.c index a5c3bc834a35..7cefac42c3be 100644 --- a/drivers/net/ovpn/socket.c +++ b/drivers/net/ovpn/socket.c@@ -31,6 +31,8 @@ static void ovpn_socket_release_kref(struct kref *kref) sockfd_put(sock->sock); kfree_rcu(sock, rcu); + + complete(&sock->peer->sock_detach); } /**@@ -181,12 +183,12 @@ struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer *peer) ovpn_sock->sock = sock; kref_init(&ovpn_sock->refcount); + ovpn_sock->peer = peer;
Unfortunately this won't work for UDP, because we are taking a reference only to the *first* peer using this specific socket (= the peer allocating the ovpn_socket object). All other peers won't get to this line. Instead, we should somewhat only take the reference to the "last" peer using this socket: the peer which will truly ignite the socket detachment. That's the peer that has to wait upon peer_del for full socket detachment. Trying to get something smart out of my brains right now ... Cheers,
quoted hunk ↗ jump to hunk
/* TCP sockets are per-peer, therefore they are linked to their unique * peer */ if (sock->sk->sk_protocol == IPPROTO_TCP) { - ovpn_sock->peer = peer; ovpn_peer_hold(peer); } else if (sock->sk->sk_protocol == IPPROTO_UDP) { /* in UDP we only link the ovpn instance since the socket isdiff --git a/drivers/net/ovpn/socket.h b/drivers/net/ovpn/socket.h index 15827e347f53..3f5a35fd9048 100644 --- a/drivers/net/ovpn/socket.h +++ b/drivers/net/ovpn/socket.h@@ -28,12 +28,12 @@ struct ovpn_peer; * @rcu: member used to schedule RCU destructor callback */ struct ovpn_socket { + struct ovpn_peer *peer; union { struct { struct ovpn_priv *ovpn; netdevice_tracker dev_tracker; }; - struct ovpn_peer *peer; }; struct socket *sock;
-- Antonio Quartulli OpenVPN Inc.