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