Re: svn commit: r1788981 - in /httpd/httpd/trunk: ./ modules/http2/

2017-03-27 Thread Stefan Eissing
Good catch. Will add a check. Should be easy. 

-Stefan

> Am 27.03.2017 um 22:51 schrieb Jacob Champion :
> 
>> On 03/27/2017 09:31 AM, ic...@apache.org wrote:
>> @@ -500,7 +533,11 @@ h2_stream *h2_stream_create(int id, apr_
>> 
>> h2_beam_create(&stream->output, pool, id, "output", H2_BEAM_OWNER_RECV, 
>> 0,
>>session->s->timeout);
>> -
>> +
>> +stream->in_window_size =
>> +nghttp2_session_get_stream_local_window_size(
>> +stream->session->ngh2, stream->id);
>> +
> 
> The configure script doesn't look for the "new" 
> get_stream_local_window_size() API, so trying to build against an earlier 
> libnghttp2 succeeds during configuration but fails at linktime. Is there a 
> good fallback to use, or should we update the version prerequisites for 
> nghttp2?
> 
> --Jacob



Re: svn commit: r1788981 - in /httpd/httpd/trunk: ./ modules/http2/

2017-03-27 Thread Jacob Champion

On 03/27/2017 09:31 AM, ic...@apache.org wrote:

@@ -500,7 +533,11 @@ h2_stream *h2_stream_create(int id, apr_

 h2_beam_create(&stream->output, pool, id, "output", H2_BEAM_OWNER_RECV, 0,
session->s->timeout);
-
+
+stream->in_window_size =
+nghttp2_session_get_stream_local_window_size(
+stream->session->ngh2, stream->id);
+


The configure script doesn't look for the "new" 
get_stream_local_window_size() API, so trying to build against an 
earlier libnghttp2 succeeds during configuration but fails at linktime. 
Is there a good fallback to use, or should we update the version 
prerequisites for nghttp2?


--Jacob


Re: svn commit: r1772331 [1/2] - in /httpd/httpd/branches/2.4.x: ./ docs/manual/ modules/cache/

2017-03-27 Thread Christophe JAILLET

The following commit also includes r1496711 but it is not listed.

Is it intentional?

CJ

Le 02/12/2016 à 12:42, j...@apache.org a écrit :

Author: jim
Date: Fri Dec  2 11:42:13 2016
New Revision: 1772331

URL: http://svn.apache.org/viewvc?rev=1772331&view=rev
Log:
Merge r1597533, r1649491, r1665216, r1756553, r1756631, r1726675, r1718496, 
r1718476, r1747469 from trunk:

mod_cache: try to use the key of a possible open but stale cache entry
if we have one in cache_try_lock(). PR 50317

Submitted by: Ruediger Pluem


* modules/cache/mod_socache_memcache.c (socache_mc_store): Pass
   through expiration time.

Submitted by: Faidon Liambotis , jorton


* mod_cache: Preserve the Content-Type in case of 304 response.
304 does not contain Content-Type and mod_mime regenerates
the Content-Type based on the r->filename. This later leads to original
Content-Type to be lost (overwriten by whatever mod_mime generates).


mod_cache: Use the actual URI path and query-string for identifying the
cached entity (key), such that rewrites are taken into account when
running afterwards (CacheQuickHandler off).  PR 21935.
  



mod_cache: follow up to r1756553: log the real/actual cached URI (debug).



better s-maxage support
  
+  *) mod_cache: Consider Cache-Control: s-maxage in expiration

+ calculations.  [Eric Covener]
+
+  *) mod_cache: Allow caching of responses with an Expires header
+ in the past that also has Cache-Control: max-age or s-maxage.
+ PR55156. [Eric Covener]




remove dead code leftover from r1023387.

Prior to this revision, there was an apr_atoi64 in this context.
Now, ap_cache_control() sets control.max_age (which is checked here) when
the maxage value was parsed OK.



duplicate debug-level AH00764 in the just-validated path.



Rename ap_casecmpstr[n]() to ap_cstr_casecmp[n](), update with APR doxygen
Submitted by: jkaluza, jorton, jkaluza, ylavic, ylavic, covener, covener, 
covener, wrowe
Reviewed/backported by: jim

Modified:
 httpd/httpd/branches/2.4.x/   (props changed)
 httpd/httpd/branches/2.4.x/CHANGES
 httpd/httpd/branches/2.4.x/STATUS
 httpd/httpd/branches/2.4.x/docs/manual/   (props changed)
 httpd/httpd/branches/2.4.x/modules/cache/cache_storage.c
 httpd/httpd/branches/2.4.x/modules/cache/cache_util.c
 httpd/httpd/branches/2.4.x/modules/cache/cache_util.h
 httpd/httpd/branches/2.4.x/modules/cache/mod_cache.c
 httpd/httpd/branches/2.4.x/modules/cache/mod_cache_disk.c
 httpd/httpd/branches/2.4.x/modules/cache/mod_file_cache.c
 httpd/httpd/branches/2.4.x/modules/cache/mod_socache_memcache.c






Re: svn commit: r1788672 - in /httpd/httpd/trunk: ./ modules/http2/

2017-03-27 Thread Christophe JAILLET

Le 25/03/2017 à 17:07, ic...@apache.org a écrit :

Author: icing
Date: Sat Mar 25 16:07:30 2017
New Revision: 1788672

URL: http://svn.apache.org/viewvc?rev=1788672&view=rev
Log:
On the trunk:

mod_http2: h2 workers with improved scalability for better scheduling
  performance. There are H2MaxWorkers threads created at start and the
  number is kept constant.


[...]


+static apr_status_t get_next(h2_slot *slot)
  {
+h2_workers *workers = slot->workers;
  apr_status_t status;
-apr_time_t wait_until = 0, now;
-h2_workers *workers = ctx;
-h2_task *task = NULL;
-
-*ptask = NULL;
-*psticky = 0;
  
-status = apr_thread_mutex_lock(workers->lock);

-if (status == APR_SUCCESS) {
-++workers->idle_workers;
-ap_log_error(APLOG_MARK, APLOG_TRACE3, 0, workers->s,
- "h2_worker(%d): looking for work", worker->id);
-
-while (!h2_worker_is_aborted(worker) && !workers->aborted
-   && !(task = next_task(workers))) {
-
-/* Need to wait for a new tasks to arrive. If we are above
- * minimum workers, we do a timed wait. When timeout occurs
- * and we have still more workers, we shut down one after
- * the other. */
-cleanup_zombies(workers, 0);
-if (workers->worker_count > workers->min_workers) {
-now = apr_time_now();
-if (now >= wait_until) {
-wait_until = now + 
apr_time_from_sec(workers->max_idle_secs);
-}
-
-ap_log_error(APLOG_MARK, APLOG_TRACE3, 0, workers->s,
- "h2_worker(%d): waiting signal, "
- "workers=%d, idle=%d", worker->id,
- (int)workers->worker_count,
- workers->idle_workers);
-status = apr_thread_cond_timedwait(workers->mplx_added,
-   workers->lock,
-   wait_until - now);
-if (status == APR_TIMEUP
-&& workers->worker_count > workers->min_workers) {
-/* waited long enough without getting a task and
- * we are above min workers, abort this one. */
-ap_log_error(APLOG_MARK, APLOG_TRACE3, 0,
- workers->s,
- "h2_workers: aborting idle worker");
-h2_worker_abort(worker);
-break;
-}
-}
-else {
-ap_log_error(APLOG_MARK, APLOG_TRACE3, 0, workers->s,
- "h2_worker(%d): waiting signal (eternal), "
- "worker_count=%d, idle=%d", worker->id,
- (int)workers->worker_count,
- workers->idle_workers);
-apr_thread_cond_wait(workers->mplx_added, workers->lock);
-}
+slot->task = NULL;
+while (!slot->aborted) {
+if (!slot->task) {
+status = h2_fifo_try_peek(workers->mplxs, mplx_peek, slot);
  }

New compilation warning with gcc 6.3.0.
   variable ‘status’ set but not used



Re: svn commit: r1788672 - in /httpd/httpd/trunk: ./ modules/http2/

2017-03-27 Thread Christophe JAILLET

Le 25/03/2017 à 17:07, ic...@apache.org a écrit :

Author: icing
Date: Sat Mar 25 16:07:30 2017
New Revision: 1788672

URL: http://svn.apache.org/viewvc?rev=1788672&view=rev
Log:
On the trunk:

mod_http2: h2 workers with improved scalability for better scheduling
  performance. There are H2MaxWorkers threads created at start and the
  number is kept constant.

[...]

+apr_status_t h2_fifo_peek(h2_fifo *fifo, h2_fifo_peek_fn *fn, void *ctx)
+{
+return fifo_peek(fifo, fn, ctx, 0);
+}

should have 1 as the last parameter (block)?


+
+apr_status_t h2_fifo_try_peek(h2_fifo *fifo, h2_fifo_peek_fn *fn, void *ctx)
+{
+return fifo_peek(fifo, fn, ctx, 0);
+}
+




Re: [users@httpd] Problem when using nested if statements in apache 2.4

2017-03-27 Thread Luca Toscano
Hi Yann,

2017-03-27 8:56 GMT+02:00 Yann Ylavic :

> Hi Luca,
>
> On Mon, Mar 20, 2017 at 1:25 PM, Luca Toscano 
> wrote:
> >
> > Documentation updated with the current status, plus the following patch
> > seems to allow nested if blocks (probably not the best one but it is a
> pof):
> >
> > http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks.patch
>
> LGTM (nit: ap_if_walk_sub() needs not be AP_DECLARE()d since it's a
> local helper only).
>

Thanks a lot for the feedback, I modified the patch to include your
suggestion and another one from Jim (checking whether or not the
ap_if_walk_sub calls return something different than OK, hope that the
implementation is what was expected). Ran also the test suite in trunk, no
failure registered (Jacob: make check is really awesome).

I'd like to perform some performance tests before proceeding, this code
adds a (probably negligible in 90% of the use case) overhead to each
ap_if_walk call (running twice for each request afaics).

Any other comment/review will be really appreciated :)

Luca


upload performance

2017-03-27 Thread Stefan Eissing
FYI, checked in some improvements in upload performance on a single connection:

curl upload of a 24.5 GB file to a static resource, so the input brigade is 
just read empty. MacBook Pro 2012. OS X 10.12.

TLS, http/1.1:348 MB/s
TLS, h2, v1.9.3:  163 MB/s  
TLS, h2, v1.10.1: 260 MB/s

clear text: all around 500 MB/s

Goes in the right direction for TLS. Curious that cleartext tops out all at the 
same speed, maybe a curl limit reached here.

The improvements are around buffering and read sizes. For such large transfers, 
adjusting the HTTP/2 flow control window size dynamically is also relevant. 
Yes, we're reinventing parts of TCP. Isn't it fun? ;-)

-Stefan

Re: mod_remoteip and mod_http2 combined

2017-03-27 Thread Sander Hoentjen


On 03/16/2017 10:34 AM, Sander Hoentjen wrote:
>
> On 03/11/2017 07:57 PM, Daniel Ruggeri wrote:
>> Thanks, all, for the patience as I finally got back to this.
>>
>> On 2/24/2017 11:05 AM, Sander Hoentjen wrote:
>>> On 02/20/2017 07:48 PM, William A Rowe Jr wrote:
 On Sat, Feb 18, 2017 at 4:25 PM, Daniel Ruggeri  
 wrote:
> On 2017-02-15 09:07 (-0600), William A Rowe Jr  
> wrote:
>> On Wed, Feb 15, 2017 at 9:02 AM, Sander Hoentjen  
>> wrote:
>>> mod_remote ip has:
>>> /* mod_proxy creates outgoing connections - we don't want those */
>>> if (!remoteip_is_server_port(c->local_addr->port)) {
>>> return DECLINED;
>>> }
>>> I am guessing something similar is needed for h2 connections?
>> I suspect that the mod_remoteip logic is wrong, that it should be 
>> guarding
>> against any subordinate connections and examining only explicitly 
>> configured
>> ports / origin IPs. the PROXY protocol is not part of the HTTP protocol 
>> and
>> incompatible with it, so the trust list logic isn't directly compatible 
>> (this is
>> clearly explained in the PROXY pseudo-RFC.)
>>
> Hi, Bill. That is what the module is doing. The original authors wrote it 
> to have a list of virtual hosts it is explicitly enabled for and 
> explicitly disabled for. I added a third list for optional vhosts. In the 
> pre_connection hook, it checks to see if the connection's local_addr 
> (which should normally be the server's IP) is explicitly configured to 
> enable PROXY handling. It then checks to see if the local port is a 
> server port.
>
> Looking at the logs shared, 192.168.122.249:84 is the server IP:Port 
> combo and is also the local IP:Port from mod_h2. If h2 sets the master of 
> this connection, then we could skip the whole ordeal with this patch:
>
> Index: modules/metadata/mod_remoteip.c
> ===
> --- modules/metadata/mod_remoteip.c (revision 1781701)
> +++ modules/metadata/mod_remoteip.c (working copy)
> @@ -862,6 +862,10 @@
>  remoteip_conn_config_t *conn_conf;
>  int optional;
>
> +if (c->master != NULL) {
> +return DECLINED;
> +}
> +
>  conf = ap_get_module_config(ap_server_conf->module_config,
>  &remoteip_module);
>
> .. but I don't know if that potentially means we are looking at the wrong 
> connection.
>>> First I'll say that with the "Optional" mode it worked, just not with On
>>> I just tried this patch and as far as I have tested this seems to work
>>> fine in On mode, as well as in Optional. I do see some other issue, but
>>> that is probably in my own code, I'll try to track that down later.
>> This is good news and about what I was expecting to happen. I will add
>> this to the commit I've got coming that incorporates a lot of Ruediger's
>> feedback.
>>
 That should be close, but need to ensure c->master is initialized for
 http as well
 where there is no master/subordinate.
>>> I am not sure what this means, how should I test this?
>> Hi, Bill - also hoping for a bit more input. Since PROXY protocol is not
>> tied to any particular layer 7 protocol, I don't think we'd have to
>> verify it is initialized for HTTP - just that there is no master at all.
>> At least, that's my understanding so I appreciate any corrections.
> Here are my changes by the way:
> https://github.com/AntagonistHQ/httpd/commit/2d208793b4494e73289477c231c79be9e0030a2b
>> Sure, to clarify, the Optional use case came from a member on one of our
>> cousin projects (Tomcat) Chris Schultz as well as my own use cases. It
>> is useful for internally accessing the site from behind the
>> loadbalancer. When there is a publicly addressed upstream loadbalancer
>> (Amazon ELB or just HAProxy itself) talking to RFC1918 addressed or
>> non-routeable backend httpd servers, it becomes impossible to enable
>> internal communication on the RFC1918 space to the backend instances.
>> If the goal is to monitor or probe the site and (httpd proxy) backends
>> internally for health, this *can* be done by duplicating the virtual
>> hosts. Depending on the complexity of the virtual hosts, what resources
>> those virtual hosts have (proxies and whatnot) and their general size,
>> this could result in a fairly unmanageable httpd configuration having a
>> vhost that requires PROXY header and a second one on a different port or
>> IP that does not.
>> It gets even more complicated when you are aiming to do management
>> tasks. If you have balancers configured at the vhost as intended, you
>> can only manage those balancers from the vhost they live in. Further,
>> you may want to view server statistics, check info about the ldap cache,
>> etc but permit access to those things only from a trusted network in
>

Re: mod_http2 file uploads slow/broken?

2017-03-27 Thread Sander Hoentjen
Hi Stefan,

With 1.9.3 file upload seems to complete every time just fine, so looks
like graceful restarts were indeed the issue. Thanks!
As to the speed, yes it needs to be improved but there is also some
weird thing:
For my testfile of about 14M,
uploads over http take about 20s
uploads over https take about 15s
(with http1.1 both take about 5s)
In my setup SSL is terminated by HAProxy in TCP mode, so Apache gets
non-SSL in both case, only in the SSL case prepended py proxy-protocol info.

Regards,
Sander

On 03/24/2017 08:14 PM, Stefan Eissing wrote:
> Hi Sander,
>
> from the logs, I think you are running into PR 59348 which was fixed in 
> v1.8.11.
>
> The bug was that during graceful restart, ongoing streams were not finished. 
> This restart could also happen, I think, if you have configured a max number 
> of connections or requests per process, for example with 
> MaxConnectionsPerChild. If that is the case, you can set this to 0 and see if 
> the upload still fails.
>
> More interesting would be to know if the current github version has the 
> problem still. With the fix, I hope it finishes the ongoing h2 requests 
> before the process exits.
>
> As to the speed: it is as I described earlier, the data is written in too 
> small chunks to the request processing thread. That needs to be improved for 
> sure.
>
> Thanks for testing, awaiting your results. If you need help with building the 
> github version, please let me know.
>
> Cheers,
>
> Stefan
>
>> Am 24.03.2017 um 16:56 schrieb Sander Hoentjen :
>>
>> Hi Stefan,
>>
>> So far I can't reproduce the breaking of the upload on a testing
>> machine. I send you logs from a production machine off-list. In the
>> meanwhile I will try to build mod_http2 from
>> https://github.com/icing/mod_h2.
>>
>> Thanks for your help!
>>
>> Sander
>>
>>
>>
>> On 03/24/2017 03:38 PM, Stefan Eissing wrote:
>>> Hi Sander,
>>>
>>> the uploads sometimes break is new to me and I sure would like
>>> to find out what is going wrong in your setup. One obvious reason
>>> for the php script failing to read its input is a timeout or
>>> abort of the main connection. mod_http2 uses the general Timeout
>>> settings for its requests as well. If you can reproduce this,
>>> a log with "LogLevel http2:trace1" or even trace2 would help.
>>>
>>> I know that input streaming is not very optimal right now. Each
>>> arriving DATA frame is send directly onward to the request thread
>>> and that should be buffered and flushed at proper times. Again,
>>> a log at trace1/2 level should show exactly what is going on.
>>>
>>> If you can, I'd be interested to hear how the current version,
>>> available at https://github.com/icing/mod_h2 fares in you 2.4.25
>>> server. If you are on Windows, apachelounge has also builds with
>>> the latest version packaged in.
>>>
>>> Cheers,
>>>
>>> Stefan
>>>
 Am 24.03.2017 um 14:10 schrieb Sander Hoentjen :

 Hi,

 I am running Apache 2.4.25 with mod_http2, and I notice that sometimes
 file uploads are broken.

 Receiving end is a php script, and it logs something like:
 Internal error on sending request(POST /upload/upload.php HTTP/2.0);
 uri(/upload/upload.php) content-length(931728): SendRequest: prepare():
 user_get_body(bodyLocalBuf, 36865): read from client failed

 With HTTP/1.1 this problem does not occur. With HTTP/2 this problems
 occurs sometimes, but not always. What I do notice though is that
 uploads via HTTP/2 are *much* slower, about 20-30s on HTTP/2 vs 4-5s on
 HTTP/1.1 for the exact same file (about 15MB)

 Is this a known issue? If not, anything I can do to help?

 Regards,
 Sander
>>> Stefan Eissing
>>>
>>> bytes GmbH
>>> Hafenstrasse 16
>>> 48155 Münster
>>> www.greenbytes.de
>>>
> Stefan Eissing
>
> bytes GmbH
> Hafenstrasse 16
> 48155 Münster
> www.greenbytes.de
>