From: Antonio Quartulli <[email protected]> __is_locked existed only to stop setenv_stats() from issuing a GET_PEER request/reply while the shared socket was still being drained for a batch of notifications, which could fail with NLE_BUSY/NLE_NOMEM or re-enter nl_recvmsgs() on the busy socket. Now that notifications are read on a dedicated socket and GET_PEER goes to the request/reply socket, the two never share an nl_recvmsgs() call: the request/reply socket no longer dispatches notifications, so multi_process_incoming_dco() (the only path that reaches dco_get_peer() during message parsing) never runs while that socket is in flight, and the flag is always false at its check. Remove the flag, its set/clear and the early return.
Change-Id: I46a9ef71359f42395d69cf899c948014fb72f65a Signed-off-by: Antonio Quartulli <[email protected]> Acked-by: Razvan Cojocaru <[email protected]> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1738 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1738 This mail reflects revision 3 of this Change. Acked-by according to Gerrit (reflected above): Razvan Cojocaru <[email protected]> diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index 27f4f99..e583696 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -49,16 +49,6 @@ #include <netlink/genl/family.h> #include <netlink/genl/ctrl.h> -/* When parsing multiple DEL_PEER notifications, openvpn tries to request stats - * for each DEL_PEER message (see setenv_stats). This triggers a GET_PEER - * request-reply while we are still parsing the rest of the initial - * notifications, which can lead to NLE_BUSY or even NLE_NOMEM. - * - * This basic lock ensures we don't bite our own tail by issuing a dco_get_peer - * while still busy receiving and parsing other messages. - */ -static bool __is_locked = false; - /* libnl < 3.5.0 does not set the NLA_F_NESTED on its own, therefore we * have to explicitly do it to prevent the kernel from failing upon * parsing of the message @@ -170,12 +160,16 @@ return ret; } +/** + * Drain the request/reply socket. Used to read command/stats replies. This + * socket is never subscribed to the multicast group, so it cannot deliver an + * asynchronous notification: a reply being parsed here can therefore never + * trigger an instance close or a re-entrant request on the same socket. + */ static int ovpn_nl_recvmsgs(dco_context_t *dco, const char *prefix) { - __is_locked = true; int ret = nl_recvmsgs(dco->nl_sock, dco->nl_cb); - __is_locked = false; return ovpn_nl_recvmsgs_report(ret, prefix); } @@ -1223,12 +1217,6 @@ { ASSERT(dco); - if (__is_locked) - { - msg(D_DCO_DEBUG, "%s: cannot request peer stats while parsing other messages", __func__); - return 0; - } - /* peer_id == -1 means "dump all peers", but this is allowed in MP mode only. * If it happens in P2P mode it means that the DCO peer was deleted and we * can simply bail out _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
