[jabberd2] Re: Rate limiting non-functional?

2008-07-10 Thread Mark Doliner
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.

-Mark

-- 
To unsubscribe send a mail to [EMAIL PROTECTED]



[jabberd2] Re: Rate limiting non-functional?

2008-07-10 Thread Mark Doliner
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.

-Mark

-- 
To unsubscribe send a mail to [EMAIL PROTECTED]