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/+/1534?usp=email
to review the following change.
Change subject: management: ensure consistent BYTECOUNT timing on server
......................................................................
management: ensure consistent BYTECOUNT timing on server
The BYTECOUNT notification is expected to be emitted every N seconds
when a management client issues the 'bytecount N' command. However, the
server currently relies on timeouts from unrelated periodic operations,
resulting in irregular notification timing.
This issue is especially noticeable with low bytecount intervals and DCO
enabled, as openvpn handles less traffic in userspace, causing the main
loop to run less frequently.
To address this, refactor the timeout logic and pass the timeval
reference to management_check_bytecount_server so that the timeout is
correctly set and notifications adhere to the specified interval.
Change-Id: I65c7225fe48c24a4734fc1b9df64b9580f6f6e5f
Signed-off-by: Ralf Lici <[email protected]>
---
M src/openvpn/manage.c
M src/openvpn/manage.h
M src/openvpn/multi.c
M src/openvpn/multi.h
4 files changed, 33 insertions(+), 18 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/34/1534/1
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 48b87e9..7bed8b8 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -4186,16 +4186,14 @@
}
void
-management_check_bytecount_server(struct multi_context *multi)
+management_check_bytecount_server(struct multi_context *multi, struct timeval
*timeval)
{
if (!(management->persist.callback.flags & MCF_SERVER))
{
return;
}
- struct timeval null;
- CLEAR(null);
- if
(event_timeout_trigger(&management->connection.bytecount_update_interval,
&null, ETT_DEFAULT))
+ if
(event_timeout_trigger(&management->connection.bytecount_update_interval,
timeval, ETT_DEFAULT))
{
/* fetch counters from dco */
if (dco_enabled(&multi->top.options))
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index d3fe84f..96d053a 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -516,7 +516,7 @@
void management_check_bytecount_client(struct context *c, struct management
*man, struct timeval *timeval);
-void management_check_bytecount_server(struct multi_context *multi);
+void management_check_bytecount_server(struct multi_context *multi, struct
timeval *timeval);
void
man_persist_client_stats(struct management *man, struct context *c);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 8654aa5..0032292 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3840,13 +3840,6 @@
{
check_stale_routes(m);
}
-
-#ifdef ENABLE_MANAGEMENT
- if (management)
- {
- management_check_bytecount_server(m);
- }
-#endif /* ENABLE_MANAGEMENT */
}
void
@@ -4199,6 +4192,28 @@
ASSERT(mi->context.c2.tls_multi->peer_id < m->max_clients);
}
+/**
+ * @brief Determines the earliest wakeup interval based on periodic operations.
+ *
+ * Updates the \c timeval to reflect the next scheduled wakeup time.
+ * Also sets \c multi->earliest_wakeup to the instance with the earliest
wakeup.
+ *
+ * @param multi Pointer to the multi context
+ * @param timeval Pointer to the timeval structure to be updated with the
+ * next wakeup time
+ */
+void
+multi_get_timeout(struct multi_context *multi, struct timeval *timeval)
+{
+ multi_get_timeout_instance(multi, timeval);
+
+#ifdef ENABLE_MANAGEMENT
+ if (management)
+ {
+ management_check_bytecount_server(multi, timeval);
+ }
+#endif /* ENABLE_MANAGEMENT */
+}
/*
* Top level event loop.
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 7167639..1f2d882 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -577,6 +577,7 @@
void multi_reap_process_dowork(const struct multi_context *m);
void multi_process_per_second_timers_dowork(struct multi_context *m);
+void multi_get_timeout(struct multi_context *multi, struct timeval *timeval);
static inline void
multi_reap_process(const struct multi_context *m)
@@ -598,15 +599,16 @@
}
/*
- * Compute earliest timeout expiry from the set of
- * all instances. Output:
+ * Updates \c dest with the earliest timeout as a delta relative to the current
+ * time and sets \c m->earliest_wakeup to the \c multi_instance with the
+ * soonest scheduled wakeup.
*
- * m->earliest_wakeup : instance needing the earliest service.
- * dest : earliest timeout as a delta in relation
- * to current time.
+ * @param m Pointer to the multi context
+ * @param dest Pointer to a timeval struct that will hold the earliest timeout
+ * delta.
*/
static inline void
-multi_get_timeout(struct multi_context *m, struct timeval *dest)
+multi_get_timeout_instance(struct multi_context *m, struct timeval *dest)
{
struct timeval tv, current;
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1534?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: release/2.6
Gerrit-Change-Id: I65c7225fe48c24a4734fc1b9df64b9580f6f6e5f
Gerrit-Change-Number: 1534
Gerrit-PatchSet: 1
Gerrit-Owner: ralf_lici <[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
