On 8 Mar 2022, at 15:16, Antonio Quartulli wrote:
> Hi Kristof,
>
> A quick question for you, see below
>
> On 24/02/2022 17:55, Kristof Provost via Openvpn-devel wrote:
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -787,7 +787,20 @@ dnl
>> AC_DEFINE(ENABLE_DCO, 1, [Enable data channel offload
>> for Linux])
>> AC_MSG_NOTICE([Enabled ovpn-dco support for Linux])
>> ;;
>> -
>> + *-*-freebsd*)
>> + DCO_CFLAGS="-I${DCO_SOURCEDIR}"
>> + saved_CFLAGS="${CFLAGS}"
>> + CFLAGS="${CFLAGS} ${DCO_CFLAGS}"
>> + AC_CHECK_HEADERS(
>> + [if_ovpn.h],
>> + ,
>> + [AC_MSG_ERROR([if_ovpn.h is missing
>> (use DCO_SOURCEDIR to set path to it, CFLAGS=${CFLAGS})])]
>> + )
>> + CFLAGS=${saved_CFLAGS}
>> + LIBS="${LIBS} -lnv"
>> + AC_DEFINE(ENABLE_DCO, 1, [Enable data channel offload
>> for FreeBSD])
>> + AC_MSG_NOTICE([Enabled ovpn-dco support for FreeBSD])
>> + ;;
>
> If you double check the latest dco branch, you will see that I have dropped
> the DCO_SOURCEDIR variable and I have rather switched to "let's include the
> kernel API header in the openvpn repository all the time and be happy with
> it".
>
> The idea is that the kernel API will always be backwards compatible,
> therefore having a stale header will never be a problem. It can be updated
> when a new feature is implemented in openvpn itself. (a similar approach is
> taken by hostapd or iw on Linux, where they ship a copy of nl80211.h)
>
> On the other hand, we drastically simplify the configure logic and avoid
> having to deal with this variable which may have different pitfalls on
> different platforms.
>
> Do you have any opinion about the above? or shipping a copy of the header
> with openvpn source code is fine with you?
>
It’s not really too relevant either way for FreeBSD, because the header
contains very little information. It’s mostly only the ioctl numbers. There are
no structures shared between the kernel and user land. Instead we use nvlists
(name/value pair serialisation).
Theoretically I’d prefer to use the OS header, but I can certainly see the
upside of not having that dependency. We’ll always have to do the runtime check
(dco_available()) anyway, so I can certainly live with that.
> p.s. as a side note, the rest of the code contains some "code style" issues.
> Nothing major, but wanted to mention it (we can discuss on IRC if you want).
>
I’m happy to fix that. Do you have a rough list of what’s wrong?
https://community.openvpn.net/openvpn/wiki/CodeStyle is the authoritative style
guide, right?
Best regards,
Kristof
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
