rlm_sql: zero session length
Hi all, I see rlm_sql complaining about stop packets with zero session length, and then just ignoring them. This misleading message actually tells that Acct-Session-Time = 0. And the code check for <= 0:) The RFC tells nothing about valid values for it, it simply states it's number of seconds. Well, these days we can do a lot in a second. I.e. we can perform a single HTTP GET, in order to download a MMS. And in that case, we get invalid radacct table, due to intentionally dropped records. Or, in VoIP, Acct-Session-Time of zero means a call was attempted, but was not accepted. (details are in another attribute) Or... anyway, it's not a big deal, though I'd rather have consistent radacct table. Why is this if statement there anyway, some buggy NASes? If so, why not just let user handle it in unlang? IMHO that needs to be removed, or if written for a good reason, made configurable. With rfc-compliant default:) Regards...
Why is this if statement there anyway, some buggy NASes?
Almost certainly.
If so, why not just let user handle it in unlang?
The rlm_sql module was written before unlang, so that's why the code is there.
IMHO that needs to be removed, or if written for a good reason, made configurable. With rfc-compliant default:)
I'll remove it. It's easy enough to do the check in unlang. Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team
On 17 Jun 2013, at 13:49, Arran Cudbard-Bell <a.cudbardb@freeradius.org> wrote:
Why is this if statement there anyway, some buggy NASes?
Almost certainly.
If so, why not just let user handle it in unlang?
The rlm_sql module was written before unlang, so that's why the code is there.
IMHO that needs to be removed, or if written for a good reason, made configurable. With rfc-compliant default:)
I'll remove it. It's easy enough to do the check in unlang.
It's already gone in 3.0, and you can disable it if you build from source in 2.x.x. https://github.com/FreeRADIUS/freeradius-server/blob/v2.x.x/src/modules/rlm_... It should probably be left as is for 2.2.1 for users which rely on this behaviour. Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team
Arran Cudbard-Bell wrote:
On 17 Jun 2013, at 13:49, Arran Cudbard-Bell <a.cudbardb@freeradius.org> wrote:
Why is this if statement there anyway, some buggy NASes?
Almost certainly.
If so, why not just let user handle it in unlang?
The rlm_sql module was written before unlang, so that's why the code is there.
IMHO that needs to be removed, or if written for a good reason, made configurable. With rfc-compliant default:)
I'll remove it. It's easy enough to do the check in unlang.
It's already gone in 3.0, and you can disable it if you build from source in 2.x.x.
https://github.com/FreeRADIUS/freeradius-server/blob/v2.x.x/src/modules/rlm_...
It should probably be left as is for 2.2.1 for users which rely on this behaviour.
Thanks. In the meantime, I've analyzed what I have. Including 2 and 3 source. And the catch is, I should never ever get that warning, because it activates only 'if (numaffected < 1)'. IOW, when update is executed, but no rows were updated. I have 8% sessions with the same starttime and stoptime. And of those, 0.13% gave us 'zero session length' warning. Turned on sqltrace, and I do see insert for start packet first, then update for stop. One right after another, amongst all other requests. But does that mean insert was executed first, and then update? I think it was only scheduled to execution in that order. Local scheduler or a database hickup might delay first query just a few milliseconds too long... Wonders of parallelization;) Regards...
participants (2)
-
Arran Cudbard-Bell -
Josip Almasi