New feauture: Add --stale-routes-check

This patch adds a stale-routes-check option that takes 2 parameters: a ageing
time (in seconds) and a check interval (in seconds). The latter defaults to the
former if it's not present.  Internally, a new "check" is added in
multi_process_per_second_timers_dowork(). This check deletes stale routes and
it is inspired to the function multi_reap_range().

We're running a very large connectivity infrastructure based on openVPN (more
than 4000 different clients connected per day per server), so we can throughly
check this patch (or, of course, any variant of it).

Signed-off-by: Davide Guerri <d.guerri@caspur.it>
Reviewed-by: David Sommerseth <davids@redhat.com>
Acked-by: Adriaan de Jong <dejong@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Signed-off-by: David Sommerseth <davids@redhat.com>

5 files changed, 97 insertions, 0 deletions

diff --git a/multi.c b/multi.c
index 1a2f86a..5e5b3df 100644
--- a/multi.c
+++ b/multi.c
@@ -368,6 +368,14 @@ multi_init (struct multi_context *m, struct context *t, bool tcp_mode, int threa
    * tun/tap interface and network stack?
    */
   m->enable_c2c = t->options.enable_c2c;
+
+  /* initialize stale routes check timer */
+  if (t->options.stale_routes_check_interval > 0)
+    {
+      msg (M_INFO, "Initializing stale route check timer to run every %i seconds and to removing routes with activity timeout older than %i seconds",
+        t->options.stale_routes_check_interval, t->options.stale_routes_ageing_time);
+      event_timeout_init (&m->stale_routes_check_et, t->options.stale_routes_check_interval, 0);
+    }
 }
 
 const char *
@@ -1197,6 +1205,32 @@ multi_delete_dup (struct multi_context *m, struct multi_instance *new_mi)
     }
 }
 
+static void
+check_stale_routes (struct multi_context *m)
+{
+
+  struct gc_arena gc = gc_new ();
+  struct hash_iterator hi;
+  struct hash_element *he;
+
+  dmsg (D_MULTI_DEBUG, "MULTI: Checking stale routes");
+  hash_iterator_init_range (m->vhash, &hi, 0, hash_n_buckets (m->vhash));
+  while ((he = hash_iterator_next (&hi)) != NULL)
+    {
+      struct multi_route *r = (struct multi_route *) he->value;
+      if (multi_route_defined (m, r) && difftime(now, r->last_reference) >= m->top.options.stale_routes_ageing_time)
+        {
+          dmsg (D_MULTI_DEBUG, "MULTI: Deleting stale route for address '%s'",
+               mroute_addr_print (&r->addr, &gc));
+          learn_address_script (m, NULL, "delete", &r->addr);
+          multi_route_del (r);
+          hash_iterator_delete_element (&hi);
+        }
+    }
+  hash_iterator_free (&hi);
+  gc_free (&gc);
+}
+
 /*
  * Ensure that endpoint to be pushed to client
  * complies with --ifconfig-push-constraint directive.
@@ -2469,6 +2503,14 @@ gremlin_flood_clients (struct multi_context *m)
 }
 #endif
 
+bool
+stale_route_check_trigger (struct multi_context *m)
+{
+  struct timeval null;
+  CLEAR (null);
+  return event_timeout_trigger (&m->stale_routes_check_et, &null, ETT_DEFAULT);
+}
+
 /*
  * Process timers in the top-level context
  */
@@ -2491,6 +2533,10 @@ multi_process_per_second_timers_dowork (struct multi_context *m)
 #ifdef ENABLE_DEBUG
   gremlin_flood_clients (m);
 #endif
+
+  /* Should we check for stale routes? */
+  if (m->top.options.stale_routes_check_interval && stale_route_check_trigger (m))
+    check_stale_routes (m);
 }
 
 void
diff --git a/multi.h b/multi.h
index 58e65a3..2bc0c8a 100644
--- a/multi.h
+++ b/multi.h
@@ -161,6 +161,11 @@ struct multi_context {
 
   struct context top;           /**< Storage structure for process-wide
                                  *   configuration. */
+
+  /*
+   * Timer object for stale route check
+   */
+  struct event_timeout stale_routes_check_et;
 };
 
 /*
diff --git a/openvpn.8 b/openvpn.8
index 55a9b80..c585da4 100644
--- a/openvpn.8
+++ b/openvpn.8
@@ -3118,6 +3118,25 @@ directive affects OpenVPN's internal routing table, not the
 kernel routing table.
 .\"*********************************************************
 .TP
+.B \-\-stale-routes-check n [t]
+Remove routes haven't had activity for
+.B n
+seconds (i.e. the ageing time).
+
+This check is ran every
+.B t
+seconds (i.e. check interval).
+
+If
+.B t
+is not present it defaults to
+.B n
+
+This option helps to keep the dynamic routing table small.
+See also
+.B \-\-max-routes-per-client
+.\"*********************************************************
+.TP
 .B \-\-connect-freq n sec
 Allow a maximum of
 .B n
diff --git a/options.c b/options.c
index d410782..68255a5 100644
--- a/options.c
+++ b/options.c
@@ -455,6 +455,9 @@ static const char usage_message[] =
   "--connect-freq n s : Allow a maximum of n new connections per s seconds.\n"
   "--max-clients n : Allow a maximum of n simultaneously connected clients.\n"
   "--max-routes-per-client n : Allow a maximum of n internal routes per client.\n"
+  "--stale-routes-check n [t] : Remove routes with a last activity timestamp\n"
+  "                             older than n seconds. Run this check every t\n"
+  "                             seconds (defaults to n).\n"
 #if PORT_SHARE
   "--port-share host port [dir] : When run in TCP mode, proxy incoming HTTPS\n"
   "                  sessions to a web server at host:port.  dir specifies an\n"
@@ -781,6 +784,7 @@ init_options (struct options *o, const bool init_gc)
   o->tcp_queue_limit = 64;
   o->max_clients = 1024;
   o->max_routes_per_client = 256;
+  o->stale_routes_check_interval = 0;
   o->ifconfig_pool_persist_refresh_freq = 600;
 #endif
 #if P2MP
@@ -2182,6 +2186,8 @@ options_postprocess_verify_ce (const struct options *options, const struct conne
 	msg (M_USAGE, "--port-share requires TCP server mode (--mode server --proto tcp-server)");
 #endif
 
+      if (options->stale_routes_check_interval)
+        msg (M_USAGE, "--stale-routes-check requires --mode server");
     }
 #endif /* P2MP_SERVER */
 
@@ -4944,6 +4950,25 @@ add_option (struct options *options,
 	}
       options->max_routes = max_routes;
     }
+  else if (streq (p[0], "stale-routes-check") && p[1])
+    {
+      int ageing_time, check_interval;
+
+      VERIFY_PERMISSION (OPT_P_GENERAL);
+      ageing_time = atoi (p[1]);
+      if (p[2])
+        check_interval = atoi (p[2]);
+      else
+        check_interval = ageing_time;
+
+      if (ageing_time < 1 || check_interval < 1)
+        {
+        msg (msglevel, "--stale-routes-check aging time and check interval must be >= 1");
+        goto err;
+        }
+      options->stale_routes_ageing_time  = ageing_time;
+      options->stale_routes_check_interval = check_interval;
+    }
   else if (streq (p[0], "route-gateway") && p[1])
     {
       VERIFY_PERMISSION (OPT_P_ROUTE_EXTRAS);
diff --git a/options.h b/options.h
index d792872..902fef4 100644
--- a/options.h
+++ b/options.h
@@ -460,6 +460,8 @@ struct options
   int cf_per;
   int max_clients;
   int max_routes_per_client;
+  int stale_routes_check_interval;
+  int stale_routes_ageing_time;
 
   const char *auth_user_pass_verify_script;
   bool auth_user_pass_verify_script_via_file;