Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1738?usp=email
to review the following change.
Change subject: dco_linux: drop the now-redundant __is_locked re-entrancy guard
......................................................................
dco_linux: drop the now-redundant __is_locked re-entrancy guard
__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]>
---
M src/openvpn/dco_linux.c
1 file changed, 4 insertions(+), 18 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/38/1738/1
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 7a0f620..199a11b 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,14 @@
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);
}
@@ -1222,12 +1214,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
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1738?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I46a9ef71359f42395d69cf899c948014fb72f65a
Gerrit-Change-Number: 1738
Gerrit-PatchSet: 1
Gerrit-Owner: ordex <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
