On 20/10/16 10:42, Antonio Quartulli wrote: > On Wed, Oct 19, 2016 at 02:22:31PM +0800, Antonio Quartulli wrote: >> Implement the functions needed by the crl-persist logic when openssl >> is enabled. Such functions are used in the ssl_verify module. >> >> Note that the CRL file is stored in an adhoc data structure and no >> openssl specific object is used. The data structure being used is a >> sorted array or serials that can later be looked up in log(N) with >> a binary search, thus guaranteeing a fast lookup operation. >> >> Such data structure may be changed in the future with an optimized >> openssl specific object. >> >> Tests have been performed by using a CRL file having size 143MB. >> Original delay upon client connection was around 5-8 seconds. >> With this patch the delay gets close to 0. >> >> Signed-off-by: Antonio Quartulli <a...@unstable.cc> > > As discussed on IRC, it might be better to first change the CRL handling code > in > the OpenSSL module to use the internal routines provided by the OpenSSL > library. > (apparently a patch to implement this change is in the work on somebody else's > desk) > > > At that point my patch could be changed to re-use the same code instead of > implementing my own optimized logic. > > > Note: also OpenSSL uses a sorted array + bsearch for CRL handling, therefore > the performance of OpenSSL vs my approach should be similar. > > > Does anybody else have any opinion against this?
First of all, thank you very much for this patch set! As I said on the IRC, this patch-set is well done when it comes to overall style and layout. I have not dug into the details, but the overall quick look gave a good impression. This new feature sounds reasonable. The improved OpenSSL CRL handling will be most welcome. So basing your work on top of that makes a lot of sense, and hopefully it can simplify your patch as well. But I also think that we should look more carefully at the whole CRL handling as a hole unit. To me the --crl-persist looks like a reasonable feature, but I'm still hoping to see more responses to this feature from others as well. But if it turns out that we can implement this feature and also improve the current CRL handling, that will hopefully give even less code duplication and an even simpler patch to when adding the --crl-persist feature. On IRC we also briefly discussed that the mbedTLS CRL code oaths is not too clever and efficient (like, no btree search, just a simple for-loop). I would encourage you to look if you can improve the mbedTLS code too, and provide that patch to them. I'm sure the mbedTLS guys will appreciate that effort. This will again indirectly benefit us, and we need to have even less specialized code to make the mbedTLS integration perform well. So it's a win-win for all of us :) And I'm happy to see more people looking into these parts of the OpenVPN code as well! So thank you! -- kind regards, David Sommerseth
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
