* [Openvpn-devel] [PATCH] Make '--comp-lzo no' the default behaviour if LZO is enabled
@ 2011-05-16 11:10 David Sommerseth
2011-05-16 11:16 ` Alon Bar-Lev
2011-05-20 9:47 ` David Sommerseth
0 siblings, 2 replies; 5+ messages in thread
From: David Sommerseth @ 2011-05-16 11:10 UTC (permalink / raw)
To: openvpn-devel; +Cc: David Sommerseth <davids@
This makes 'comp-lzo' pushable without requiring clients to have
--comp-lzo defined in the client configs. To make 'comp-lzo' not
pushable on the client, a new 'disabled' argument have been added.
Trac-ticket: 128
Signed-off-by: David Sommerseth <davids@...1347...>
---
openvpn.8 | 12 ++++++------
options.c | 10 +++++++---
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/openvpn.8 b/openvpn.8
index 5cbf4c5..89759b6 100644
--- a/openvpn.8
+++ b/openvpn.8
@@ -2284,7 +2284,9 @@ limit repetitive logging of similar message types.
Use fast LZO compression \-\- may add up to 1 byte per
packet for incompressible data.
.B mode
-may be "yes", "no", or "adaptive" (default).
+may be "yes", "adaptive" or "disabled".
+If \-\-comp-lzo is used without an argument, it defaults to
+"adaptive".
In a server mode setup, it is possible to selectively turn
compression on or off for individual clients.
@@ -2293,11 +2295,9 @@ First, make sure the client-side config file enables selective
compression by having at least one
.B \-\-comp-lzo
directive, such as
-.B \-\-comp-lzo no.
-This will turn off compression by default,
-but allow a future directive push from the server to
-dynamically change the
-on/off/adaptive setting.
+
+If the "disabled" argument is given, \-\-comp-lzo can not be pushed
+from the server to the client.
Next in a
.B \-\-client-config-dir
diff --git a/options.c b/options.c
index f643acc..dc45282 100644
--- a/options.c
+++ b/options.c
@@ -816,6 +816,10 @@ init_options (struct options *o, const bool init_gc)
o->tmp_dir = "/tmp";
}
#endif /* WIN32 */
+
+#ifdef USE_LZO
+ o->lzo = LZO_SELECTED;
+#endif
}
void
@@ -5889,13 +5893,13 @@ add_option (struct options *options,
{
if (streq (p[1], "yes"))
options->lzo = LZO_SELECTED|LZO_ON;
- else if (streq (p[1], "no"))
- options->lzo = LZO_SELECTED;
else if (streq (p[1], "adaptive"))
options->lzo = LZO_SELECTED|LZO_ON|LZO_ADAPTIVE;
+ else if (streq (p[1], "disabled"))
+ options->lzo = 0;
else
{
- msg (msglevel, "bad comp-lzo option: %s -- must be 'yes', 'no', or 'adaptive'", p[1]);
+ msg (msglevel, "bad comp-lzo option: %s -- must be 'yes', 'adaptive' or 'disabled", p[1]);
goto err;
}
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Openvpn-devel] [PATCH] Make '--comp-lzo no' the default behaviour if LZO is enabled
2011-05-16 11:10 [Openvpn-devel] [PATCH] Make '--comp-lzo no' the default behaviour if LZO is enabled David Sommerseth
@ 2011-05-16 11:16 ` Alon Bar-Lev
2011-05-20 9:47 ` David Sommerseth
1 sibling, 0 replies; 5+ messages in thread
From: Alon Bar-Lev @ 2011-05-16 11:16 UTC (permalink / raw)
To: David Sommerseth <davids@; +Cc: openvpn-devel
If you have a problem in what can or cannot be pushed to client, why
not do this generic?
Something like:
---
push-allowed="comp-lzo ..."
push-disallow="comp-lzo ..."
---
On Mon, May 16, 2011 at 2:10 PM, David Sommerseth <davids@...1347...> wrote:
>
> This makes 'comp-lzo' pushable without requiring clients to have
> --comp-lzo defined in the client configs. To make 'comp-lzo' not
> pushable on the client, a new 'disabled' argument have been added.
>
> Trac-ticket: 128
> Signed-off-by: David Sommerseth <davids@...1347...>
> ---
> openvpn.8 | 12 ++++++------
> options.c | 10 +++++++---
> 2 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/openvpn.8 b/openvpn.8
> index 5cbf4c5..89759b6 100644
> --- a/openvpn.8
> +++ b/openvpn.8
> @@ -2284,7 +2284,9 @@ limit repetitive logging of similar message types.
> Use fast LZO compression \-\- may add up to 1 byte per
> packet for incompressible data.
> .B mode
> -may be "yes", "no", or "adaptive" (default).
> +may be "yes", "adaptive" or "disabled".
> +If \-\-comp-lzo is used without an argument, it defaults to
> +"adaptive".
>
> In a server mode setup, it is possible to selectively turn
> compression on or off for individual clients.
> @@ -2293,11 +2295,9 @@ First, make sure the client-side config file enables selective
> compression by having at least one
> .B \-\-comp-lzo
> directive, such as
> -.B \-\-comp-lzo no.
> -This will turn off compression by default,
> -but allow a future directive push from the server to
> -dynamically change the
> -on/off/adaptive setting.
> +
> +If the "disabled" argument is given, \-\-comp-lzo can not be pushed
> +from the server to the client.
>
> Next in a
> .B \-\-client-config-dir
> diff --git a/options.c b/options.c
> index f643acc..dc45282 100644
> --- a/options.c
> +++ b/options.c
> @@ -816,6 +816,10 @@ init_options (struct options *o, const bool init_gc)
> o->tmp_dir = "/tmp";
> }
> #endif /* WIN32 */
> +
> +#ifdef USE_LZO
> + o->lzo = LZO_SELECTED;
> +#endif
> }
>
> void
> @@ -5889,13 +5893,13 @@ add_option (struct options *options,
> {
> if (streq (p[1], "yes"))
> options->lzo = LZO_SELECTED|LZO_ON;
> - else if (streq (p[1], "no"))
> - options->lzo = LZO_SELECTED;
> else if (streq (p[1], "adaptive"))
> options->lzo = LZO_SELECTED|LZO_ON|LZO_ADAPTIVE;
> + else if (streq (p[1], "disabled"))
> + options->lzo = 0;
> else
> {
> - msg (msglevel, "bad comp-lzo option: %s -- must be 'yes', 'no', or 'adaptive'", p[1]);
> + msg (msglevel, "bad comp-lzo option: %s -- must be 'yes', 'adaptive' or 'disabled", p[1]);
> goto err;
> }
> }
> --
> 1.7.4.4
>
>
> ------------------------------------------------------------------------------
> Achieve unprecedented app performance and reliability
> What every C/C++ and Fortran developer should know.
> Learn how Intel has extended the reach of its next-generation tools
> to help boost performance applications - inlcuding clusters.
> http://p.sf.net/sfu/intel-dev2devmay
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Openvpn-devel] [PATCH] Make '--comp-lzo no' the default behaviour if LZO is enabled
2011-05-16 11:10 [Openvpn-devel] [PATCH] Make '--comp-lzo no' the default behaviour if LZO is enabled David Sommerseth
2011-05-16 11:16 ` Alon Bar-Lev
@ 2011-05-20 9:47 ` David Sommerseth
2011-05-20 9:55 ` Jan Just Keijser
1 sibling, 1 reply; 5+ messages in thread
From: David Sommerseth @ 2011-05-20 9:47 UTC (permalink / raw)
To: openvpn-devel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 16/05/11 13:10, David Sommerseth wrote:
> This makes 'comp-lzo' pushable without requiring clients to have
> --comp-lzo defined in the client configs. To make 'comp-lzo' not
> pushable on the client, a new 'disabled' argument have been added.
>
> Trac-ticket: 128
> Signed-off-by: David Sommerseth <davids@...1347...>
> ---
> openvpn.8 | 12 ++++++------
> options.c | 10 +++++++---
> 2 files changed, 13 insertions(+), 9 deletions(-)
>
As announced on the developers meeting yesterday (Samuli will paste the
summary with the discussion), I'm withdrawing this patch in the current form.
The reason is that the OpenVPN wire protocol changes slightly when using
'--comp-lzo no' versus not having --comp-lzo in the config file. This
change will cause server/client incompatibilities.
Alon proposed another approach with white/blacklist of pushed options which
should be processed. And this approach do make a lot of sense, and we
should implement this as well. But this will be a bigger change.
However, I'm looking at a different approach right now isolated to
- --comp-lzo, where OpenVPN can detect incompatible --comp-lzo settings and
rather provide a better error and stop OpenVPN. This I consider better
than to try to reconnect indefinitely when critical config options on
client and server is incompatible.
- From a quick brain storm, for --comp-lzo, the only compatible/valid
combinations are:
Client Server
(no --comp-lzo) (no --comp-lzo)
--comp-lzo {,adaptive} (*) --comp-lzo {,adaptive}
--comp-lzo yes (*) --comp-lzo yes
--comp-lzo no (*) --comp-lzo no
(*) In these combinations, comp-lzo should be pushable which can change the
client setting. If client does not have comp-lzo in its config, it should
disconnect from the server if the server pushes --comp-lzo settings, as the
wire protocol from the server will be different from what the client expects.
kind regards,
David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk3WOJ4ACgkQDC186MBRfrq8/gCfYJz7aoPQwNrSTIaz9letMTTC
ufsAnR7MdJtsZz6wvRyJOxCwKOU3tZcs
=SaAA
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Openvpn-devel] [PATCH] Make '--comp-lzo no' the default behaviour if LZO is enabled
2011-05-20 9:47 ` David Sommerseth
@ 2011-05-20 9:55 ` Jan Just Keijser
2011-05-20 10:28 ` David Sommerseth
0 siblings, 1 reply; 5+ messages in thread
From: Jan Just Keijser @ 2011-05-20 9:55 UTC (permalink / raw)
To: David Sommerseth <openvpn.list@; +Cc: openvpn-devel
Hi *,
David Sommerseth wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 16/05/11 13:10, David Sommerseth wrote:
>
>> This makes 'comp-lzo' pushable without requiring clients to have
>> --comp-lzo defined in the client configs. To make 'comp-lzo' not
>> pushable on the client, a new 'disabled' argument have been added.
>>
>> Trac-ticket: 128
>> Signed-off-by: David Sommerseth <davids@...1347...>
>> ---
>> openvpn.8 | 12 ++++++------
>> options.c | 10 +++++++---
>> 2 files changed, 13 insertions(+), 9 deletions(-)
>>
>>
>
> As announced on the developers meeting yesterday (Samuli will paste the
> summary with the discussion), I'm withdrawing this patch in the current form.
>
> The reason is that the OpenVPN wire protocol changes slightly when using
> '--comp-lzo no' versus not having --comp-lzo in the config file. This
> change will cause server/client incompatibilities.
>
> Alon proposed another approach with white/blacklist of pushed options which
> should be processed. And this approach do make a lot of sense, and we
> should implement this as well. But this will be a bigger change.
>
> However, I'm looking at a different approach right now isolated to
> - --comp-lzo, where OpenVPN can detect incompatible --comp-lzo settings and
> rather provide a better error and stop OpenVPN. This I consider better
> than to try to reconnect indefinitely when critical config options on
> client and server is incompatible.
>
> - From a quick brain storm, for --comp-lzo, the only compatible/valid
> combinations are:
>
> Client Server
> (no --comp-lzo) (no --comp-lzo)
> --comp-lzo {,adaptive} (*) --comp-lzo {,adaptive}
> --comp-lzo yes (*) --comp-lzo yes
> --comp-lzo no (*) --comp-lzo no
>
> (*) In these combinations, comp-lzo should be pushable which can change the
> client setting. If client does not have comp-lzo in its config, it should
> disconnect from the server if the server pushes --comp-lzo settings, as the
> wire protocol from the server will be different from what the client expects.
>
>
please remember the reason for this patch: bug
https://community.openvpn.net/openvpn/ticket/128
if there is a 'comp-lzo' mismatch and the server pushes out 'push
"comp-lzo yes"' then upon reconnecting it works all of a sudden - this
inconsistency needs to be addressed one way or the other.
If we don't want to change the 'comp-lzo' behaviour then at the very
least this "reconnect-makes-it-work" feature/bug should be fixed in a
different manner.
JM2CW,
JJK
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Openvpn-devel] [PATCH] Make '--comp-lzo no' the default behaviour if LZO is enabled
2011-05-20 9:55 ` Jan Just Keijser
@ 2011-05-20 10:28 ` David Sommerseth
0 siblings, 0 replies; 5+ messages in thread
From: David Sommerseth @ 2011-05-20 10:28 UTC (permalink / raw)
To: Jan Just Keijser <janjust@; +Cc: openvpn-devel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 20/05/11 11:55, Jan Just Keijser wrote:
>> - From a quick brain storm, for --comp-lzo, the only compatible/valid
>> combinations are:
>>
>> Client Server
>> (no --comp-lzo) (no --comp-lzo)
>> --comp-lzo {,adaptive} (*) --comp-lzo {,adaptive}
>> --comp-lzo yes (*) --comp-lzo yes
>> --comp-lzo no (*) --comp-lzo no
>>
>> (*) In these combinations, comp-lzo should be pushable which can change the
>> client setting. If client does not have comp-lzo in its config, it should
>> disconnect from the server if the server pushes --comp-lzo settings, as the
>> wire protocol from the server will be different from what the client
>> expects.
>>
>>
> please remember the reason for this patch: bug
> https://community.openvpn.net/openvpn/ticket/128
>
> if there is a 'comp-lzo' mismatch and the server pushes out 'push "comp-lzo
> yes"' then upon reconnecting it works all of a sudden - this inconsistency
> needs to be addressed one way or the other.
> If we don't want to change the 'comp-lzo' behaviour then at the very least
> this "reconnect-makes-it-work" feature/bug should be fixed in a different
> manner.
Hi Jan,
Yeah, that is the reason why I think a disconnect should happen when the
client is not using comp-lzo (no --comp-lzo defined), while the server
pushes it. This is a misconfiguration to start with. However, when the
client disconnects and stops running, it should give a sensible error
message - to help solve this issue.
Why it "works" with a reconnect, is that the first connect fails because
client config is not using --comp-lzo. But the server managed to get a
- --comp-lzo push through, which then enables it on the client on the next
reconnect. This is actually a the real misbehaviour, and is a bug.
When this bug is fixed, we can consider an "auto-fix feature", where it
will reconnect with '--comp-lzo no' in this kind of situation, without
giving odd errors in the log files. This auto-fix feature should probably
be possible to disable, for those wanting more control over what's happening.
kind regards,
David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk3WQjAACgkQDC186MBRfroqZQCeOMiuIGsYgze9omfQpjiL9Zfg
6IQAn1ffLU1b1LV9a5nhmuWCWm4XCoKH
=dILR
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-05-20 10:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-16 11:10 [Openvpn-devel] [PATCH] Make '--comp-lzo no' the default behaviour if LZO is enabled David Sommerseth
2011-05-16 11:16 ` Alon Bar-Lev
2011-05-20 9:47 ` David Sommerseth
2011-05-20 9:55 ` Jan Just Keijser
2011-05-20 10:28 ` David Sommerseth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.