On Fri, May 08, 2020 at 04:34:27PM +0200, William Dauchy wrote:
> experimental patch to make use of IO_URING to batch load certificates;
> this drastically reduces the number of syscall and might benefit to
> setup with large number of certificates.
> it uses liburing in order to batch operation as follow:
> for each certificate directory, we apply the same operations on each
> file:
>  - statx
>  - openat
>  - read
>  - close
> the results are stored in an ebtree. Then when we need to load them
> with the SSL lib, instead of providing a path, we provide a buffer to
> be consumed. The tree is freed after each directory.
> 
> for now it requires a quite large limit of file descriptors, as all
> operations types are done one after another; so the limit of fd should
> be set higher than the number of certificates you have to load. This
> part is probably going to evolve very soon as IO_URING plans are to be
> able to chain operations with a given pre-defined file descriptor.
> 
> on a setup with 25k certificates I was able to measure a minimum of 20%
> gain on the init time when the filesystem cache is empty. My testing is
> as follow; for each directory:
> - put a timer before `ssl_sock_load_cert_list_file`, including
>   `ssl_load_certiodir` in the case of io_uring case.
> - measure the time at the end of those operations, just after
>   `ssl_free_certiodir` in the case of io_uring.

I'm wondering if we coud have the timing directly in the logs or on the
CLI, because I'm actually also doing this kind of things, and people
could probably optimize the loading if they could see the loading time.

Your patch is really interesting in my opinion, a lot of people could
benefits of something like that. Regarding the reload part, I have some
ideas to pass the ckch structures to the new processes without reloading
the files one the filesystem, but I didn't implement anything yet.

I'm currently reworking the ssl_sock.c and .h files, and splitting them
in several files, so you will probably need to rebase your patch on top
of this. At the moment I separated this in 3 parts: 1 part for the
crt-list/directories, 1 part for the ckch, and 1 part for the
SSL_CTX/bind. I'm not happy yet with what I have, but I'll do my best to
push that as soon as possible in the git repository, I don't want the
2.2 to be released in the current state of the files.

I don't really know the io_uring API in details so I trust you on the
implementation. 

Regarding the name of the file, ssl_load.c, it is really generic, I
don't mind having a more specific file only for loading the SSL
certificates with io_uring.

I'm wondering if we couldn't push directly the certificate in an X509
struture instead of storing it in a intermediate buffer, and use the
ckch instead of a specific cert_iobuf. That just some thoughts, I don't
know io_uring enough.

The crtlist files could also benefits from this, since they can contain
a lot of certificates too.

I'm seeing that you are using in ssl_load_preparetree() the
SSL_SOCK_KEYTYPES_NAME array, which is only used for merging a
certificate bundle. I know that this can be confusing in the code
because the bundle implementation is crappy. But in a directory we
don't use any filename extension to load a PEM, we load everything as a
PEM file with the exception of .key, .ocsp, .sctl and .issuer, which are
loaded separatly with their own functions. (The cert_exts[] array is
used for that in ssl_sock.c)

I'm going to remove the support for multi-cert bundle once 2.2 is
released, so it will simplify a lot of things. I encourage people
writing new features to not support multi-cert bundles, more
particularly on the CLI.

Unfortunately your patch is too late for 2.2, but I think it could be
great for the next release!

Regards,

-- 
William Lallemand

Reply via email to