Re: Universal setting for APR_HOOK_PROBES_ENABLED
On Tue, May 10, 2011 at 7:52 AM, Jim Jagielski wrote:
>
> On May 9, 2011, at 5:06 PM, Jeff Trawick wrote:
>>
>> This patch disables hook probes for our two hooks which don't have args:
>>
>
>> /* which gen? */
>>
>> ugly but effective
>>
>
> +1 (on both counts ;)
>
>
>>
>> I rehashed that with the latest code and the ugly no-arg-hook patch
>> above. Attached is a .c file implementation and a corresponding
>> ap_hook_probes.h for this sample feature set. The .c file has to be
>> included from some httpd source file to get it linked in. It won't
>> work without the workaround for our two no-arg hooks.
>>
>> What's a cleaner way to add the code to the server, assuming that
>> somebody implementing hook probes needs to add
>>
>> 1) a .h file that gets included by ap_hooks.h
>> 2) a .c file that should be linked in to httpd
>>
>> ?
>>
>> Maybe --enable-hook-probes=/path/to/ap_hook_probes.{c,h}
>>
>> Alternately, the code that needs to get linked in could be implemented
>> with a drop-in modules/myprobes/{config.m4,mod_myprobes.c,etc.} and
>> then --enable the module statically.
>>
>
> My thoughts are to hopefully make it as self-contained w/i a
> module as possible... That is, the dev must enable-hook-probes and
> then enable/link-in a module which implements them, so I'm
> more consdering the latter.
How about this:
1. --enable-hook-probes=dtrace:
use this to enable existing dtrace code; we'd remove the existing
dtrace configure option; dtrace hook probes and non-dtrace hook probes
necessarily conflict
the dtrace probe list has to be brought up to date and the user has to
add uncommitted build hacks to make it work; while I can't see
committing the build hacks that Theo had, I wouldn't like to see it
get into worse shape
2. --enable-hook-probes=api:
provide a simple API which allows a module to specify request hook
probe callbacks and/or connection hook probe callbacks; the API is
first-come-first-served, as this special mechanism to probe our
primary extensibility mechanism is not itself extensible in the usual
manner ;)
other hooks (those other than connection-based or request-based)
wouldn't be supported
distinguishing request vs. connection would help a little with type
safety and, for some such modules, allow bypassing one of these
categories if it isn't interesting to the module; I guess a module
which cared only for request hooks could coexist with one which cared
only for connection hooks
I predict ugly preprocessing work for this, unless the hook macros are
forked. On the bright side, it should be trivial to exploit the API
given the reduced set of probed hooks (request and/or connection) and
known context within a given probe (request_rec * or conn_rec *)
3.--enable-hooks-probes=custom: provide your own ap_hook_probes.h, do
what you want
Thoughts?
Re: Universal setting for APR_HOOK_PROBES_ENABLED
On May 9, 2011, at 5:06 PM, Jeff Trawick wrote:
>
> This patch disables hook probes for our two hooks which don't have args:
>
> /* which gen? */
>
> ugly but effective
>
+1 (on both counts ;)
>
> I rehashed that with the latest code and the ugly no-arg-hook patch
> above. Attached is a .c file implementation and a corresponding
> ap_hook_probes.h for this sample feature set. The .c file has to be
> included from some httpd source file to get it linked in. It won't
> work without the workaround for our two no-arg hooks.
>
> What's a cleaner way to add the code to the server, assuming that
> somebody implementing hook probes needs to add
>
> 1) a .h file that gets included by ap_hooks.h
> 2) a .c file that should be linked in to httpd
>
> ?
>
> Maybe --enable-hook-probes=/path/to/ap_hook_probes.{c,h}
>
> Alternately, the code that needs to get linked in could be implemented
> with a drop-in modules/myprobes/{config.m4,mod_myprobes.c,etc.} and
> then --enable the module statically.
>
My thoughts are to hopefully make it as self-contained w/i a
module as possible... That is, the dev must enable-hook-probes and
then enable/link-in a module which implements them, so I'm
more consdering the latter.
Re: Universal setting for APR_HOOK_PROBES_ENABLED
On Sun, May 8, 2011 at 7:58 AM, Jeff Trawick wrote:
> a potential concern is that by doing it that way (really, that's the
> only way httpd can help out, whether the include of third-party code
> is in ap_hooks.h or elsewhere) the provider of the probe macros is "on
> the hook" to handle absolutely every hook, which can a bit tedious to
> do
>
> * having separate macros for every individual hook gives you type
> safety and the ability to ignore some or, in general, have different
> functionality for different hooks (e.g., connection-based vs.
> request-based, ignoring everything else); but that gets out of date
> (not a killer)
> * having common macros for all hooks breaks because of challenges with
> the parameter lists (a couple of hooks have 0 parameters!!!) and type
> safety (first parm isn't always a pointer, though you can look at the
> name of the hook)
> ** shall we give optional_fn_retrieve a server_rec & ? (I suggested
> this previously on-list, but only niq responded, and not possitively
> IIRC)
> ** I forget what the other one was
the other one was mpm_name, not a likely candidate for a parameter
This patch disables hook probes for our two hooks which don't have args:
Index: server/config.c
===
--- server/config.c (revision 1101166)
+++ server/config.c (working copy)
@@ -167,6 +167,20 @@
AP_IMPLEMENT_HOOK_RUN_FIRST(int, quick_handler, (request_rec *r, int lookup),
(r, lookup), DECLINED)
+/* hooks with no args are implemented last, after disabling APR hook probes */
+#if defined(APR_HOOK_PROBES_ENABLED)
+#undef APR_HOOK_PROBES_ENABLED
+#undef APR_HOOK_PROBE_ENTRY
+#define APR_HOOK_PROBE_ENTRY(ud,ns,name,args)
+#undef APR_HOOK_PROBE_RETURN
+#define APR_HOOK_PROBE_RETURN(ud,ns,name,rv,args)
+#undef APR_HOOK_PROBE_INVOKE
+#define APR_HOOK_PROBE_INVOKE(ud,ns,name,src,args)
+#undef APR_HOOK_PROBE_COMPLETE
+#define APR_HOOK_PROBE_COMPLETE(ud,ns,name,src,rv,args)
+#undef APR_HOOK_INT_DCL_UD
+#define APR_HOOK_INT_DCL_UD
+#endif
AP_IMPLEMENT_HOOK_VOID(optional_fn_retrieve, (void), ())
/
Index: server/mpm_common.c
===
--- server/mpm_common.c (revision 1101166)
+++ server/mpm_common.c (working copy)
@@ -98,9 +98,6 @@
AP_IMPLEMENT_HOOK_RUN_FIRST(apr_status_t, mpm_register_timed_callback,
(apr_time_t t, ap_mpm_callback_fn_t
*cbfn, void *baton),
(t, cbfn, baton), APR_ENOTIMPL)
-AP_IMPLEMENT_HOOK_RUN_FIRST(const char *, mpm_get_name,
-(void),
-(), NULL)
AP_IMPLEMENT_HOOK_VOID(end_generation,
(server_rec *s, ap_generation_t gen),
(s, gen))
@@ -108,6 +105,24 @@
(server_rec *s, pid_t pid, ap_generation_t
gen, int slot, mpm_child_status status),
(s,pid,gen,slot,status))
+/* hooks with no args are implemented last, after disabling APR hook probes */
+#if defined(APR_HOOK_PROBES_ENABLED)
+#undef APR_HOOK_PROBES_ENABLED
+#undef APR_HOOK_PROBE_ENTRY
+#define APR_HOOK_PROBE_ENTRY(ud,ns,name,args)
+#undef APR_HOOK_PROBE_RETURN
+#define APR_HOOK_PROBE_RETURN(ud,ns,name,rv,args)
+#undef APR_HOOK_PROBE_INVOKE
+#define APR_HOOK_PROBE_INVOKE(ud,ns,name,src,args)
+#undef APR_HOOK_PROBE_COMPLETE
+#define APR_HOOK_PROBE_COMPLETE(ud,ns,name,src,rv,args)
+#undef APR_HOOK_INT_DCL_UD
+#define APR_HOOK_INT_DCL_UD
+#endif
+AP_IMPLEMENT_HOOK_RUN_FIRST(const char *, mpm_get_name,
+(void),
+(), NULL)
+
typedef struct mpm_gen_info_t {
APR_RING_ENTRY(mpm_gen_info_t) link;
int gen; /* which gen? */
ugly but effective
>
> but there's probably practical no way to individually select classes
> of hooks (e.g., it would be nice to enable only for connection-based
> and request-based) other than invasive preprocessor work within .c
> files
>
> should a distinction be made for core+bundled modules vs. third-party
> code? maybe the define to include the third-party .h file is for
> internal CPPFLAGS only, so that it doesn't blow (or get the hook probe
> implementer on the hook to handle stuff she's never heard of) when
> building third-party modules against a probe-enabled build?
fixed now
>
> as far as potential bundled exploitation:
>
> * DTrace is one flavor (not kept out of date, horrible build changes
> never cleaned up/committed)
> * it would be cool to have some teaching/debugging mode available at
> configure time which resulted in a lot of crap written to the error
> log during hook execution which a script could create a nice display
> from, combining error and access log information
> ** perhaps enabled at runtime via -Dfoo
> * another flavor I've played with is just maintaining
> r->notes{"ActiveModule"} and r->no
Re: Universal setting for APR_HOOK_PROBES_ENABLED
Let me just impl the below for now for the upcoming next beta and we'll take it from there... On May 8, 2011, at 9:43 AM, Jim Jagielski wrote: >>> On May 6, 2011, at 8:38 PM, Jim Jagielski wrote: >>> APR_HOOK_PROBES_ENABLED is pretty nice, the rub is that, esp in httpd, we have lots of modules which go ahead and include apr_hooks.h on their own, making the setup of "universal" probe hooks difficult. Anyone have ideas on how to make it easier, either in APR directly or in httpd? Maybe making some kind of httpd_hooks.h file that includes some local local_hooks_probes.h file (depending on some config-time setting) assuming APR_HOOK_PROBES_ENABLED is set (ie: --enable-hook-probes sets APR_HOOK_PROBES_ENABLED and httpd_hooks.h is basically: #ifdef APR_HOOK_PROBES_ENABLED #include "local_hooks_probes.h" #endif #include "apr_hooks.h" and no httpd files include apr_hooks.h directly... That seems a very rough way to do it... :/ >>> >>> >> >> >> >> -- >> Born in Roswell... married an alien... >> >
Re: Universal setting for APR_HOOK_PROBES_ENABLED
Agreed... In some ways it would be cool to actually implement
a ap_hook_probe_entry_* (ie: ap_hook_probe_entry_check_config)
(et.al., that is ap_hook_probe_invoke, ...) that actually moves
them into the actual function list itself instead of using
the macros at all.
This would involve, unfortunately, afaict, simply dropping
APR_HOOKs totally for an AP_HOOKs fork (instead of just AP_HOOKS
using APR_HOOKS)... To be honest, as much as I hate just
duplication of code, I kind of like this idea better since
it's easier for how we wish httpd would use hooks...
Comments? Ideas?
On May 8, 2011, at 7:58 AM, Jeff Trawick wrote:
>
> a potential concern is that by doing it that way (really, that's the
> only way httpd can help out, whether the include of third-party code
> is in ap_hooks.h or elsewhere) the provider of the probe macros is "on
> the hook" to handle absolutely every hook, which can a bit tedious to
> do
>
> * having separate macros for every individual hook gives you type
> safety and the ability to ignore some or, in general, have different
> functionality for different hooks (e.g., connection-based vs.
> request-based, ignoring everything else); but that gets out of date
> (not a killer)
> * having common macros for all hooks breaks because of challenges with
> the parameter lists (a couple of hooks have 0 parameters!!!) and type
> safety (first parm isn't always a pointer, though you can look at the
> name of the hook)
> ** shall we give optional_fn_retrieve a server_rec & ? (I suggested
> this previously on-list, but only niq responded, and not possitively
> IIRC)
> ** I forget what the other one was
>
> but there's probably practical no way to individually select classes
> of hooks (e.g., it would be nice to enable only for connection-based
> and request-based) other than invasive preprocessor work within .c
> files
>
> should a distinction be made for core+bundled modules vs. third-party
> code? maybe the define to include the third-party .h file is for
> internal CPPFLAGS only, so that it doesn't blow (or get the hook probe
> implementer on the hook to handle stuff she's never heard of) when
> building third-party modules against a probe-enabled build?
>
> as far as potential bundled exploitation:
>
> * DTrace is one flavor (not kept out of date, horrible build changes
> never cleaned up/committed)
> * it would be cool to have some teaching/debugging mode available at
> configure time which resulted in a lot of crap written to the error
> log during hook execution which a script could create a nice display
> from, combining error and access log information
> ** perhaps enabled at runtime via -Dfoo
> * another flavor I've played with is just maintaining
> r->notes{"ActiveModule"} and r->notes{"RequestFailer"} for access from
> mod_whatkilledus or access log, respectively
>
>> On May 6, 2011, at 8:38 PM, Jim Jagielski wrote:
>>
>>> APR_HOOK_PROBES_ENABLED is pretty nice, the rub is that, esp
>>> in httpd, we have lots of modules which go ahead and include
>>> apr_hooks.h on their own, making the setup of "universal"
>>> probe hooks difficult.
>>>
>>> Anyone have ideas on how to make it easier, either in
>>> APR directly or in httpd? Maybe making some kind of
>>> httpd_hooks.h file that includes some local local_hooks_probes.h
>>> file (depending on some config-time setting) assuming
>>> APR_HOOK_PROBES_ENABLED is set (ie: --enable-hook-probes
>>> sets APR_HOOK_PROBES_ENABLED and httpd_hooks.h is
>>> basically:
>>>
>>> #ifdef APR_HOOK_PROBES_ENABLED
>>> #include "local_hooks_probes.h"
>>> #endif
>>> #include "apr_hooks.h"
>>>
>>> and no httpd files include apr_hooks.h directly...
>>>
>>> That seems a very rough way to do it... :/
>>>
>>
>>
>
>
>
> --
> Born in Roswell... married an alien...
>
Re: Universal setting for APR_HOOK_PROBES_ENABLED
On Sat, May 7, 2011 at 7:33 PM, Jim Jagielski wrote:
> Here's my idea:
>
> o create ap_hooks.h which moves the AP_IMPLEMENT_HOOKS* from
> ap_config.h to itself.
> o Fold in the APR_HOOK_PROBES_ENABLED logic as
> described below.
> o change configure to accept --enable-hooks-probes=
> which points to the location of the ap_hooks_probes.h
> file.
> o Adjust all files to include ap_hooks.h instead
> of apr_hooks.h
>
> I'll start coding this tomorrow; my hope is to have this
> in for the next beta (as RM, I'm gonna hold off a T&R
> until then) since I think it's something that we want
> to really promote as a cool 2.4 feature.
sounds right in general; one of my prior patches (Oakland or Atlanta?)
had ap_hooks.h or similar
a potential concern is that by doing it that way (really, that's the
only way httpd can help out, whether the include of third-party code
is in ap_hooks.h or elsewhere) the provider of the probe macros is "on
the hook" to handle absolutely every hook, which can a bit tedious to
do
* having separate macros for every individual hook gives you type
safety and the ability to ignore some or, in general, have different
functionality for different hooks (e.g., connection-based vs.
request-based, ignoring everything else); but that gets out of date
(not a killer)
* having common macros for all hooks breaks because of challenges with
the parameter lists (a couple of hooks have 0 parameters!!!) and type
safety (first parm isn't always a pointer, though you can look at the
name of the hook)
** shall we give optional_fn_retrieve a server_rec & ? (I suggested
this previously on-list, but only niq responded, and not possitively
IIRC)
** I forget what the other one was
but there's probably practical no way to individually select classes
of hooks (e.g., it would be nice to enable only for connection-based
and request-based) other than invasive preprocessor work within .c
files
should a distinction be made for core+bundled modules vs. third-party
code? maybe the define to include the third-party .h file is for
internal CPPFLAGS only, so that it doesn't blow (or get the hook probe
implementer on the hook to handle stuff she's never heard of) when
building third-party modules against a probe-enabled build?
as far as potential bundled exploitation:
* DTrace is one flavor (not kept out of date, horrible build changes
never cleaned up/committed)
* it would be cool to have some teaching/debugging mode available at
configure time which resulted in a lot of crap written to the error
log during hook execution which a script could create a nice display
from, combining error and access log information
** perhaps enabled at runtime via -Dfoo
* another flavor I've played with is just maintaining
r->notes{"ActiveModule"} and r->notes{"RequestFailer"} for access from
mod_whatkilledus or access log, respectively
> On May 6, 2011, at 8:38 PM, Jim Jagielski wrote:
>
>> APR_HOOK_PROBES_ENABLED is pretty nice, the rub is that, esp
>> in httpd, we have lots of modules which go ahead and include
>> apr_hooks.h on their own, making the setup of "universal"
>> probe hooks difficult.
>>
>> Anyone have ideas on how to make it easier, either in
>> APR directly or in httpd? Maybe making some kind of
>> httpd_hooks.h file that includes some local local_hooks_probes.h
>> file (depending on some config-time setting) assuming
>> APR_HOOK_PROBES_ENABLED is set (ie: --enable-hook-probes
>> sets APR_HOOK_PROBES_ENABLED and httpd_hooks.h is
>> basically:
>>
>> #ifdef APR_HOOK_PROBES_ENABLED
>> #include "local_hooks_probes.h"
>> #endif
>> #include "apr_hooks.h"
>>
>> and no httpd files include apr_hooks.h directly...
>>
>> That seems a very rough way to do it... :/
>>
>
>
--
Born in Roswell... married an alien...
Re: Universal setting for APR_HOOK_PROBES_ENABLED
Here's my idea: o create ap_hooks.h which moves the AP_IMPLEMENT_HOOKS* from ap_config.h to itself. o Fold in the APR_HOOK_PROBES_ENABLED logic as described below. o change configure to accept --enable-hooks-probes= which points to the location of the ap_hooks_probes.h file. o Adjust all files to include ap_hooks.h instead of apr_hooks.h I'll start coding this tomorrow; my hope is to have this in for the next beta (as RM, I'm gonna hold off a T&R until then) since I think it's something that we want to really promote as a cool 2.4 feature. On May 6, 2011, at 8:38 PM, Jim Jagielski wrote: > APR_HOOK_PROBES_ENABLED is pretty nice, the rub is that, esp > in httpd, we have lots of modules which go ahead and include > apr_hooks.h on their own, making the setup of "universal" > probe hooks difficult. > > Anyone have ideas on how to make it easier, either in > APR directly or in httpd? Maybe making some kind of > httpd_hooks.h file that includes some local local_hooks_probes.h > file (depending on some config-time setting) assuming > APR_HOOK_PROBES_ENABLED is set (ie: --enable-hook-probes > sets APR_HOOK_PROBES_ENABLED and httpd_hooks.h is > basically: > > #ifdef APR_HOOK_PROBES_ENABLED > #include "local_hooks_probes.h" > #endif > #include "apr_hooks.h" > > and no httpd files include apr_hooks.h directly... > > That seems a very rough way to do it... :/ >
Universal setting for APR_HOOK_PROBES_ENABLED
APR_HOOK_PROBES_ENABLED is pretty nice, the rub is that, esp in httpd, we have lots of modules which go ahead and include apr_hooks.h on their own, making the setup of "universal" probe hooks difficult. Anyone have ideas on how to make it easier, either in APR directly or in httpd? Maybe making some kind of httpd_hooks.h file that includes some local local_hooks_probes.h file (depending on some config-time setting) assuming APR_HOOK_PROBES_ENABLED is set (ie: --enable-hook-probes sets APR_HOOK_PROBES_ENABLED and httpd_hooks.h is basically: #ifdef APR_HOOK_PROBES_ENABLED #include "local_hooks_probes.h" #endif #include "apr_hooks.h" and no httpd files include apr_hooks.h directly... That seems a very rough way to do it... :/
