[Openvpn-devel] dco: print FreeBSD version

Message ID 20230309092635.7450-1-kprovost@netgate.com
State Superseded
Headers show
Series [Openvpn-devel] dco: print FreeBSD version | expand

Commit Message

Kristof Provost March 9, 2023, 9:26 a.m. UTC
From: Kristof Provost <kp@FreeBSD.org>

Implement dco_version_string() for FreeBSD.
Unlike Linux and Windows the DCO driver is built into the operating
system itself, so we log the OS version as a proxy for the DCO version.
---
 src/openvpn/dco_freebsd.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Arne Schwabe March 9, 2023, 12:06 p.m. UTC | #1
Am 09.03.23 um 10:26 schrieb Kristof Provost via Openvpn-devel:
> From: Kristof Provost <kp@FreeBSD.org>
> 
> Implement dco_version_string() for FreeBSD.
> Unlike Linux and Windows the DCO driver is built into the operating
> system itself, so we log the OS version as a proxy for the DCO version.
> ---
>   src/openvpn/dco_freebsd.c | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
> index 2b94b2a2..a5e96bb2 100644
> --- a/src/openvpn/dco_freebsd.c
> +++ b/src/openvpn/dco_freebsd.c
> @@ -31,6 +31,8 @@
>   #include <sys/param.h>
>   #include <sys/linker.h>
>   #include <sys/nv.h>
> +#include <sys/utsname.h>
> +
>   #include <netinet/in.h>
>   
>   #include "dco_freebsd.h"
> @@ -627,7 +629,17 @@ out:
>   const char *
>   dco_version_string(struct gc_arena *gc)
>   {
> -    return "v0";
> +    struct utsname name;
> +    struct buffer out = alloc_buf_gc(256, gc);
> +    int ret;
> +
> +    ret = uname(&name);
> +    if (ret != 0)
> +        return "N/A";

There should be { around the return

> +
> +    buf_printf(&out, "%s", name.version);
> +
> +    return (char *)out.data;

This should use BSTR(data) instead. Instead of copying/printf, you could 
also just do:

     struct utsname* uts;
     ALLOC_OBJ_GC(uts, struct utsname, gc);


     if (uname(&name) != 0)
     {
         return "N/A";
     }

     return uts->name;

Arne
Kristof Provost March 9, 2023, 12:13 p.m. UTC | #2
On 9 Mar 2023, at 13:06, Arne Schwabe wrote:
> Am 09.03.23 um 10:26 schrieb Kristof Provost via Openvpn-devel:
>> From: Kristof Provost <kp@FreeBSD.org>
>>
>> Implement dco_version_string() for FreeBSD.
>> Unlike Linux and Windows the DCO driver is built into the operating
>> system itself, so we log the OS version as a proxy for the DCO version.
>> ---
>>   src/openvpn/dco_freebsd.c | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
>> index 2b94b2a2..a5e96bb2 100644
>> --- a/src/openvpn/dco_freebsd.c
>> +++ b/src/openvpn/dco_freebsd.c
>> @@ -31,6 +31,8 @@
>>   #include <sys/param.h>
>>   #include <sys/linker.h>
>>   #include <sys/nv.h>
>> +#include <sys/utsname.h>
>> +
>>   #include <netinet/in.h>
>>    #include "dco_freebsd.h"
>> @@ -627,7 +629,17 @@ out:
>>   const char *
>>   dco_version_string(struct gc_arena *gc)
>>   {
>> -    return "v0";
>> +    struct utsname name;
>> +    struct buffer out = alloc_buf_gc(256, gc);
>> +    int ret;
>> +
>> +    ret = uname(&name);
>> +    if (ret != 0)
>> +        return "N/A";
>
> There should be { around the return

I’ll fix that.

>> +
>> +    buf_printf(&out, "%s", name.version);
>> +
>> +    return (char *)out.data;
>
> This should use BSTR(data) instead.
>
I copied Antonio’s code here, but that is better, so I’ll fix that too.

> Instead of copying/printf, you could also just do:
>
>     struct utsname* uts;
>     ALLOC_OBJ_GC(uts, struct utsname, gc);
>
>
>     if (uname(&name) != 0)
>     {
>         return "N/A";
>     }
>
>     return uts->name;
>
I was going to object that we can’t return stack variables, but that’s an allocation of utsname, and it removes an intermediate step, so yeah, that’s better too (and means I can just copy your code, call it my own and pretend I did work ;))

Thanks.
Kristof
Antonio Quartulli March 9, 2023, 12:16 p.m. UTC | #3
Hi,

On 09/03/2023 13:13, Kristof Provost via Openvpn-devel wrote:
>> This should use BSTR(data) instead.
>>
> I copied Antonio’s code here, but that is better, so I’ll fix that too.
> 

dang! with one email Arne spoiled two patches!

Cheers,

Patch

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 2b94b2a2..a5e96bb2 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -31,6 +31,8 @@ 
 #include <sys/param.h>
 #include <sys/linker.h>
 #include <sys/nv.h>
+#include <sys/utsname.h>
+
 #include <netinet/in.h>
 
 #include "dco_freebsd.h"
@@ -627,7 +629,17 @@  out:
 const char *
 dco_version_string(struct gc_arena *gc)
 {
-    return "v0";
+    struct utsname name;
+    struct buffer out = alloc_buf_gc(256, gc);
+    int ret;
+
+    ret = uname(&name);
+    if (ret != 0)
+        return "N/A";
+
+    buf_printf(&out, "%s", name.version);
+
+    return (char *)out.data;
 }
 
 void