| Message ID | 20230309131419.29157-1-a@unstable.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v3] dco: print version to log if available | expand |
This is being discussed on Gerrit at: https://gerrit.openvpn.net/c/openvpn/+/28 On 09/03/2023 14:14, Antonio Quartulli wrote: > In order to provide better support in case of troubleshooting issues, > it's important to know what exact DCO version is loaded on the user > system. > > Therefore print the DCO version during bootup. > > For Windows and FreeBSD we currently implement a placeholder printing 'v0'. > This should be improved with a follow-up patch. > > For Linux we directly fetch the module version from /sys and print > something like: > > DCO version: 0.1.20230206-15-g580608ec7c59 > > Change-Id: Ie1f6fa5d12a473d353d84fd119c2430b638e8bcd > Signed-off-by: Antonio Quartulli <a@unstable.cc> > --- > Changes from v1: > * beautify usage of buf with some helpers > * don't apply -1 to buffer capacity as fgets will already read one byte > less than provided length and will add a null terminator there > > Changes from v2: > * fix style of type returned by dco_version_string() (uncrustify doesn't > work anymore on my box..due to incompatibler config) > --- > src/openvpn/dco.h | 15 +++++++++++++++ > src/openvpn/dco_freebsd.c | 6 ++++++ > src/openvpn/dco_linux.c | 27 +++++++++++++++++++++++++++ > src/openvpn/dco_win.c | 6 ++++++ > src/openvpn/openvpn.c | 2 ++ > src/openvpn/options.c | 11 +++++++++++ > src/openvpn/options.h | 2 ++ > 7 files changed, 69 insertions(+) > > diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h > index 2fe671bf..96d95c21 100644 > --- a/src/openvpn/dco.h > +++ b/src/openvpn/dco.h > @@ -58,6 +58,15 @@ struct tuntap; > */ > bool dco_available(int msglevel); > > + > +/** > + * Return a human readable string representing the DCO version > + * > + * @param gc the garbage collector to use for any dynamic allocation > + * @return a pointer to the string (allocated via gc) containing the string > + */ > +const char *dco_version_string(struct gc_arena *gc); > + > /** > * Check whether the options struct has any option that is not supported by > * our current dco implementation. If so print a warning at warning level > @@ -250,6 +259,12 @@ dco_available(int msglevel) > return false; > } > > +static inline const char * > +dco_version_string(struct gc_arena *gc) > +{ > + return "not-compiled"; > +} > + > static inline bool > dco_check_option(int msglevel, const struct options *o) > { > diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c > index 92de5f04..cbd2ce20 100644 > --- a/src/openvpn/dco_freebsd.c > +++ b/src/openvpn/dco_freebsd.c > @@ -614,6 +614,12 @@ out: > return available; > } > > +const char * > +dco_version_string(struct gc_arena *gc) > +{ > + return "v0"; > +} > + > void > dco_event_set(dco_context_t *dco, struct event_set *es, void *arg) > { > diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c > index 308abfc4..9f3531d5 100644 > --- a/src/openvpn/dco_linux.c > +++ b/src/openvpn/dco_linux.c > @@ -841,9 +841,36 @@ dco_available(int msglevel) > "Note: Kernel support for ovpn-dco missing, disabling data channel offload."); > return false; > } > + > return true; > } > > +const char * > +dco_version_string(struct gc_arena *gc) > +{ > + struct buffer out = alloc_buf_gc(256, gc); > + FILE *fp = fopen("/sys/module/ovpn_dco_v2/version", "r"); > + if (!fp) > + { > + return "N/A"; > + } > + > + if (!fgets(BSTR(&out), BCAP(&out), fp)) > + { > + return "ERR"; > + } > + > + /* remove potential newline at the end of the string */ > + char *str = BSTR(&out); > + char *nl = strchr(str, '\n'); > + if (nl) > + { > + *nl = '\0'; > + } > + > + return BSTR(&out); > +} > + > void > dco_event_set(dco_context_t *dco, struct event_set *es, void *arg) > { > diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c > index a805c2a0..26463b38 100644 > --- a/src/openvpn/dco_win.c > +++ b/src/openvpn/dco_win.c > @@ -385,6 +385,12 @@ dco_available(int msglevel) > return false; > } > > +const char * > +dco_version_string(struct gc_arena *gc) > +{ > + return "v0"; > +} > + > int > dco_do_read(dco_context_t *dco) > { > diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c > index cba58276..1aaddcdf 100644 > --- a/src/openvpn/openvpn.c > +++ b/src/openvpn/openvpn.c > @@ -262,6 +262,8 @@ openvpn_main(int argc, char *argv[]) > #endif > show_library_versions(M_INFO); > > + show_dco_version(M_INFO); > + > /* misc stuff */ > pre_setup(&c.options); > > diff --git a/src/openvpn/options.c b/src/openvpn/options.c > index ce7bd0a7..8f0e2194 100644 > --- a/src/openvpn/options.c > +++ b/src/openvpn/options.c > @@ -4816,6 +4816,16 @@ show_windows_version(const unsigned int flags) > } > #endif > > +void > +show_dco_version(const unsigned int flags) > +{ > +#ifdef ENABLE_DCO > + struct gc_arena gc = gc_new(); > + msg(flags, "DCO version: %s", dco_version_string(&gc)); > + gc_free(&gc); > +#endif > +} > + > void > show_library_versions(const unsigned int flags) > { > @@ -4839,6 +4849,7 @@ usage_version(void) > #ifdef _WIN32 > show_windows_version( M_INFO|M_NOPREFIX ); > #endif > + show_dco_version(M_INFO | M_NOPREFIX); > msg(M_INFO|M_NOPREFIX, "Originally developed by James Yonan"); > msg(M_INFO|M_NOPREFIX, "Copyright (C) 2002-2023 OpenVPN Inc <sales@openvpn.net>"); > #ifndef ENABLE_SMALL > diff --git a/src/openvpn/options.h b/src/openvpn/options.h > index 7df717f7..2f25f5d0 100644 > --- a/src/openvpn/options.h > +++ b/src/openvpn/options.h > @@ -796,6 +796,8 @@ void show_windows_version(const unsigned int flags); > > #endif > > +void show_dco_version(const unsigned int flags); > + > void init_options(struct options *o, const bool init_gc); > > void uninit_options(struct options *o);
Ignore the last message - it was meant for another patch *shrug* On 09/03/2023 16:02, Antonio Quartulli wrote: > This is being discussed on Gerrit at: > > https://gerrit.openvpn.net/c/openvpn/+/28 > > On 09/03/2023 14:14, Antonio Quartulli wrote: >> In order to provide better support in case of troubleshooting issues, >> it's important to know what exact DCO version is loaded on the user >> system. >> >> Therefore print the DCO version during bootup. >> >> For Windows and FreeBSD we currently implement a placeholder printing >> 'v0'. >> This should be improved with a follow-up patch. >> >> For Linux we directly fetch the module version from /sys and print >> something like: >> >> DCO version: 0.1.20230206-15-g580608ec7c59 >> >> Change-Id: Ie1f6fa5d12a473d353d84fd119c2430b638e8bcd >> Signed-off-by: Antonio Quartulli <a@unstable.cc> >> --- >> Changes from v1: >> * beautify usage of buf with some helpers >> * don't apply -1 to buffer capacity as fgets will already read one byte >> less than provided length and will add a null terminator there >> >> Changes from v2: >> * fix style of type returned by dco_version_string() (uncrustify doesn't >> work anymore on my box..due to incompatibler config) >> --- >> src/openvpn/dco.h | 15 +++++++++++++++ >> src/openvpn/dco_freebsd.c | 6 ++++++ >> src/openvpn/dco_linux.c | 27 +++++++++++++++++++++++++++ >> src/openvpn/dco_win.c | 6 ++++++ >> src/openvpn/openvpn.c | 2 ++ >> src/openvpn/options.c | 11 +++++++++++ >> src/openvpn/options.h | 2 ++ >> 7 files changed, 69 insertions(+) >> >> diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h >> index 2fe671bf..96d95c21 100644 >> --- a/src/openvpn/dco.h >> +++ b/src/openvpn/dco.h >> @@ -58,6 +58,15 @@ struct tuntap; >> */ >> bool dco_available(int msglevel); >> + >> +/** >> + * Return a human readable string representing the DCO version >> + * >> + * @param gc the garbage collector to use for any dynamic allocation >> + * @return a pointer to the string (allocated via gc) containing >> the string >> + */ >> +const char *dco_version_string(struct gc_arena *gc); >> + >> /** >> * Check whether the options struct has any option that is not >> supported by >> * our current dco implementation. If so print a warning at warning >> level >> @@ -250,6 +259,12 @@ dco_available(int msglevel) >> return false; >> } >> +static inline const char * >> +dco_version_string(struct gc_arena *gc) >> +{ >> + return "not-compiled"; >> +} >> + >> static inline bool >> dco_check_option(int msglevel, const struct options *o) >> { >> diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c >> index 92de5f04..cbd2ce20 100644 >> --- a/src/openvpn/dco_freebsd.c >> +++ b/src/openvpn/dco_freebsd.c >> @@ -614,6 +614,12 @@ out: >> return available; >> } >> +const char * >> +dco_version_string(struct gc_arena *gc) >> +{ >> + return "v0"; >> +} >> + >> void >> dco_event_set(dco_context_t *dco, struct event_set *es, void *arg) >> { >> diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c >> index 308abfc4..9f3531d5 100644 >> --- a/src/openvpn/dco_linux.c >> +++ b/src/openvpn/dco_linux.c >> @@ -841,9 +841,36 @@ dco_available(int msglevel) >> "Note: Kernel support for ovpn-dco missing, disabling >> data channel offload."); >> return false; >> } >> + >> return true; >> } >> +const char * >> +dco_version_string(struct gc_arena *gc) >> +{ >> + struct buffer out = alloc_buf_gc(256, gc); >> + FILE *fp = fopen("/sys/module/ovpn_dco_v2/version", "r"); >> + if (!fp) >> + { >> + return "N/A"; >> + } >> + >> + if (!fgets(BSTR(&out), BCAP(&out), fp)) >> + { >> + return "ERR"; >> + } >> + >> + /* remove potential newline at the end of the string */ >> + char *str = BSTR(&out); >> + char *nl = strchr(str, '\n'); >> + if (nl) >> + { >> + *nl = '\0'; >> + } >> + >> + return BSTR(&out); >> +} >> + >> void >> dco_event_set(dco_context_t *dco, struct event_set *es, void *arg) >> { >> diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c >> index a805c2a0..26463b38 100644 >> --- a/src/openvpn/dco_win.c >> +++ b/src/openvpn/dco_win.c >> @@ -385,6 +385,12 @@ dco_available(int msglevel) >> return false; >> } >> +const char * >> +dco_version_string(struct gc_arena *gc) >> +{ >> + return "v0"; >> +} >> + >> int >> dco_do_read(dco_context_t *dco) >> { >> diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c >> index cba58276..1aaddcdf 100644 >> --- a/src/openvpn/openvpn.c >> +++ b/src/openvpn/openvpn.c >> @@ -262,6 +262,8 @@ openvpn_main(int argc, char *argv[]) >> #endif >> show_library_versions(M_INFO); >> + show_dco_version(M_INFO); >> + >> /* misc stuff */ >> pre_setup(&c.options); >> diff --git a/src/openvpn/options.c b/src/openvpn/options.c >> index ce7bd0a7..8f0e2194 100644 >> --- a/src/openvpn/options.c >> +++ b/src/openvpn/options.c >> @@ -4816,6 +4816,16 @@ show_windows_version(const unsigned int flags) >> } >> #endif >> +void >> +show_dco_version(const unsigned int flags) >> +{ >> +#ifdef ENABLE_DCO >> + struct gc_arena gc = gc_new(); >> + msg(flags, "DCO version: %s", dco_version_string(&gc)); >> + gc_free(&gc); >> +#endif >> +} >> + >> void >> show_library_versions(const unsigned int flags) >> { >> @@ -4839,6 +4849,7 @@ usage_version(void) >> #ifdef _WIN32 >> show_windows_version( M_INFO|M_NOPREFIX ); >> #endif >> + show_dco_version(M_INFO | M_NOPREFIX); >> msg(M_INFO|M_NOPREFIX, "Originally developed by James Yonan"); >> msg(M_INFO|M_NOPREFIX, "Copyright (C) 2002-2023 OpenVPN Inc >> <sales@openvpn.net>"); >> #ifndef ENABLE_SMALL >> diff --git a/src/openvpn/options.h b/src/openvpn/options.h >> index 7df717f7..2f25f5d0 100644 >> --- a/src/openvpn/options.h >> +++ b/src/openvpn/options.h >> @@ -796,6 +796,8 @@ void show_windows_version(const unsigned int flags); >> #endif >> +void show_dco_version(const unsigned int flags); >> + >> void init_options(struct options *o, const bool init_gc); >> void uninit_options(struct options *o); >
Am 09.03.23 um 14:14 schrieb Antonio Quartulli: > In order to provide better support in case of troubleshooting issues, > it's important to know what exact DCO version is loaded on the user > system. > > Therefore print the DCO version during bootup. > > For Windows and FreeBSD we currently implement a placeholder printing 'v0'. > This should be improved with a follow-up patch. > > For Linux we directly fetch the module version from /sys and print > something like: > > DCO version: 0.1.20230206-15-g580608ec7c59 > > Change-Id: Ie1f6fa5d12a473d353d84fd119c2430b638e8bcd > Signed-off-by: Antonio Quartulli <a@unstable.cc> > --- > Changes from v1: > * beautify usage of buf with some helpers > * don't apply -1 to buffer capacity as fgets will already read one byte > less than provided length and will add a null terminator there > > Changes from v2: > * fix style of type returned by dco_version_string() (uncrustify doesn't > work anymore on my box..due to incompatibler config) any more, incompatible > return true; > } > > +const char * > +dco_version_string(struct gc_arena *gc) > +{ > + struct buffer out = alloc_buf_gc(256, gc); > + FILE *fp = fopen("/sys/module/ovpn_dco_v2/version", "r"); > + if (!fp) Note: this patch should only merged after "dco: don't use NetLink to exchange control packets" as it uses the same API version as that patch. Acked-By: Arne Schwabe <arne@rfc2549.org>
I briefly tested this on a system with the dco-v2 kernel module
(and with the module unloaded). It did what I expected:
DCO version: 0.1.20230206-15-g580608e
or
DCO version: N/A
On FreeBSD 12 (no DCO), it does not print anything, on FreeBSD 14
it prints
DCO version: v0
(which will be addressed in a subsequent patch).
The code itself looks a bit overcomplicated, using a "struct buffer"
with all the macros, just to have something for fgets() to fill...
(it could use a on-stack char[] + capacity provided by the caller,
getting rid of the gc in the process...) but it has an ACK, it
works, and I want the FreeBSD version patch done...
Your patch has been applied to the master and release/2.6 branch.
commit 3da238e677b7801607e6777d9d23eb61e38034c2 (master)
commit 9bd6fff74cf34892339fcab2fb3fc3cee54a2051 (release/2.6)
Author: Antonio Quartulli
Date: Thu Mar 9 14:14:19 2023 +0100
dco: print version to log if available
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230309131419.29157-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26370.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h index 2fe671bf..96d95c21 100644 --- a/src/openvpn/dco.h +++ b/src/openvpn/dco.h @@ -58,6 +58,15 @@ struct tuntap; */ bool dco_available(int msglevel); + +/** + * Return a human readable string representing the DCO version + * + * @param gc the garbage collector to use for any dynamic allocation + * @return a pointer to the string (allocated via gc) containing the string + */ +const char *dco_version_string(struct gc_arena *gc); + /** * Check whether the options struct has any option that is not supported by * our current dco implementation. If so print a warning at warning level @@ -250,6 +259,12 @@ dco_available(int msglevel) return false; } +static inline const char * +dco_version_string(struct gc_arena *gc) +{ + return "not-compiled"; +} + static inline bool dco_check_option(int msglevel, const struct options *o) { diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index 92de5f04..cbd2ce20 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -614,6 +614,12 @@ out: return available; } +const char * +dco_version_string(struct gc_arena *gc) +{ + return "v0"; +} + void dco_event_set(dco_context_t *dco, struct event_set *es, void *arg) { diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index 308abfc4..9f3531d5 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -841,9 +841,36 @@ dco_available(int msglevel) "Note: Kernel support for ovpn-dco missing, disabling data channel offload."); return false; } + return true; } +const char * +dco_version_string(struct gc_arena *gc) +{ + struct buffer out = alloc_buf_gc(256, gc); + FILE *fp = fopen("/sys/module/ovpn_dco_v2/version", "r"); + if (!fp) + { + return "N/A"; + } + + if (!fgets(BSTR(&out), BCAP(&out), fp)) + { + return "ERR"; + } + + /* remove potential newline at the end of the string */ + char *str = BSTR(&out); + char *nl = strchr(str, '\n'); + if (nl) + { + *nl = '\0'; + } + + return BSTR(&out); +} + void dco_event_set(dco_context_t *dco, struct event_set *es, void *arg) { diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c index a805c2a0..26463b38 100644 --- a/src/openvpn/dco_win.c +++ b/src/openvpn/dco_win.c @@ -385,6 +385,12 @@ dco_available(int msglevel) return false; } +const char * +dco_version_string(struct gc_arena *gc) +{ + return "v0"; +} + int dco_do_read(dco_context_t *dco) { diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c index cba58276..1aaddcdf 100644 --- a/src/openvpn/openvpn.c +++ b/src/openvpn/openvpn.c @@ -262,6 +262,8 @@ openvpn_main(int argc, char *argv[]) #endif show_library_versions(M_INFO); + show_dco_version(M_INFO); + /* misc stuff */ pre_setup(&c.options); diff --git a/src/openvpn/options.c b/src/openvpn/options.c index ce7bd0a7..8f0e2194 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -4816,6 +4816,16 @@ show_windows_version(const unsigned int flags) } #endif +void +show_dco_version(const unsigned int flags) +{ +#ifdef ENABLE_DCO + struct gc_arena gc = gc_new(); + msg(flags, "DCO version: %s", dco_version_string(&gc)); + gc_free(&gc); +#endif +} + void show_library_versions(const unsigned int flags) { @@ -4839,6 +4849,7 @@ usage_version(void) #ifdef _WIN32 show_windows_version( M_INFO|M_NOPREFIX ); #endif + show_dco_version(M_INFO | M_NOPREFIX); msg(M_INFO|M_NOPREFIX, "Originally developed by James Yonan"); msg(M_INFO|M_NOPREFIX, "Copyright (C) 2002-2023 OpenVPN Inc <sales@openvpn.net>"); #ifndef ENABLE_SMALL diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 7df717f7..2f25f5d0 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -796,6 +796,8 @@ void show_windows_version(const unsigned int flags); #endif +void show_dco_version(const unsigned int flags); + void init_options(struct options *o, const bool init_gc); void uninit_options(struct options *o);
In order to provide better support in case of troubleshooting issues, it's important to know what exact DCO version is loaded on the user system. Therefore print the DCO version during bootup. For Windows and FreeBSD we currently implement a placeholder printing 'v0'. This should be improved with a follow-up patch. For Linux we directly fetch the module version from /sys and print something like: DCO version: 0.1.20230206-15-g580608ec7c59 Change-Id: Ie1f6fa5d12a473d353d84fd119c2430b638e8bcd Signed-off-by: Antonio Quartulli <a@unstable.cc> --- Changes from v1: * beautify usage of buf with some helpers * don't apply -1 to buffer capacity as fgets will already read one byte less than provided length and will add a null terminator there Changes from v2: * fix style of type returned by dco_version_string() (uncrustify doesn't work anymore on my box..due to incompatibler config) --- src/openvpn/dco.h | 15 +++++++++++++++ src/openvpn/dco_freebsd.c | 6 ++++++ src/openvpn/dco_linux.c | 27 +++++++++++++++++++++++++++ src/openvpn/dco_win.c | 6 ++++++ src/openvpn/openvpn.c | 2 ++ src/openvpn/options.c | 11 +++++++++++ src/openvpn/options.h | 2 ++ 7 files changed, 69 insertions(+)