[Openvpn-devel,v5] management: resync timer on bytecount interval change

Message ID 20250902160050.18640-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v5] management: resync timer on bytecount interval change | expand

Commit Message

Gert Doering Sept. 2, 2025, 4 p.m. UTC
From: Ralf Lici <ralf@mandelbit.com>

coarse_timer_wakeup tracks when the next timer-driven task will occur.
If a user issues `bytecount n` via the management interface, but the
next scheduled wakeup is more than n seconds away, bandwidth logging
will be delayed until that timer fires.

To ensure timely logging, reset the timer whenever a new `bytecount`
command is received. This guarantees that logging begins exactly n
seconds after the command, matching the user-defined interval.

Change-Id: Ic0035d52e0ea123398318870d2f4d21af927a602
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
---

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/+/1113
This mail reflects revision 5 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Sept. 2, 2025, 4:12 p.m. UTC | #1
I have tested this on client and server, including lots of extra msg()
printing to verify that "the arg that we think should be a multi* or
context* pointer" indeed is what we expect.

v4 was good to go, but it crashes the server if --management-hold is
active and "bytecount 1" is issued (tun not open yet, so parts of the
management infra are not initalized yet - in particular *.callback.arg)

So, v5 is the same as v3, just having an if() clause there.  ASSERT()
for "can't happen" will tell you what you overlooked ;-)

As for the commit before that, mail-archive.org refuses to acknowledge
existence of the "patch on the list!" mail, so sourceforge.net is 
linked.

Your patch has been applied to the master branch.

commit 3bc0b2d0aea742640a1acf97fc4b41726b88ce96
Author: Ralf Lici
Date:   Tue Sep 2 18:00:44 2025 +0200

     management: resync timer on bytecount interval change

     Signed-off-by: Ralf Lici <ralf@mandelbit.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20250902160050.18640-1-gert@greenie.muc.de>
     URL: https://sourceforge.net/p/openvpn/mailman/message/59228306/
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index aed04f5..53a3e4e 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -41,6 +41,7 @@ 
 #include "manage.h"
 #include "openvpn.h"
 #include "dco.h"
+#include "multi.h"
 
 #include "memdbg.h"
 
@@ -513,6 +514,28 @@ 
         man->connection.bytecount_update_seconds = 0;
         event_timeout_clear(&man->connection.bytecount_update_interval);
     }
+
+    /* The newly received bytecount interval may be sooner than the existing
+     * coarse timer wakeup. Reset the timer to ensure it fires at the correct,
+     * earlier time.
+     */
+    if (man->persist.callback.arg)
+    {
+        struct context *c;
+
+        if (man->settings.flags & MF_SERVER)
+        {
+            struct multi_context *m = man->persist.callback.arg;
+            c = &m->top;
+        }
+        else
+        {
+            c = man->persist.callback.arg;
+        }
+
+        reset_coarse_timers(c);
+    }
+
     msg(M_CLIENT, "SUCCESS: bytecount interval changed");
 }