Attention is currently required from: flichtenheld, plaisthos.
Hello plaisthos, flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1351?usp=email
to review the following change.
Change subject: FreeBSD DCO: repair --inactive
......................................................................
FreeBSD DCO: repair --inactive
--inactive on DCO requires a working DCO counters query function
(dco_get_peer_stats(), implemented in the previous commit) and
that the DCO implementation in use fills the "tun_{read,write}_bytes"
fields for the peer context.
FreeBSD DCO only fills the "dco_{read,write}_bytes" counters - which is
something we can't fix in OpenVPN, this needs kernel enhancements.
So, to make the feature (mostly) work, check the other set of counters
on FreeBSD. Caveat: this will count encryption overhead and keepalives,
so it will still not work for `--inactive <n>` without a byte count, or
for byte counts with too tight thresholds.
Adding the #ifdef to forward.c was considered the least bad alternative.
Github: OpenVPN/openvpn#898
Change-Id: I48c877843d24144450af1282b7524bb3ba18232e
Signed-off-by: Gert Doering <[email protected]>
---
M doc/man-sections/client-options.rst
M src/openvpn/forward.c
2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/51/1351/1
diff --git a/doc/man-sections/client-options.rst
b/doc/man-sections/client-options.rst
index 0aee9e2..4513ff3 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -274,6 +274,11 @@
counted as traffic, as they are used internally by OpenVPN and are not
an indication of actual user activity.
+ NOTE: on FreeBSD with DCO, due to platform limits, the previous paragraph
+ is not correct. In that case, encapsulation overhead and keepalives are
+ counted, so using this feature needs a sufficiently-high `bytes` value to
+ take these extra numbers into account.
+
--proto-force p
When iterating through connection profiles, only consider profiles using
protocol ``p`` (:code:`tcp` \| :code:`udp`).
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 5bbac13..c355f66 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -476,13 +476,20 @@
* and the logic needs to change - we permit the event to trigger and check
* kernel DCO counters here, returning and rearming the timer if there was
* sufficient traffic.
+ *
+ * NOTE: FreeBSD DCO does not supply "tun bytes" (= decrypted payload) today,
+ * so "dco bytes" (encrypted bytes, including keepalives) is used instead
*/
static void
check_inactivity_timeout(struct context *c)
{
if (dco_enabled(&c->options) && dco_get_peer_stats(c, true) == 0)
{
+#ifdef TARGET_FREEBSD
+ int64_t tot_bytes = c->c2.dco_read_bytes + c->c2.dco_write_bytes;
+#else
int64_t tot_bytes = c->c2.tun_read_bytes + c->c2.tun_write_bytes;
+#endif
int64_t new_bytes = tot_bytes - c->c2.inactivity_bytes;
if (new_bytes > c->options.inactivity_minimum_bytes)
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1351?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: I48c877843d24144450af1282b7524bb3ba18232e
Gerrit-Change-Number: 1351
Gerrit-PatchSet: 1
Gerrit-Owner: cron2 <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel