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/+/1729?usp=email
to review the following change.
Change subject: multi: don't let stale-routes-check delete permanent routes
......................................................................
multi: don't let stale-routes-check delete permanent routes
When --stale-routes-check is enabled the periodic check removed every
aged entry from the virtual address routing table, including the
permanent routes installed from --iroute (e.g. via --client-config-dir)
and the client's pushed ifconfig address. Once those entries were gone,
traffic towards the iroute networks was no longer forwarded and the
affected client suffered an outage until it reconnected.
Routes learned from configuration (iroutes and pushed ifconfig
addresses) and genuinely dynamic routes (TAP source addresses learned
from the data channel) were both added with flags == 0, so
check_stale_routes() could not tell them apart and aged out all of
them. Their last_reference is only refreshed when they are hit as a
packet destination, which never happens for an iroute network (CIDR
lookups create a separate cached child route instead) nor for an idle
client's pushed address, so both were eventually deleted.
Mark the config-derived routes with a new MULTI_ROUTE_PERMANENT flag in
the two learn helpers that install them, and skip permanent routes in
check_stale_routes(). Dynamically learned routes keep flags == 0
and are still aged out, preserving the documented purpose of the
option. Cleanup on disconnect is unaffected: a halted instance makes
multi_route_defined() return false, so the reaper still removes the
permanent routes when the client goes away.
Change-Id: I3f9834cc13c49b9249653d7d8637383f50c2fb87
Github: closes OpenVPN/openvpn#1063
Signed-off-by: Antonio Quartulli <[email protected]>
---
M doc/man-sections/server-options.rst
M src/openvpn/multi.c
M src/openvpn/multi.h
3 files changed, 11 insertions(+), 5 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/29/1729/1
diff --git a/doc/man-sections/server-options.rst
b/doc/man-sections/server-options.rst
index eb8e273..c9d29f6 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -671,6 +671,11 @@
If ``t`` is not present it defaults to ``n``.
+ Only dynamically learned routes are subject to this check. Routes added from
+ configuration, such as ``--iroute`` entries and a client's pushed ifconfig
+ address, are never removed by it; they are dropped only when the client
+ disconnects.
+
This option helps to keep the dynamic routing table small. See also
``--max-routes-per-client``
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index a957fdf..b08ff08 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1191,7 +1191,7 @@
addr.netbits = (uint8_t)netbits;
}
- struct multi_instance *owner = multi_learn_addr(m, mi, &addr, 0);
+ struct multi_instance *owner = multi_learn_addr(m, mi, &addr,
MULTI_ROUTE_PERMANENT);
#ifdef ENABLE_MANAGEMENT
if (management && owner)
{
@@ -1236,7 +1236,7 @@
mroute_addr_mask_host_bits(&addr);
}
- struct multi_instance *owner = multi_learn_addr(m, mi, &addr, 0);
+ struct multi_instance *owner = multi_learn_addr(m, mi, &addr,
MULTI_ROUTE_PERMANENT);
#ifdef ENABLE_MANAGEMENT
if (management && owner)
{
@@ -1360,7 +1360,7 @@
while ((he = hash_iterator_next(&hi)) != NULL)
{
struct multi_route *r = (struct multi_route *)he->value;
- if (multi_route_defined(m, r)
+ if (multi_route_defined(m, r) && !(r->flags & MULTI_ROUTE_PERMANENT)
&& difftime(now, r->last_reference) >=
m->top.options.stale_routes_ageing_time)
{
dmsg(D_MULTI_DEBUG, "MULTI: Deleting stale route for address '%s'",
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 3ed08d4..22cfeee 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -237,8 +237,9 @@
struct mroute_addr addr;
struct multi_instance *instance;
-#define MULTI_ROUTE_CACHE (1 << 0)
-#define MULTI_ROUTE_AGEABLE (1 << 1)
+#define MULTI_ROUTE_CACHE (1 << 0)
+#define MULTI_ROUTE_AGEABLE (1 << 1)
+#define MULTI_ROUTE_PERMANENT (1 << 2) /* config-derived (iroute / pushed
ifconfig); never stale-aged */
unsigned int flags;
unsigned int cache_generation;
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1729?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: I3f9834cc13c49b9249653d7d8637383f50c2fb87
Gerrit-Change-Number: 1729
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
