sqlcounter returning wrong value?
Venkatesh K
kaevee at gmail.com
Sat Oct 25 18:42:54 CEST 2008
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
>>>
>>
>
> -
> List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
>
--
Venkatesh. K
More information about the Freeradius-Users
mailing list