Re: mpm_winnt lingering close

2017-04-03 Thread William A Rowe Jr
On Mon, Apr 3, 2017 at 8:21 AM, Eric Covener  wrote:
> On Mon, Apr 3, 2017 at 9:07 AM, Stefan Eissing
>  wrote:
>> Question is: do we "fix" mpm_winnt or is there a better way for mod_http2 to 
>> shutdown the connection before mod_ssl does. This would need to work in 
>> async mpms for any connection state.
>
> I think it's okay to add the prep cal when short-circuiting lingering
> close, but it seems like very little extra will be running in the full
> call either.

Jeff Trawick knew this bit of logic better than most any of us, I'd love
to hear his thoughts on the cleanest solution. But fixing mpm_winnt
to behave as the other MPM's would be worthwhile. It's also worth
looking at third party MPM's such as mpm_itk to see if we have an
underlying bug that must be fixed.

I presume we still allow the disconnected socket to be recycled, which
was the underlying idea behind the shortcut/optimization. It seems that
the shortcut isn't (valid).


Re: mpm_winnt lingering close

2017-04-03 Thread Eric Covener
On Mon, Apr 3, 2017 at 9:07 AM, Stefan Eissing
 wrote:
> Question is: do we "fix" mpm_winnt or is there a better way for mod_http2 to 
> shutdown the connection before mod_ssl does. This would need to work in async 
> mpms for any connection state.

I think it's okay to add the prep cal when short-circuiting lingering
close, but it seems like very little extra will be running in the full
call either.

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


mpm_winnt lingering close

2017-04-03 Thread Stefan Eissing
Steffen started testing and immediately found sth. Yeah! He sees many

[Mon Apr 03 14:20:28.390474 2017] [http2:warn] [pid 3180:tid 2880] [client 
5.80.147.209:61342] AH10020: h2_session(106,DONE,0): session cleanup triggered 
by pool cleanup. this should have happened earlier already.

in his logs. The problem here is that mod_http2 would like to shutdown in 
ap_prep_lingering_close()
which is invoked by ap_lingering_close(). This is always done by mpm_event, 
worker and prefork AFAIK.

mpm_winnt has an optimization in line 814:
ap_process_connection(c, context->sock);

apr_socket_opt_get(context->sock, APR_SO_DISCONNECTED, );

if (!disconnected) {
context->accept_socket = INVALID_SOCKET;
if (!c->aborted) { 
ap_lingering_close(c);
}
}

So, if the connection is already gone or was aborted, it will not call
this and the "safe" shutdown is not triggered. "safe" insofar as
mod_http2 needs to shutdown the connection *before* mod_ssl does.

Question is: do we "fix" mpm_winnt or is there a better way for mod_http2 to 
shutdown the connection before mod_ssl does. This would need to work in async 
mpms for any connection state.

-Stefan

Re: Upgrade header

2017-04-03 Thread Stefan Eissing
https://tools.ietf.org/html/rfc7230#section-6.7:

"A server MAY send an Upgrade header field in any other response to
   advertise that it implements support for upgrading to the listed
   protocols, in order of descending preference, when appropriate for a
   future request."

Yes, it is intentional to advertise this to clients on the first request
of a connection. To disable this, you can "unset" such a header via
mod_headers.

Hope this helps.

-Stefan

> Am 03.04.2017 um 13:10 schrieb Sander Hoentjen :
> 
> Hi Stefan (and others),
> 
> Right now when I enable h2 and/or h2c Apache will respond with headers
> "Upgrade: h2,h2c" and Connection: Upgrade
> As I understand it this is the wrong way around. Only the client should
> send the Upgrade headers, and only then should the server respond with a
> 101 with the Upgrade header only specifying the proto that is being
> switched to.
> Is this a bug, or does Apache respond intentionally with the Upgrade
> header? If intentional, then what is the reason?
> 
> Regards,
> Sander



mod_http2 v1.10.0

2017-04-03 Thread Stefan Eissing
Hi there,

a new version of mod_http2 has been backported to 2.4.x and there is also a 
github release, as usual: https://github.com/icing/mod_h2/releases/tag/v1.10.0

I rewrote some key parts of scheduling and slave connection handling for better 
performance. It shows good results. However it needs more testing than just by 
myself. If you have a HTTP/2 setup where you can risk it, please give it a try. 
The github builds against a 2.4.25 server, so you do not need to got totally 
into the unknown. Thanks!

-Stefan

PS. I plan to write a blog about it, but the basic store is in this picture.  
This is hammering localhost with a list of 180 urls over and over with h2load. 
Most interesting are the changes to the green bars. The yellow shows that there 
is still a large gap between what HTTP/1.1 can do on 6 connections compared to 
HTTP/2 on a single one. But we're getting closer.


Upgrade header

2017-04-03 Thread Sander Hoentjen
Hi Stefan (and others),

Right now when I enable h2 and/or h2c Apache will respond with headers
"Upgrade: h2,h2c" and Connection: Upgrade
As I understand it this is the wrong way around. Only the client should
send the Upgrade headers, and only then should the server respond with a
101 with the Upgrade header only specifying the proto that is being
switched to.
Is this a bug, or does Apache respond intentionally with the Upgrade
header? If intentional, then what is the reason?

Regards,
Sander


Re: mod_remoteip and mod_http2 combined

2017-04-03 Thread Stefan Eissing
I can see that a flat directive namespace has its drawbacks... ;-)

> Am 01.04.2017 um 19:12 schrieb Daniel Ruggeri :
> 
> 
> On 4/1/2017 11:18 AM, Yann Ylavic wrote:
>> Hi Daniel,
>> 
>> On Sat, Apr 1, 2017 at 3:56 PM, Daniel Ruggeri  wrote:
>>> I went with the directive name
>>> RemoteIPProxyProtocolDisableHosts to align more with the fact that a
>>> single host or range can be disabled.
>> How about RemoteIPProxyProtocolExceptions since one can configure
>> either an IP or a network?
>> Host looks a bit ambiguous to me, especially with HTTP...
>> 
>> 
>> Regards,
>> Yann.
> 
> Oh, yeah, I like that even better. I'll stage this up and will commit in
> a day or two unless we have some better suggestions.
> 
> -- 
> Daniel Ruggeri
>