DHCP + sqlpippol test, v2.1.x
Hi, I'm testing some modifications to make DHCP able to allocate dynamic IP address using sqlippool. It's only tested with one client, but it should be able to assign dynamic IP address (smallest ip address first, due to sorting). The modification is purely config files addition/modification, no source code change. Current version is here: https://github.com/fajarnugraha/freeradius-server/commit/8af7142 Static IP allocation SHOULD also be possible by having expiration time far in the future (e.g. 2100-01-01 00:00:00) on radippool table, and modifying the query to NOT overwrite the expiration time on that scenario. This hasn't been implemented though. Any thoughts on these changes? Perhaps there a simpler way to implement it. -- Fajar
Fajar A. Nugraha wrote:
I'm testing some modifications to make DHCP able to allocate dynamic IP address using sqlippool. It's only tested with one client, but it should be able to assign dynamic IP address (smallest ip address first, due to sorting). The modification is purely config files addition/modification, no source code change. Current version is here: https://github.com/fajarnugraha/freeradius-server/commit/8af7142
It looks very nice. I'm a bit surprised that it's that simple.
Static IP allocation SHOULD also be possible by having expiration time far in the future (e.g. 2100-01-01 00:00:00) on radippool table, and modifying the query to NOT overwrite the expiration time on that scenario. This hasn't been implemented though.
Any thoughts on these changes? Perhaps there a simpler way to implement it.
The changes could be simpler. A lot of the code in policy.conf is site-specific, and doesn't need to go into the server core. My $0.02: dhcp.sqlippool_request - this is good, it "patches" the DHCP packet so that the SQLIPPool module understands it dhcp.sqlippool_reply - this is good, too The sites-available/dhcp-sqliipool text is not necessary. Just put the sqlippool examples into the existing DHCP virtual server. The SQL sample configurations for MySQL are good. I'll pull part of it into the v2.1.x branch. Please check that in a day or so, to see what needs to be done in order for it to work. Alan DeKok.
On Thu, Jan 19, 2012 at 2:36 PM, Alan DeKok <aland@deployingradius.com> wrote:
Fajar A. Nugraha wrote:
I'm testing some modifications to make DHCP able to allocate dynamic IP address using sqlippool. It's only tested with one client, but it should be able to assign dynamic IP address (smallest ip address first, due to sorting). The modification is purely config files addition/modification, no source code change. Current version is here: https://github.com/fajarnugraha/freeradius-server/commit/8af7142
It looks very nice. I'm a bit surprised that it's that simple.
Glad to hear it :D Most of my changes are simple ones,
Static IP allocation SHOULD also be possible by having expiration time far in the future (e.g. 2100-01-01 00:00:00) on radippool table, and modifying the query to NOT overwrite the expiration time on that scenario. This hasn't been implemented though.
I have updated the code so that static IP assignment works fine as well by setting expiration in the future. The only essential change to the last one is sql query change: https://github.com/fajarnugraha/freeradius-server/commit/b657ce3#diff-6
Any thoughts on these changes? Perhaps there a simpler way to implement it.
The changes could be simpler. A lot of the code in policy.conf is site-specific, and doesn't need to go into the server core. My $0.02:
Yes. Which brings me to two other questions: 1) dhcp functionality is a good example that uses policy. It SHOULD be site specific, but currently all policies must be defined on policy.conf. So currently I must either: a. write the same block twice (as in your original sites-available/dhcp example. OR b. put all changes on policy.conf. OR c. Implement something like polcies directory (or, following the recent changes on modules, policies-available and policies-enabled). Option (1.c) would be cleaner, but we ended up with many of -enabled directory. Especially since the next question also needs that. Currently I use (1.b) 2) I need a place to store centralized configuration variable, where the same variable is used by policy, module, and sites. It can be: a. write the same value many times. Not good. b. edit radiusd.conf (or policy.conf) and place it there. Messy. c. create a new file included by radiusd.conf just before modules section. Works, but it still requires adding 1 line to radiusd.conf. d. Implement configs-available and configs-enabled directory, loaded just before modules section on radiusd.conf. In this code update I use (2.c.) to store dhcp-related configuration variables (config-dhcp.conf). Option (2.d) would've been much cleaner. Current version of dhcp config code (with static IP, central config, and comment changes), diff against v2.1.x: https://github.com/fajarnugraha/freeradius-server/commit/fdff057 ... or diff against the previous version that you reviewed: https://github.com/fajarnugraha/freeradius-server/commit/b657ce3
dhcp.sqlippool_request - this is good, it "patches" the DHCP packet so that the SQLIPPool module understands it
dhcp.sqlippool_reply - this is good, too
I wasn't sure this approach was acceptable. Glad you like it.
The sites-available/dhcp-sqliipool text is not necessary. Just put the sqlippool examples into the existing DHCP virtual server.
The original intention was to push the changes in my ppa build as an alternative example, leaving the existing example untouched. If both examples can be merged then it's great.
The SQL sample configurations for MySQL are good.
I'll pull part of it into the v2.1.x branch. Please check that in a day or so, to see what needs to be done in order for it to work.
Please also take a look at my last update, at least the sql change part since it enables static IP assignment. As a side note, I noticed you closed pull request https://github.com/alandekok/freeradius-server/pull/40 , but it hasn't been merged in v2.1.x. Without that, debian package build still fails. Are you planning to commit an alternate fix? This is different btw from pull request #41, which you've applied (thanks for that). -- Fajar
Fajar A. Nugraha wrote:
Glad to hear it :D Most of my changes are simple ones,
I've committed a number of the changes.
Yes. Which brings me to two other questions: 1) dhcp functionality is a good example that uses policy. It SHOULD be site specific, but currently all policies must be defined on policy.conf. So currently I must either: a. write the same block twice (as in your original sites-available/dhcp example. OR b. put all changes on policy.conf. OR c. Implement something like polcies directory (or, following the recent changes on modules, policies-available and policies-enabled).
The policies can go anywhere you want. If you're only running one DHCP server, you can put them in sites-enabled/dhcp
2) I need a place to store centralized configuration variable, where the same variable is used by policy, module, and sites. It can be: a. write the same value many times. Not good. b. edit radiusd.conf (or policy.conf) and place it there. Messy. c. create a new file included by radiusd.conf just before modules section. Works, but it still requires adding 1 line to radiusd.conf. d. Implement configs-available and configs-enabled directory, loaded just before modules section on radiusd.conf.
Yeah, that needs addressing in a general way for 3.0.
Current version of dhcp config code (with static IP, central config, and comment changes), diff against v2.1.x: https://github.com/fajarnugraha/freeradius-server/commit/fdff057
I've pulled in the changes && pushed updates back. The changes I made are much simpler than what you had. The extra policies you created were site-specific, and not suitable for general use. The MySQL example, and the dhcp_sqlippool module went in pretty much unchanged. The rest of the changes are much simpler, and should be easier to understand for most people. Please test them out.
As a side note, I noticed you closed pull request https://github.com/alandekok/freeradius-server/pull/40 , but it hasn't been merged in v2.1.x. Without that, debian package build still fails. Are you planning to commit an alternate fix? This is different btw from pull request #41, which you've applied (thanks for that).
It was pulled in, but not pushed back to github. I've done that now. Alan DeKok.
On Fri, Jan 20, 2012 at 7:39 PM, Alan DeKok <aland@deployingradius.com> wrote:
Fajar A. Nugraha wrote:
c. Implement something like polcies directory (or, following the recent changes on modules, policies-available and policies-enabled).
The policies can go anywhere you want. If you're only running one DHCP server, you can put them in sites-enabled/dhcp
Really? How do I put policies it (e.g.) sites-enabled/dhcp? I tried using another policy {} block (but also on policy.conf), the second policy block is not loaded.
Current version of dhcp config code (with static IP, central config, and comment changes), diff against v2.1.x: https://github.com/fajarnugraha/freeradius-server/commit/fdff057
I've pulled in the changes && pushed updates back.
Thanks.
The changes I made are much simpler than what you had. The extra policies you created were site-specific, and not suitable for general use. The MySQL example, and the dhcp_sqlippool module went in pretty much unchanged.
The rest of the changes are much simpler, and should be easier to understand for most people. Please test them out.
Just did. You still need some minor changes though: (1) Pool-Name. It's needed for sqlipool. something like this works ... # Do some minor hacks to the request so that it looks # like a RADIUS request to the SQL IP Pool module. update control { Pool-Name = "DHCP-default" } update request { ... (2) policy vs method In sites-available/dhcp, you used "dhcp_sqlippool". In my test, that would call the dhcp_sqlipool module instance, bypassing the policy, so the compatibility code was never used. So I change the policy name to dhcp_sqlippool.compat, and call that instead in sites-available/dhcp. (3) NAS-IP-Address is not quoted on policy.conf, and FR fails to start with /etc/freeradius/policy.conf[210]: ERROR: Failed to find IP address for %{%{DHCP-Gateway-IP-Address}:-127.0.0.1} /etc/freeradius/policy.conf[207]: Failed to parse "update" subsection. Adding quotes fixed the problem: https://github.com/fajarnugraha/freeradius-server/commit/543ce28#diff-3 Also, I have two questions regarding modules and method: (1) calling the instance dhcp_sqlippool works, without having to force it to call the method post-auth (dhcp_sqlippool.post-auth). Does this mean post-auth is the default method? Is it safe to just call it like that, or should we explicitly call dhcp_sqlippool.post-auth instead? (2) Is calling the policy dhcp_sqlippool.post-auth (in your commit) or dhcp_sqlippool.compat (in my lastest change) considered "safe" enough? e.g. should the policy be called something else (e.g. dhcp_sqlippool_compat) to avoid namespace conflict with dhcp_sqlippool (the module instance)? -- Fajar
Fajar A. Nugraha wrote:
Really? How do I put policies it (e.g.) sites-enabled/dhcp? I tried using another policy {} block (but also on policy.conf), the second policy block is not loaded.
You can just write "unlang" blocks. My main objection to your code was that it's fairly site-specific. Other people may want to do something else.
Just did. You still need some minor changes though: (1) Pool-Name. It's needed for sqlipool. something like this works
I'll go fix that.
(2) policy vs method In sites-available/dhcp, you used "dhcp_sqlippool". In my test, that would call the dhcp_sqlipool module instance, bypassing the policy, so the compatibility code was never used. So I change the policy name to dhcp_sqlippool.compat, and call that instead in sites-available/dhcp.
That's a typo. The policy *should* be "dhcp_sqlippool.post-auth". It will take care of over-riding the dhcp_sqlippool call.
(3) NAS-IP-Address is not quoted on policy.conf, and FR fails to start with
I've committed a fix.
Also, I have two questions regarding modules and method: (1) calling the instance dhcp_sqlippool works, without having to force it to call the method post-auth (dhcp_sqlippool.post-auth). Does this mean post-auth is the default method? Is it safe to just call it like that, or should we explicitly call dhcp_sqlippool.post-auth instead?
It's the default method for DHCP. You should just use the module name.
(2) Is calling the policy dhcp_sqlippool.post-auth (in your commit) or dhcp_sqlippool.compat (in my lastest change) considered "safe" enough? e.g. should the policy be called something else (e.g. dhcp_sqlippool_compat) to avoid namespace conflict with dhcp_sqlippool (the module instance)?
The idea is that the DHCP SQL stuff is in dhcp_sqlippool. The user sees that, and manages DHCP like that. The "magic" to glue DHCP to RADIUS is in policy.conf, in the dhcp_sqlippool.post-auth method Alan DeKok.
On Tue, Jan 24, 2012 at 9:57 PM, Alan DeKok <aland@deployingradius.com> wrote:
Fajar A. Nugraha wrote:
Really? How do I put policies it (e.g.) sites-enabled/dhcp? I tried using another policy {} block (but also on policy.conf), the second policy block is not loaded.
You can just write "unlang" blocks.
Do you mean I can assign a name to an unlang block and call it elsewhere, without it needing to be on policy section? Or do you mean I need to put the unlang blocks wherever I need to call it (thus the possible code duplication)?
(2) policy vs method In sites-available/dhcp, you used "dhcp_sqlippool". In my test, that would call the dhcp_sqlipool module instance, bypassing the policy, so the compatibility code was never used. So I change the policy name to dhcp_sqlippool.compat, and call that instead in sites-available/dhcp.
That's a typo. The policy *should* be "dhcp_sqlippool.post-auth". It will take care of over-riding the dhcp_sqlippool call.
I see
Also, I have two questions regarding modules and method: (1) calling the instance dhcp_sqlippool works, without having to force it to call the method post-auth (dhcp_sqlippool.post-auth). Does this mean post-auth is the default method? Is it safe to just call it like that, or should we explicitly call dhcp_sqlippool.post-auth instead?
It's the default method for DHCP. You should just use the module name.
Noted.
(2) Is calling the policy dhcp_sqlippool.post-auth (in your commit) or dhcp_sqlippool.compat (in my lastest change) considered "safe" enough? e.g. should the policy be called something else (e.g. dhcp_sqlippool_compat) to avoid namespace conflict with dhcp_sqlippool (the module instance)?
The idea is that the DHCP SQL stuff is in dhcp_sqlippool. The user sees that, and manages DHCP like that. The "magic" to glue DHCP to RADIUS is in policy.conf, in the dhcp_sqlippool.post-auth method
Thanks, will try it again tomorrow. -- Fajar
On Tue, Jan 24, 2012 at 10:13 PM, Fajar A. Nugraha <list@fajar.net> wrote:
On Tue, Jan 24, 2012 at 9:57 PM, Alan DeKok <aland@deployingradius.com> wrote:
(2) policy vs method In sites-available/dhcp, you used "dhcp_sqlippool". In my test, that would call the dhcp_sqlipool module instance, bypassing the policy, so the compatibility code was never used. So I change the policy name to dhcp_sqlippool.compat, and call that instead in sites-available/dhcp.
That's a typo. The policy *should* be "dhcp_sqlippool.post-auth". It will take care of over-riding the dhcp_sqlippool call.
I see
Just tested, and now it works as expected. Thanks. -- Fajar
Fajar A. Nugraha wrote:
Just tested, and now it works as expected. Thanks.
Great! We can now say that the server has full support for dynamic assignment of addresses via DHCP. It's time for everyone to toss the ISC server. It doesn't support magic technology called "databases", which have been around for decades. Alan DeKok.
Fajar, If you want to split policies into distinct files, use a $INCLUDE statement in the policy.conf policy stanza and then create a directory that contains your additional policy files. Use a distinct namespace prefix if the unlang policies are for a specific purpose. The configuration preprocessor will take care of including all the files in the subdirectory before parsing the configuration, so you just write the unlang as if it already had a policy {} wrapper around it. I've been meaning to do that for a while. -Arran On 24 Jan 2012, at 15:57, Alan DeKok wrote:
Fajar A. Nugraha wrote:
Really? How do I put policies it (e.g.) sites-enabled/dhcp? I tried using another policy {} block (but also on policy.conf), the second policy block is not loaded.
You can just write "unlang" blocks.
My main objection to your code was that it's fairly site-specific. Other people may want to do something else.
Just did. You still need some minor changes though: (1) Pool-Name. It's needed for sqlipool. something like this works
I'll go fix that.
(2) policy vs method In sites-available/dhcp, you used "dhcp_sqlippool". In my test, that would call the dhcp_sqlipool module instance, bypassing the policy, so the compatibility code was never used. So I change the policy name to dhcp_sqlippool.compat, and call that instead in sites-available/dhcp.
That's a typo. The policy *should* be "dhcp_sqlippool.post-auth". It will take care of over-riding the dhcp_sqlippool call.
(3) NAS-IP-Address is not quoted on policy.conf, and FR fails to start with
I've committed a fix.
Also, I have two questions regarding modules and method: (1) calling the instance dhcp_sqlippool works, without having to force it to call the method post-auth (dhcp_sqlippool.post-auth). Does this mean post-auth is the default method? Is it safe to just call it like that, or should we explicitly call dhcp_sqlippool.post-auth instead?
It's the default method for DHCP. You should just use the module name.
(2) Is calling the policy dhcp_sqlippool.post-auth (in your commit) or dhcp_sqlippool.compat (in my lastest change) considered "safe" enough? e.g. should the policy be called something else (e.g. dhcp_sqlippool_compat) to avoid namespace conflict with dhcp_sqlippool (the module instance)?
The idea is that the DHCP SQL stuff is in dhcp_sqlippool. The user sees that, and manages DHCP like that. The "magic" to glue DHCP to RADIUS is in policy.conf, in the dhcp_sqlippool.post-auth method
Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
Arran Cudbard-Bell a.cudbardb@networkradius.com Technical consultant and solutions architect 15 Ave. du Granier, Meylan, France +33 4 69 66 54 50
On Tue, Jan 24, 2012 at 11:16 PM, Arran Cudbard-Bell <a.cudbardb@freeradius.org> wrote:
Fajar,
If you want to split policies into distinct files, use a $INCLUDE statement in the policy.conf policy stanza and then create a directory that contains your additional policy files.
Yes, that's what I meant earlier by policies-available and policies-enabled directory :) -- Fajar
participants (3)
-
Alan DeKok -
Arran Cudbard-Bell -
Fajar A. Nugraha