FW: [PATCH] when we need to transfer data between file and socket we prefer to use sendfile instead of write because we save the copy to a buffer

[Maxim Dounin]

Hello!

On Mon, May 06, 2019 at 06:02:10AM +0000, Ben Ben Ishay wrote:

[...]

> > - The SSL_sendfile() call you are using in this version does not
> >    seem to exists in any published version of OpenSSL, including
> >    github repo.  This is not going to work.
> > 
> we attach a documentation link about this 
> function(https://github.com/Mellanox/openssl/blob/tls_sendfile/doc/man3/SSL_write.pod), 
> we have a pull request at advanced stage for adding this function.
> 
> > - The approach you are using - that is, introducing changes into
> >    ngx_linux_sendfile_chain.c - is not portable, and is not going
> >    to work on other platforms if/when appropriate kernel level and
> >    OpenSSL level support will be available.  As suggested in the
> >    previous thread, this should be something handled at the
> >    ngx_event_openssl.c level.
> > 
> we think that this is the best solution because there is a unique 
> function for every OS  and there is a difference between them for 
> example ngx_linux_sendfile_chain use TCP_CORK option while 
> ngx_freebsd_sendfile_chain dosent(in addition in solaris function 
> ngx_solaris_sendfilev_chain there is a call to sendfilev that dosent 
> exists in linux) , we can create a function that is constructed from the 
> diffrenet flow for OS's but we think this option include copying code 
> and thus the best option is to change the sendfile call in every 
> ngx_sendfile function when the OS and OPENSSL will support SSL_Sendfile.

While handling of the sendfile() syscall is certainly OS-specific, 
the SSL_sendfile() interface as provided by OpenSSL is certainly 
not going to follow these OS-specific code paths.  For example, I 
cannot reasonably assume SSL_sendfile() will support headers and 
trailers on FreeBSD, or will be similar to sendfilev() interface 
on Solaris.

That is, SSL_sendfile() is going to be a separate interface, 
different from all the OS-specific interfaces, with its own 
features and limitations.  And most likely it will need different 
error handling, including handling of SSL-specific errors.

As such, trying to change OS-specific sendfile chains in nginx 
looks wrong to me.  Not to mention it will require additional 
application-level work upon introduction of in-kernel SSL/TLS in 
other OSes.  Rather, it should be an SSL-level changes, 
introducing an SSL-level send chain, similar to one we already 
provide for SSL_write().

(Note well that using BIO_get_ktls_send() to find out if 
SSL_sendfile() can be used or not also looks strange and 
non-portable.  If the route taken is to provide SSL_sendfile(), an 
OS-independent interface to use sendfile() when supported by the 
OpenSSL library, there should be a simplier / more portable way to 
test if SSL_sendfile() can be used on a particular SSL 
connection.)

-- 
Maxim Dounin
http://mdounin.ru/