Re: rlm_sql.c in 2.0.0-pre2
Unfortunately whoever modified rlm_sql in cvs head chose a very inefficient querying system. So change it - stored procedures maybe? First you query to pull out group membership, second you query to get each groups check items, then to get each groups reply items ... It just doesn't scale when a users a member of lots of groups. Previously you pulled out all the records for all the groups a user was a member of in two queries, one for check items and one for reply items.. Eh? I've got to strongly disagree with that - the old code was a DISASTROUS scheme. If you had 2 groups with check items: RESIDENCES: check: Calling-Station-Id ~ 192.168. reply: Filter-Id = resnet CONFERENCES: check: Calling-Station-Id ~ 10. reply: Filter-Id = conferencenet ...and johndoe was in BOTH, NEITHER of them would *ever* match. Merging the groups' check items was just idiotic. The new version is far, far better. --- Still think it would be a nice idea to have the option to disable single Not sure what you mean by that? user lookups, SQL queries really are very expensive . Expensive: we're doing ~260k authentications a day, each involving at least 1 SQL SELECT and 1 SQL INSERT and we've no problems. Hardware is nothing massively silly - dual-proc DL380 running both the SQL and Radius server. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: rlm_sql.c in 2.0.0-pre2
Phil Mayers wrote:
Unfortunately whoever modified rlm_sql in cvs head chose a very
inefficient querying system.
So change it - stored procedures maybe?
Stored procedures ... but then you lose the only advantage of using the
SQL modules in authorisation, which is that it can evaluate the check
items when assigning users to groups.
First you query to pull out group membership, second you query to get
each groups check items, then to get each groups reply items ... It just
doesn't scale when a users a member of lots of groups.
Previously you pulled out all the records for all the groups a user was
a member of in two queries, one for check items and one for reply items..
Eh? I've got to strongly disagree with that - the old code was a
DISASTROUS scheme. If you had 2 groups with check items:
RESIDENCES:
check: Calling-Station-Id ~ 192.168.
reply: Filter-Id = resnet
CONFERENCES:
check: Calling-Station-Id ~ 10.
reply: Filter-Id = conferencenet
...and johndoe was in BOTH, NEITHER of them would *ever* match.
Merging the groups' check items was just idiotic. The new version is
far, far better.
I agree the old way of doing things was just plain broken, but splitting
the querying scheme into two parts, while making processing the SQL
result far easier, is inefficient.
You can pull out all rows relating to the groups a user is a member of,
and process them on the RADIUS server using the group name as a key.
This could be done in one query if you were feeling adventurous, or
probably more easily with two.
SELECT * FROM `radgroupcheck`,`radusergroup` WHERE radusergroup.UserName
= '%{User-Name}' AND radgroupcheck.GroupName = radusergroup.GroupName
SELECT * FROM `radgroupreply`,`radusergroup` WHERE radusergroup.UserName
= '%{User-Name}' AND radgroupcheck.GroupName = radusergroup.GroupName
This is far more scalable... Think of a user who belongs to 5 groups, we
don't know which groups will actually be valid so all have to be pulled
down from the SQL server ...
Thats one radcheck
one radreply
one membership query.
5 check items queries.
5 reply items queries.
If you've configured your server very efficiently you'll need to grab
these at least twice for PEAP...
So thats 26 Select queries every time that user authenticates... as
opposed to 8, (or 4 without radcheck,radreply).
Another possible alternative improvement using the current scheme,would
be to bring all the rows relating to a group down, then evaluate them,
and only continue querying if fall-through was true, or the check items
didn't match.
Not sure what you mean by that?
Only do group lookups as opposed to checking for the users existence
radcheck and radreply...
Not everyone uses SQL for storing attributes in Authentication.
We use it because it provides a flexible means of overriding defaults in
the users file, and allows special authorisation groups to be added, and
memberships to be changed, without restarting the server.
user lookups, SQL queries really are very expensive .
Expensive: we're doing ~260k authentications a day, each involving at
least 1 SQL SELECT and 1 SQL INSERT and we've no problems. Hardware is
nothing massively silly - dual-proc DL380 running both the SQL and
Radius server.
Our SQL server is running on the campus webserver, it's horribly
overloaded and horribly slow.
Your running your SQL server on the same box as the RADIUS server which
gives you a significant advantage in that your queries aren't affected
by network latency.
Don't get me wrong, I appreciate you fixing the horribly broken SQL
code, I was just suggesting some things that might improve efficiency.
Thanks,
Arran
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: rlm_sql.c in 2.0.0-pre2
You can pull out all rows relating to the groups a user is a member of, and process them on the RADIUS server using the group name as a key. This could be done in one query if you were feeling adventurous, or probably more easily with two. That's a very reasonable suggestion, and would not I suspect be hard to code. However, I think it would prevent certain types of dynamic processing in the SQL side, so it would ideally be an optional configuration e.g. bulk_query_groups = yes/no or something. Another possible alternative improvement using the current scheme,would be to bring all the rows relating to a group down, then evaluate them, and only continue querying if fall-through was true, or the check items didn't match. Good suggestion - but it already does that. See lines 610 (the for loop dependent on the dofallthrough flag being 1) and 665/699 (setting of the dofallthrough flag based on the per-group Fall-Through reply items) of rlm_sql.c in todays CVS checkout. I'm fairly sure the CVS module has done that for months Our SQL server is running on the campus webserver, it's horribly overloaded and horribly slow. Then frankly I'm surprised you aren't experiencing more problems Don't get me wrong, I appreciate you fixing the horribly broken SQL code, I was just suggesting some things that might improve efficiency. I didn't fix it - the only code I've ever contributed to FreeRadius has been tiny 1-liners in the bug tracker. I was speaking up in support of whoever did. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: rlm_sql.c in 2.0.0-pre2
Alexander Serkin wrote: Hi, Is the read_groups configuration paramter reading strings intentionally removed from rlm_sql.c? Why? I don't think it was ever added. I'm not sure the functionality is even tested. i.e. Does it work? Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: rlm_sql.c in 2.0.0-pre2
Alan DeKok wrote: Alexander Serkin wrote: Hi, Is the read_groups configuration paramter reading strings intentionally removed from rlm_sql.c? Why? I don't think it was ever added. I'm not sure the functionality is even tested. i.e. Does it work? Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html Read Groups in SQL ? Yes, very very well tested. It's horribly broken in 1.*.* though, or at least it was for me. Unfortunately whoever modified rlm_sql in cvs head chose a very inefficient querying system. First you query to pull out group membership, second you query to get each groups check items, then to get each groups reply items ... It just doesn't scale when a users a member of lots of groups. Previously you pulled out all the records for all the groups a user was a member of in two queries, one for check items and one for reply items.. --- Still think it would be a nice idea to have the option to disable single user lookups, SQL queries really are very expensive . -- Arran Cudbard-Bell ([EMAIL PROTECTED]) Authentication, Authorisation and Accounting Officer Infrastructure Services | ENG1 E1-1-08 University Of Sussex, Brighton EXT:01273 873900 | INT: 3900 - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: rlm_sql.c in 2.0.0-pre2
Arran Cudbard-Bell wrote: Alan DeKok wrote: I don't think it was ever added. I'm not sure the functionality is even tested. i.e. Does it work? Alan DeKok. Read Groups in SQL ? Yes, very very well tested. It's horribly broken in 1.*.* though, or at least it was for me. Unfortunately whoever modified rlm_sql in cvs head chose a very inefficient querying system. First you query to pull out group membership, second you query to get each groups check items, then to get each groups reply items ... It just doesn't scale when a users a member of lots of groups. Previously you pulled out all the records for all the groups a user was a member of in two queries, one for check items and one for reply items.. Yes. It worked for me this way until at least 1.1.6. You are right, Alan, - read_grops configuration checks were not in 1.1.x also, but they worked somehow. Starting from 2.0.0-pre only user checks are performed by default. The only way to make groups to be checked was the supposed patch. Or adding Fall-Through=yes for all user profiles in radcheck table which is not good. -- Sincerely Yours, Alexander - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
rlm_sql.c in 2.0.0-pre2
Hi,
Is the read_groups configuration paramter reading strings
intentionally removed from rlm_sql.c? Why?
Let me suggest a patch:
*** rlm_sql.c.orig 2007-05-15 14:10:35.0 +0400
--- rlm_sql.c 2007-06-18 19:46:59.0 +0400
***
*** 57,62
--- 57,64
offsetof(SQL_CONFIG,tracefile), NULL, SQLTRACEFILE},
{readclients, PW_TYPE_BOOLEAN,
offsetof(SQL_CONFIG,do_clients), NULL, no},
+ {read_groups, PW_TYPE_BOOLEAN,
+ offsetof(SQL_CONFIG,read_groups), NULL, yes},
{deletestalesessions, PW_TYPE_BOOLEAN,
offsetof(SQL_CONFIG,deletestalesessions), NULL, yes},
{num_sql_socks, PW_TYPE_INTEGER,
read_groups is checked on line 959 of rlm_sql.c, but it's not set
anywhere before.
thanks,
--
Alexander
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
