[Openvpn-devel,19/25] dco-win: implement GetOverlappedResultEx for mingw32

Message ID 20220624083809.23487-20-a@unstable.cc
State Rejected
Headers show
Series
  • ovpn-dco: introduce data-channel offload support
Related show

Commit Message

Antonio Quartulli June 24, 2022, 8:38 a.m.
GetOverlappedResultEx is not available on ming32 therefore we must
provide some compat layer before being able to use this function.

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

Comments

Selva Nair June 24, 2022, 6:53 p.m. | #1
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 &lt;<a href="mailto:a@unstable.cc" target="_blank">a@unstable.cc</a>&gt; 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 &quot;mingw32&quot; 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&#39;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 &lt;<a href="mailto:a@unstable.cc" target="_blank">a@unstable.cc</a>&gt;<br>
Signed-off-by: Lev Stipakov &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<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 &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<br>
+ *  Copyright (C) 2021-2022 OpenVPN Inc &lt;<a href="mailto:sales@openvpn.net" target="_blank">sales@openvpn.net</a>&gt;<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 &quot;config.h&quot;<br>
+#elif defined(_MSC_VER)<br>
+#include &quot;config-msvc.h&quot;<br>
+#endif<br>
+<br>
+#include &quot;compat.h&quot;<br>
+<br>
+#if defined(__MINGW32__) &amp;&amp; !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) &amp;&amp; (GetLastError() == ERROR_IO_INCOMPLETE))<br>
+    {<br>
+        Sleep(delay_millisec);</blockquote><div><br></div><div>While this may be ok assuming it&#39;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>
Lev Stipakov June 27, 2022, 10:30 a.m. | #2
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
Antonio Quartulli June 27, 2022, 6:50 p.m. | #3
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,

Patch

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">