Re: httpd 2.4.25, mpm_event, ssl: segfaults

2017-02-17 Thread William A Rowe Jr
On Feb 17, 2017 2:52 PM, "William A Rowe Jr"  wrote:

On Feb 17, 2017 1:02 PM, "Jacob Champion"  wrote:

`EnableMMAP on` appears to boost performance for static files, yes, but is
that because of mmap() itself, or because our bucket brigades configure
themselves more optimally in the mmap() code path? Yann's research is
starting to point towards the latter IMO.


This may be as simple as the page manager caching and reusing the
un-cleared page mapping on subsequent hits. You would need to
overwhelmingly vary the page content served to test this theory. But the
same caching wins for libld[l] ... Which doesn't segv during os updates.
Probably due to copy-on-write mechanics.


(With traditional and sendfile, you still have copy once on read - even if
that file is sitting in FS cache.)


Re: httpd 2.4.25, mpm_event, ssl: segfaults

2017-02-17 Thread William A Rowe Jr
On Feb 17, 2017 1:02 PM, "Jacob Champion"  wrote:

`EnableMMAP on` appears to boost performance for static files, yes, but is
that because of mmap() itself, or because our bucket brigades configure
themselves more optimally in the mmap() code path? Yann's research is
starting to point towards the latter IMO.


This may be as simple as the page manager caching and reusing the
un-cleared page mapping on subsequent hits. You would need to
overwhelmingly vary the page content served to test this theory. But the
same caching wins for libld[l] ... Which doesn't segv during os updates.
Probably due to copy-on-write mechanics.


Re: httpd 2.4.25, mpm_event, ssl: segfaults

2017-02-17 Thread Jacob Champion

On 02/17/2017 07:04 AM, Daniel Lescohier wrote:

Is the high-level issue that: for serving static content over HTTP, you
can use sendfile() from the OS filesystem cache, avoiding extra
userspace copying; but if it's SSL, or any other dynamic filtering of
content, you have to do extra work in userspace?


Yes -- there are a bunch of potential high-level issues, but the one 
you've highlighted is the reason that I wouldn't expect our HTTPS 
implementation to ever get as fast as HTTP for static responses. At 
least not given the current architecture.


(There are potential kernel-level encryption APIs that are popping up; I 
keep hoping someone will start playing around with AF_ALG sockets. But 
those aren't magic, either; we still have to layer TLS around the 
encrypted data.)


That said, that's not what I'm trying to focus on with this thread. I 
have a feeling our performance is being artificially limited. 
`EnableMMAP on` appears to boost performance for static files, yes, but 
is that because of mmap() itself, or because our bucket brigades 
configure themselves more optimally in the mmap() code path? Yann's 
research is starting to point towards the latter IMO.


--Jacob


Re: svn commit: r1783413 - in /httpd/httpd/branches/2.4.x: ./ STATUS server/mpm/event/event.c

2017-02-17 Thread Marion & Christophe JAILLET

No CHANGES entry?

CJ


Le 17/02/2017 à 16:36, j...@apache.org a écrit :

Author: jim
Date: Fri Feb 17 15:36:02 2017
New Revision: 1783413

URL: http://svn.apache.org/viewvc?rev=1783413=rev
Log:
Merge r1774541 from trunk:

event: close a race condition where we might re-enable listeners while they
are already or about to be closed.


Submitted by: ylavic
Reviewed by: ylavic, jim, wrowe

Modified:
 httpd/httpd/branches/2.4.x/   (props changed)
 httpd/httpd/branches/2.4.x/STATUS
 httpd/httpd/branches/2.4.x/server/mpm/event/event.c


Re: [2.2 PATCH] fix HttpProtocolOptions (etc) merging

2017-02-17 Thread Eric Covener
+1

On Fri, Feb 17, 2017 at 12:37 PM, William A Rowe Jr  wrote:
> Great catch; +1 to commit to 2.2.x and
> http://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x-merge-http-strict/
> branches.
>
> And thanks for adding the breadcrumb for the next sucker to miss this :-O
>
> On Fri, Feb 17, 2017 at 3:30 AM, Joe Orton  wrote:
>> Found during QA of the CVE-2016-8743 patch here.  The merging logic in
>> merge_core_server_configs is (confusingly) inverted from 2.2 to 2.4, so
>> e.g. HttpProtocolOptions doesn't inherit from global to vhost configs in
>> 2.2.32. :(
>>
>> Index: server/core.c
>> ===
>> --- server/core.c   (revision 1783354)
>> +++ server/core.c   (working copy)
>> @@ -546,15 +546,19 @@
>> ? virt->merge_trailers
>> : base->merge_trailers;
>>
>> -if (virt->http09_enable != AP_HTTP09_UNSET)
>> -conf->http09_enable = virt->http09_enable;
>> +if (conf->http09_enable == AP_HTTP09_UNSET)
>> +conf->http09_enable = base->http09_enable;
>>
>> -if (virt->http_conformance != AP_HTTP_CONFORMANCE_UNSET)
>> -conf->http_conformance = virt->http_conformance;
>> +if (conf->http_conformance == AP_HTTP_CONFORMANCE_UNSET)
>> +conf->http_conformance = base->http_conformance;
>>
>> -if (virt->http_methods != AP_HTTP_METHODS_UNSET)
>> -conf->http_methods = virt->http_methods;
>> +if (conf->http_methods == AP_HTTP_METHODS_UNSET)
>> +conf->http_methods = base->http_methods;
>>
>> +/* N.B. If you backport things here from 2.4, note that the
>> + * merging logic needs to be inverted, since conf is initially a
>> + * copy of vertv not basev. */
>> +
>>  return conf;
>>  }
>>
>>



-- 
Eric Covener
cove...@gmail.com


Re: [2.2 PATCH] fix HttpProtocolOptions (etc) merging

2017-02-17 Thread William A Rowe Jr
Great catch; +1 to commit to 2.2.x and
http://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x-merge-http-strict/
branches.

And thanks for adding the breadcrumb for the next sucker to miss this :-O

On Fri, Feb 17, 2017 at 3:30 AM, Joe Orton  wrote:
> Found during QA of the CVE-2016-8743 patch here.  The merging logic in
> merge_core_server_configs is (confusingly) inverted from 2.2 to 2.4, so
> e.g. HttpProtocolOptions doesn't inherit from global to vhost configs in
> 2.2.32. :(
>
> Index: server/core.c
> ===
> --- server/core.c   (revision 1783354)
> +++ server/core.c   (working copy)
> @@ -546,15 +546,19 @@
> ? virt->merge_trailers
> : base->merge_trailers;
>
> -if (virt->http09_enable != AP_HTTP09_UNSET)
> -conf->http09_enable = virt->http09_enable;
> +if (conf->http09_enable == AP_HTTP09_UNSET)
> +conf->http09_enable = base->http09_enable;
>
> -if (virt->http_conformance != AP_HTTP_CONFORMANCE_UNSET)
> -conf->http_conformance = virt->http_conformance;
> +if (conf->http_conformance == AP_HTTP_CONFORMANCE_UNSET)
> +conf->http_conformance = base->http_conformance;
>
> -if (virt->http_methods != AP_HTTP_METHODS_UNSET)
> -conf->http_methods = virt->http_methods;
> +if (conf->http_methods == AP_HTTP_METHODS_UNSET)
> +conf->http_methods = base->http_methods;
>
> +/* N.B. If you backport things here from 2.4, note that the
> + * merging logic needs to be inverted, since conf is initially a
> + * copy of vertv not basev. */
> +
>  return conf;
>  }
>
>


Re: mod_proxy_http2 sni ?

2017-02-17 Thread Steffen
Looks like the same, is not looking for the host as with 1.1

It is on my wish list for 2.4.26 



> Op 16 feb. 2017 om 11:38 heeft Stefan Eissing  
> het volgende geschreven:
> 
> Is this the same as https://github.com/icing/mod_h2/issues/124 ?
> 
> It seems that the ProxyPreserveHost is not (correctly) implemented.
> 
>> Am 16.02.2017 um 10:42 schrieb Steffen :
>> 
>> 
>> Have an Apache ssl only in front of an Apache on port 80 with several vhosts.
>> 
>> In front have:
>> 
>> 
>> ProtocolsHonorOrder On
>> Protocols h2 http/1.1
>> LoadModule http2_module modules/mod_http2.so
>> 
>> 
>> 
>> ProxyPass / http://127.0.0.1:80/
>> ProxyPassReverse / http://127.0.0.1:80/
>> 
>> 
>> In backend have:
>> 
>> 
>> ProtocolsHonorOrder On
>> Protocols h2c http/1.1
>> LoadModule http2_module modules/mod_http2.so
>> 
>> This is working great and with all the vhosts.
>> 
>> 
>> When I add/change the front to:
>> 
>> 
>> ProtocolsHonorOrder On
>> Protocols h2 http/1.1
>> LoadModule http2_module modules/mod_http2.so
>> LoadModule proxy_http2_module modules/mod_proxy_http2.so
>> 
>> 
>> ProxyPass / h2c://127.0.0.1:80/
>> ProxyPassReverse / h2c://127.0.0.1:80/
>> 
>> 
>> This is not working as expected, all is going to the default/first vhost.
>> 
>> a log line from the backend gives is all cases not found .
>> 
>> default 127.0.0.1 - - [16/Feb/2017:10:22:00 +0100] "GET /index.php HTTP/2.0" 
>> 404 207 ...
>> 
>> 
>> Cheers,
>> 
>> Steffenal
>> 
>> 
> 
> Stefan Eissing
> 
> bytes GmbH
> Hafenstrasse 16
> 48155 Münster
> www.greenbytes.de
> 



Re: httpd 2.4.25, mpm_event, ssl: segfaults

2017-02-17 Thread Daniel Lescohier
Is the high-level issue that: for serving static content over HTTP, you can
use sendfile() from the OS filesystem cache, avoiding extra userspace
copying; but if it's SSL, or any other dynamic filtering of content, you
have to do extra work in userspace?


On Thu, Feb 16, 2017 at 6:01 PM, Yann Ylavic  wrote:

> On Thu, Feb 16, 2017 at 10:51 PM, Jacob Champion 
> wrote:
> > On 02/16/2017 02:49 AM, Yann Ylavic wrote:
> >>
> >> +#define FILE_BUCKET_BUFF_SIZE (64 * 1024 - 64) /* >
> APR_BUCKET_BUFF_SIZE
> >> */
> >
> >
> > So, I had already hacked my O_DIRECT bucket case to just be a copy of
> APR's
> > file bucket, minus the mmap() logic. I tried making this change on top of
> > it...
> >
> > ...and holy crap, for regular HTTP it's *faster* than our current mmap()
> > implementation. HTTPS is still slower than with mmap, but faster than it
> was
> > without the change. (And the HTTPS performance has been really variable.)
> >
> > Can you confirm that you see a major performance improvement with the
> with
> > the new 64K file buffer?
>
> I can't test speed for now (stick with my laptop/localhost, which
> won't be relevant enough I guess).
>
> > I'm pretty skeptical of my own results at this
> > point... but if you see it too, I think we need to make *all* these
> > hard-coded numbers tunable in the config.
>
> We could also improve the apr_bucket_alloc()ator to recycle more
> order-n allocations possibilities (saving as much
> {apr_allocator_,m}alloc() calls), along with configurable/higher
> orders in httpd that'd be great I think.
>
> I can try this patch...
>


Re: svn commit: r1783317 - /httpd/httpd/trunk/modules/ssl/ssl_engine_init.c

2017-02-17 Thread Ruediger Pluem


On 02/16/2017 11:27 PM, wr...@apache.org wrote:
> Author: wrowe
> Date: Thu Feb 16 22:27:24 2017
> New Revision: 1783317
> 
> URL: http://svn.apache.org/viewvc?rev=1783317=rev
> Log:
> Avoid unnecessary code (the deprecation macro wrapper itself emits unused args
> warnings) in OpenSSL 1.1.0 and avoid _free()ing NULL references.
> 
> 
> Modified:
> httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
> 
> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_init.c?rev=1783317=1783316=1783317=diff
> ==
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_init.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_init.c Thu Feb 16 22:27:24 2017
> @@ -1320,19 +1320,24 @@ static apr_status_t ssl_init_server_cert
>   OBJ_nid2sn(nid), vhost_id, certfile);
>  }
>  /*
> - * ...otherwise, enable auto curve selection (OpenSSL 1.0.2 and later)
> + * ...otherwise, enable auto curve selection (OpenSSL 1.0.2)
>   * or configure NIST P-256 (required to enable ECDHE for earlier 
> versions)
> + * ECDH is always enabled in 1.0.2 unless excluded from SSLCipherList

Shouldn't that be 1.1.0 above instead of 1.0.2?

>   */
> +#if (OPENSSL_VERSION_NUMBER < 0x1010L)
>  else {
> -#if defined(SSL_CTX_set_ecdh_auto)
> +#elif defined(SSL_CTX_set_ecdh_auto)
>  SSL_CTX_set_ecdh_auto(mctx->ssl_ctx, 1);
>  #else
>  eckey = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);
>  SSL_CTX_set_tmp_ecdh(mctx->ssl_ctx, eckey);
>  #endif
>  }
> -EC_KEY_free(eckey);
> -EC_GROUP_free(ecparams);
> +#endif
> +if (eckey)
> +EC_KEY_free(eckey);
> +if (ecparams)
> +EC_GROUP_free(ecparams);
>  #endif
>  
>  return APR_SUCCESS;
> 
> 
> 


Regards

Rüdiger


Re: Topic for discussion... 2.4.26

2017-02-17 Thread Jim Jagielski
All IMHO:

> On Feb 16, 2017, at 6:46 PM, William A Rowe Jr  wrote:
> 
> With the passing of OpenSSL 1.0.1, is OpenSSL 1.1.0 on our radar for the next 
> release?

Depends on the status of the patch support...

> 
> I'm not clear how that merge branch is intended to be used, I'm don't 
> understand whether we propose to adopt every feature and API change commit to 
> modules/ssl/* - and why it has been rebased, unless we intend to svn cp the 
> resulting tree on top of modules/ssl/.
> 
> I'm set up to review it against 1.1.0, if I understood how that branch would 
> be applied.
> 
> 
> On Feb 16, 2017 11:25 AM, "Jim Jagielski"  wrote:
> Would be nice, I think, to start discussion on a T of 2.4.26 and
> to open the doors to who wants to RM. Note, that if *nobody*
> offers to RM, I will... and no matter what, I offer to help
> whoever wishes to RM.



Re: Topic for discussion... 2.4.26

2017-02-17 Thread Luca Toscano
My personal wishlist:

1) Openssl 1.1.x support, a lot of people are asking for it in various
support channels and it seems important to catch up with others project
that already support it :)

2) Yann's work on mpm-event to remove the unnecessary 100ms of polling even
when idling. I am really looking forward to see this patch on 2.4.x, but it
might need more testing on platform different from Linux.

3) New mod_http2 features from Stefan to fix recent issues reported in dev@

4) mod_proxy_fcgi rework from Jim, Jacob and Eric.


Luca

2017-02-17 10:12 GMT+01:00 Stefan Eissing :

> Also interested in the state of the openssl 1.1.0 support. Having it in
> the next release would be great. OpenSSL has promised TLS 1.3 beginning of
> April as a drop in against the 1.1.0 ABI - which remains to be seem if that
> works, but would be nice to be ready for it.
>
> > Am 17.02.2017 um 00:46 schrieb William A Rowe Jr :
> >
> > With the passing of OpenSSL 1.0.1, is OpenSSL 1.1.0 on our radar for the
> next release?
> >
> > I'm not clear how that merge branch is intended to be used, I'm don't
> understand whether we propose to adopt every feature and API change commit
> to modules/ssl/* - and why it has been rebased, unless we intend to svn cp
> the resulting tree on top of modules/ssl/.
> >
> > I'm set up to review it against 1.1.0, if I understood how that branch
> would be applied.
> >
> >
> > On Feb 16, 2017 11:25 AM, "Jim Jagielski"  wrote:
> > Would be nice, I think, to start discussion on a T of 2.4.26 and
> > to open the doors to who wants to RM. Note, that if *nobody*
> > offers to RM, I will... and no matter what, I offer to help
> > whoever wishes to RM.
>
> Stefan Eissing
>
> bytes GmbH
> Hafenstrasse 16
> 48155 Münster
> www.greenbytes.de
>
>


[2.2 PATCH] fix HttpProtocolOptions (etc) merging

2017-02-17 Thread Joe Orton
Found during QA of the CVE-2016-8743 patch here.  The merging logic in 
merge_core_server_configs is (confusingly) inverted from 2.2 to 2.4, so 
e.g. HttpProtocolOptions doesn't inherit from global to vhost configs in 
2.2.32. :(

Index: server/core.c
===
--- server/core.c   (revision 1783354)
+++ server/core.c   (working copy)
@@ -546,15 +546,19 @@
? virt->merge_trailers
: base->merge_trailers;
 
-if (virt->http09_enable != AP_HTTP09_UNSET)
-conf->http09_enable = virt->http09_enable;
+if (conf->http09_enable == AP_HTTP09_UNSET)
+conf->http09_enable = base->http09_enable;
 
-if (virt->http_conformance != AP_HTTP_CONFORMANCE_UNSET)
-conf->http_conformance = virt->http_conformance;
+if (conf->http_conformance == AP_HTTP_CONFORMANCE_UNSET)
+conf->http_conformance = base->http_conformance;
 
-if (virt->http_methods != AP_HTTP_METHODS_UNSET)
-conf->http_methods = virt->http_methods;
+if (conf->http_methods == AP_HTTP_METHODS_UNSET)
+conf->http_methods = base->http_methods;
 
+/* N.B. If you backport things here from 2.4, note that the
+ * merging logic needs to be inverted, since conf is initially a
+ * copy of vertv not basev. */
+
 return conf;
 }
 



Re: Topic for discussion... 2.4.26

2017-02-17 Thread Stefan Eissing
Also interested in the state of the openssl 1.1.0 support. Having it in the 
next release would be great. OpenSSL has promised TLS 1.3 beginning of April as 
a drop in against the 1.1.0 ABI - which remains to be seem if that works, but 
would be nice to be ready for it.

> Am 17.02.2017 um 00:46 schrieb William A Rowe Jr :
> 
> With the passing of OpenSSL 1.0.1, is OpenSSL 1.1.0 on our radar for the next 
> release?
> 
> I'm not clear how that merge branch is intended to be used, I'm don't 
> understand whether we propose to adopt every feature and API change commit to 
> modules/ssl/* - and why it has been rebased, unless we intend to svn cp the 
> resulting tree on top of modules/ssl/.
> 
> I'm set up to review it against 1.1.0, if I understood how that branch would 
> be applied.
> 
> 
> On Feb 16, 2017 11:25 AM, "Jim Jagielski"  wrote:
> Would be nice, I think, to start discussion on a T of 2.4.26 and
> to open the doors to who wants to RM. Note, that if *nobody*
> offers to RM, I will... and no matter what, I offer to help
> whoever wishes to RM.

Stefan Eissing

bytes GmbH
Hafenstrasse 16
48155 Münster
www.greenbytes.de