[S] Change in openvpn[master]: multi: don't let stale-routes-check delete permanent routes

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1729?usp=email )

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]>
Acked-by: Arne Schwabe <[email protected]>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1729
Message-Id: <[email protected]>
URL: https://www.mail-archive.com/[email protected]/msg38102.html
Signed-off-by: Gert Doering <[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(-)




diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst
index f420588..9dfa6bd 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -673,6 +673,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 74939bf..d8dd99b 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1190,7 +1190,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)
     {
@@ -1235,7 +1235,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)
     {
@@ -1359,7 +1359,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 8b837fa..6cb86c7 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -233,8 +233,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: merged
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I3f9834cc13c49b9249653d7d8637383f50c2fb87
Gerrit-Change-Number: 1729
Gerrit-PatchSet: 2
Gerrit-Owner: ordex <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel