Ruediger Pluem wrote:
+ * The major difference is that mod_antiloris checks the scoreboard
+ * on every request. This implies a per-request overhead that grows
+ * with the scoreboard, and gets very expensive on a big server.
+ * On the other hand, this module (mod_noloris) may be slower to
+ * react to a DoS attack, and in the case of a very small server
+ * it might be too late.
I am not sure if doing this for each connection (not each *request*)
is really that much of a performace hit. I used a modified version of
mod_limitipcon and do just that. So far I haven't noticed any performance
issues with this approach. But maybe with a maximum of 1040 clients
I am just having a small server :-).
I'm also thinking of simpler versions that set per-process limits
(no use of scoreboard/shm). Obviously for servers with high numbers
of threads-per-server. As and when round tuits permit.
+ apr_socket_close(csd);
This looks like an ugly thing to do for an module. Why not running the whole
thing in the pre_connection hook and return something different then OK or DONE.
This lets the core perform this dirty work in the predefined way.
I did have a good reason for process_connection over pre_connection,
but I can't bring it to mind right now.
+ /* store this client IP for the monitor to pick up */
+ /* under traditional scoreboard, none of this happens until
+ * there's a request_rec. This is where we use the illegally-
+ * obtained private info from the scoreboard.
+ */
+
+ ws = &ap_scoreboard_image->servers[sbh->child_num][sbh->thread_num];
Shouldn't we use ap_get_scoreboard_worker here instead?
Probably, but it does nothing about the badness of how we got
child_num and thread_num.
+ ip = apr_palloc(pool, 18);
Harcoded values are IMHO ugly. Why 18 at all?
Erm .. 'cos I can't count. Needs to hold the max size of
an (IPv6) address string.
+ memset(shm_rec, NULL, shm_size);
Why NULL and not 0?
Erm ... 'cos we have a heatwave, and I'm in an office in an
attic apartment with a south-facing window and low ceiling.
Storing the configuration in global variables looks kind of ugly.
But I get the point that it is hard to get them in the monitor hook.
But at least a struct for them and a global pointer to the struct would
make them look a little nicer :-).
Feel free to change it. I don't really have any strong preference
there, and with GLOBAL_ONLY there's no context or hierarchy to
worry about.
Since it's now in svn, it's open to hack on
