Attention is currently required from: cron2, ordex, plaisthos.
Hello cron2, ordex, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1635?usp=email
to look at the new patch set (#7).
Change subject: Multisocket: use event engine rwflags for UDP I/O
......................................................................
Multisocket: use event engine rwflags for UDP I/O
udp_flags does not guarantee correct association with the socket being
processed. Use the rwflags delivered by the event engine along with the
event to ensure proper per-socket I/O handling.
Remove udp_flags entirely.
Replace the previous global-style flag computation with a per-socket
decision model in p2mp_iow_flags(), which derives I/O flags
directly from the current multi_context state and the specific socket
being processed. This ensures that read/write decisions are correctly
bound to the active socket rather than shared or implicit global state.
This change is based on an investigation triggered by a report from
Joshua Rogers using ZeroPath.
Change-Id: I6b303805a3688b6f6363140c76853a58badecd8f
Signed-off-by: Gianmarco De Gregori <[email protected]>
---
M src/openvpn/forward.c
M src/openvpn/forward.h
M src/openvpn/mudp.c
M src/openvpn/mudp.h
M src/openvpn/multi_io.c
M src/openvpn/multi_io.h
6 files changed, 74 insertions(+), 83 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/35/1635/7
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index d24f534..ab76b9e 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -2030,9 +2030,8 @@
check_timeout_random_component(c);
}
-static void
-multi_io_process_flags(struct context *c, struct event_set *es, const unsigned
int flags,
- unsigned int *out_socket, unsigned int *out_tuntap)
+void
+multi_io_process_flags(struct context *c, struct event_set *es, struct
link_socket *sock, const unsigned int flags)
{
unsigned int socket = 0;
unsigned int tuntap = 0;
@@ -2123,50 +2122,19 @@
* (for TCP server sockets this happens in
* socket_set_listen_persistent()).
*/
- for (int i = 0; i < c->c1.link_sockets_num; i++)
- {
- if ((c->options.mode != MODE_SERVER) ||
(proto_is_dgram(c->c2.link_sockets[i]->info.proto)))
- {
- socket_set(c->c2.link_sockets[i], es, socket,
&c->c2.link_sockets[i]->ev_arg, NULL);
- }
- }
-
+ socket_set(sock, es, socket, &sock->ev_arg, NULL);
tun_set(c->c1.tuntap, es, tuntap, (void *)tun_shift, NULL);
-
- if (out_socket)
- {
- *out_socket = socket;
- }
-
- if (out_tuntap)
- {
- *out_tuntap = tuntap;
- }
}
/*
- * Wait for I/O events. Used for UDP sockets in
- * point-to-multipoint mode.
- */
-
-void
-get_io_flags_udp(struct context *c, struct multi_io *multi_io, const unsigned
int flags)
-{
- unsigned int out_socket;
-
- multi_io_process_flags(c, multi_io->es, flags, &out_socket, NULL);
- multi_io->udp_flags = (out_socket << SOCKET_SHIFT);
-}
-
-/*
- * This is the core I/O wait function, used for all I/O waits except
- * for the top-level server sockets.
+ * This is the core I/O wait function, used for all I/O waits.
+ *
+ * Invoked by P2P instances in tunnel_point_to_point() or by P2MP
+ * instances within the per-client context in multi_io.c.
*/
void
io_wait(struct context *c, const unsigned int flags)
{
- unsigned int out_socket;
- unsigned int out_tuntap;
struct event_set_return esr[4];
/* These shifts all depend on EVENT_READ and EVENT_WRITE */
@@ -2185,7 +2153,7 @@
*/
event_reset(c->c2.event_set);
- multi_io_process_flags(c, c->c2.event_set, flags, &out_socket,
&out_tuntap);
+ multi_io_process_flags(c, c->c2.event_set, c->c2.link_sockets[0], flags);
#if defined(TARGET_LINUX) || defined(TARGET_FREEBSD)
if (c->c1.tuntap)
diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h
index 0d3e492..324c0b4 100644
--- a/src/openvpn/forward.h
+++ b/src/openvpn/forward.h
@@ -68,7 +68,22 @@
extern counter_type link_write_bytes_global;
-void get_io_flags_udp(struct context *c, struct multi_io *multi_io, const
unsigned int flags);
+/**
+ * @brief Processes I/O flags to configure socket and TUN/TAP event monitors.
+ *
+ * Evaluates the current I/O state (`flags`) and context mode to determine
+ * which READ or WRITE events to register in the event loop.
+ *
+ * @note This function gets called from client instances from @c io_wait()
+ * and from p2mp (Point-to-multipoint) instances in @c multi_io_wait().
+ *
+ * @param c Pointer to the core context.
+ * @param es The event set where descriptors are registered.
+ * @param sock The link socket to configure.
+ * @param flags Bitmask of active I/O operation requests (e.g., @ref
IOW_TO_LINK,
+ * @ref IOW_READ_TUN, @ref IOW_TO_TUN, @ref IOW_SHAPER).
+ */
+void multi_io_process_flags(struct context *c, struct event_set *es, struct
link_socket *sock, const unsigned int flags);
void io_wait(struct context *c, const unsigned int flags);
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index dded1ee..143a330 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -340,18 +340,17 @@
* Process a UDP socket event.
*/
void
-multi_process_io_udp(struct multi_context *m, struct link_socket *sock)
+multi_process_io_udp(struct multi_context *m, struct link_socket *sock,
unsigned int rwflags)
{
- const unsigned int status = m->multi_io->udp_flags;
const unsigned int mpp_flags = (MPP_PRE_SELECT | MPP_CLOSE_ON_SIGNAL);
/* UDP port ready to accept write */
- if (status & SOCKET_WRITE)
+ if (rwflags & SOCKET_WRITE)
{
multi_process_outgoing_link(m, mpp_flags);
}
/* Incoming data on UDP port */
- else if (status & SOCKET_READ)
+ else if (rwflags & SOCKET_READ)
{
read_incoming_link(&m->top, sock);
if (!IS_SIG(&m->top))
@@ -359,36 +358,4 @@
multi_process_incoming_link(m, NULL, mpp_flags, sock);
}
}
-
- m->multi_io->udp_flags = ES_ERROR;
-}
-
-/*
- * Return the io_wait() flags appropriate for
- * a point-to-multipoint tunnel.
- */
-unsigned int
-p2mp_iow_flags(const struct multi_context *m)
-{
- unsigned int flags = IOW_WAIT_SIGNAL;
- if (m->pending)
- {
- if (TUN_OUT(&m->pending->context))
- {
- flags |= IOW_TO_TUN;
- }
- if (LINK_OUT(&m->pending->context))
- {
- flags |= IOW_TO_LINK;
- }
- }
- else if (mbuf_defined(m->mbuf))
- {
- flags |= IOW_MBUF;
- }
- else
- {
- flags |= IOW_READ;
- }
- return flags;
}
diff --git a/src/openvpn/mudp.h b/src/openvpn/mudp.h
index 005ee10..6567539 100644
--- a/src/openvpn/mudp.h
+++ b/src/openvpn/mudp.h
@@ -30,9 +30,7 @@
struct context;
struct multi_context;
-unsigned int p2mp_iow_flags(const struct multi_context *m);
-
-void multi_process_io_udp(struct multi_context *m, struct link_socket *sock);
+void multi_process_io_udp(struct multi_context *m, struct link_socket *sock,
unsigned int rwflags);
/**************************************************************************/
/**
* Get, and if necessary create, the multi_instance associated with a
diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c
index e6f4e9c..d8cc708 100644
--- a/src/openvpn/multi_io.c
+++ b/src/openvpn/multi_io.c
@@ -154,6 +154,40 @@
}
}
+/*
+ * Return the io flags appropriate for
+ * a point-to-multipoint tunnel.
+ */
+static unsigned int
+p2mp_iow_flags(const struct multi_context *m, struct link_socket *sock)
+{
+ unsigned int flags = IOW_WAIT_SIGNAL;
+
+ if (m->pending)
+ {
+ if (TUN_OUT(&m->pending->context))
+ {
+ flags |= IOW_TO_TUN;
+ }
+
+ /* If the to be processed socket happens to be the same one on the
pending
+ * UDP client instance, we set that socket ready for write */
+ if (LINK_OUT(&m->pending->context) &&
m->pending->context.c2.link_sockets[0] == sock)
+ {
+ flags |= IOW_TO_LINK;
+ }
+ }
+ else if (mbuf_defined(m->mbuf))
+ {
+ flags |= IOW_MBUF;
+ }
+ else
+ {
+ flags |= IOW_READ;
+ }
+ return flags;
+}
+
int
multi_io_wait(struct multi_context *m)
{
@@ -171,7 +205,17 @@
if (has_udp_in_local_list(&m->top.options))
{
- get_io_flags_udp(&m->top, m->multi_io, p2mp_iow_flags(m));
+ for (int i = 0; i < m->top.c1.link_sockets_num; i++)
+ {
+ struct link_socket *sock = m->top.c2.link_sockets[i];
+
+ if ((m->top.options.mode == MODE_SERVER) &&
proto_is_dgram(sock->info.proto))
+ {
+ unsigned int flags = p2mp_iow_flags(m, sock);
+
+ multi_io_process_flags(&m->top, m->multi_io->es, sock, flags);
+ }
+ }
}
tun_set(m->top.c1.tuntap, m->multi_io->es, EVENT_READ, MULTI_IO_TUN,
persistent);
@@ -457,7 +501,7 @@
}
else
{
- multi_process_io_udp(m, ev_arg->u.sock);
+ multi_process_io_udp(m, ev_arg->u.sock, e->rwflags);
mi = m->pending;
}
/* monitor and/or handle events that are
diff --git a/src/openvpn/multi_io.h b/src/openvpn/multi_io.h
index 6b2f59a..d6734bd 100644
--- a/src/openvpn/multi_io.h
+++ b/src/openvpn/multi_io.h
@@ -55,7 +55,6 @@
int n_esr;
int maxevents;
unsigned int tun_rwflags;
- unsigned int udp_flags;
#ifdef ENABLE_MANAGEMENT
unsigned int management_persist_flags;
#endif
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1635?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6b303805a3688b6f6363140c76853a58badecd8f
Gerrit-Change-Number: 1635
Gerrit-PatchSet: 7
Gerrit-Owner: its_Giaan <[email protected]>
Gerrit-Reviewer: cron2 <[email protected]>
Gerrit-Reviewer: ordex <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: cron2 <[email protected]>
Gerrit-Attention: ordex <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
