Re: [Openvpn-devel] [PATCH v2] configure: enable DCO by default on FreeBSD/Linux
Hi,
On Mon, Feb 6, 2023 at 6:24 AM Frank Lichtenheld
wrote:
> Automatically disabled when
> - iproute2 is enabled
> (Don't want to force people specifying --disable-dco explicitely)
> - libnv is missing on FreeBSD
> (FreeBSD version too old anyway)
>
> Will still error out if libnl-genl is missing on Linux to
> make people aware of new dependency.
>
> Signed-off-by: Frank Lichtenheld
> ---
> configure.ac | 78
> 1 file changed, 61 insertions(+), 17 deletions(-)
>
> v2: error out when libnl-genl is missing as discussed with ordex on
> IRC.
>
>
> diff --git a/configure.ac b/configure.ac
> index 91500087..acfa4bc1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -144,14 +144,27 @@ AC_ARG_ENABLE(
>
> AC_ARG_ENABLE(
> [dco],
> - [AS_HELP_STRING([--enable-dco], [enable data channel offload
> support using the ovpn-dco kernel module (always enabled on Windows)
> @<:@default=no@:>@])],
> + [AS_HELP_STRING([--disable-dco], [enable data channel offload
> support using the ovpn-dco kernel module (always enabled on Windows)
> @<:@default=yes@:>@])],
>
The help string should be
"disable data channel offload support ..." instead of "enable data channel
offload ..." as it's now prefixed with "--disable-dco" .
The "default=yes" in this case may be better written as "default=auto" (or
"platform specific") but it's not critical. Here, default always refers to
the "--enable-FEATURE" form of the option. Confusing but consistent with
how other options are described.
We could be more helpful by describing both enable and disable forms of all
options, as well as custom args that the enable form takes (if any), but
that's beyond the scope of this patch.
-if test "$enable_dco" = "yes"; then
> +if test "$enable_dco" != "no"; then
> + enable_dco_arg="$enable_dco"
> + if test "${enable_iproute2}" = "yes"; then
> + AC_MSG_WARN([iproute2 support cannot be enabled when using
> DCO])
>
This is mildly misleading as here iproute2 is going to win over DCO but the
message appears to indicate otherwise. It would result in outputs like:
WARNING: iproute2 support cannot be enabled when using DCO
WARNING: DCO support disabled
(or in an error)
Phrasing the first as "DCO cannot be enabled when using iproute2" would be
better.
> + enable_dco="no"
> + fi
> + case "$host" in
> + *-*-linux*)
I'll defer to those who understand DCO better to ACK the technical part.
Selva
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] configure: fix formatting of --disable-lz4 and --enable-comp-stub
Make consistent with the other options. Signed-off-by: Frank Lichtenheld --- configure.ac | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) A small issue I noticed when staring at configure.ac for "enable DCO by default" patch. diff --git a/configure.ac b/configure.ac index acfa4bc1..7ce4c826 100644 --- a/configure.ac +++ b/configure.ac @@ -67,14 +67,16 @@ AC_ARG_ENABLE( [enable_lzo="yes"] ) -AC_ARG_ENABLE(lz4, - [ --disable-lz4 Disable LZ4 compression support], +AC_ARG_ENABLE( + [lz4], + [AS_HELP_STRING([--disable-lz4], [disable LZ4 compression support @<:@default=yes@:>@])], [enable_lz4="$enableval"], [enable_lz4="yes"] ) -AC_ARG_ENABLE(comp-stub, - [ --enable-comp-stub Don't compile compression support but still allow limited interoperability with compression-enabled peers], +AC_ARG_ENABLE( + [comp-stub], + [AS_HELP_STRING([--enable-comp-stub], [disable compression support but still allow limited interoperability with compression-enabled peers @<:@default=no@:>@])], [enable_comp_stub="$enableval"], [enable_comp_stub="no"] ) -- 2.34.1 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] configure: enable DCO by default on FreeBSD/Linux
Automatically disabled when
- iproute2 is enabled
(Don't want to force people specifying --disable-dco explicitely)
- libnv is missing on FreeBSD
(FreeBSD version too old anyway)
Will still error out if libnl-genl is missing on Linux to
make people aware of new dependency.
Signed-off-by: Frank Lichtenheld
---
configure.ac | 78
1 file changed, 61 insertions(+), 17 deletions(-)
v2: error out when libnl-genl is missing as discussed with ordex on
IRC.
diff --git a/configure.ac b/configure.ac
index 91500087..acfa4bc1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -144,14 +144,27 @@ AC_ARG_ENABLE(
AC_ARG_ENABLE(
[dco],
- [AS_HELP_STRING([--enable-dco], [enable data channel offload support
using the ovpn-dco kernel module (always enabled on Windows)
@<:@default=no@:>@])],
+ [AS_HELP_STRING([--disable-dco], [enable data channel offload support
using the ovpn-dco kernel module (always enabled on Windows)
@<:@default=yes@:>@])],
,
- [enable_dco="no"]
+ [
+ case "$host" in
+ *-*-linux*)
+ enable_dco="auto"
+ ;;
+ *-*-freebsd*)
+ enable_dco="auto"
+ ;;
+ *)
+ # note that this does not disable it for Windows
+ enable_dco="no"
+ ;;
+ esac
+ ]
)
AC_ARG_ENABLE(
[iproute2],
- [AS_HELP_STRING([--enable-iproute2], [enable support for iproute2
@<:@default=no@:>@])],
+ [AS_HELP_STRING([--enable-iproute2], [enable support for iproute2
(disables DCO) @<:@default=no@:>@])],
,
[enable_iproute2="no"]
)
@@ -534,7 +547,7 @@ AC_CHECK_DECLS(
,
[[${SOCKET_INCLUDES}]]
)
-AC_CHECKING([anonymous union support])
+AC_MSG_CHECKING([anonymous union support])
AC_COMPILE_IFELSE(
[AC_LANG_PROGRAM(
[[
@@ -769,28 +782,59 @@ PKG_CHECK_MODULES(
)
-if test "$enable_dco" = "yes"; then
+if test "$enable_dco" != "no"; then
+ enable_dco_arg="$enable_dco"
+ if test "${enable_iproute2}" = "yes"; then
+ AC_MSG_WARN([iproute2 support cannot be enabled when using DCO])
+ enable_dco="no"
+ fi
+ case "$host" in
+ *-*-linux*)
dnl
dnl Include generic netlink library used to talk to ovpn-dco
dnl
- case "$host" in
- *-*-linux*)
PKG_CHECK_MODULES([LIBNL_GENL],
[libnl-genl-3.0 >= 3.4.0],
[have_libnl="yes"],
- [AC_MSG_ERROR([libnl-genl-3.0 package
not found or too old. Is the development package and pkg-config installed? Must
be version 3.4.0 or newer])]
+ [
+ AC_MSG_ERROR([libnl-genl-3.0 package
not found or too old. Is the development package and pkg-config installed? Must
be version 3.4.0 or newer for DCO])
+ ]
)
-
- CFLAGS="${CFLAGS} ${LIBNL_GENL_CFLAGS}"
- LIBS="${LIBS} ${LIBNL_GENL_LIBS}"
-
- AC_DEFINE(ENABLE_DCO, 1, [Enable shared data channel
offload])
- AC_MSG_NOTICE([Enabled ovpn-dco support for Linux])
+ if test "$enable_dco" = "no"; then
+ if test "$enable_dco_arg" = "auto"; then
+ AC_MSG_WARN([DCO support disabled])
+ else
+ AC_MSG_ERROR([DCO support can't be
enabled])
+ fi
+ else
+ CFLAGS="${CFLAGS} ${LIBNL_GENL_CFLAGS}"
+ LIBS="${LIBS} ${LIBNL_GENL_LIBS}"
+
+ AC_DEFINE(ENABLE_DCO, 1, [Enable shared data
channel offload])
+ AC_MSG_NOTICE([Enabled ovpn-dco support for
Linux])
+ fi
;;
*-*-freebsd*)
- LIBS="${LIBS} -lnv"
- AC_DEFINE(ENABLE_DCO, 1, [Enable data channel offload
for FreeBSD])
- AC_MSG_NOTICE([Enabled ovpn-dco support for FreeBSD])
+ AC_CHECK_LIB(
+ [nv],
+ [nvlist_create],
+ [
+LIBS="${LIBS} -lnv"
+AC_DEFINE(ENABLE_DCO, 1, [Enable data channel
offload for FreeBSD])
+AC_MSG_NOTICE([Enabled ovpn-dco support for
FreeBSD])
+
