| Message ID | 20240213194050.1590143-4-i.maximets@ovn.org |
|---|---|
| State | Superseded |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | Windows: Fix OpenSSL build and ovs-pki. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On Tue, Feb 13, 2024 at 08:40:17PM +0100, Ilya Maximets wrote: > OpenSSL 1.1.0 changed the library names from libeay32 and ssleay32 to > standard libssl and libcrypto. All the versions of OpenSSL that used > old names reached their official EoL, so it should be safe to just > migrate to new names. They can still be supported via premium support > option, but I don't think that is important for us. > > Also, OpenSSL installers for older versions had the following folder > structure: ... > Basically, instead of one generic library in the lib folder and a bunch > of differently named versions of it for different type of linkage, we > now have multiple instances of the library located in different folders > based on the linkage type. So, we have to provide an exact path in > order to find the library. > > 'lib/VC/x64/MD' was chosen in this patch and it seems to work fine. > MD stands for dynamic linking, MT is static, 'd' stands for debug > versions of the libraries. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Simon Horman <horms@ovn.org> ...
Thank you so much for the patches and dealing with this, Ilya! Regarding the MD / MT linking for a given binary, this can work, usually, without any issues, although it will increase the memory used by the process because both runtimes have to be loaded. We are linking the rest of the binaries with MT / MTd inside OVS: https://github.com/openvswitch/ovs/blob/master/build-aux/cccl#L92-L98 . Can you please change the OpenSSL to link with MT instead of MD? This will ensure that we test with what will be built in the end. Because we are changing the defaults to msys2 / openssl3 we need to also update the documentation. I'm okay with changing it to a later date. If you prefer, I can pick this one up. I will insert the snippets where the documentation has to be changed for OpenSSL: https://github.com/openvswitch/ovs/blob/master/Documentation/intro/install/windows.rst?plain=1#L109-L115 https://github.com/openvswitch/ovs/blob/master/Documentation/intro/install/windows.rst?plain=1#L179-L185 https://github.com/openvswitch/ovs/blob/master/Documentation/intro/install/windows.rst?plain=1#L191-L199 Alin.
On 2/20/24 23:33, Alin Serdean wrote: > Thank you so much for the patches and dealing with this, Ilya! > > Regarding the MD / MT linking for a given binary, this can work, usually, without any issues, although it will increase the memory used by the process because both runtimes have to be loaded. > > We are linking the rest of the binaries with MT / MTd inside OVS: https://github.com/openvswitch/ovs/blob/master/build-aux/cccl#L92-L98 <https://github.com/openvswitch/ovs/blob/master/build-aux/cccl#L92-L98> . > Can you please change the OpenSSL to link with MT instead of MD? Ah, good point, I wasn't aware that we're linking other stuff with MT. Sure, I can change that. Should I send a v2 with this change? > This will ensure that we test with what will be built in the end. > > Because we are changing the defaults to msys2 / openssl3 we need to also update the documentation. > > I'm okay with changing it to a later date. If you prefer, I can pick this one up. Looking at the links below I'm actually not sure what should be changed there beside replacing OpenSSL-Win32 with OpenSSL-Win64. I suppose we need to change the instructions at the top from MinGW to MSYS, but I actually have no idea how to install them. I've only been using what AppVeyor provides. :) So, if you can take this one that would be great. It can definitely be a separate change. Best regards, Ilya Maximets. > > I will insert the snippets where the documentation has to be changed for OpenSSL: > https://github.com/openvswitch/ovs/blob/master/Documentation/intro/install/windows.rst?plain=1#L109-L115 <https://github.com/openvswitch/ovs/blob/master/Documentation/intro/install/windows.rst?plain=1#L109-L115> > https://github.com/openvswitch/ovs/blob/master/Documentation/intro/install/windows.rst?plain=1#L179-L185 <https://github.com/openvswitch/ovs/blob/master/Documentation/intro/install/windows.rst?plain=1#L179-L185> > https://github.com/openvswitch/ovs/blob/master/Documentation/intro/install/windows.rst?plain=1#L191-L199 <https://github.com/openvswitch/ovs/blob/master/Documentation/intro/install/windows.rst?plain=1#L191-L199> > > > Alin. > ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ > *From:* dev <ovs-dev-bounces@openvswitch.org> on behalf of Simon Horman <horms@ovn.org> > *Sent:* Thursday, February 15, 2024 2:08 PM > *To:* Ilya Maximets <i.maximets@ovn.org> > *Cc:* ovs-dev@openvswitch.org <ovs-dev@openvswitch.org> > *Subject:* Re: [ovs-dev] [PATCH 3/4] m4: Fix linking with OpenSSL 1.1.0+ and 3+ on Windows. > > On Tue, Feb 13, 2024 at 08:40:17PM +0100, Ilya Maximets wrote: >> OpenSSL 1.1.0 changed the library names from libeay32 and ssleay32 to >> standard libssl and libcrypto. All the versions of OpenSSL that used >> old names reached their official EoL, so it should be safe to just >> migrate to new names. They can still be supported via premium support >> option, but I don't think that is important for us. >> >> Also, OpenSSL installers for older versions had the following folder >> structure: > > ... > >> Basically, instead of one generic library in the lib folder and a bunch >> of differently named versions of it for different type of linkage, we >> now have multiple instances of the library located in different folders >> based on the linkage type. So, we have to provide an exact path in >> order to find the library. >> >> 'lib/VC/x64/MD' was chosen in this patch and it seems to work fine. >> MD stands for dynamic linking, MT is static, 'd' stands for debug >> versions of the libraries. >> >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > > Acked-by: Simon Horman <horms@ovn.org> > > ... > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
On 2/20/24 23:52, Ilya Maximets wrote: > On 2/20/24 23:33, Alin Serdean wrote: >> Thank you so much for the patches and dealing with this, Ilya! >> >> Regarding the MD / MT linking for a given binary, this can work, usually, without any issues, although it will increase the memory used by the process because both runtimes have to be loaded. >> >> We are linking the rest of the binaries with MT / MTd inside OVS: https://github.com/openvswitch/ovs/blob/master/build-aux/cccl#L92-L98 <https://github.com/openvswitch/ovs/blob/master/build-aux/cccl#L92-L98> . >> Can you please change the OpenSSL to link with MT instead of MD? > > Ah, good point, I wasn't aware that we're linking other stuff > with MT. Sure, I can change that. > > Should I send a v2 with this change? > >> This will ensure that we test with what will be built in the end. >> >> Because we are changing the defaults to msys2 / openssl3 we need to also update the documentation. >> >> I'm okay with changing it to a later date. If you prefer, I can pick this one up. > > Looking at the links below I'm actually not sure what should be changed > there beside replacing OpenSSL-Win32 with OpenSSL-Win64. > I suppose we need to change the instructions at the top from MinGW to > MSYS, but I actually have no idea how to install them. I've only been > using what AppVeyor provides. :) > > So, if you can take this one that would be great. It can definitely > be a separate change. > > Best regards, Ilya Maximets. > >> >> I will insert the snippets where the documentation has to be changed for OpenSSL: >> https://github.com/openvswitch/ovs/blob/master/Documentation/intro/install/windows.rst?plain=1#L109-L115 >> https://github.com/openvswitch/ovs/blob/master/Documentation/intro/install/windows.rst?plain=1#L179-L185 >> https://github.com/openvswitch/ovs/blob/master/Documentation/intro/install/windows.rst?plain=1#L191-L199 I posted a v2 of the patch set here: https://patchwork.ozlabs.org/project/openvswitch/list/?series=397413&state=* I updated the Win32 references with Win64 in the doc that you pointed at above. Larger change to update the doc from MinGW to MSYS is a little out of scope for this patch set and I'm also not entirely sure what the step should be. So, I didn't touch that. Best regards, Ilya Maximets. >> >> >> Alin. >> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ >> *From:* dev <ovs-dev-bounces@openvswitch.org> on behalf of Simon Horman <horms@ovn.org> >> *Sent:* Thursday, February 15, 2024 2:08 PM >> *To:* Ilya Maximets <i.maximets@ovn.org> >> *Cc:* ovs-dev@openvswitch.org <ovs-dev@openvswitch.org> >> *Subject:* Re: [ovs-dev] [PATCH 3/4] m4: Fix linking with OpenSSL 1.1.0+ and 3+ on Windows. >> >> On Tue, Feb 13, 2024 at 08:40:17PM +0100, Ilya Maximets wrote: >>> OpenSSL 1.1.0 changed the library names from libeay32 and ssleay32 to >>> standard libssl and libcrypto. All the versions of OpenSSL that used >>> old names reached their official EoL, so it should be safe to just >>> migrate to new names. They can still be supported via premium support >>> option, but I don't think that is important for us. >>> >>> Also, OpenSSL installers for older versions had the following folder >>> structure: >> >> ... >> >>> Basically, instead of one generic library in the lib folder and a bunch >>> of differently named versions of it for different type of linkage, we >>> now have multiple instances of the library located in different folders >>> based on the linkage type. So, we have to provide an exact path in >>> order to find the library. >>> >>> 'lib/VC/x64/MD' was chosen in this patch and it seems to work fine. >>> MD stands for dynamic linking, MT is static, 'd' stands for debug >>> versions of the libraries. >>> >>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> >> >> Acked-by: Simon Horman <horms@ovn.org> >> >> ... >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> >
diff --git a/m4/ax_check_openssl.m4 b/m4/ax_check_openssl.m4 index 281d4dc65..e3d361d63 100644 --- a/m4/ax_check_openssl.m4 +++ b/m4/ax_check_openssl.m4 @@ -81,7 +81,8 @@ AC_DEFUN([AX_CHECK_OPENSSL], [ SSL_INCLUDES="-I$ssldir/include" SSL_LDFLAGS="-L$ssldir/lib" if test "$WIN32" = "yes"; then - SSL_LIBS="-lssleay32 -llibeay32" + SSL_LDFLAGS="$SSL_LDFLAGS -L$ssldir/lib/VC/x64/MD" + SSL_LIBS="-llibssl -llibcrypto" SSL_DIR=/$(echo ${ssldir} | ${SED} -e 's/://') else SSL_LIBS="-lssl -lcrypto"
OpenSSL 1.1.0 changed the library names from libeay32 and ssleay32 to standard libssl and libcrypto. All the versions of OpenSSL that used old names reached their official EoL, so it should be safe to just migrate to new names. They can still be supported via premium support option, but I don't think that is important for us. Also, OpenSSL installers for older versions had the following folder structure: C:\OPENSSL-WIN64\ +---bin +---include | +---openssl +---lib | libeay32.lib | ssleay32.lib +---VC libeay32MD.lib libeay32MDd.lib libeay32MT.lib libeay32MTd.lib ssleay32MD.lib ssleay32MDd.lib ssleay32MT.lib ssleay32MTd.lib With newer OpenSSL 3+ the structure is different: C:\OPENSSL-WIN64 +---bin +---include | +---openssl +---lib +---VC +---x64 +---MD | libcrypto.lib | libssl.lib +---MDd | libcrypto.lib | libssl.lib +---MT | libcrypto.lib | libssl.lib +---MTd libcrypto.lib libssl.lib Basically, instead of one generic library in the lib folder and a bunch of differently named versions of it for different type of linkage, we now have multiple instances of the library located in different folders based on the linkage type. So, we have to provide an exact path in order to find the library. 'lib/VC/x64/MD' was chosen in this patch and it seems to work fine. MD stands for dynamic linking, MT is static, 'd' stands for debug versions of the libraries. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- m4/ax_check_openssl.m4 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)