Info on internal routes aging and MULTI_ROUTE_AGEABLE

Openvpn-devel In review Matteo Latini 4 participants 9 messages Sep 14, 2011 – Oct 9, 2011

Share & tools

Matteo Latini

author

2011-09-14 13:42 UTC #0 lore raw

Hi,
we (openwisp.it) are an open source wireless ISP using openvpn's tap devices to bridge access points with our main servers.

We recently ran into a problem with openvpn's internal routing table getting filled up by "old and stale" routes. Some routes are kept for months (till the vpn gets restarted). This and the --max-routes-per-client option makes the internal routing table fill up and never let newer clients add new internal routes.

Since we were unable to find a way to clean stale routes by reading the manpages, we began exploring openvpn's code to create a patch that would clean old and inactive routes. The patch is working but, by looking through code, we found some reference to MULTI_ROUTE_AGEABLE and ageable_ttl_secs in the multi_route_defined method (multi.h).

Are those in any way meant to clean, in any way (maybe yet to implement), aged routes? We tried using multi_route_defined to keep track of such routes without success.

Should we keep trying with MULTI_ROUTE_AGEABLE or find other ways (we implemented the patch by using a dedicated timer that triggers a function that does something similar to what happens on teardown)?

We are currently patching OpenVPN 2.1.0

Regards,
Matteo Latini

Davide Guerri

2011-09-14 14:58 UTC #1 lore raw

Hi all,
please find attached the patch Matteo is referring to.

Since we're not expert of the openvpn source code, we think it should be considered as a starting point for a discussion.

Basically 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).

Regards,
Matteo and Davide.

multi.c +46 -0

diff -ruN openvpn-2.1.0-orig/multi.c openvpn-2.1.0/multi.c
@@ -375,6 +375,14 @@
    * 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 *

@@ -1159,6 +1167,32 @@
     }
 }
 
+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, true, 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.

@@ -2349,6 +2383,14 @@
 }
 #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
  */

@@ -2371,6 +2413,10 @@
 #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

multi.h +5 -0

diff -ruN openvpn-2.1.0-orig/multi.h openvpn-2.1.0/multi.h
@@ -129,6 +129,11 @@
   time_t per_second_trigger;
 
   struct context top;
+  
+  /*
+   * Timer object for stale route check
+   */
+  struct event_timeout stale_routes_check_et;
 };
 
 /*

openvpn.8 +20 -1

diff -ruN openvpn-2.1.0-orig/openvpn.8 openvpn-2.1.0/openvpn.8
@@ -1,4 +1,4 @@
-.\"  OpenVPN -- An application to securely tunnel IP networks
+f.\"  OpenVPN -- An application to securely tunnel IP networks
 .\"             over a single TCP/UDP port, with support for SSL/TLS-based
 .\"             session authentication and key exchange,
 .\"             packet encryption, packet authentication, and

@@ -2976,6 +2976,25 @@
 kernel routing table.
 .\"*********************************************************
 .TP
+.B ----stale-routes-check n [t]
+Remove routes older than
+.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

options.c +25 -1

diff -ruN openvpn-2.1.0-orig/options.c openvpn-2.1.0/options.c
@@ -416,6 +416,9 @@
   "--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 : When run in TCP mode, proxy incoming HTTPS sessions\n"
   "                  to a web server at host:port.\n"

@@ -725,6 +728,7 @@
   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

@@ -1788,7 +1792,8 @@
       if (options->port_share_host || options->port_share_port)
 	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 */

@@ -4373,6 +4378,25 @@
 	}
       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 ageing 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);

options.h +2 -0

diff -ruN openvpn-2.1.0-orig/options.h openvpn-2.1.0/options.h
@@ -383,6 +383,8 @@
   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;

--
dott. Davide Guerri - C.A.S.P.U.R. Interuniversity Consortium
Lead R&D Eng. OpenWISP (http://openwisp.caspur.it)
CTO CASPUR per Provinciawifi, Pratowifi, FreeWiFiGenova, Maremmawifi, ProvinciaGoriziaWiFi, IX-WiFi

e-mail: [email protected]...
phone: +39 06.44486.404
www: http://openwisp.caspur.it/projects/ow/wiki/Davide_Guerri

Quoted 23 lines · Matteo Latini

Hi,
we (openwisp.it) are an open source wireless ISP using openvpn's tap devices to bridge access points with our main servers.
We recently ran into a problem with openvpn's internal routing table getting filled up by "old and stale" routes. Some routes are kept for months (till the vpn gets restarted). This and the --max-routes-per-client option makes the internal routing table fill up and never let newer clients add new internal routes.
Since we were unable to find a way to clean stale routes by reading the manpages, we began exploring openvpn's code to create a patch that would clean old and inactive routes. The patch is working but, by looking through code, we found some reference to MULTI_ROUTE_AGEABLE and ageable_ttl_secs in the multi_route_defined method (multi.h).
Are those in any way meant to clean, in any way (maybe yet to implement), aged routes? We tried using multi_route_defined to keep track of such routes without success.
Should we keep trying with MULTI_ROUTE_AGEABLE or find other ways (we implemented the patch by using a dedicated timer that triggers a function that does something similar to what happens on teardown)?
We are currently patching OpenVPN 2.1.0
Regards,
Matteo Latini

Davide Guerri

2011-09-15 21:42 UTC #2 lore raw

PATCH 1/1 Client's routes ageing timer

Please find attached a patch for a new stale-routes-check (and remove) option.

As requested on IRC #openvpn-devel channel, I've fixed my previously submitted patch against version 2.1.0 (i.e. the version currently used by ubuntu 10.04.3 LTS) and I'm also submitting a patch against the latest master branch.

I've been testing both versions for a while with no issues so far.

multi.c +46 -0

diff -ruN openvpn-2.1.0-orig/multi.c openvpn-2.1.0/multi.c
@@ -375,6 +375,14 @@
    * 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 *

@@ -1159,6 +1167,32 @@
     }
 }
 
+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, true, 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.

@@ -2349,6 +2383,14 @@
 }
 #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
  */

@@ -2371,6 +2413,10 @@
 #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

multi.h +5 -0

diff -ruN openvpn-2.1.0-orig/multi.h openvpn-2.1.0/multi.h
@@ -129,6 +129,11 @@
   time_t per_second_trigger;
 
   struct context top;
+  
+  /*
+   * Timer object for stale route check
+   */
+  struct event_timeout stale_routes_check_et;
 };
 
 /*

openvpn.8 +19 -0

diff -ruN openvpn-2.1.0-orig/openvpn.8 openvpn-2.1.0/openvpn.8
@@ -2976,6 +2976,25 @@
 kernel routing table.
 .\"*********************************************************
 .TP
+.B \-\-stale-routes-check n [t]
+Remove routes older than
+.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

options.c +6 -0

diff -ruN openvpn-2.1.0-orig/options.c openvpn-2.1.0/options.c
@@ -416,6 +416,9 @@
   "--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 : When run in TCP mode, proxy incoming HTTPS sessions\n"
   "                  to a web server at host:port.\n"

@@ -725,6 +728,7 @@
   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

@@ -1788,7 +1792,8 @@
       if (options->port_share_host || options->port_share_port)
 	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 */

@@ -4373,6 +4378,25 @@
}

       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);

options.h +2 -0

diff -ruN openvpn-2.1.0-orig/options.h openvpn-2.1.0/options.h
@@ -383,6 +383,8 @@
   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;

multi.c +46 -0

70 lines not on this page · raw message

multi.h +5 -0

index 58e65a3..2bc0c8a 100644
@@ -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;
 };
 
 /*

openvpn.8 +19 -0

index 55a9b80..c585da4 100644
@@ -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

options.c +25 -0

index d410782..68255a5 100644
@@ -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);

options.h +2 -0

index d792872..902fef4 100644
@@ -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;

Signed-off-by Davide Guerri <[email protected]...>

Quoted 38 lines · Davide Guerri

Hi all,
please find attached the patch Matteo is referring to.
Since we're not expert of the openvpn source code, we think it should be considered as a starting point for a discussion.
Basically 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).
Regards,
Matteo and Davide.
Quoted 23 lines · Matteo Latini
Hi,
we (openwisp.it) are an open source wireless ISP using openvpn's tap devices to bridge access points with our main servers.
We recently ran into a problem with openvpn's internal routing table getting filled up by "old and stale" routes. Some routes are kept for months (till the vpn gets restarted). This and the --max-routes-per-client option makes the internal routing table fill up and never let newer clients add new internal routes.
Since we were unable to find a way to clean stale routes by reading the manpages, we began exploring openvpn's code to create a patch that would clean old and inactive routes. The patch is working but, by looking through code, we found some reference to MULTI_ROUTE_AGEABLE and ageable_ttl_secs in the multi_route_defined method (multi.h).
Are those in any way meant to clean, in any way (maybe yet to implement), aged routes? We tried using multi_route_defined to keep track of such routes without success.
Should we keep trying with MULTI_ROUTE_AGEABLE or find other ways (we implemented the patch by using a dedicated timer that triggers a function that does something similar to what happens on teardown)?
We are currently patching OpenVPN 2.1.0
Regards,
Matteo Latini

David Sommerseth

2011-09-30 09:12 UTC #3 lore raw

Re: [Openvpn-devel] [PATCH] Client's routes ageing timer

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 10 lines · Davide Guerri

Please find attached a patch for a new stale-routes-check (and remove) option.
As requested on IRC #openvpn-devel channel, I've fixed my previously submitted patch against version 2.1.0 (i.e. the version currently used by ubuntu 10.04.3 LTS) and I'm also submitting a patch against the latest master branch.
I've been testing both versions for a while with no issues so far.

Applied to the master branch in testing and stable trees. Next time, could you please
commit this to your local git tree (in a separate branch, if you like) and then use
'git format-patch' when sending patches? That makes the inclusion easier, as I don't
have to start writing commit messages and tweaking with git other :)

I've added this commit message to this patch:

commit 3a957aaef3ae512b217dd475a846a0ea35aae49c

Author: Davide Guerri <[email protected]...>
Date:   Thu Sep 15 23:42:22 2011 +0200

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 <[email protected]...>
    Reviewed-by: David Sommerseth <[email protected]...>
    Acked-by: Adriaan de Jong <[email protected]...>
    Acked-by: Gert Doering <[email protected]...>
    Signed-off-by: David Sommerseth <[email protected]...>

(I set myself as Reviewed-by this time, because I struggled to remember the details - but remember having reviewed it and found it generally useful and good. Anyway Gert and Adriaan was clear in their ACKs.)

kind regards,

David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk6Fh+kACgkQDC186MBRfrrwdQCfXFu8hh7TdRwLcOVCUNr+PQuO zkAAn0L58KNlcnHU9JhgT2lPLkd8P+Qp
=MtGN
-----END PGP SIGNATURE-----

Gert Doering

2011-10-08 10:31 UTC #4 lore raw

Re: [Openvpn-devel] [PATCH] Client's routes ageing timer

Hi,

Quoted 5 lines · David Sommerseth

Quoted 2 lines · Davide Guerri
Please find attached a patch for a new stale-routes-check (and remove) option.
Applied to the master branch in testing and stable trees.

The way this is currently implemented breaks compilation with --disable-crypto --disable-ssl (glad we have buildbot to test this :) ), because in that case, P2MP_SERVER is not enabled, and options->stale_routes_ageing_time and options->stale_routes_check_interval are not defined.

The patch below moves the block inside the #ifdef P2MP_SERVER block inside options.c - the code is not changed, just moved elsewhere.

Smoke-tested on FreeBSD 8.2 (just compilation, not even started).

Please verify, ACK, and commit :-)

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

From 6e7fdb22e361f6c3d0c617b2d633be4dc3e8607a Mon Sep 17 00:00:00 2001
From: Gert Doering <[email protected]...>
Date: Sat, 8 Oct 2011 12:26:52 +0200
Subject: [PATCH] move block for "stale-routes-check" config inside #ifdef
P2MP_SERVER block - options->stale_routes_ageing_time etc. are not defined otherwise, and compilation fails.

Signed-off-by Gert Doering <[email protected]...>

---
 options.c |   38 +++++++++++++++++++-------------------
 1 files changed, 19 insertions(+), 19 deletions(-)

options.c +19 ~~-19~~

index 68255a5..c1a0ba6 100644
@@ -4950,25 +4950,6 @@ 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);

@@ -5598,6 +5579,25 @@ add_option (struct options *options,
       VERIFY_PERMISSION (OPT_P_GENERAL);
       options->server_flags |= SF_TCP_NODELAY_HELPER;
     }
+  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;
+    }
 #endif /* P2MP_SERVER */
 
   else if (streq (p[0], "client"))

Signature · 1 line

1.7.7

Davide Guerri

2011-10-08 15:50 UTC #5 lore raw

Re: [Openvpn-devel] [PATCH] Client's routes ageing timer

Hi,
sorry for that.

I've just double checked the patch and there is no other code outside an #ifdef P2MP_SERVER

Davide.

Quoted 29 lines · Gert Doering

Hi,
Quoted 5 lines · David Sommerseth
Quoted 2 lines · Davide Guerri
Please find attached a patch for a new stale-routes-check (and remove) option.
Applied to the master branch in testing and stable trees.
The way this is currently implemented breaks compilation with --disable-crypto --disable-ssl (glad we have buildbot to test this :) ), because in that case, P2MP_SERVER is not enabled, and options->stale_routes_ageing_time and options->stale_routes_check_interval are not defined.
The patch below moves the block inside the #ifdef P2MP_SERVER block inside options.c - the code is not changed, just moved elsewhere.
Smoke-tested on FreeBSD 8.2 (just compilation, not even started).
Please verify, ACK, and commit :-)
gert
Signature · 5 lines
USENET is *not* the non-clickable part of WWW!
                                                          //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
<0001-move-block-for-stale-routes-check-config-inside-ifde.patch>

Gert Doering

2011-10-08 20:57 UTC #6 lore raw

Re: [Openvpn-devel] [PATCH] Client's routes ageing timer

Hi,

Quoted 3 lines · Davide Guerri

sorry for that.
I've just double checked the patch and there is no other code outside an #ifdef P2MP_SERVER

Could you test and formally ACK my patch to your patch? :-)

Thanks,

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Davide Guerri

2011-10-09 08:31 UTC #7 lore raw

Re: [Openvpn-devel] [PATCH] Client's routes ageing timer

Hi,
compiled _with default flags_ (i.e. autoreconf -v -i; ./configure; make) and tested w/ and w/o stale-routes-check option on Ubuntu 10.10 x86_64 and on Ubuntu 8.04.4 LTS x86_64.

So, I formally ACK your patched patch :)

Quoted 5 lines · Gert Doering

Hi,
Quoted 2 lines · Davide Guerri
sorry for that.

Quoted 13 lines

Quoted 1 line
I've just double checked the patch and there is no other code outside an #ifdef P2MP_SERVER
Could you test and formally ACK my patch to your patch? :-)
Thanks,
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                          //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

--
dott. Davide Guerri - C.A.S.P.U.R. Interuniversity Consortium Lead R&D Eng. OpenWISP (http://openwisp.caspur.it)
CTO CASPUR per Provinciawifi, Pratowifi, FreeWiFiGenova, Maremmawifi, ProvinciaGoriziaWiFi, IX-WiFi

e-mail: [email protected]...
phone: +39 06.44486.404
www: http://openwisp.caspur.it/projects/ow/wiki/Davide_Guerri

David Sommerseth

2011-10-09 11:36 UTC #8 lore raw

Re: [Openvpn-devel] [PATCH] Client's routes ageing timer

In reply to Gert Doering

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/10/11 12:31, Gert Doering wrote:

| Hi,
|
| On Fri, Sep 30, 2011 at 11:12:12AM +0200, David Sommerseth wrote:
|> On 15/09/11 23:42, Davide Guerri wrote:
|>> Please find attached a patch for a new stale-routes-check (and remove)
|>> option.
|>
|> Applied to the master branch in testing and stable trees.
|
| The way this is currently implemented breaks compilation with
| --disable-crypto --disable-ssl (glad we have buildbot to test this :) ),
| because in that case, P2MP_SERVER is not enabled, and
| options->stale_routes_ageing_time and options->stale_routes_check_interval
| are not defined.
|
| The patch below moves the block inside the #ifdef P2MP_SERVER block
| inside options.c - the code is not changed, just moved elsewhere.
|
| Smoke-tested on FreeBSD 8.2 (just compilation, not even started).
|
| Please verify, ACK, and commit :-)

Applied to master branches in testing and stable.
Commit 32ab329bc69c6292c205d4f33a4b8069341798d3

~    Signed-off-by: Gert Doering <[email protected]...>
~    Acked-by: Davide Guerri <[email protected]...>
~    Signed-off-by: David Sommerseth <[email protected]...>

kind regards,

David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk6RhyYACgkQDC186MBRfrpUhQCglzfsZwAJpPLgGP8pC4T9cFev /8oAoKrVgg7IjXNqhACSHp9qJciBB6qS
=R7tm
-----END PGP SIGNATURE-----