Re: RFC: Documenting changes in the CHANGES file

2020-06-02 Thread Stefan Eissing


> Am 02.06.2020 um 14:11 schrieb Daniel Ruggeri :
> 
> On 6/1/2020 6:23 AM, Yann Ylavic wrote:
>> On Fri, May 29, 2020 at 9:30 PM Ruediger Pluem 
>>  wrote:
>> 
>>> Reviewing our backport process I noticed that in many cases a clean merge 
>>> via svn merge fails due to conflicts in CHANGES. While
>>> these are easy to solve it puts IMHO unnecessary extra work on the backport 
>>> process, both for reviewing and for actually doing the
>>> backport. How about if we change the way we document changes the following 
>>> way:
>>> 
>>> 1. We create a changes-fragments directory (name to be determined) at the 
>>> top level.
>>> 2. For each release we create a subdirectory such that we end up with the 
>>> following structure:
>>> 
>>>changes-fragments/
>>>  2.4.41/
>>>  2.4.42/
>>>  2.4.43/
>>>  2.4.44/
>>> 
>>> 3. Each directory contains the changes for each release and each change 
>>> entry is a single file.
>>> 4. We have a script that builds our current CHANGES file from the content 
>>> in changes-fragments directories with the help of
>>>a template or at least some sort of header / footer that is static.
>>> 5. This script can be called either manually and we commit the resulting 
>>> CHANGES file as we like just like the x-forms commits
>>>for documentation plus this script is called by the release scripts from 
>>> Daniel as part of the preparation of rolling a
>>>release.
>>> 
>> +1 from me, I don't volonteer for the scripts though :)
>> 
>> Regards;
>> Yann.
>> 
> Hi, Yann;
> 
> I'm open to whatever... and don't mind writing or tweaking scripts once we 
> decide on an approach :-)
> 
> While we are discussing ideas in this neighborhood, one thing to keep in mind 
> is that during release of security fixes, sometimes there are items added to 
> CHANGES and sometimes CHANGES is modified to add CVE information. There have 
> been minor bumps in the road where these patches don't always apply cleanly. 
> So, if possible, it would be great to consider. There may be nothing to do, 
> though, since that happens way after backport.
> 


+1 from me as well. CHANGES is annoying atm, any automation appreciated.

Cheers, Stefan

Re: RFC: Documenting changes in the CHANGES file

2020-06-02 Thread Daniel Ruggeri
On 6/1/2020 6:23 AM, Yann Ylavic wrote:
> On Fri, May 29, 2020 at 9:30 PM Ruediger Pluem  wrote:
>> Reviewing our backport process I noticed that in many cases a clean merge 
>> via svn merge fails due to conflicts in CHANGES. While
>> these are easy to solve it puts IMHO unnecessary extra work on the backport 
>> process, both for reviewing and for actually doing the
>> backport. How about if we change the way we document changes the following 
>> way:
>>
>> 1. We create a changes-fragments directory (name to be determined) at the 
>> top level.
>> 2. For each release we create a subdirectory such that we end up with the 
>> following structure:
>>
>>changes-fragments/
>>  2.4.41/
>>  2.4.42/
>>  2.4.43/
>>  2.4.44/
>>
>> 3. Each directory contains the changes for each release and each change 
>> entry is a single file.
>> 4. We have a script that builds our current CHANGES file from the content in 
>> changes-fragments directories with the help of
>>a template or at least some sort of header / footer that is static.
>> 5. This script can be called either manually and we commit the resulting 
>> CHANGES file as we like just like the x-forms commits
>>for documentation plus this script is called by the release scripts from 
>> Daniel as part of the preparation of rolling a
>>release.
> +1 from me, I don't volonteer for the scripts though :)
>
> Regards;
> Yann.

Hi, Yann;

I'm open to whatever... and don't mind writing or tweaking scripts once
we decide on an approach :-)

While we are discussing ideas in this neighborhood, one thing to keep in
mind is that during release of security fixes, sometimes there are items
added to CHANGES and sometimes CHANGES is modified to add CVE
information. There have been minor bumps in the road where these patches
don't always apply cleanly. So, if possible, it would be great to
consider. There may be nothing to do, though, since that happens way
after backport.

-- 
Daniel Ruggeri



Increase rwnd size

2020-06-02 Thread Sébastien Mizrahi
Hi Apache teams,

In my httpd module, I would like to increase the receive window (rwnd) size.
For some mysterious reasons, if I set a value greater than 4 Mb, the rwnd 
stucks to 4193 Kb.

Here is my investigation states :

  *   This is for upload (client uploads to httpd)
  *   The correct TCP options are enabled on the server
  *   I got another homemade app on the same server and the rwnd can go up to 8 
Mb - so this exclude the server
  *   The sizeof(int) is 4 bytes

Ton increase receive window size, I do :
int opt = 500;
if (setsockopt(this->mSocket, SOL_SOCKET, SO_RCVBUF, , sizeof(opt)) < 0) {
throw invalid_argument(string(CLASSNAME) + " Could not set SO_RCVBUF option : " 
+ string(strerror(errno)));
}

I can check the window size status using getsockopt, and values are correct at 
the beginning and at the end of the upload.
In wireshark, I can see this :

Frame 36818: 66 bytes on wire (528 bits), 66 bytes captured (528 bits)
Ethernet II, Src: X, Dst: Y
Internet Protocol Version 4, Src: X, Dst: Y
Transmission Control Protocol, Src Port: 8443, Dst Port: 65168, Seq: 6136, Ack: 
31241018, Len: 0
Source Port: 8443
Destination Port: 65168
[Stream index: 0]
[TCP Segment Len: 0]
Sequence number: 6136 (relative sequence number)
[Next sequence number: 6136 (relative sequence number)]
Acknowledgment number: 31241018 (relative ack number)
1000  = Header Length: 32 bytes (8)
Flags: 0x010 (ACK)
Window size value: 4095
[Calculated window size: 4193280]
[Window size scaling factor: 1024]
Checksum: 0x048c [unverified]
[Checksum Status: Unverified]
Urgent pointer: 0
Options: (12 bytes), No-Operation (NOP), No-Operation (NOP), Timestamps
[SEQ/ACK analysis]
[Timestamps]

Does a specific mechanism in Apache cause this behaviour ?

Thanks a lot for helping.

Seb.


Re: Trying to understand the logic of mod_proxy_http2 with regards to timeouts

2020-06-02 Thread Stefan Eissing
Looks good!

> Am 29.05.2020 um 21:14 schrieb Ruediger Pluem :
> 
> 
> 
> On 5/29/20 11:41 AM, Stefan Eissing wrote:
>> 
>> 
>>> Am 29.05.2020 um 11:23 schrieb Ruediger Pluem :
>>> 
>>> 
>>> 
>>> On 5/29/20 10:09 AM, Stefan Eissing wrote:
 Looks good. Now I learned about the "ping" parameter...
>>> 
>>> Committed as r1878264 with a tweaked comment to make clear what I do.
> 
> Thanks for backporting both
> 
>>> 
>>> Getting me to the next possible enhancement. I already had a patch but when 
>>> putting it to the mail I got doubts that it could work
>>> due to the fact, that in most use cases HTTP/2 is encrypted.
>>> In AJP a set ping parameter on the worker will also cause an AJP ping to be 
>>> sent as the first thing even on fresh connections and
>>> we only wait for the timeout set in the parameter for a reply. The idea 
>>> behind this is that the backend might be able to deal with
>>> a TCP handshake, but not with processing a request, because maybe all 
>>> processing threads / processes on the backend application
>>> are busy. This way this can be detected quickly and early and we can sent 
>>> our request to a different backend in case of a load
>>> balancing scenario.
>>> With HTTP/2 we will likely have a TLS handshake first which likely already 
>>> requires a responding backend application. So it would
>>> not work to wait only for ping timeout time on a ping reply as we will 
>>> already wait for the timeout set on the socket to get an
>>> answer to our TLS client Hello. So the idea would be to already lower the 
>>> socket timeout to ping timeout before the TLS handshake
>>> starts and reset it back after we received the ping from the backend. 
>>> Opinions?
>> 
>> HTTP/2 also has an initial SETTINGS frame handshake. We could use the ping 
>> timeout on the socket until the first NGHTTP2_SETTINGS frame from the 
>> backend arrives on a new connection.
> 
> And now I learned about the HTTP/2 handshake :-). I haven't though about the 
> SETTINGS frame. So the high level idea would be:
> 
> 1. If a fresh session in h2_proxy_session_setup is created and the ping 
> parameter is set on the worker change the socket timeout
> to the one in ping, but tuck away the current socket timeout.
> 2. In on_frame_recv in the NGHTTP2_SETTINGS restore the old timeout (if it 
> was changed) and continue.
> 
> So something like the below?
> 
> Index: modules/http2/h2_proxy_session.c
> ===
> --- modules/http2/h2_proxy_session.c  (revision 1878265)
> +++ modules/http2/h2_proxy_session.c  (working copy)
> @@ -203,6 +203,21 @@
> case NGHTTP2_PUSH_PROMISE:
> break;
> case NGHTTP2_SETTINGS:
> +/*
> + * Check if we have saved a socket timeout before sending the
> + * SETTINGS frame in h2_proxy_session_setup to perform a quick
> + * "ping" on the backend. If yes, restore the saved timeout to
> + * the socket.
> + */
> +if (session->save_timeout != -1) {
> +apr_socket_t *socket = NULL;
> +
> +socket = ap_get_conn_socket(session->c);
> +if (socket) {
> +apr_socket_timeout_set(socket, session->save_timeout);
> +session->save_timeout = -1;
> +}
> +}
> if (frame->settings.niv > 0) {
> n = nghttp2_session_get_remote_settings(ngh2, 
> NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
> if (n > 0) {
> @@ -634,6 +649,8 @@
> session->input = apr_brigade_create(session->pool, 
> session->c->bucket_alloc);
> session->output = apr_brigade_create(session->pool, 
> session->c->bucket_alloc);
> 
> +session->save_timeout = -1;
> +
> nghttp2_session_callbacks_new();
> nghttp2_session_callbacks_set_on_frame_recv_callback(cbs, 
> on_frame_recv);
> nghttp2_session_callbacks_set_on_data_chunk_recv_callback(cbs, 
> stream_response_data);
> @@ -654,6 +671,22 @@
> nghttp2_option_del(option);
> nghttp2_session_callbacks_del(cbs);
> 
> +/*
> + * If a ping parameter is set on the worker set the socket timeout to
> + * it to "use" the possible TLS handshake and the initial SETTINGS
> + * frame as kind of ping. Tuck away the old timeout to restore it, 
> once
> + * the SETTING frame arrived from the backend.
> + */
> +if (p_conn->worker->s->ping_timeout_set) {
> +apr_socket_t *socket = NULL;
> +
> +socket = ap_get_conn_socket(session->c);
> +if (socket) {
> +apr_socket_timeout_get(socket, >save_timeout);
> +apr_socket_timeout_set(socket, 
> p_conn->worker->s->ping_timeout);
> +}
> +}
> +
> ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, APLOGNO(03362)
>   "setup session for %s",