diff mbox series

[Openvpn-devel,v3] dco_linux: drop the now-redundant __is_locked re-entrancy guard

Message ID 20260925131830.10902-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v3] dco_linux: drop the now-redundant __is_locked re-entrancy guard |

Commit Message

Gert Doering Sept. 25, 2026, 1:18 p.m. UTC 
  From: Antonio Quartulli <antonio@mandelbit.com>

__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 <antonio@mandelbit.com>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
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 <razvanc@mailbox.org>
diff mbox series

Patch

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