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>

diff --git a/multi.c b/multi.c
index 1a2f86ab2f8800bf887d895199fe632aa69d3733..5e5b3df69ac433ae29176ab04a269129a0a6b6e2 100644 (file)
--- 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 58e65a3884b6139fdd334fb1b39c072024dc4b4a..2bc0c8a1c41014b2f03ed46fbc1787201f72c54a 100644 (file)
--- 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 55a9b80c026c5282a2c1e65db570d6ddcc07ac3d..c585da4aea1bd05315697a10bc8404e08b52f3f6 100644 (file)
--- 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 d410782ace4cc29e817426e4b07a4a76de19ad2b..68255a5a14a34cc95f052ca7a7b1e5c54e880711 100644 (file)
--- 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 d79287217f787fe715b4c5807d03e1b110cb7b9b..902fef4b08606402ebcd07b88c619d0d78d286b5 100644 (file)
--- 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;