Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On 24 Jan 2022, at 19:39, Yann Ylavic wrote:
--- httpd/httpd/trunk/server/mpm/event/event.c (original)
+++ httpd/httpd/trunk/server/mpm/event/event.c Fri Jan 21 00:09:24 2022
@@ -1142,6 +1145,7 @@ read_request:
*/
if (rc != OK || (cs->pub.state >= CONN_STATE_NUM)
|| (cs->pub.state < CONN_STATE_LINGER
+ && cs->pub.state != CONN_STATE_READ_REQUEST_LINE
&& cs->pub.state != CONN_STATE_WRITE_COMPLETION
&& cs->pub.state !=
CONN_STATE_CHECK_REQUEST_LINE_READABLE
&& cs->pub.state != CONN_STATE_SUSPENDED)) {
>>>
>>> The issue is that we've never asked process_connection hooks that
>>> don't care about async and co to change cs->pub.state when they are
>>> done, and for instance mod_echo's process_echo_connection() will
>>> consume the whole connection but leave CONN_STATE_READ_REQUEST_LINE as
>>> is, and here we'll no longer turn it to CONN_STATE_LINGER to close the
>>> connection as before hence loop indefinitely between mod_echo and
>>> mpm_event (eating a CPU) for a connection that is EOF already.
>>
>> True - fixed this in r1897423.
>>
>> Instead of assuming that all async MPM’s can handle AGAIN, added a dedicated
>> MPM check for it. This mean no surprises for existing code.
>
> Why? Really I think you are complicating this. Every async MPM should
> be able to handle EAGAIN, i.e. AP_MPMQ_IS_ASYNC == AP_MPM_CAN_AGAIN.
Alas no, for the reason you pointed out above.
There are three MPMs in trunk that can AP_MPMQ_IS_ASYNC, only one of them
understands AGAIN.
Little-Net:httpd-trunk6 minfrin$ grep -r IS_ASYNC server/mpm
server/mpm/motorz/motorz.c:case AP_MPMQ_IS_ASYNC:
server/mpm/simple/simple_api.c:case AP_MPMQ_IS_ASYNC:
server/mpm/event/event.c:case AP_MPMQ_IS_ASYNC:
Remember that the SUSPEND function and the AGAIN function are two completely
separate functions. SUSPEND is all about making sure that some other task can
happen without blocking (proxy, DNS, etc). AGAIN is about making sure the
primary connection itself doesn't block.
Regards,
Graham
—
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On Mon, Jan 24, 2022 at 5:19 PM Graham Leggett wrote:
>
> On 24 Jan 2022, at 13:56, Yann Ylavic wrote:
>
> >> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
> >> +++ httpd/httpd/trunk/server/mpm/event/event.c Fri Jan 21 00:09:24 2022
> >> @@ -1142,6 +1145,7 @@ read_request:
> >> */
> >> if (rc != OK || (cs->pub.state >= CONN_STATE_NUM)
> >> || (cs->pub.state < CONN_STATE_LINGER
> >> + && cs->pub.state != CONN_STATE_READ_REQUEST_LINE
> >> && cs->pub.state != CONN_STATE_WRITE_COMPLETION
> >> && cs->pub.state !=
> >> CONN_STATE_CHECK_REQUEST_LINE_READABLE
> >> && cs->pub.state != CONN_STATE_SUSPENDED)) {
> >
> > The issue is that we've never asked process_connection hooks that
> > don't care about async and co to change cs->pub.state when they are
> > done, and for instance mod_echo's process_echo_connection() will
> > consume the whole connection but leave CONN_STATE_READ_REQUEST_LINE as
> > is, and here we'll no longer turn it to CONN_STATE_LINGER to close the
> > connection as before hence loop indefinitely between mod_echo and
> > mpm_event (eating a CPU) for a connection that is EOF already.
>
> True - fixed this in r1897423.
>
> Instead of assuming that all async MPM’s can handle AGAIN, added a dedicated
> MPM check for it. This mean no surprises for existing code.
Why? Really I think you are complicating this. Every async MPM should
be able to handle EAGAIN, i.e. AP_MPMQ_IS_ASYNC == AP_MPM_CAN_AGAIN.
So far (as you said in your other response) there was only one way to
go async: register a callback onto the MPM with
ap_mpm_register_{timed,poll}_callback() and return SUSPENDED to httpd
for process_connection to set CONN_STATE_SUSPENDED before returning
(with OK!) to the MPM. This tells the MPM that the connection has been
queued by other means (nothing to do on its side).
Yet the connection is suspended (be it to the MPM or not), and the MPM
will notify_suspend() before ignoring it for now (meaning that if the
module suspended it to something else than the MPM it'd need to
ap_run_suspend_connection() by itself and notify_resume() too when
it's rescheduled).
A callback was registered though so whenever the event fires the MPM
will unregister it, notify_resume() and call it.
If the callback wants to be scheduled by the MPM again after running
it needs to call ap_mpm_register_*_callback() again and finally
ap_mpm_resume_suspended() (i.e. suspend the connection again and call
me back etc, or set CONN_STATE_LINGER to let the MPM kill it).
In dialup_handler() we are not in a position where we can simply
return SUSPENDED to the MPM (from process_connection) and expect that
it calls process_connection() again when the timer/IO fires and we can
come back to dialup_handler() where it left without anything else
interacting until then, that why there is a callback mechanism.
Now we want to be able to suspend in early connection processing,
before anything happened on the connection or any request was
created/handled (i.e. AP_MODE_INIT) and know that the MPM can simply
call process_connection() again when it's ready.
I argue that returning SUSPENDED from a process_connection hook that
knows what it's doing is the simpler thing to do, without breaking any
semantics..
Regards;
Yann.
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On 24 Jan 2022, at 18:07, Yann Ylavic wrote: >> Well, this is after check_time_left().. > > Yet we should update the socket timeout for AP_MODE_INIT since it's > how handshakes are performed.. > Done in r1897422. Thank you for this. Regards, Graham —
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On 24 Jan 2022, at 13:56, Yann Ylavic wrote:
>> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
>> +++ httpd/httpd/trunk/server/mpm/event/event.c Fri Jan 21 00:09:24 2022
>> @@ -1142,6 +1145,7 @@ read_request:
>> */
>> if (rc != OK || (cs->pub.state >= CONN_STATE_NUM)
>> || (cs->pub.state < CONN_STATE_LINGER
>> + && cs->pub.state != CONN_STATE_READ_REQUEST_LINE
>> && cs->pub.state != CONN_STATE_WRITE_COMPLETION
>> && cs->pub.state !=
>> CONN_STATE_CHECK_REQUEST_LINE_READABLE
>> && cs->pub.state != CONN_STATE_SUSPENDED)) {
>
> The issue is that we've never asked process_connection hooks that
> don't care about async and co to change cs->pub.state when they are
> done, and for instance mod_echo's process_echo_connection() will
> consume the whole connection but leave CONN_STATE_READ_REQUEST_LINE as
> is, and here we'll no longer turn it to CONN_STATE_LINGER to close the
> connection as before hence loop indefinitely between mod_echo and
> mpm_event (eating a CPU) for a connection that is EOF already.
True - fixed this in r1897423.
Instead of assuming that all async MPM’s can handle AGAIN, added a dedicated
MPM check for it. This mean no surprises for existing code.
Regards,
Graham
—
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On Mon, Jan 24, 2022 at 2:49 PM Yann Ylavic wrote: > > On Mon, Jan 24, 2022 at 2:02 PM Graham Leggett wrote: > > > > Looks like mod_reqtimeout’s capabilities are overstated at this point - the > > module neatly steps out the way and does nothing the moment it sees a non > > blocking connection: > > > > https://github.com/apache/httpd/blob/trunk/modules/filters/mod_reqtimeout.c#L226 > > Well, this is after check_time_left().. Yet we should update the socket timeout for AP_MODE_INIT since it's how handshakes are performed.. Done in r1897422.
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On 24 Jan 2022, at 17:21, Yann Ylavic wrote: > Maybe the resume_suspended hook is a misnomer? We also have the > suspend_connection and resume_connection hooks which mean the opposite > (called when added to and removed from the queues respectively). The ap_mpm_resume_suspended() is a function rather than a hook. This is what gets called when you’ve decided your external condition has been met (DNS lookup finished, etc), and the connection should carry on back to pumping data like it did before. The two hooks suspend_connection and resume_connection allow code that cares to know when something is being paused and unpaused. Right now what’s confusing is there is no ap_mpm_suspend() function, the module is expected to do the suspend stuff by hand: https://github.com/apache/httpd/blob/3ec0ffb9e1ac05622b97a7afd6992dd2bd41ce38/modules/http/http_request.c#L476 > It sounds more natural to me to think that the connection (processing) > is suspended when it's in the MPM, than suspended from any MPM action > when it's outside the MPM (all the connections being processed outside > the MPM are not in any MPM queue anyway), but YMMV ;) There are two types of suspended: - AGAIN/APR_EAGAIN: The socket does not have data for me to read, or the write buffer is full and I can no longer write, go into select/poll in the MPM and when I can either read or write call me again so I can carry on. In the mean time, I’m not hogging the connection, other requests get a go. This is new, previously we just blocked and clogged up everything, hogging the slot. Yuck. - SUSPENDED/CONN_STATE_SUSPENDED: I don't care about my socket, don’t call me. If there is data there to read or space on the buffer for me to write I don’t care, completely ignore me for now. Maybe I am throttling a connection, maybe I’m waiting for DNS, maybe it’s the backend proxy’s turn to read. At some point in the future a condition will get fulfilled, and I’ll un-suspend/resume this connection, but in the mean time, pretend I don’t exist. Here is mod_dialup telling the core that it wants to go to sleep: https://github.com/apache/httpd/blob/1032155c5720e167446efceb4b8e0b545851b7a4/modules/test/mod_dialup.c#L99 Here is mod_dialup telling the core that enough delay has passed, it wants to wake back up: https://github.com/apache/httpd/blob/1032155c5720e167446efceb4b8e0b545851b7a4/modules/test/mod_dialup.c#L138 Regards, Graham —
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On Mon, Jan 24, 2022 at 4:21 PM Yann Ylavic wrote: > > On Mon, Jan 24, 2022 at 3:49 PM Graham Leggett wrote: > > > > On 24 Jan 2022, at 15:52, Yann Ylavic wrote: > > > > >> SUSPENDED already has the opposite meaning, in that a suspended > > >> connection will be excluded from run_process_connection() until such > > >> time as some other event has occurred (response on DNS, a proxy > > >> connection received something, etc) and the connection resumed. > > > > > > Hm, where is that? (All the uses of SUSPENDED I see in httpd seem to > > > be "defer to the MPM".) > > > > It does defer to the MPM, which then makes sure that connection stays out > > of the queues until some other out-of-band process is triggered for the > > connection to put back in the queue using ap_mpm_resume_suspended(). In > > other words “don’t call me ever, even if data is available, as I’m waiting > > for something else to happen, like say the result of a DNS lookup or a > > response from a backend connection”. > > > > In the case of AGAIN, it means “call me again when you have more data, > > serve other connections while we wait”. > > Maybe the resume_suspended hook is a misnomer? Or it means put it in suspended state again (at least that's how I understood it so far). > We also have the > suspend_connection and resume_connection hooks which mean the opposite > (called when added to and removed from the queues respectively). > > It sounds more natural to me to think that the connection (processing) > is suspended when it's in the MPM, than suspended from any MPM action > when it's outside the MPM (all the connections being processed outside > the MPM are not in any MPM queue anyway), but YMMV ;) > > > Regards; > Yann.
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On Mon, Jan 24, 2022 at 3:49 PM Graham Leggett wrote: > > On 24 Jan 2022, at 15:52, Yann Ylavic wrote: > > >> SUSPENDED already has the opposite meaning, in that a suspended connection > >> will be excluded from run_process_connection() until such time as some > >> other event has occurred (response on DNS, a proxy connection received > >> something, etc) and the connection resumed. > > > > Hm, where is that? (All the uses of SUSPENDED I see in httpd seem to > > be "defer to the MPM".) > > It does defer to the MPM, which then makes sure that connection stays out of > the queues until some other out-of-band process is triggered for the > connection to put back in the queue using ap_mpm_resume_suspended(). In other > words “don’t call me ever, even if data is available, as I’m waiting for > something else to happen, like say the result of a DNS lookup or a response > from a backend connection”. > > In the case of AGAIN, it means “call me again when you have more data, serve > other connections while we wait”. Maybe the resume_suspended hook is a misnomer? We also have the suspend_connection and resume_connection hooks which mean the opposite (called when added to and removed from the queues respectively). It sounds more natural to me to think that the connection (processing) is suspended when it's in the MPM, than suspended from any MPM action when it's outside the MPM (all the connections being processed outside the MPM are not in any MPM queue anyway), but YMMV ;) Regards; Yann.
Re: svn commit: r1897387 - in /httpd/httpd/trunk: docs/log-message-tags/next-number modules/ssl/mod_ssl.c
On Mon, Jan 24, 2022 at 3:56 PM Graham Leggett wrote: > > On 24 Jan 2022, at 16:45, Yann Ylavic wrote: > > >> True - changed to AP_FILTER_ERROR in r1897418, which is the correct code > >> for this. > > > > Would it work if ssl_hook_process_connection() handled only EAGAIN and > > otherwise always returned DECLINED (expecting that the usual > > processing would re-catch the error and still behave correctly)? > > It’s not ideal. > > This means on error, we would then attempt to read again (this time a > blocking read), and then possibly error again (but maybe not). Yes, but it could be the duty of mod_ssl to not let further reading succeed if the handshake failed (which may be the case already).
Re: svn commit: r1897387 - in /httpd/httpd/trunk: docs/log-message-tags/next-number modules/ssl/mod_ssl.c
On 24 Jan 2022, at 16:45, Yann Ylavic wrote: >> True - changed to AP_FILTER_ERROR in r1897418, which is the correct code for >> this. > > Would it work if ssl_hook_process_connection() handled only EAGAIN and > otherwise always returned DECLINED (expecting that the usual > processing would re-catch the error and still behave correctly)? It’s not ideal. This means on error, we would then attempt to read again (this time a blocking read), and then possibly error again (but maybe not). The existing behaviour where we read and then throw away the error result is definitely a bug. Regards, Graham —
Re: http and http/1.x separation
> Am 24.01.2022 um 15:40 schrieb Yann Ylavic : > > On Mon, Jan 24, 2022 at 3:28 PM Stefan Eissing wrote: >> >> FYI: I am busy hacking away at the separation between our HTTP and HTTP/1.x >> handling code. I'll make a PR once all tests have passed, so we can talk >> about the changes. > > Looking forward to seeing it ;) 🙈 > >> >> The goals are pretty simple: >> >> 1. have mod_http focus on HTTP semantics, header checks, ap_die conversions, >> not chunking and CRLFs. >> 2. have a mod_http1 that installs TRANSCODE filters when the connection is >> h1. >> those do the chunking and the header serialization. >> 3. Have interim responses that send META buckets, not DATA to the transcode >> filters. >> No content filter will be confused by those. >> 4. Pass FOOTERS in/out as META buckets in a similar way. >> >> The main idea is to introduce a META "HEADERS" bucket that is used for >> final/interim responses and footers as well. This will safely pass through >> all filters that do not know about it. This is similar to the ERROR bucket >> type we already have. > > There are some assumptions in httpd (and possibly third-party modules) > that meta buckets have a no length, so if "HEADERS" buckets have one > you might need to embed it in the bucket->data struct.. Yes, it's 0 length like ERROR and has a struct as that holds: - int status, opt 0 - const char *reason, opt NULL - apr_table_t *headers - apr_table_t *notes on responses, shallow copied from the request_rec, etc. The nice thing is that the HTTP/1.x transcode out filter can now check in one place that - there is indeed a HEADERS before data buckets - there is only one HEADERS with status >= 200 And "above" the transcode layer, a data bucket is content, period. > > > Cheers; > Yann.
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On 24 Jan 2022, at 15:52, Yann Ylavic wrote: >> SUSPENDED already has the opposite meaning, in that a suspended connection >> will be excluded from run_process_connection() until such time as some other >> event has occurred (response on DNS, a proxy connection received something, >> etc) and the connection resumed. > > Hm, where is that? (All the uses of SUSPENDED I see in httpd seem to > be "defer to the MPM".) It does defer to the MPM, which then makes sure that connection stays out of the queues until some other out-of-band process is triggered for the connection to put back in the queue using ap_mpm_resume_suspended(). In other words “don’t call me ever, even if data is available, as I’m waiting for something else to happen, like say the result of a DNS lookup or a response from a backend connection”. In the case of AGAIN, it means “call me again when you have more data, serve other connections while we wait”. Regards, Graham —
Re: svn commit: r1897387 - in /httpd/httpd/trunk: docs/log-message-tags/next-number modules/ssl/mod_ssl.c
On Mon, Jan 24, 2022 at 3:39 PM Graham Leggett wrote: > > On 24 Jan 2022, at 14:33, Yann Ylavic wrote: > > > I mean not only mod_ssl could return an error for > > ap_get_brigade(AP_MODE_INIT), APR_EGENERAL is not distinctive enough > > IMHO. > > True - changed to AP_FILTER_ERROR in r1897418, which is the correct code for > this. Would it work if ssl_hook_process_connection() handled only EAGAIN and otherwise always returned DECLINED (expecting that the usual processing would re-catch the error and still behave correctly)?
Re: http and http/1.x separation
On Mon, Jan 24, 2022 at 3:28 PM Stefan Eissing wrote: > > FYI: I am busy hacking away at the separation between our HTTP and HTTP/1.x > handling code. I'll make a PR once all tests have passed, so we can talk > about the changes. Looking forward to seeing it ;) > > The goals are pretty simple: > > 1. have mod_http focus on HTTP semantics, header checks, ap_die conversions, >not chunking and CRLFs. > 2. have a mod_http1 that installs TRANSCODE filters when the connection is h1. >those do the chunking and the header serialization. > 3. Have interim responses that send META buckets, not DATA to the transcode > filters. >No content filter will be confused by those. > 4. Pass FOOTERS in/out as META buckets in a similar way. > > The main idea is to introduce a META "HEADERS" bucket that is used for > final/interim responses and footers as well. This will safely pass through > all filters that do not know about it. This is similar to the ERROR bucket > type we already have. There are some assumptions in httpd (and possibly third-party modules) that meta buckets have a no length, so if "HEADERS" buckets have one you might need to embed it in the bucket->data struct.. Cheers; Yann.
Re: svn commit: r1897387 - in /httpd/httpd/trunk: docs/log-message-tags/next-number modules/ssl/mod_ssl.c
On 24 Jan 2022, at 14:33, Yann Ylavic wrote: > I mean not only mod_ssl could return an error for > ap_get_brigade(AP_MODE_INIT), APR_EGENERAL is not distinctive enough > IMHO. True - changed to AP_FILTER_ERROR in r1897418, which is the correct code for this. Regards, Graham —
Re: http and http/1.x separation
On 24 Jan 2022, at 16:28, Stefan Eissing wrote: > The main idea is to introduce a META "HEADERS" bucket that is used for > final/interim responses and footers as well. This will safely pass through > all filters that do not know about it. This is similar to the ERROR bucket > type we already have. Big +1. We don’t use metadata buckets enough, and life is made significantly easier when used. Regards, Graham —
http and http/1.x separation
FYI: I am busy hacking away at the separation between our HTTP and HTTP/1.x handling code. I'll make a PR once all tests have passed, so we can talk about the changes. The goals are pretty simple: 1. have mod_http focus on HTTP semantics, header checks, ap_die conversions, not chunking and CRLFs. 2. have a mod_http1 that installs TRANSCODE filters when the connection is h1. those do the chunking and the header serialization. 3. Have interim responses that send META buckets, not DATA to the transcode filters. No content filter will be confused by those. 4. Pass FOOTERS in/out as META buckets in a similar way. The main idea is to introduce a META "HEADERS" bucket that is used for final/interim responses and footers as well. This will safely pass through all filters that do not know about it. This is similar to the ERROR bucket type we already have. It will eliminate the remaining code duplications in mod_http2, make it leaner and use the standard HTTP filters just like for HTTP/1.x requests. Also, h2 will no longer need to parse interim responses back into header tables or deal with chunked encodings. Kind Regards, Stefan
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On Mon, Jan 24, 2022 at 2:11 PM Graham Leggett wrote: > > SUSPENDED already has the opposite meaning, in that a suspended connection > will be excluded from run_process_connection() until such time as some other > event has occurred (response on DNS, a proxy connection received something, > etc) and the connection resumed. Hm, where is that? (All the uses of SUSPENDED I see in httpd seem to be "defer to the MPM".) Regards; Yann.
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On Mon, Jan 24, 2022 at 2:02 PM Graham Leggett wrote: > > Looks like mod_reqtimeout’s capabilities are overstated at this point - the > module neatly steps out the way and does nothing the moment it sees a non > blocking connection: > > https://github.com/apache/httpd/blob/trunk/modules/filters/mod_reqtimeout.c#L226 Well, this is after check_time_left().. > > I imagine what we need in the async case is to have the ability to override > the timeout on the connection, Which is what mod_reqtimeout does already by applying a "time self" to the socket timeout. > and have the MPM react to that timeout. The pattern would then be set the > timeout and return APR_EAGAIN, let the mpm handle the timeout. Or let the MPM apply the (dynamic-)timeout set on the socket. > > We also need to teach mod_reqtimeout’s code that SSL/TLS libraries sometimes > want to write: > > https://github.com/apache/httpd/blob/trunk/modules/filters/mod_reqtimeout.c#L293 Yeah, mod_reqtimeout is missing context and states transitions, that's why I was talking about better interaction. Btw, I'm not sure what the SSL_WANT_READ/WRITE semantics are in openssl/mod_ssl but with the async handshake we can either always poll() in the MPM before trying to actually read/write on the socket (which could be quite painful for usual cases), or first try to read/write and set the WANT_READ/WRITE flag afterward only if EAGAIN. AIUI, mod_ssl currently FLUSHes all the packets during handshake which means that writes can block still, how is that handled? Regards; Yann.
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On 24 Jan 2022, at 13:56, Yann Ylavic wrote:
>> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
>> +++ httpd/httpd/trunk/server/mpm/event/event.c Fri Jan 21 00:09:24 2022
>> @@ -1142,6 +1145,7 @@ read_request:
>> */
>> if (rc != OK || (cs->pub.state >= CONN_STATE_NUM)
>> || (cs->pub.state < CONN_STATE_LINGER
>> + && cs->pub.state != CONN_STATE_READ_REQUEST_LINE
>> && cs->pub.state != CONN_STATE_WRITE_COMPLETION
>> && cs->pub.state !=
>> CONN_STATE_CHECK_REQUEST_LINE_READABLE
>> && cs->pub.state != CONN_STATE_SUSPENDED)) {
>
> The issue is that we've never asked process_connection hooks that
> don't care about async and co to change cs->pub.state when they are
> done, and for instance mod_echo's process_echo_connection() will
> consume the whole connection but leave CONN_STATE_READ_REQUEST_LINE as
> is, and here we'll no longer turn it to CONN_STATE_LINGER to close the
> connection as before hence loop indefinitely between mod_echo and
> mpm_event (eating a CPU) for a connection that is EOF already.
This is true.
> That's why I'm proposing above that ssl_hook_process_connection()
> returns SUSPENDED and that we handle it as keep calling
> run_process_connection() after poll()ing here in the MPM. Or have a
> new CONN_STATE_WAIT_IO or something set by the hooks, though since we
> have SUSPENDED already it looks unnecessary.
SUSPENDED already has the opposite meaning, in that a suspended connection will
be excluded from run_process_connection() until such time as some other event
has occurred (response on DNS, a proxy connection received something, etc) and
the connection resumed.
I think we need an AGAIN, like this:
Index: include/httpd.h
===
--- include/httpd.h (revision 1897407)
+++ include/httpd.h (working copy)
@@ -464,6 +464,9 @@
*/
#define SUSPENDED -3 /**< Module will handle the remainder of the request.
* The core will never invoke the request again, */
+#define AGAIN -4/**< Module wants to be called again when
+ * more data is availble.
+ */
/** Returned by the bottom-most filter if no data was written.
* @see ap_pass_brigade(). */
Regards,
Graham
—
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On 24 Jan 2022, at 14:13, Yann Ylavic wrote: >> Maybe it is not needed for the handshake any longer, but it allows fine >> grained timeouts on reading speed for headers and request >> bodies which I am not sure event could deal with. At least it cannot now. > > It's not needed for the handshake *if* we have a HandshakeTimeout or > something (accept() is already done at this point so AcceptTimeout > would be a bit misleading), but currently using s->timeout is not > really the same. > > So far this series is only about handshakes, but indeed if/when all > the request read phases can go async we probably want the fine grained > timeouts/rates from mod_reqtimeout to be available still (so either a > better interaction between the MPM and reqtimeout, or general > HeaderTimeout/BodyTimeout/... directives applicable by the MPM but > we'd still need something to count for bytes/rates in the different > states). Looks like mod_reqtimeout’s capabilities are overstated at this point - the module neatly steps out the way and does nothing the moment it sees a non blocking connection: https://github.com/apache/httpd/blob/trunk/modules/filters/mod_reqtimeout.c#L226 I imagine what we need in the async case is to have the ability to override the timeout on the connection, and have the MPM react to that timeout. The pattern would then be set the timeout and return APR_EAGAIN, let the mpm handle the timeout. We also need to teach mod_reqtimeout’s code that SSL/TLS libraries sometimes want to write: https://github.com/apache/httpd/blob/trunk/modules/filters/mod_reqtimeout.c#L293 Regards, Graham —
Re: svn commit: r1897387 - in /httpd/httpd/trunk: docs/log-message-tags/next-number modules/ssl/mod_ssl.c
On Mon, Jan 24, 2022 at 1:30 PM Yann Ylavic wrote:
>
> On Sun, Jan 23, 2022 at 10:16 PM wrote:
> >
> > --- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
> > +++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Sun Jan 23 21:16:06 2022
> > @@ -723,17 +723,37 @@ static int ssl_hook_process_connection(c
> >
> > if (rv == APR_SUCCESS) {
> > /* great news, lets continue */
> > +
> > +ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(10370)
> > + "SSL handshake completed, continuing");
> > +
> > status = DECLINED;
> > }
> > else if (rv == APR_EAGAIN) {
> > /* we've been asked to come around again, don't block */
> > -status = OK;
> > +
> > +ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(10371)
> > + "SSL handshake in progress, continuing");
> > +
> > + status = OK;
> > +}
> > +else if (rv == APR_EGENERAL) {
> > +/* handshake error, but mod_ssl handled it */
> > +
> > +ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(10372)
> > + "SSL handshake failed, returning error
> > response");
> > +
> > + status = DECLINED;
> > }
> > else {
> > /* we failed, give up */
> >
> > cs->state = CONN_STATE_LINGER;
> >
> > +ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c, APLOGNO(10373)
> > + "SSL handshake was not completed, "
> > + "closing connection");
> > +
> > status = OK;
> > }
> > }
>
> Why is APR_EGENERAL a special error, didn't mod_ssl handle the other errors
> too?
> Is this about special HTTP_SPOKEN_ON_HTTPS error handling?
I mean not only mod_ssl could return an error for
ap_get_brigade(AP_MODE_INIT), APR_EGENERAL is not distinctive enough
IMHO.
>
>
> Regards;
> Yann.
Re: svn commit: r1897387 - in /httpd/httpd/trunk: docs/log-message-tags/next-number modules/ssl/mod_ssl.c
On Sun, Jan 23, 2022 at 10:16 PM wrote:
>
> --- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
> +++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Sun Jan 23 21:16:06 2022
> @@ -723,17 +723,37 @@ static int ssl_hook_process_connection(c
>
> if (rv == APR_SUCCESS) {
> /* great news, lets continue */
> +
> +ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(10370)
> + "SSL handshake completed, continuing");
> +
> status = DECLINED;
> }
> else if (rv == APR_EAGAIN) {
> /* we've been asked to come around again, don't block */
> -status = OK;
> +
> +ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(10371)
> + "SSL handshake in progress, continuing");
> +
> + status = OK;
> +}
> +else if (rv == APR_EGENERAL) {
> +/* handshake error, but mod_ssl handled it */
> +
> +ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(10372)
> + "SSL handshake failed, returning error
> response");
> +
> + status = DECLINED;
> }
> else {
> /* we failed, give up */
>
> cs->state = CONN_STATE_LINGER;
>
> +ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c, APLOGNO(10373)
> + "SSL handshake was not completed, "
> + "closing connection");
> +
> status = OK;
> }
> }
Why is APR_EGENERAL a special error, didn't mod_ssl handle the other errors too?
Is this about special HTTP_SPOKEN_ON_HTTPS error handling?
Regards;
Yann.
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On Mon, Jan 24, 2022 at 10:29 AM Ruediger Pluem wrote:
>
> On 1/24/22 12:43 AM, Graham Leggett wrote:
> > On 22 Jan 2022, at 15:01, Yann Ylavic wrote:
> >
> >>> @@ -268,12 +268,14 @@ struct timeout_queue {
> >>> /*
> >>> * Several timeout queues that use different timeouts, so that we always
> >>> can
> >>> * simply append to the end.
> >>> + * read_line_quses vhost's TimeOut FIXME - we can use a short
> >>> timeout here
> >>
> >> I think we need a better interaction with mod_reqtimeout if we add
> >> client side read timeouts in the MPM.
> >> For instance we document handshake=5 in mod_reqtimeout and it does not
> >> fit the fixed s->timeout here (handshake= is disabled by default for
> >> compatibility because it was added late(ly) in 2.4, but if it's
> >> configured we should honor it in the MPM too).
> >>
> >> Not sure how to do that but the single timeout per timeout_queue model
> >> is probably not the right one here.
> >> Maybe we could queue a timer event based on the actual connection's
> >> socket timeout?
> >
> > The present approach is to extend the existing model of queues in the MPM
> > from two queues to three, I don’t want to make too many changes to the
> > existing structure of the event MPM at this stage if I can get away with it.
> >
> > In theory, there should be an AcceptTimeout directive alongside Timeout and
> > KeepaliveTimeout that mod_reqtimeout should defer to. Or even a case where
> > if an async MPM exists, mod_reqtimeout shouldn’t be needed at all?
>
> Maybe it is not needed for the handshake any longer, but it allows fine
> grained timeouts on reading speed for headers and request
> bodies which I am not sure event could deal with. At least it cannot now.
It's not needed for the handshake *if* we have a HandshakeTimeout or
something (accept() is already done at this point so AcceptTimeout
would be a bit misleading), but currently using s->timeout is not
really the same.
So far this series is only about handshakes, but indeed if/when all
the request read phases can go async we probably want the fine grained
timeouts/rates from mod_reqtimeout to be available still (so either a
better interaction between the MPM and reqtimeout, or general
HeaderTimeout/BodyTimeout/... directives applicable by the MPM but
we'd still need something to count for bytes/rates in the different
states).
Regards;
Yann.
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On Fri, Jan 21, 2022 at 1:09 AM wrote:
>
> Author: minfrin
> Date: Fri Jan 21 00:09:24 2022
> New Revision: 1897281
>
> URL: http://svn.apache.org/viewvc?rev=1897281&view=rev
> Log:
> event: Add support for non blocking behaviour in the
> CONN_STATE_READ_REQUEST_LINE phase, in addition to the existing
> CONN_STATE_WRITE_COMPLETION phase. Update mod_ssl to perform non blocking
> TLS handshakes.
It looks like reusing CONN_STATE_READ_REQUEST_LINE for that is causing
issues, see below.
> --- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
> +++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Fri Jan 21 00:09:24 2022
> @@ -691,6 +692,8 @@ static int ssl_hook_process_connection(c
> {
> SSLConnRec *sslconn = myConnConfig(c);
>
> +int status = DECLINED;
> +
> if (sslconn && !sslconn->disabled) {
> /* On an active SSL connection, let the input filters initialize
> * themselves which triggers the handshake, which again triggers
> @@ -698,13 +701,48 @@ static int ssl_hook_process_connection(c
> */
> apr_bucket_brigade* temp;
>
> +int async_mpm = 0;
> +
> temp = apr_brigade_create(c->pool, c->bucket_alloc);
> -ap_get_brigade(c->input_filters, temp,
> - AP_MODE_INIT, APR_BLOCK_READ, 0);
> +
> +if (ap_mpm_query(AP_MPMQ_IS_ASYNC, &async_mpm) != APR_SUCCESS) {
> +async_mpm = 0;
> +}
> +
> +if (async_mpm) {
> +
> +/* Take advantage of an async MPM. If we see an EAGAIN,
> + * loop round and don't block.
> + */
> +apr_status_t rv;
> +
> +rv = ap_get_brigade(c->input_filters, temp,
> + AP_MODE_INIT, APR_NONBLOCK_READ, 0);
> +
> +if (rv == APR_SUCCESS) {
> +/* great news, lets continue */
> +status = DECLINED;
> +}
> +else if (rv == APR_EAGAIN) {
> +/* we've been asked to come around again, don't block */
> +status = OK;
Maybe we could return SUSPENDED here to tell the MPM that it should
call run_process_connection() again after poll()ing.
> +}
> +else {
> +/* we failed, give up */
> +status = DONE;
> +
> +c->aborted = 1;
> +}
> +}
> +else {
> +ap_get_brigade(c->input_filters, temp,
> + AP_MODE_INIT, APR_BLOCK_READ, 0);
> +}
> +
> apr_brigade_destroy(temp);
> }
> -
> -return DECLINED;
> +
> +return status;
> }
> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
> +++ httpd/httpd/trunk/server/mpm/event/event.c Fri Jan 21 00:09:24 2022
> @@ -1142,6 +1145,7 @@ read_request:
> */
> if (rc != OK || (cs->pub.state >= CONN_STATE_NUM)
> || (cs->pub.state < CONN_STATE_LINGER
> + && cs->pub.state != CONN_STATE_READ_REQUEST_LINE
> && cs->pub.state != CONN_STATE_WRITE_COMPLETION
> && cs->pub.state !=
> CONN_STATE_CHECK_REQUEST_LINE_READABLE
> && cs->pub.state != CONN_STATE_SUSPENDED)) {
The issue is that we've never asked process_connection hooks that
don't care about async and co to change cs->pub.state when they are
done, and for instance mod_echo's process_echo_connection() will
consume the whole connection but leave CONN_STATE_READ_REQUEST_LINE as
is, and here we'll no longer turn it to CONN_STATE_LINGER to close the
connection as before hence loop indefinitely between mod_echo and
mpm_event (eating a CPU) for a connection that is EOF already.
That's why I'm proposing above that ssl_hook_process_connection()
returns SUSPENDED and that we handle it as keep calling
run_process_connection() after poll()ing here in the MPM. Or have a
new CONN_STATE_WAIT_IO or something set by the hooks, though since we
have SUSPENDED already it looks unnecessary.
> @@ -1153,6 +1157,41 @@ read_request:
> cs->pub.state = CONN_STATE_LINGER;
> }
Regards;
Yann.
Re: svn commit: r1897336 - in /httpd/httpd/trunk: modules/ssl/mod_ssl.c support/ab.c
On 24 Jan 2022, at 11:37, Ruediger Pluem wrote: > How is the above related to this commit? Yuck, it’s not. Taken out in r1897408. Regards, Graham —
Re: svn commit: r1897388 - in /httpd/httpd/trunk: docs/log-message-tags/next-number server/mpm/event/event.c
On 1/23/22 10:16 PM, minf...@apache.org wrote: > Author: minfrin > Date: Sun Jan 23 21:16:58 2022 > New Revision: 1897388 > > URL: http://svn.apache.org/viewvc?rev=1897388&view=rev > Log: > Add missing log message tag. There are still failures: https://app.travis-ci.com/github/apache/httpd/jobs/556673001 Regards Rüdiger
Re: svn commit: r1897336 - in /httpd/httpd/trunk: modules/ssl/mod_ssl.c support/ab.c
On 1/22/22 1:09 PM, minf...@apache.org wrote:
> Author: minfrin
> Date: Sat Jan 22 12:09:12 2022
> New Revision: 1897336
>
> URL: http://svn.apache.org/viewvc?rev=1897336&view=rev
> Log:
> When failing, we need to explicitly set the connection state.
>
> Modified:
> httpd/httpd/trunk/modules/ssl/mod_ssl.c
> httpd/httpd/trunk/support/ab.c
>
>
> Modified: httpd/httpd/trunk/support/ab.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/support/ab.c?rev=1897336&r1=1897335&r2=1897336&view=diff
> ==
> --- httpd/httpd/trunk/support/ab.c (original)
> +++ httpd/httpd/trunk/support/ab.c Sat Jan 22 12:09:12 2022
> @@ -1434,7 +1434,7 @@ static void start_connect(struct connect
> }
> #endif
> if ((rv = apr_socket_connect(c->aprsock, destsa)) != APR_SUCCESS) {
> -if (APR_STATUS_IS_EINPROGRESS(rv)) {
> +if (APR_STATUS_IS_EINPROGRESS(rv) || APR_STATUS_IS_EINTR(rv)) {
> set_conn_state(c, STATE_CONNECTING);
> c->rwrite = 0;
> return;
>
How is the above related to this commit?
Regards
Rüdiger
Re: svn commit: r1897281 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c server/mpm/event/event
On 1/24/22 12:43 AM, Graham Leggett wrote:
> On 22 Jan 2022, at 15:01, Yann Ylavic wrote:
>
>>> @@ -268,12 +268,14 @@ struct timeout_queue {
>>> /*
>>> * Several timeout queues that use different timeouts, so that we always can
>>> * simply append to the end.
>>> + * read_line_quses vhost's TimeOut FIXME - we can use a short
>>> timeout here
>>
>> I think we need a better interaction with mod_reqtimeout if we add
>> client side read timeouts in the MPM.
>> For instance we document handshake=5 in mod_reqtimeout and it does not
>> fit the fixed s->timeout here (handshake= is disabled by default for
>> compatibility because it was added late(ly) in 2.4, but if it's
>> configured we should honor it in the MPM too).
>>
>> Not sure how to do that but the single timeout per timeout_queue model
>> is probably not the right one here.
>> Maybe we could queue a timer event based on the actual connection's
>> socket timeout?
>
> The present approach is to extend the existing model of queues in the MPM
> from two queues to three, I don’t want to make too many changes to the
> existing structure of the event MPM at this stage if I can get away with it.
>
> In theory, there should be an AcceptTimeout directive alongside Timeout and
> KeepaliveTimeout that mod_reqtimeout should defer to. Or even a case where if
> an async MPM exists, mod_reqtimeout shouldn’t be needed at all?
Maybe it is not needed for the handshake any longer, but it allows fine grained
timeouts on reading speed for headers and request
bodies which I am not sure event could deal with. At least it cannot now.
Regards
Rüdiger
Re: svn commit: r1897240 - in /httpd/httpd/trunk/server: main.c util_pcre.c
On 1/24/22 9:42 AM, Ruediger Pluem wrote:
>
>
> On 1/20/22 5:14 PM, Yann Ylavic wrote:
>> Index: server/util.c
>> ===
>> --- server/util.c(revision 1897156)
>> +++ server/util.c(working copy)
>> @@ -3261,6 +3261,56 @@ AP_DECLARE(void *) ap_realloc(void *ptr, size_t si
>> return p;
>> }
>>
>> +#if defined(AP_THREAD_LOCAL) && !APR_VERSION_AT_LEAST(1,8,0)
>> +struct thread_ctx {
>> +apr_thread_start_t func;
>> +void *data;
>> +};
>> +
>> +static AP_THREAD_LOCAL apr_thread_t *current_thread;
>
> Shouldn't we do
>
> static AP_THREAD_LOCAL apr_thread_t *current_thread = NULL;
>
> for the sake of clarity?
Otherwise looks good.
Regards
Rüdiger
Re: svn commit: r1897240 - in /httpd/httpd/trunk/server: main.c util_pcre.c
On 1/20/22 5:14 PM, Yann Ylavic wrote:
> Index: server/util.c
> ===
> --- server/util.c (revision 1897156)
> +++ server/util.c (working copy)
> @@ -3261,6 +3261,56 @@ AP_DECLARE(void *) ap_realloc(void *ptr, size_t si
> return p;
> }
>
> +#if defined(AP_THREAD_LOCAL) && !APR_VERSION_AT_LEAST(1,8,0)
> +struct thread_ctx {
> +apr_thread_start_t func;
> +void *data;
> +};
> +
> +static AP_THREAD_LOCAL apr_thread_t *current_thread;
Shouldn't we do
static AP_THREAD_LOCAL apr_thread_t *current_thread = NULL;
for the sake of clarity?
Regards
Rüdiger
Re: svn commit: r1897240 - in /httpd/httpd/trunk/server: main.c util_pcre.c
On 1/20/22 3:52 PM, William A Rowe Jr wrote: > On Thu, Jan 20, 2022 at 5:09 AM wrote: >> >> Author: ylavic >> Date: Thu Jan 20 11:09:34 2022 >> New Revision: 1897240 >> >> URL: http://svn.apache.org/viewvc?rev=1897240&view=rev >> Log: >> ap_regex: Use Thread Local Storage (if efficient) to avoid allocations. >> >> PCRE2 wants an opaque context by providing the API to allocate and free it, >> so >> to minimize these calls we maintain one opaque context per thread (in Thread >> Local Storage, TLS) grown as needed, and while at it we do the same for PCRE1 >> ints vectors. Note that this requires a fast TLS mechanism to be worth it, >> which is the case of apr_thread_data_get/set() from/to apr_thread_current() >> when APR_HAS_THREAD_LOCAL; otherwise we'll do the allocation and freeing for >> each ap_regexec(). >> >> The small stack vector is used for PCRE1 && !APR_HAS_THREAD_LOCAL only now. > > It's good to keep iterating on this, for now, but when I wrote the > patch both pcre1 & pcre2 > were supported by an author, if not a whole community. > > I don't believe the project can or should ship support for httpd > 2.6/3.0/next with support > for a dead library. But it's better not to rip it out just yet. I guess we can rip it out of trunk once we backported the PCRE2 support to 2.4.x and some month without issues in 2.4.x have passed. Regards Rüdiger
Re: svn commit: r1897240 - in /httpd/httpd/trunk/server: main.c util_pcre.c
On 1/20/22 5:14 PM, Yann Ylavic wrote: > On Thu, Jan 20, 2022 at 2:41 PM Ruediger Pluem wrote: >> >> On 1/20/22 2:24 PM, Yann Ylavic wrote: >> >>> >>> All good points, thanks Rüdiger, should be fixed in r1897250. >> >> Great. I guess next we need to think what we do for 2.4.x. >> Even when 1.8.x is released, we cannot demand it for 2.4.x (for trunk we >> could). >> I guess we have two general choices: >> >> 1. We implement it differently on 2.4.x also using TLS when available, but >> not requiring APR 1.8.x. >> 2. We recommend that people who switch over to PCRE2 to use APR 1.8.x for >> performance reasons. >> >> As using PCRE2 make some sense and should be encouraged 2. would make it >> difficult for people tied to older APR versions like some >> distributions. >> >> For 1. my rough idea would be that in case of a threaded MPM we could store >> the apr_thread_t pointer of a worker_thread in a TLS. >> That would solve the performance issue in most cases. > > How about the attached patch? > For APR < 1.8 it defines ap_thread_create()/ap_thread_current() as > wrappers around the APR ones plus storing/loading the current > apr_thread_t* from the TLS as APR 1.8 does (if implemented by the > compiler still). Then it applies a simple > s/apr_thread_create/ap_thread_create/g in the code base.. > Finally ap_regex is adapted to use that if available at compile time > and runtime, otherwise it falls back to allocation/free. > >> >> BTW: I think the current code does not work well in the case of >> !APR_HAS_THREADS, but in this case we could just a static variable >> to store the pointer, correct? > > AP[R]_HAS_THREAD_LOCAL are defined on if APR_HAS_THREADS, so it should > be fine for the compilation. > I'm not sure we should optimize for the !APR_HAS_THREADS case with a > static though, but I could be convinced.. Rethinking as well and I guess even users of prefork will likely compile on systems with APR_HAS_THREADS and probably AP[R]_HAS_THREAD_LOCAL. Hence it might be a very small group of users that would benefit from this. Hence lets park this for a while and resurrect it if my assumption on the amount of users benefiting from this was wrong. Regards Rüdiger
