Hi,

On Fri, Dec 30, 2011 at 08:12:56PM +0100, Gert Doering wrote:
> On Wed, Nov 23, 2011 at 04:56:22PM +1300, Michal Ludvig wrote:
> > I'm using the latest openvpn from GIT on OpenSUSE 11.4 and am 
> > experiencing a problem with IPv6 payload setup. It works but openvpn 
> > seems to be somewhat confused when setting up the v6 route.
[..]
> Bug.  For workarounds, see above :-) and a patch will follow as soon as
> I have cornered the bug (or have understood why this is not happening for
> IPv4).

"lazy coder error".  OpenVPN in p2mp client mode saves certain(!) options 
before pulling options from the server (pre_pull_save() etc. in options.c)
and this only happened for IPv4 - because, when adding IPv6 support, I
just overlooked this code path.

So on reconnection, the IPv4 "route_option_list" was reset, and the
corresponding IPv6 "route_ipv6_option_list" wasn't - and thus, all the
IPv6 routes were added to whatever was pushed by the server previously,
leading to duplicate routes.

A patch is attached.  I have tested this here for various cases, and
with the patch, the problem is gone (as expected) and I haven't seen
any adverse cases.

Michal, if you could test that this fixes the problem for you, it would
be great.

David, this patch is pretty much straightforward duplication of the
corresponding IPv4 code path.  It needs helpers in route.c, thus the
patch is somewhat bigger than expected...  but it should be "fairly
easy" to review.

gert
-- 
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             g...@greenie.muc.de
fax: +49-89-35655025                        g...@net.informatik.tu-muenchen.de
From 1a8e83b89a828cfc80a229309e30d8b3b6c59928 Mon Sep 17 00:00:00 2001
From: Gert Doering <g...@mobile.greenie.muc.de>
List-Post: openvpn-devel@lists.sourceforge.net
Date: Fri, 30 Dec 2011 21:08:49 +0100
Subject: [PATCH] Fix build-up of duplicate IPv6 routes on reconnect.

options.c: extend pre_pull_save() and pre_pull_restore() to
           save/restore options->routes_ipv6 as well
options.h: add routes_ipv6 to "struct options_pre_pull"
route.h, route.c: add clone_route_ipv6_option_list() and
           copy_route_ipv6_option_list() helper functions

Signed-off-by: Gert Doering <g...@greenie.muc.de>
---
 options.c |   13 +++++++++++++
 options.h |    3 +++
 route.c   |   19 +++++++++++++++++++
 route.h   |    3 +++
 4 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/options.c b/options.c
index 736495e..a186276 100644
--- a/options.c
+++ b/options.c
@@ -2778,6 +2778,11 @@ pre_pull_save (struct options *o)
          o->pre_pull->routes = clone_route_option_list(o->routes, &o->gc);
          o->pre_pull->routes_defined = true;
        }
+      if (o->routes_ipv6)
+       {
+         o->pre_pull->routes_ipv6 = 
clone_route_ipv6_option_list(o->routes_ipv6, &o->gc);
+         o->pre_pull->routes_ipv6_defined = true;
+       }
 #ifdef ENABLE_CLIENT_NAT
       if (o->client_nat)
        {
@@ -2806,6 +2811,14 @@ pre_pull_restore (struct options *o)
       else
        o->routes = NULL;
 
+      if (pp->routes_ipv6_defined)
+       {
+         rol6_check_alloc (o);
+         copy_route_ipv6_option_list (o->routes_ipv6, pp->routes_ipv6);
+       }
+      else
+       o->routes_ipv6 = NULL;
+
 #ifdef ENABLE_CLIENT_NAT
       if (pp->client_nat_defined)
        {
diff --git a/options.h b/options.h
index 06c4dcb..03fd197 100644
--- a/options.h
+++ b/options.h
@@ -68,6 +68,9 @@ struct options_pre_pull
   bool routes_defined;
   struct route_option_list *routes;
 
+  bool routes_ipv6_defined;
+  struct route_ipv6_option_list *routes_ipv6;
+
 #ifdef ENABLE_CLIENT_NAT
   bool client_nat_defined;
   struct client_nat_option_list *client_nat;
diff --git a/route.c b/route.c
index be23a89..e95292c 100644
--- a/route.c
+++ b/route.c
@@ -108,6 +108,15 @@ clone_route_option_list (const struct route_option_list 
*src, struct gc_arena *a
   return ret;
 }
 
+struct route_ipv6_option_list *
+clone_route_ipv6_option_list (const struct route_ipv6_option_list *src, struct 
gc_arena *a)
+{
+  const size_t rl_size = array_mult_safe (sizeof(struct route_ipv6_option), 
src->capacity, sizeof(struct route_ipv6_option_list));
+  struct route_ipv6_option_list *ret = gc_malloc (rl_size, false, a);
+  memcpy (ret, src, rl_size);
+  return ret;
+}
+
 void
 copy_route_option_list (struct route_option_list *dest, const struct 
route_option_list *src)
 {
@@ -117,6 +126,16 @@ copy_route_option_list (struct route_option_list *dest, 
const struct route_optio
   memcpy (dest, src, src_size);
 }
 
+void
+copy_route_ipv6_option_list (struct route_ipv6_option_list *dest, 
+                            const struct route_ipv6_option_list *src)
+{
+  const size_t src_size = array_mult_safe (sizeof(struct route_ipv6_option), 
src->capacity, sizeof(struct route_ipv6_option_list));
+  if (src->n > dest->capacity)
+    msg (M_FATAL, PACKAGE_NAME " ROUTE: (copy) number of route options in src 
(%d) is greater than route list capacity in dest (%d)", src->n, dest->capacity);
+  memcpy (dest, src, src_size);
+}
+
 struct route_list *
 new_route_list (const int max_routes, struct gc_arena *a)
 {
diff --git a/route.h b/route.h
index 9953478..3e1e84e 100644
--- a/route.h
+++ b/route.h
@@ -211,7 +211,10 @@ struct route_option_list *new_route_option_list (const int 
max_routes, struct gc
 struct route_ipv6_option_list *new_route_ipv6_option_list (const int 
max_routes, struct gc_arena *a);
 
 struct route_option_list *clone_route_option_list (const struct 
route_option_list *src, struct gc_arena *a);
+struct route_ipv6_option_list *clone_route_ipv6_option_list (const struct 
route_ipv6_option_list *src, struct gc_arena *a);
 void copy_route_option_list (struct route_option_list *dest, const struct 
route_option_list *src);
+void copy_route_ipv6_option_list (struct route_ipv6_option_list *dest, 
+                                 const struct route_ipv6_option_list *src);
 
 struct route_list *new_route_list (const int max_routes, struct gc_arena *a);
 struct route_ipv6_list *new_route_ipv6_list (const int max_routes, struct 
gc_arena *a);
-- 
1.7.3.4

Attachment: pgplWWb1NOmv6.pgp
Description: PGP signature

Reply via email to