Re: [Openvpn-devel] [PATCH v5 04/14] client-connect: Move multi_client_connect_setenv into early_setup

2020-07-14 Thread Antonio Quartulli
Hi,

On 11/07/2020 11:36, Arne Schwabe wrote:
> From: Fabian Knittel 
> 
> This patch moves multi_client_connect_setenv into
> multi_client_connect_early_setup and makes sure that every client-connect
> handling function updates the virtual address selection.
> 
> Background: This unifies how the client-connect handling functions work.
> 
> Signed-off-by: Fabian Knittel 
> Signed-off-by: Arne Schwabe 
> 
> Patch V5: Rebase on master
> 
> Signed-off-by: Arne Schwabe 
> ---
>  src/openvpn/multi.c | 22 +-
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
> index 35e0bd10..539ebfc0 100644
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -2150,6 +2150,12 @@ multi_client_connect_early_setup(struct multi_context 
> *m,
>  
>  /* reset pool handle to null */
>  mi->vaddr_handle = -1;
> +
> +/* do --client-connect setenvs */
> +multi_select_virtual_addr(m, mi);
> +
> +multi_client_connect_setenv(m, mi);
> +
>  }
>  
>  /**
> @@ -2195,6 +2201,13 @@ multi_client_connect_source_ccd(struct multi_context 
> *m,
>CLIENT_CONNECT_OPT_MASK,
>option_types_found,
>mi->context.c2.es);
> +/*
> + * Select a virtual address from either --ifconfig-push in
> + * --client-config-dir file or --ifconfig-pool.
> + */
> +multi_select_virtual_addr(m, mi);
> +
> +multi_client_connect_setenv(m, mi);
>  }
>  gc_free();
>  }


With this patch applied we are moving these 2 lines to:
- multi_client_connect_source_ccd()
- multi_client_connect_early_setup()

In multi_connection_established() we call the two aforementioned
functions one after the other. This means we are really executing the
select_virtual_addr() twice in a row. Am I wrong?

Maybe this gets fixed in a later patch, but are we sure we are not
breaking anything? If this double call gets rearranged in a later
patch...maybe it's better to merge these 2 patches? Bisect may become
quite ugly otherwise.

Cheers,


> @@ -2236,15 +2249,6 @@ multi_connection_established(struct multi_context *m, 
> struct multi_instance *mi)
>  
>  multi_client_connect_source_ccd(m, mi, _types_found);
>  
> -/*
> - * Select a virtual address from either --ifconfig-push in
> - * --client-config-dir file or --ifconfig-pool.
> - */
> -multi_select_virtual_addr(m, mi);
> -
> -/* do --client-connect setenvs */
> -multi_client_connect_setenv(m, mi);
> -
>  multi_client_connect_call_plugin_v1(m, mi, _types_found,
>  _succeeded,
>  _succeeded_count);
> 

-- 
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v5 04/14] client-connect: Move multi_client_connect_setenv into early_setup

2020-07-13 Thread Gert Doering
On Sat, Jul 11, 2020 at 11:36:45AM +0200, Arne Schwabe wrote:
> From: Fabian Knittel 
> 
> This patch moves multi_client_connect_setenv into
> multi_client_connect_early_setup and makes sure that every client-connect
> handling function updates the virtual address selection.
> 
> Background: This unifies how the client-connect handling functions work.


Tested on the server side framework, all tests succeed

23...
Test sets succeeded: 1 1a 1b 1d 2 2a 2b 2c 2d 3 4 5 6 8 8a 9.
Test sets failed: none.
24...
Test sets succeeded: 1 1a 1b 1c 1d 1e 2 2a 2b 2c 2d 2e 3 4 4a 5 6 8 8a 9.
Test sets failed: none.
master...
Test sets succeeded: 1 1a 1b 1c 1d 1e 2 2a 2b 2c 2d 2e 3 4 5 6 7 7a 8 8a 9 2f 4b
.
Test sets failed: none.
 

The patch itself looks a bit strange, now code seems to be duplicated...

... hopefully this gets better in the next patches.
 
gert

-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v5 04/14] client-connect: Move multi_client_connect_setenv into early_setup

2020-07-11 Thread Arne Schwabe
From: Fabian Knittel 

This patch moves multi_client_connect_setenv into
multi_client_connect_early_setup and makes sure that every client-connect
handling function updates the virtual address selection.

Background: This unifies how the client-connect handling functions work.

Signed-off-by: Fabian Knittel 
Signed-off-by: Arne Schwabe 

Patch V5: Rebase on master

Signed-off-by: Arne Schwabe 
---
 src/openvpn/multi.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 35e0bd10..539ebfc0 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2150,6 +2150,12 @@ multi_client_connect_early_setup(struct multi_context *m,
 
 /* reset pool handle to null */
 mi->vaddr_handle = -1;
+
+/* do --client-connect setenvs */
+multi_select_virtual_addr(m, mi);
+
+multi_client_connect_setenv(m, mi);
+
 }
 
 /**
@@ -2195,6 +2201,13 @@ multi_client_connect_source_ccd(struct multi_context *m,
   CLIENT_CONNECT_OPT_MASK,
   option_types_found,
   mi->context.c2.es);
+/*
+ * Select a virtual address from either --ifconfig-push in
+ * --client-config-dir file or --ifconfig-pool.
+ */
+multi_select_virtual_addr(m, mi);
+
+multi_client_connect_setenv(m, mi);
 }
 gc_free();
 }
@@ -2236,15 +2249,6 @@ multi_connection_established(struct multi_context *m, 
struct multi_instance *mi)
 
 multi_client_connect_source_ccd(m, mi, _types_found);
 
-/*
- * Select a virtual address from either --ifconfig-push in
- * --client-config-dir file or --ifconfig-pool.
- */
-multi_select_virtual_addr(m, mi);
-
-/* do --client-connect setenvs */
-multi_client_connect_setenv(m, mi);
-
 multi_client_connect_call_plugin_v1(m, mi, _types_found,
 _succeeded,
 _succeeded_count);
-- 
2.26.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel