Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos,

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1682?usp=email

to look at the new patch set (#3).


Change subject: multi/dco: simplify dco_delete_iroutes call chain
......................................................................

multi/dco: simplify dco_delete_iroutes call chain

In order to be able to delete DCO iroutes from areas of the code
where the multi_context and the multi_instance objects may not be
available, let's simplify the call chain by passing the smallest
scoped context required.

This is a refactoring only and does not include any functional change.

This patch is required in preparation of fixing DCO iroutes removal
upon client exit, without waiting for the delayed exit routine
to kick in.

Change-Id: Ib5832dc56eaeca1b17016396769b84bdf2c1513a
Signed-off-by: Antonio Quartulli <[email protected]>
---
M src/openvpn/dco.c
M src/openvpn/dco.h
M src/openvpn/multi.c
M src/openvpn/multi.h
4 files changed, 29 insertions(+), 36 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/82/1682/3

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index f5b7081..1e6638b 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -724,26 +724,25 @@
 }

 void
-dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi)
+dco_delete_iroutes(openvpn_net_ctx_t *net_ctx, struct context *c)
 {
 #if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) || defined(_WIN32)
-    if (!dco_enabled(&m->top.options))
+    if (!dco_enabled(&c->options))
     {
         return;
     }
-    ASSERT(TUNNEL_TYPE(mi->context.c1.tuntap) == DEV_TYPE_TUN);
+    ASSERT(TUNNEL_TYPE(c->c1.tuntap) == DEV_TYPE_TUN);

-    struct context *c = &mi->context;

-    if (mi->context.c2.push_ifconfig_defined)
+    if (c->c2.push_ifconfig_defined)
     {
         for (const struct iroute *ir = c->options.iroutes; ir; ir = ir->next)
         {
 #if defined(_WIN32)
             dco_win_del_iroute_ipv4(&c->c1.tuntap->dco, htonl(ir->network), 
ir->netbits);
 #else
-            net_route_v4_del(&m->top.net_ctx, &ir->network, ir->netbits,
-                             &mi->context.c2.push_ifconfig_local, 
c->c1.tuntap->actual_name, 0,
+            net_route_v4_del(net_ctx, &ir->network, ir->netbits,
+                             &c->c2.push_ifconfig_local, 
c->c1.tuntap->actual_name, 0,
                              DCO_IROUTE_METRIC);
 #endif
         }
@@ -751,27 +750,26 @@
 #if !defined(_WIN32)
         /* Check if we added a host route as the assigned client IP address was
          * not in the on link scope defined by --ifconfig */
-        in_addr_t ifconfig_local = mi->context.c2.push_ifconfig_local;
+        in_addr_t ifconfig_local = c->c2.push_ifconfig_local;

-        if (multi_check_push_ifconfig_extra_route(mi, htonl(ifconfig_local)))
+        if (multi_check_push_ifconfig_extra_route(&c->options, 
htonl(ifconfig_local)))
         {
             /* On windows we do not install these routes, so we also do not 
need to delete them */
-            net_route_v4_del(&m->top.net_ctx, &ifconfig_local,
-                             32, NULL, c->c1.tuntap->actual_name, 0,
-                             DCO_IROUTE_METRIC);
+            net_route_v4_del(net_ctx, &ifconfig_local, 32, NULL,
+                             c->c1.tuntap->actual_name, 0, DCO_IROUTE_METRIC);
         }
 #endif
     }

-    if (mi->context.c2.push_ifconfig_ipv6_defined)
+    if (c->c2.push_ifconfig_ipv6_defined)
     {
         for (const struct iroute_ipv6 *ir6 = c->options.iroutes_ipv6; ir6; ir6 
= ir6->next)
         {
 #if defined(_WIN32)
             dco_win_del_iroute_ipv6(&c->c1.tuntap->dco, ir6->network, 
ir6->netbits);
 #else
-            net_route_v6_del(&m->top.net_ctx, &ir6->network, ir6->netbits,
-                             &mi->context.c2.push_ifconfig_ipv6_local, 
c->c1.tuntap->actual_name, 0,
+            net_route_v6_del(net_ctx, &ir6->network, ir6->netbits,
+                             &c->c2.push_ifconfig_ipv6_local, 
c->c1.tuntap->actual_name, 0,
                              DCO_IROUTE_METRIC);
 #endif
         }
@@ -779,11 +777,11 @@
         /* Checked if we added a host route as the assigned client IP address 
was
          * outside the --ifconfig-ipv6 tun interface config */
 #if !defined(_WIN32)
-        struct in6_addr *dest = &mi->context.c2.push_ifconfig_ipv6_local;
-        if (multi_check_push_ifconfig_ipv6_extra_route(mi, dest))
+        struct in6_addr *dest = &c->c2.push_ifconfig_ipv6_local;
+        if (multi_check_push_ifconfig_ipv6_extra_route(&c->options, dest))
         {
             /* On windows we do not install these routes, so we also do not 
need to delete them */
-            net_route_v6_del(&m->top.net_ctx, dest, 128, NULL,
+            net_route_v6_del(net_ctx, dest, 128, NULL,
                              c->c1.tuntap->actual_name, 0, DCO_IROUTE_METRIC);
         }
 #endif
diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 4e5aad5..733f59a1 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -220,10 +220,10 @@
 /**
  * Remove all routes added through the specified client
  *
- * @param m         the server context
- * @param mi        the client instance for which routes have to be removed
+ * @param net_ctx   the iface networking context
+ * @param c         the client context for which routes have to be removed
  */
-void dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi);
+void dco_delete_iroutes(openvpn_net_ctx_t *net_ctx, struct context *c);

 /**
  * Update traffic statistics for all peers
@@ -361,7 +361,7 @@
 }

 static inline void
-dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi)
+dco_delete_iroutes(openvpn_net_ctx_t *net_ctx, struct context *c)
 {
 }

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index a957fdf..a72dcd1 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -479,7 +479,7 @@
     const struct iroute *ir;
     const struct iroute_ipv6 *ir6;

-    dco_delete_iroutes(m, mi);
+    dco_delete_iroutes(&m->top.net_ctx, &mi->context);

     if (TUNNEL_TYPE(mi->context.c1.tuntap) == DEV_TYPE_TUN)
     {
@@ -1198,7 +1198,7 @@
         management_learn_addr(management, &mi->context.c2.mda_context, &addr, 
primary);
     }
 #endif
-    if (primary && multi_check_push_ifconfig_extra_route(mi, addr.v4.addr))
+    if (primary && multi_check_push_ifconfig_extra_route(&mi->context.options, 
addr.v4.addr))
     {
         /* "primary" is the VPN ifconfig address of the peer */
         /* if it does not fall into the network defined by ifconfig_local
@@ -1243,7 +1243,7 @@
         management_learn_addr(management, &mi->context.c2.mda_context, &addr, 
primary);
     }
 #endif
-    if (primary && multi_check_push_ifconfig_ipv6_extra_route(mi, 
&addr.v6.addr))
+    if (primary && 
multi_check_push_ifconfig_ipv6_extra_route(&mi->context.options, &addr.v6.addr))
     {
         /* "primary" is the VPN ifconfig address of the peer */
         /* if it does not fall into the network defined by ifconfig_local
@@ -4373,9 +4373,8 @@
 }

 bool
-multi_check_push_ifconfig_extra_route(struct multi_instance *mi, in_addr_t 
dest)
+multi_check_push_ifconfig_extra_route(struct options *o, in_addr_t dest)
 {
-    struct options *o = &mi->context.options;
     in_addr_t local_addr, local_netmask;

     if (!o->ifconfig_local || !o->ifconfig_remote_netmask)
@@ -4394,11 +4393,8 @@
 }

 bool
-multi_check_push_ifconfig_ipv6_extra_route(struct multi_instance *mi,
-                                           struct in6_addr *dest)
+multi_check_push_ifconfig_ipv6_extra_route(struct options *o, struct in6_addr 
*dest)
 {
-    struct options *o = &mi->context.options;
-
     if (!o->ifconfig_ipv6_local || !o->ifconfig_ipv6_netbits)
     {
         /* If we do not have a local address, we just return false as
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 3ed08d4..f4459d2 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -673,28 +673,27 @@
  * Determines if the ifconfig_push_local address falls into the range of the 
local
  * IP addresses of the VPN interface (ifconfig_local with 
ifconfig_remote_netmask)
  *
- * @param mi           The multi-instance to check this condition for
+ * @param o            The instance wide options
  * @param dest         The destination IP address to check
  *
  * @return Returns true if ifconfig_push is outside that range and requires an 
extra
  * route to be installed.
  */
 bool
-multi_check_push_ifconfig_extra_route(struct multi_instance *mi, in_addr_t 
dest);
+multi_check_push_ifconfig_extra_route(struct options *o, in_addr_t dest);

 /**
  * Determines if the ifconfig_ipv6_local address falls into the range of the 
local
  * IP addresses of the VPN interface (ifconfig_local with 
ifconfig_remote_netmask)
  *
- * @param mi           The multi-instance to check this condition for
+ * @param o            The instance wide options
  * @param dest         The destination IPv6 address to check
  *
  * @return Returns true if ifconfig_push is outside that range and requires an 
extra
  * route to be installed.
  */
 bool
-multi_check_push_ifconfig_ipv6_extra_route(struct multi_instance *mi,
-                                           struct in6_addr *dest);
+multi_check_push_ifconfig_ipv6_extra_route(struct options *o, struct in6_addr 
*dest);

 /*
  * Check for signals.

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1682?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ib5832dc56eaeca1b17016396769b84bdf2c1513a
Gerrit-Change-Number: 1682
Gerrit-PatchSet: 3
Gerrit-Owner: ordex <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: flichtenheld <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to