| Message ID | 20220624083809.23487-20-a@unstable.cc |
|---|---|
| State | Rejected |
| Headers | show |
| Series | ovpn-dco: introduce data-channel offload support | expand |
Hi, On Fri, Jun 24, 2022 at 5:10 AM Antonio Quartulli <a@unstable.cc> wrote: > GetOverlappedResultEx is not available on ming32 therefore we must > provide some compat layer before being able to use this function. > I suppose "mingw32" here refers to I mingw-w64 for 32 bit (i686) target. This symbol has been exported in kernel32.lib since version 7.0 release of mingw-w64 (~ 2019). Current release is version 10. Do we need to support older versions given that mingw build is now used only for development by a few of us? I use debian which probably is the slowest to get new versions -- bullseye has version 8 (gcc version 10), on buster I use unstable which gives version 8. Ubuntu 20.04 has version 7, 22.04 has version 8. Even if we must support 6.0 and older, let's not unconditionally use the compat layer proposed here. > > Signed-off-by: Antonio Quartulli <a@unstable.cc> > Signed-off-by: Lev Stipakov <lev@openvpn.net> > --- > src/compat/Makefile.am | 3 +- > src/compat/compat-dco_get_overlapped_result.c | 46 +++++++++++++++++++ > src/compat/compat.h | 8 ++++ > src/compat/compat.vcxproj | 1 + > src/compat/compat.vcxproj.filters | 3 ++ > 5 files changed, 60 insertions(+), 1 deletion(-) > create mode 100644 src/compat/compat-dco_get_overlapped_result.c > > diff --git a/src/compat/Makefile.am b/src/compat/Makefile.am > index 6eb991dc..6dba08aa 100644 > --- a/src/compat/Makefile.am > +++ b/src/compat/Makefile.am > @@ -28,4 +28,5 @@ libcompat_la_SOURCES = \ > compat-gettimeofday.c \ > compat-daemon.c \ > compat-strsep.c \ > - compat-versionhelpers.h > + compat-versionhelpers.h \ > + compat-dco_get_overlapped_result.c > diff --git a/src/compat/compat-dco_get_overlapped_result.c > b/src/compat/compat-dco_get_overlapped_result.c > new file mode 100644 > index 00000000..e14ce976 > --- /dev/null > +++ b/src/compat/compat-dco_get_overlapped_result.c > @@ -0,0 +1,46 @@ > +/* > + * OpenVPN -- An application to securely tunnel IP networks > + * over a single UDP port, with support for SSL/TLS-based > + * session authentication and key exchange, > + * packet encryption, packet authentication, and > + * packet compression. > + * > + * Copyright (C) 2021-2022 Lev Stipakov <lev@openvpn.net> > + * Copyright (C) 2021-2022 OpenVPN Inc <sales@openvpn.net> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 > + * as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program (see the file COPYING included with this > + * distribution); if not, write to the Free Software Foundation, Inc., > + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > + */ > + > +#ifdef HAVE_CONFIG_H > +#include "config.h" > +#elif defined(_MSC_VER) > +#include "config-msvc.h" > +#endif > + > +#include "compat.h" > + > +#if defined(__MINGW32__) && !defined(__MINGW64__) > +BOOL > +dco_get_overlapped_result(HANDLE handle, OVERLAPPED *ov, DWORD > *transferred, > + DWORD delay_millisec, BOOL unused) > +{ > + BOOL res = GetOverlappedResult(handle, ov, transferred, FALSE); > + if ((res == 0) && (GetLastError() == ERROR_IO_INCOMPLETE)) > + { > + Sleep(delay_millisec); While this may be ok assuming it's not inside any performance critical loops, no need to force this on newer versions of mingw-w64. Alternatively, we could use AC_CHECK_FUNC to make this conditional or find the symbol GetOverlappedResultEx at runtime. But, imo, we do not need any of this as supporting mingw-w64 version 7+ should be enough. Regards, Selva <div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jun 24, 2022 at 5:10 AM Antonio Quartulli <<a href="mailto:a@unstable.cc" target="_blank">a@unstable.cc</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">GetOverlappedResultEx is not available on ming32 therefore we must<br> provide some compat layer before being able to use this function.<br></blockquote><div><br></div><div>I suppose "mingw32" here refers to I mingw-w64 for 32 bit (i686) target.</div><div><br></div><div>This symbol has been exported in kernel32.lib since version 7.0 release of mingw-w64 (~ 2019). Current release is version 10. Do we need to support older versions given that mingw build is now used only for development by a few of us?</div><div><br></div><div>I use debian which probably is the slowest to get new versions -- bullseye has version 8 (gcc version 10), on buster I use unstable which gives version 8. Ubuntu 20.04 has version 7, 22.04 has version 8.</div><div><br></div><div>Even if we must support 6.0 and older, let's not unconditionally use the compat layer proposed here.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br> Signed-off-by: Antonio Quartulli <<a href="mailto:a@unstable.cc" target="_blank">a@unstable.cc</a>><br> Signed-off-by: Lev Stipakov <<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>><br> ---<br> src/compat/Makefile.am | 3 +-<br> src/compat/compat-dco_get_overlapped_result.c | 46 +++++++++++++++++++<br> src/compat/compat.h | 8 ++++<br> src/compat/compat.vcxproj | 1 +<br> src/compat/compat.vcxproj.filters | 3 ++<br> 5 files changed, 60 insertions(+), 1 deletion(-)<br> create mode 100644 src/compat/compat-dco_get_overlapped_result.c<br> <br> diff --git a/src/compat/Makefile.am b/src/compat/Makefile.am<br> index 6eb991dc..6dba08aa 100644<br> --- a/src/compat/Makefile.am<br> +++ b/src/compat/Makefile.am<br> @@ -28,4 +28,5 @@ libcompat_la_SOURCES = \<br> compat-gettimeofday.c \<br> compat-daemon.c \<br> compat-strsep.c \<br> - compat-versionhelpers.h<br> + compat-versionhelpers.h \<br> + compat-dco_get_overlapped_result.c<br> diff --git a/src/compat/compat-dco_get_overlapped_result.c b/src/compat/compat-dco_get_overlapped_result.c<br> new file mode 100644<br> index 00000000..e14ce976<br> --- /dev/null<br> +++ b/src/compat/compat-dco_get_overlapped_result.c<br> @@ -0,0 +1,46 @@<br> +/*<br> + * OpenVPN -- An application to securely tunnel IP networks<br> + * over a single UDP port, with support for SSL/TLS-based<br> + * session authentication and key exchange,<br> + * packet encryption, packet authentication, and<br> + * packet compression.<br> + *<br> + * Copyright (C) 2021-2022 Lev Stipakov <<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>><br> + * Copyright (C) 2021-2022 OpenVPN Inc <<a href="mailto:sales@openvpn.net" target="_blank">sales@openvpn.net</a>><br> + *<br> + * This program is free software; you can redistribute it and/or modify<br> + * it under the terms of the GNU General Public License version 2<br> + * as published by the Free Software Foundation.<br> + *<br> + * This program is distributed in the hope that it will be useful,<br> + * but WITHOUT ANY WARRANTY; without even the implied warranty of<br> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the<br> + * GNU General Public License for more details.<br> + *<br> + * You should have received a copy of the GNU General Public License<br> + * along with this program (see the file COPYING included with this<br> + * distribution); if not, write to the Free Software Foundation, Inc.,<br> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA<br> + */<br> +<br> +#ifdef HAVE_CONFIG_H<br> +#include "config.h"<br> +#elif defined(_MSC_VER)<br> +#include "config-msvc.h"<br> +#endif<br> +<br> +#include "compat.h"<br> +<br> +#if defined(__MINGW32__) && !defined(__MINGW64__)<br> +BOOL<br> +dco_get_overlapped_result(HANDLE handle, OVERLAPPED *ov, DWORD *transferred,<br> + DWORD delay_millisec, BOOL unused)<br> +{<br> + BOOL res = GetOverlappedResult(handle, ov, transferred, FALSE);<br> + if ((res == 0) && (GetLastError() == ERROR_IO_INCOMPLETE))<br> + {<br> + Sleep(delay_millisec);</blockquote><div><br></div><div>While this may be ok assuming it's not inside any performance critical loops, no need to force this on newer versions of mingw-w64. </div><div><br></div><div>Alternatively, we could use AC_CHECK_FUNC to make this conditional or find the symbol GetOverlappedResultEx at runtime. But, imo, we do not need any of this as supporting mingw-w64 version 7+ should be enough.</div><div><br></div><div>Regards,</div><div><br></div><div>Selva</div></div></div>
Hi, The problem this patch solves appeared on mingw-w64 for 32bit version 6.0.0, which is the latest version of mingw on Windows. When this patch was written (April 2021), we still used travis/appveyor which did mingw builds on Windows so back then patch made sense. Since we moved to GHa since the last summer and mingw builds are done on Linux with new enough mingw, this patch could be safely removed. I dropped this commit and pushed changes to my fork and GHa is happy: https://github.com/lstipakov/openvpn/actions/runs/2567640010 -Lev
Hi, On 27/06/2022 12:30, Lev Stipakov wrote: > Hi, > > The problem this patch solves appeared on mingw-w64 for 32bit version > 6.0.0, which is the latest version of mingw on Windows. When this > patch was written (April 2021), we still used travis/appveyor which > did mingw builds on Windows so back then patch made sense. > > Since we moved to GHa since the last summer and mingw builds are done > on Linux with new enough mingw, this patch could be safely removed. > > I dropped this commit and pushed changes to my fork and GHa is happy: > https://github.com/lstipakov/openvpn/actions/runs/2567640010 > The 'dco' branch has been modified accordingly: * compat patch 19/25 removed; * code now uses GetOverlappedResultEx() directly. Cheers,
diff --git a/src/compat/Makefile.am b/src/compat/Makefile.am index 6eb991dc..6dba08aa 100644 --- a/src/compat/Makefile.am +++ b/src/compat/Makefile.am @@ -28,4 +28,5 @@ libcompat_la_SOURCES = \ compat-gettimeofday.c \ compat-daemon.c \ compat-strsep.c \ - compat-versionhelpers.h + compat-versionhelpers.h \ + compat-dco_get_overlapped_result.c diff --git a/src/compat/compat-dco_get_overlapped_result.c b/src/compat/compat-dco_get_overlapped_result.c new file mode 100644 index 00000000..e14ce976 --- /dev/null +++ b/src/compat/compat-dco_get_overlapped_result.c @@ -0,0 +1,46 @@ +/* + * OpenVPN -- An application to securely tunnel IP networks + * over a single UDP port, with support for SSL/TLS-based + * session authentication and key exchange, + * packet encryption, packet authentication, and + * packet compression. + * + * Copyright (C) 2021-2022 Lev Stipakov <lev@openvpn.net> + * Copyright (C) 2021-2022 OpenVPN Inc <sales@openvpn.net> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program (see the file COPYING included with this + * distribution); if not, write to the Free Software Foundation, Inc., + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#elif defined(_MSC_VER) +#include "config-msvc.h" +#endif + +#include "compat.h" + +#if defined(__MINGW32__) && !defined(__MINGW64__) +BOOL +dco_get_overlapped_result(HANDLE handle, OVERLAPPED *ov, DWORD *transferred, + DWORD delay_millisec, BOOL unused) +{ + BOOL res = GetOverlappedResult(handle, ov, transferred, FALSE); + if ((res == 0) && (GetLastError() == ERROR_IO_INCOMPLETE)) + { + Sleep(delay_millisec); + } + return res; +} +#endif diff --git a/src/compat/compat.h b/src/compat/compat.h index 026974a8..2b1ad6a6 100644 --- a/src/compat/compat.h +++ b/src/compat/compat.h @@ -62,4 +62,12 @@ char *strsep(char **stringp, const char *delim); #endif +#if defined(__MINGW32__) && !defined(__MINGW64__) +BOOL dco_get_overlapped_result(HANDLE handle, OVERLAPPED *ov, DWORD *transferred, + DWORD delay_millisec, BOOL unused); + +#else +#define dco_get_overlapped_result GetOverlappedResultEx +#endif + #endif /* COMPAT_H */ diff --git a/src/compat/compat.vcxproj b/src/compat/compat.vcxproj index fe03a51a..1dacb503 100644 --- a/src/compat/compat.vcxproj +++ b/src/compat/compat.vcxproj @@ -159,6 +159,7 @@ </ItemDefinitionGroup> <ItemGroup> <ClCompile Include="compat-basename.c" /> + <ClCompile Include="compat-dco_get_overlapped_result.c" /> <ClCompile Include="compat-dirname.c" /> <ClCompile Include="compat-gettimeofday.c" /> <ClCompile Include="compat-daemon.c" /> diff --git a/src/compat/compat.vcxproj.filters b/src/compat/compat.vcxproj.filters index 96ca026a..73fc9f91 100644 --- a/src/compat/compat.vcxproj.filters +++ b/src/compat/compat.vcxproj.filters @@ -30,6 +30,9 @@ <ClCompile Include="compat-strsep.c"> <Filter>Source Files</Filter> </ClCompile> + <ClCompile Include="compat-dco_get_overlapped_result.c"> + <Filter>Source Files</Filter> + </ClCompile> </ItemGroup> <ItemGroup> <ClInclude Include="compat.h">