sqlcounter returning wrong value?
Venkatesh K
kaevee at gmail.com
Thu Oct 30 04:24:47 CET 2008
The patch I posted was out of the hat just to skip the routine
which adds additional quota.
We have stopped using rlm_sqlcounter long back as we moved
from radius schema to our own custom schema and patched
the rlm_sql to work with our custom schema.
If you or someone list out what one would like to see in rlm_sql_counter,
I will try to come up with the patch. Back porting to previous versions
of radius should not be a big problem as rlm_sql and rlm_sqlcounter
have't changed much.
Thanks,
Venkatesh K
2008/10/30 liran tal <liransgarage at gmail.com>:
>
> Thanks for the patch, I have not checked it yet.
> What is the status on this issue though? A patch is nice but I'd like to see
> support for data counters
> to find it's place in the newer version as well as the old one (well, a
> backport would be nice. Many of us still have
> production systems running 1.1.X)
>
> What still disturbs me is that many of the hotspot forums (coovachilli and
> chillispot) has post on how to add an sqlcounter
> which performs these data queries "successfully" and user's feedback is that
> everything is good and working. Not that it's
> something to count on but I'd guess if somsone had an issue then we'd know
> about it...
>
>
> Regards,
> Liran.
>
>
> 2008/10/25 Venkatesh K <kaevee at gmail.com>
>>
>> I have had hard time using sqlcounter for data. It has (at least had)
>> limitations for counting data like 2GB limit. I ended up writing my
>> own code for counting data.
>>
>> Here is a small patch which will skip adding additional quota if
>> "counter-type=time" in sqlcounter.conf file.
>>
>>
>> ----------------------------------------------------------------------------------------------------------------
>> diff -u
>> ../../../freeradius-1.1.7.orig/src/modules/rlm_sqlcounter/rlm_sqlcounter.c
>> rlm_sqlcounter/rlm_sqlcounter.c
>> ---
>> ../../../freeradius-1.1.7.orig/src/modules/rlm_sqlcounter/rlm_sqlcounter.c
>> 2007-04-08
>> 04:51:42.000000000 +0530
>> +++ rlm_sqlcounter/rlm_sqlcounter.c 2008-10-25 22:05:58.000000000
>> +0530
>> @@ -72,6 +72,7 @@
>> char *sqlmod_inst; /* instance of SQL module to use, usually
>> just 'sql' */
>> char *query; /* SQL query to retrieve current session
>> time */
>> char *reset; /* daily, weekly, monthly, never or user
>> defined */
>> + char *counter_type; /* Type of counter (data / time) */
>> char *allowed_chars; /* safe characters list for SQL queries */
>> time_t reset_time;
>> time_t last_reset;
>> @@ -97,6 +98,7 @@
>> { "sqlmod-inst", PW_TYPE_STRING_PTR,
>> offsetof(rlm_sqlcounter_t,sqlmod_inst), NULL, NULL },
>> { "query", PW_TYPE_STRING_PTR, offsetof(rlm_sqlcounter_t,query),
>> NULL, NULL },
>> { "reset", PW_TYPE_STRING_PTR, offsetof(rlm_sqlcounter_t,reset),
>> NULL, NULL },
>> + { "counter-type", PW_TYPE_STRING_PTR,
>> offsetof(rlm_sqlcounter_t,counter_type), NULL, NULL },
>> { "safe-characters", PW_TYPE_STRING_PTR,
>> offsetof(rlm_sqlcounter_t,allowed_chars), NULL,
>> "@abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_:
>> /"},
>> { NULL, -1, 0, NULL, NULL }
>> };
>> @@ -675,10 +677,13 @@
>> * limit, so that the user will not need to
>> * login again
>> */
>> - if (data->reset_time && (
>> - res >= (data->reset_time - request->timestamp))) {
>> - res = data->reset_time - request->timestamp;
>> - res += check_vp->lvalue;
>> + if(strcmp(data->counter_type,"time")==0)
>> + {
>> + if (data->reset_time && (
>> + res >= (data->reset_time -
>> request->timestamp))) {
>> + res = data->reset_time -
>> request->timestamp;
>> + res += check_vp->lvalue;
>> + }
>> }
>>
>> if ((reply_item = pairfind(request->reply->vps,
>> data->reply_attr)) != NULL) {
>>
>> ---------------------------------------------------------------------------------------------------------------------------------------------
>>
>> I did compile the code successfully. I have not checked it. Let me
>> know if you have
>> any issues.
>>
>> Thanks,
>>
>> Venkatesh K
>>
>> 2008/10/25 <tnt at kalik.net>:
>> > And they won't. It's nothing to do with the settings - it's this peace
>> > of the code.
>> >
>> > Let's take your example. Limit was 26MB and about 2MB was left.
>> > 2,000,000 seconds is about 23 days. So this part of the code will "kick
>> > in" (there are 6 days left in this month) and returned value will be
>> > 26MB + number of seconds untill 1.11. Run debug twice. You will see that
>> > the returned value will be reduced by the number of seconds between two
>> > requests.
>> >
>> > Allowance left would have to be well below 1MB for data counter to start
>> > working properly for monthly reset towards the end of the month, 100KB
>> > for weekly reset and 10KB for daily. Which is of no practical use.
>> >
>> > I don't know when was this part of the code added but data counters
>> > would work fine without it. I was testing time counter and still
>> > reproduced this only because limit value was so high (10 million). When
>> > I reduced it to 10,000 it worked fine.
>> >
>> > Ivan Kalik
>> > Kalik Informatika ISP
>> >
>> >
>> > Dana 25/10/2008, "liran tal" <liransgarage at gmail.com> piše:
>> >
>> >>I've actually tested the sqlcounter for a data counter for both weekly
>> >> and
>> >>monthly resets and that doesn't work well either...
>> >>This is odd because people have reported this working so... it is either
>> >>something in the configuration that I'm missing
>> >>or I really don't know what's going on but the counter seem to behave as
>> >>expected.
>> >>
>> >>
>> >>Regards,
>> >>Liran.
>> >>
>> >>2008/10/25 <tnt at kalik.net>
>> >>
>> >>> OK. This where the "problem" comes from:
>> >>>
>> >>> /*
>> >>> * If we are near a reset then add the next
>> >>> * limit, so that the user will not need to
>> >>> * login again
>> >>> */
>> >>> if (data->reset_time &&
>> >>> (res >= (data->reset_time - request->timestamp))) {
>> >>> res = data->reset_time - request->timestamp;
>> >>> res += check_vp->vp_integer;
>> >>> }
>> >>>
>> >>> (that's rlm_sqlcounter.c line about 710 in 2.0.5)
>> >>>
>> >>> Sqlcounter was designed for time counters. When your allowance (limit
>> >>> -
>> >>> time used) is greater than the time left in the period, time left in
>> >>> the
>> >>> period is added to the limit and sent as reply.
>> >>>
>> >>> This will create problems for data counters since limit values are
>> >>> much
>> >>> greater than those for time counters. As a workaround I would suggest
>> >>> that you comment this out if you are using data counters.
>> >>>
>> >>> Perhaps there is a case for adding another configuration item to
>> >>> sqlcounter that would determine what is counted time or data. Then for
>> >>> data counters this can be skipped or replaced with something like:
>> >>>
>> >>> if (data->reset_time && (trigger >= (data->reset_time -
>> >>> request->timestamp))) {
>> >>> res += check_vp->vp_integer;
>> >>> }
>> >>>
>> >>> where trigger would be set to a minute for hourly counter and hour for
>> >>> daily, weekly and monthly counters. I am sorry but I don't know how to
>> >>> write a patch.
>> >>>
>> >>> Ivan Kalik
>> >>> Kalik Informatika ISP
>> >>>
>> >>> Dana 24/10/2008, "liran tal" <liransgarage at gmail.com> piše:
>> >>>
>> >>> >Hey Ivan
>> >>> >
>> >>> >2008/10/24 <tnt at kalik.net>
>> >>> >
>> >>> >> It (daily sqlcounter) does the same in 2.0.5:
>> >>> >>
>> >>> >> rlm_sqlcounter: Authorized user jagoda, check_item=10000000,
>> >>> counter=2635
>> >>> >> rlm_sqlcounter: Sent Reply-Item for user jagoda,
>> >>> >> Type=Session-Timeout,
>> >>> >> value=10027850
>> >>> >>
>> >>> >> Returns value that is greater than the limit. I am using noreset
>> >>> >> sqlcounter and that one works fine.
>> >>> >
>> >>> >
>> >>> >Thanks for confirming this on a more up to date version.
>> >>> >Alan, this smells like a bug (unless we missed something along the
>> >>> > way),
>> >>> >should I open up a bug ticket?
>> >>> >And what would be the chances it can be backported to 1.1.7?
>> >>> >
>> >>> >Thanks,
>> >>> >Liran.
>> >>>
>> >>> -
>
> -
> List info/subscribe/unsubscribe? See
> http://www.freeradius.org/list/users.html
>
--
Venkatesh. K
More information about the Freeradius-Users
mailing list