You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2015/09/25 09:27:06 UTC
svn commit: r1705236 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Author: ylavic
Date: Fri Sep 25 07:27:04 2015
New Revision: 1705236
URL: http://svn.apache.org/viewvc?rev=1705236&view=rev
Log:
mod_ssl: don't FLUSH first for non blocking reads.
Such readers are prepared to received empty data anyway (and take appropriate
action), while e.g. check_pipeline() is not expecting the pipe to be flushed
under it.
Modified:
httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c?rev=1705236&r1=1705235&r2=1705236&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Fri Sep 25 07:27:04 2015
@@ -472,9 +472,12 @@ static int bio_filter_in_read(BIO *bio,
* connection or for a proxy connection. Calling _out_flush
* should be very cheap in cases where it is unnecessary (and no
* output is buffered) so the performance impact of doing it
- * unconditionally should be minimal.
+ * unconditionally should be minimal, but in non blocking mode
+ * where the caller may not expect the flush round trip (e.g.
+ * check_pipeline()'s speculative and non-blocking read).
*/
- if (bio_filter_out_flush(inctx->bio_out) < 0) {
+ if (block != APR_NONBLOCK_READ &&
+ bio_filter_out_flush(inctx->bio_out) < 0) {
bio_filter_out_ctx_t *outctx = inctx->bio_out->ptr;
inctx->rc = outctx->rc;
return -1;
Re: svn commit: r1705236 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Sep 29, 2015 at 10:38 AM, Joe Orton <jo...@redhat.com> wrote:
> On Fri, Sep 25, 2015 at 03:54:56PM +0200, Yann Ylavic wrote:
>> Couldn't we completely remove the flush from bio_filter_in_read() then?
>> Or make it conditional to OpenSSL < 0.9.8m (since we support 0.9.8a at least)?
>
> That looks correct to me, +1.
Thanks, committed in r1705823, along with your patch (metadata
passthru) in r1705828, and finally to make a backport proposal in
r1705836.
Regards,
Yann.
Re: svn commit: r1705236 -
/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Joe Orton <jo...@redhat.com>.
On Fri, Sep 25, 2015 at 03:54:56PM +0200, Yann Ylavic wrote:
> Couldn't we completely remove the flush from bio_filter_in_read() then?
> Or make it conditional to OpenSSL < 0.9.8m (since we support 0.9.8a at least)?
That looks correct to me, +1.
Regards, Joe
Re: svn commit: r1705236 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Sep 25, 2015 at 10:30 AM, Yann Ylavic <yl...@gmail.com> wrote:
> On Fri, Sep 25, 2015 at 10:20 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> Any other way around?
>
> Couldn't mod_ssl know by openssl when SSL "administrative" data are
> out and then bucket-FLUSH?
Actually, the discussion in PR 46952 (mentioned in the comment around
the flush) shows that OpenSSL should ask for the flush when necessary,
and that it should have been fixed uptream with:
Changes between 0.9.8l and 0.9.8m [25 Feb 2010]
*) The code that handled flushing of data in SSL/TLS originally used the
BIO_CTRL_INFO ctrl to see if any data was pending first. This caused
the problem outlined in PR#1949. The fix suggested there however can
trigger problems with buggy BIO_CTRL_WPENDING (e.g. some versions
of Apache). So instead simplify the code to flush unconditionally.
This should be fine since flushing with no data to flush is a no op.
[Steve Henson]
Couldn't we completely remove the flush from bio_filter_in_read() then?
Or make it conditional to OpenSSL < 0.9.8m (since we support 0.9.8a at least)?
Re: svn commit: r1705236 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Sep 25, 2015 at 10:20 AM, Yann Ylavic <yl...@gmail.com> wrote:
>
> Any other way around?
Couldn't mod_ssl know by openssl when SSL "administrative" data are
out and then bucket-FLUSH?
Re: svn commit: r1705236 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Sep 25, 2015 at 10:01 AM, Ruediger Pluem <rp...@apache.org> wrote:
>
> On 09/25/2015 09:27 AM, ylavic@apache.org wrote:
>>
>> URL: http://svn.apache.org/viewvc?rev=1705236&view=rev
>>
>
> Can't we run in endless loops here, because no new input will arrive until the output buffer was flushed (e.g. due to
> pending SSL layer data the client is waiting for before it sents new data)? Or do you expect the non blocking readers
> to switch to blocking next time they read?
The idea was that the reader either expects data and blocks, or is
just polling what's available now.
But hm, event/motorz may not follow that scheme, and poll() the socket
directly (w/o flushing) after some previous non-blocking read is
EAGAIN.
Flushing defeats pipelining, so maybe it's not achievable with SSL/TLS...
Any other way around?
Regards,
Yann.
Re: svn commit: r1705236 -
/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 09/25/2015 09:27 AM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Fri Sep 25 07:27:04 2015
> New Revision: 1705236
>
> URL: http://svn.apache.org/viewvc?rev=1705236&view=rev
> Log:
> mod_ssl: don't FLUSH first for non blocking reads.
> Such readers are prepared to received empty data anyway (and take appropriate
> action), while e.g. check_pipeline() is not expecting the pipe to be flushed
> under it.
>
> Modified:
> httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
>
> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c?rev=1705236&r1=1705235&r2=1705236&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Fri Sep 25 07:27:04 2015
> @@ -472,9 +472,12 @@ static int bio_filter_in_read(BIO *bio,
> * connection or for a proxy connection. Calling _out_flush
> * should be very cheap in cases where it is unnecessary (and no
> * output is buffered) so the performance impact of doing it
> - * unconditionally should be minimal.
> + * unconditionally should be minimal, but in non blocking mode
> + * where the caller may not expect the flush round trip (e.g.
> + * check_pipeline()'s speculative and non-blocking read).
> */
> - if (bio_filter_out_flush(inctx->bio_out) < 0) {
> + if (block != APR_NONBLOCK_READ &&
> + bio_filter_out_flush(inctx->bio_out) < 0) {
> bio_filter_out_ctx_t *outctx = inctx->bio_out->ptr;
> inctx->rc = outctx->rc;
> return -1;
>
>
>
Can't we run in endless loops here, because no new input will arrive until the output buffer was flushed (e.g. due to
pending SSL layer data the client is waiting for before it sents new data)? Or do you expect the non blocking readers
to switch to blocking next time they read?
Regards
Rüdiger