On Thu, Jul 10, 2008 at 5:51 PM, Mark Doliner <[EMAIL PROTECTED]> wrote: > On Thu, Jul 10, 2008 at 5:38 PM, Mark Doliner <[EMAIL PROTECTED]> wrote: >> On Mon, Jul 7, 2008 at 10:25 AM, Mark Doliner <[EMAIL PROTECTED]> wrote: >>> I've been looking at the rate limiting options in c2s.xml, and looking >>> at the code that intends to enforce those limits. It looks like the >>> connection rate limit would work (but I haven't tested it), but it >>> looks like the byte limit code has a few major problems that would >>> keep it from doing anything useful. Has anyone successfully used byte >>> limiting? >>> >>> For starters there's no call to rate_add(), which means jabberd checks >>> if the rate has been exceeded, but it always checks if 0 > rate_limit, >>> which will never happen. Then there's a call to rate_left(), which I >>> think is intended to have the recv() call only attempt to read the >>> number of bytes that are allowed before you would be rate limited, but >>> the return value from rate_left() is never used. >>> >>> Those two problems are fairly easy to fix. The third problem I've >>> noticed, which is more difficult, is what to do once someone has been >>> rate limited. Ideally jabberd would just stop reading data from that >>> user until the desired number of seconds has elapsed. But that's hard >>> to do because I think our mio code uses readiness change >>> notification/edge-triggered readiness notification instead of >>> level-triggered readiness notification. So we can't just not read >>> from the socket during this function call because >>> _c2s_client_sx_callback() won't be called again. We could disconnect >>> the user pretty easily, but that's a bit clunky. >> >> I noticed that router has similar rate limiting, but there the byte >> rate limit is a little more effective, but I think it has the same >> problem with ceasing to read from the socket once it gets limited. It >> seems weird to me that the router would limit the traffic sent between >> the components and the rate at which new components can connect. I'm >> in favor of removing this limiting from router for the sake of >> simplifying the code and simplifying router.xml. How do other people >> feel? >> >> I noticed another flaw in the rate implementation. rate_t->bad is set >> when rate_add() is called, and rate_t->time is only reset in >> rate_reset(). What ends up happening is that the rate count gradually >> increases and approaches the limit. It eventually calls rate_add() >> which pushes the count over the limit and you get rate limited. >> >> This isn't so bad, except that it happens regardless of the amount of >> time that has elapsed since the first message. > > Oh, and there's another problem which is that the window doesn't > slide. It starts when you increment the count for the first time, > then resets at the end of the timeout. I guess this is a pretty minor > problem when the window is so short, but it's possible for someone to > be limited when they should not have been, and not limited when they > should have been. > > For example, say you have two windows back to back. If someone bursts > a lot of traffic at the end of the first window, and then a lot of > traffic at the beginning of the second window (but not enough to go > over the threshold in either window), they probably SHOULD be limited > because they've sent a lot of bytes within the time span of one > window, but they won't be.
Sorry for replying to myself a bunch of times, I've been thinking out loud. I checked in a change that fixes the bigger problems here. There are still two things that could be improved: 1. Don't disconnect people when they hit the rate limit. Instead, stop reading from their socket until the limit has expired. Do we have a way to call a function after a certain amount of time has passed? We would need one for this. 2. The window still doesn't slide, it just starts over. This is negligible if the window is small. If you wanted to limit people to sending no more than 100MB/hour then this might be more of an issue. -Mark -- To unsubscribe send a mail to [EMAIL PROTECTED]
