diff mbox series

[Openvpn-devel,20/25] dco-win: add platform dependant check on incompatible options

Message ID 20220624083809.23487-21-a@unstable.cc
State Changes Requested
Headers
Series ovpn-dco: introduce data-channel offload support |

Commit Message

Antonio Quartulli June 23, 2022, 10:38 p.m. UTC 
  Some platforms may have different constraints in terms of incompatible
opions, therefore we add a function that explicitly checks those.

Also, add generic option check for when ovpn-dco-win is in use.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 src/openvpn/dco.c     | 25 +++++++++++++++++++++++++
 src/openvpn/options.c |  5 +++++
 2 files changed, 30 insertions(+)

Comments

Heiko Hund July 5, 2022, 2:31 a.m. UTC | #1 
On Freitag, 24. Juni 2022 10:38:04 CEST Antonio Quartulli wrote:
> Some platforms may have different constraints in terms of incompatible
> opions, therefore we add a function that explicitly checks those.
> 
> Also, add generic option check for when ovpn-dco-win is in use.
> 
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> Signed-off-by: Lev Stipakov <lev@openvpn.net>

Acked-by: Heiko Hund <heiko@ist.eigentlich.net>
Gert Doering July 19, 2022, 10:17 a.m. UTC | #2 
Hi,

this one has an ACK, but needs a v2 still, because...

On Fri, Jun 24, 2022 at 10:38:04AM +0200, Antonio Quartulli wrote:
> Some platforms may have different constraints in terms of incompatible
> opions, therefore we add a function that explicitly checks those.
> 
> Also, add generic option check for when ovpn-dco-win is in use.
> 
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> Signed-off-by: Lev Stipakov <lev@openvpn.net>
[..]
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -2376,6 +2376,11 @@ options_postprocess_verify_ce(const struct options *options,
>      {
>          msg(M_USAGE, "--windows-driver wintun requires --dev tun");
>      }
> +
> +    if (options->windows_driver == WINDOWS_DRIVER_WINDCO)
> +    {
> +        dco_check_option_conflict(M_USAGE, options);
> +    }
>  #endif /* ifdef _WIN32 */

... the WINDOWS_DRIVER_WINDCO enum is only introduced in one of the
later hunks.

gert
Gert Doering Aug. 11, 2022, 10:58 a.m. UTC | #3 
Hi,

On Fri, Jun 24, 2022 at 10:38:04AM +0200, Antonio Quartulli wrote:
> Some platforms may have different constraints in terms of incompatible
> opions, therefore we add a function that explicitly checks those.
> 
> Also, add generic option check for when ovpn-dco-win is in use.

Looking more closely at this, this patch needs a carefuly rebase :-)

> --- a/src/openvpn/dco.c
> +++ b/src/openvpn/dco.c
> @@ -190,6 +190,26 @@ dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
>      }
>  }
>  
> +static bool
> +dco_check_option_conflict_platform(int msglevel, const struct options *o)
> +{
> +#if defined(_WIN32)
> +    if (o->mode == MODE_SERVER)
> +    {

... it *introduces* dco_check_option_conflict_platform() with "only
windows" content, but in the current master tree, this function already
exists, with Linux content...

gert
diff mbox series

Patch

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 1612a64e..448f29b5 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -190,6 +190,26 @@  dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
     }
 }
 
+static bool
+dco_check_option_conflict_platform(int msglevel, const struct options *o)
+{
+#if defined(_WIN32)
+    if (o->mode == MODE_SERVER)
+    {
+        msg(msglevel, "Only client and p2p data channel offload is supported "
+            "with ovpn-dco-win.");
+        return false;
+    }
+
+    if (o->persist_tun)
+    {
+        msg(msglevel, "--persist-tun is not supported with ovpn-dco-win.");
+        return false;
+    }
+#endif
+    return true;
+}
+
 static bool
 dco_check_option_conflict_ce(const struct connection_entry *ce, int msglevel)
 {
@@ -228,6 +248,11 @@  dco_check_option_conflict(int msglevel, const struct options *o)
         return false;
     }
 
+    if (!dco_check_option_conflict_platform(msglevel, o))
+    {
+        return false;
+    }
+
     if (dev_type_enum(o->dev, o->dev_type) != DEV_TYPE_TUN)
     {
         msg(msglevel, "Note: dev-type not tun, disabling data channel offload.");
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 7b450296..d9da8b8b 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2376,6 +2376,11 @@  options_postprocess_verify_ce(const struct options *options,
     {
         msg(M_USAGE, "--windows-driver wintun requires --dev tun");
     }
+
+    if (options->windows_driver == WINDOWS_DRIVER_WINDCO)
+    {
+        dco_check_option_conflict(M_USAGE, options);
+    }
 #endif /* ifdef _WIN32 */
 
     /*