unlock_ovpn() released each queued peer's socket via ovpn_socket_release(), which runs a synchronize_rcu() per call before freeing the socket. Tearing down N peers serializes N grace periods.
Split ovpn_socket_release() around the grace period. The new ovpn_socket_release_prepare() detaches peer->sock and drops the ref, returning the socket once its refcount hits zero. ovpn_socket_release_finish() frees it afterwards. unlock_ovpn() now prepares all sockets, waits once, then finishes them and drops the peer refs. The wait is skipped when nothing needs freeing. ovpn_socket_release() chains both halves to keep its behaviour. Switch synchronize_rcu() to synchronize_net() while at it. Signed-off-by: Marco Baffo <[email protected]> --- drivers/net/ovpn/peer.c | 22 +++++++-- drivers/net/ovpn/socket.c | 96 +++++++++++++++++++++++++++------------ drivers/net/ovpn/socket.h | 4 ++ 3 files changed, 87 insertions(+), 35 deletions(-) diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c index a21d02ac715e..c2110d8435e6 100644 --- a/drivers/net/ovpn/peer.c +++ b/drivers/net/ovpn/peer.c @@ -26,15 +26,27 @@ static void unlock_ovpn(struct ovpn_priv *ovpn, struct llist_head *release_list) __releases(&ovpn->lock) { - struct ovpn_peer *peer, *next; + struct ovpn_peer *peer, *next_peer; + struct ovpn_socket *sock, *next_sock; + LIST_HEAD(sock_release_list); spin_unlock_bh(&ovpn->lock); - llist_for_each_entry_safe(peer, next, release_list->first, - release_entry) { - ovpn_socket_release(peer); - ovpn_peer_put(peer); + llist_for_each_entry(peer, release_list->first, release_entry) { + sock = ovpn_socket_release_prepare(peer); + if (!sock) + continue; + list_add_tail(&sock->release_entry, &sock_release_list); } + + if (!list_empty(&sock_release_list)) + synchronize_net(); + + list_for_each_entry_safe(sock, next_sock, &sock_release_list, release_entry) + ovpn_socket_release_finish(sock); + + llist_for_each_entry_safe(peer, next_peer, release_list->first, release_entry) + ovpn_peer_put(peer); } /** diff --git a/drivers/net/ovpn/socket.c b/drivers/net/ovpn/socket.c index 517caa64a4fe..dd27b79a062e 100644 --- a/drivers/net/ovpn/socket.c +++ b/drivers/net/ovpn/socket.c @@ -45,6 +45,69 @@ static bool ovpn_socket_put(struct ovpn_peer *peer, struct ovpn_socket *sock) return kref_put(&sock->refcount, ovpn_socket_release_kref); } +/** + * ovpn_socket_release_prepare - detach peer from its socket + * @peer: peer whose socket should be detached + * + * Return: the detached socket if its refcount reached zero, NULL otherwise. + * If a socket is returned, the caller must call synchronize_net() before + * invoking ovpn_socket_release_finish(). + */ +struct ovpn_socket *ovpn_socket_release_prepare(struct ovpn_peer *peer) +{ + struct ovpn_socket *sock; + bool released; + + might_sleep(); + + sock = rcu_replace_pointer(peer->sock, NULL, true); + /* release may be invoked after socket was detached */ + if (!sock) + return NULL; + + /* Drop the reference while holding the sock lock to avoid + * concurrent ovpn_socket_new call to mess up with a partially + * detached socket. + * + * Holding the lock ensures that a socket with refcnt 0 is fully + * detached before it can be picked by a concurrent reader. + */ + lock_sock(sock->sk); + released = ovpn_socket_put(peer, sock); + release_sock(sock->sk); + + if (released) + return sock; + + return NULL; +} + +/** + * ovpn_socket_release_finish - complete release of a detached socket + * @sock: socket whose refcount reached zero + * + * The caller must have called synchronize_net() after + * ovpn_socket_release_prepare() returned this socket. + */ +void ovpn_socket_release_finish(struct ovpn_socket *sock) +{ + might_sleep(); + + if (sock->sk->sk_protocol == IPPROTO_UDP) { + netdev_put(sock->ovpn->dev, &sock->dev_tracker); + } else if (sock->sk->sk_protocol == IPPROTO_TCP) { + /* wait for TCP jobs to terminate */ + ovpn_tcp_socket_wait_finish(sock); + ovpn_peer_put(sock->peer); + } + /* drop reference acquired in ovpn_socket_new() */ + sock_put(sock->sk); + /* we can call plain kfree() because we already waited one RCU + * period due to synchronize_net() + */ + kfree(sock); +} + /** * ovpn_socket_release - release resources owned by socket user * @peer: peer whose socket should be released @@ -66,45 +129,18 @@ static bool ovpn_socket_put(struct ovpn_peer *peer, struct ovpn_socket *sock) void ovpn_socket_release(struct ovpn_peer *peer) { struct ovpn_socket *sock; - bool released; might_sleep(); - sock = rcu_replace_pointer(peer->sock, NULL, true); - /* release may be invoked after socket was detached */ + sock = ovpn_socket_release_prepare(peer); if (!sock) return; - /* Drop the reference while holding the sock lock to avoid - * concurrent ovpn_socket_new call to mess up with a partially - * detached socket. - * - * Holding the lock ensures that a socket with refcnt 0 is fully - * detached before it can be picked by a concurrent reader. - */ - lock_sock(sock->sk); - released = ovpn_socket_put(peer, sock); - release_sock(sock->sk); - /* align all readers with sk_user_data being NULL */ - synchronize_rcu(); + synchronize_net(); /* following cleanup should happen with lock released */ - if (released) { - if (sock->sk->sk_protocol == IPPROTO_UDP) { - netdev_put(sock->ovpn->dev, &sock->dev_tracker); - } else if (sock->sk->sk_protocol == IPPROTO_TCP) { - /* wait for TCP jobs to terminate */ - ovpn_tcp_socket_wait_finish(sock); - ovpn_peer_put(sock->peer); - } - /* drop reference acquired in ovpn_socket_new() */ - sock_put(sock->sk); - /* we can call plain kfree() because we already waited one RCU - * period due to synchronize_rcu() - */ - kfree(sock); - } + ovpn_socket_release_finish(sock); } static bool ovpn_socket_hold(struct ovpn_socket *sock) diff --git a/drivers/net/ovpn/socket.h b/drivers/net/ovpn/socket.h index 4afcec71040d..b4c7aa9b6179 100644 --- a/drivers/net/ovpn/socket.h +++ b/drivers/net/ovpn/socket.h @@ -24,6 +24,7 @@ struct ovpn_peer; * @peer: unique peer transmitting over this socket (TCP only) * @sk: the low level sock object * @refcount: amount of contexts currently referencing this object + * @release_entry: entry for the deferred release list * @work: member used to schedule release routine (it may block) * @tcp_tx_work: work for deferring outgoing packet processing (TCP only) */ @@ -38,6 +39,7 @@ struct ovpn_socket { struct sock *sk; struct kref refcount; + struct list_head release_entry; struct work_struct work; struct work_struct tcp_tx_work; }; @@ -45,5 +47,7 @@ struct ovpn_socket { struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer *peer); void ovpn_socket_release(struct ovpn_peer *peer); +struct ovpn_socket *ovpn_socket_release_prepare(struct ovpn_peer *peer); +void ovpn_socket_release_finish(struct ovpn_socket *sock); #endif /* _NET_OVPN_SOCK_H_ */ -- 2.43.0 _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
