Hi! Small feature request: please make it possible to specify fractional values of seconds for Reject delay values. I need this for dhcp module, 1 second is too small for me but 2 is too large :-) I'd like to specify 1.5 or 1.8 seconds. 1/100th second precision ought be enough :-) Eugene Grosbein
Eugene Grosbein wrote:
Small feature request: please make it possible to specify fractional values of seconds for Reject delay values. I need this for dhcp module, 1 second is too small for me but 2 is too large :-) I'd like to specify 1.5 or 1.8 seconds.
1/100th second precision ought be enough :-)
Sure. Send a patch. :) It's probably not hard to do. There is already code to do similar things for home servers in 3.0.4. Alan DeKok.
On Tue, Oct 28, 2014 at 11:51:11AM -0400, Alan DeKok wrote:
Eugene Grosbein wrote:
Small feature request: please make it possible to specify fractional values of seconds for Reject delay values. I need this for dhcp module, 1 second is too small for me but 2 is too large :-) I'd like to specify 1.5 or 1.8 seconds.
1/100th second precision ought be enough :-)
Sure. Send a patch. :)
It's probably not hard to do. There is already code to do similar things for home servers in 3.0.4.
Well, here it is. Not sure about "#include <sys/time.h>" policy in the FreeRADIUS code, though it compiles and works for me. The patch is for 3.0.4 release source code. --- raddb/radiusd.conf.in.orig 2014-10-29 00:10:21.000000000 +0700 +++ raddb/radiusd.conf.in 2014-10-29 00:10:32.000000000 +0700 @@ -453,7 +453,7 @@ # is deleted from the internal cache of requests. # # Useful ranges: 1 to 5 - reject_delay = 1 + reject_delay = 1.0 # # status_server: Whether or not the server will respond --- src/include/libradius.h.orig 2014-10-29 00:43:21.000000000 +0700 +++ src/include/libradius.h 2014-10-29 00:50:38.000000000 +0700 @@ -67,6 +67,7 @@ /* * Include system headers. */ +#include <sys/time.h> #include <stdio.h> #include <stdlib.h> #include <stdarg.h> @@ -262,6 +263,8 @@ uint8_t *tlv; //!< Nested TLV (should go away). void const *ptr; //!< generic pointer. + + struct timeval tv; //!< Timer } value_data_t; /** The type of value a VALUE_PAIR contains @@ -356,6 +359,7 @@ #define vp_integer64 data.integer64 #define vp_ipv4prefix data.ipv4prefix #define vp_tlv data.tlv +#define vp_tv data.tv typedef struct fr_ipaddr_t { int af; /* address family */ --- src/include/radiusd.h.orig 2014-10-29 00:14:23.000000000 +0700 +++ src/include/radiusd.h 2014-10-29 00:58:53.000000000 +0700 @@ -47,6 +47,8 @@ # define REQUEST_MAGIC (0xdeadbeef) #endif +#include <sys/time.h> + /* * WITH_VMPS is handled by src/include/features.h */ @@ -277,7 +279,7 @@ rad_child_state_t child_state; RAD_LISTEN_TYPE priority; - int response_delay; + struct timeval response_delay; int timer_action; fr_event_t *ev; @@ -435,7 +437,7 @@ #ifdef WITH_PROXY bool proxy_requests; #endif - uint32_t reject_delay; + struct timeval reject_delay; bool status_server; char const *allow_vulnerable_openssl; --- src/main/mainconfig.c.orig 2014-10-29 00:15:57.000000000 +0700 +++ src/main/mainconfig.c 2014-10-29 01:04:26.000000000 +0700 @@ -97,7 +97,7 @@ static char const *radius_dir = NULL; // */ static const CONF_PARSER security_config[] = { { "max_attributes", FR_CONF_POINTER(PW_TYPE_INTEGER, &fr_max_attributes), STRINGIFY(0) }, - { "reject_delay", FR_CONF_POINTER(PW_TYPE_INTEGER, &main_config.reject_delay), STRINGIFY(0) }, + { "reject_delay", FR_CONF_POINTER(PW_TYPE_TIMEVAL, &main_config.reject_delay), STRINGIFY(0) }, { "status_server", FR_CONF_POINTER(PW_TYPE_BOOLEAN, &main_config.status_server), "no"}, { "allow_vulnerable_openssl", FR_CONF_POINTER(PW_TYPE_STRING, &main_config.allow_vulnerable_openssl), "no"}, { NULL, -1, 0, NULL, NULL } @@ -908,7 +908,7 @@ do {\ fr_debug_flag = debug_flag; FR_INTEGER_COND_CHECK("max_request_time", main_config.max_request_time, (main_config.max_request_time != 0), 100); - FR_INTEGER_BOUND_CHECK("reject_delay", main_config.reject_delay, <=, 10); + FR_TIMEVAL_BOUND_CHECK("reject_delay", &main_config.reject_delay, <=, 10, 0); FR_INTEGER_BOUND_CHECK("cleanup_delay", main_config.cleanup_delay, <=, 10); /* @@ -955,7 +955,7 @@ do {\ * Sanity check the configuration for internal * consistency. */ - FR_INTEGER_BOUND_CHECK("reject_delay", main_config.reject_delay, <=, main_config.cleanup_delay); + FR_TIMEVAL_BOUND_CHECK("reject_delay", &main_config.reject_delay, <=, main_config.cleanup_delay, 0); if (chroot_dir) { if (chdir(radlog_dir) < 0) { --- src/main/process.c.orig 2014-09-10 20:57:22.000000000 +0700 +++ src/main/process.c 2014-10-29 01:12:27.000000000 +0700 @@ -353,6 +353,17 @@ static void tv_add(struct timeval *tv, i } } +static void tv_inc(struct timeval *tv, struct timeval *inc) +{ + tv->tv_sec += inc->tv_sec; + tv->tv_usec += inc->tv_usec; + + if (tv->tv_usec >= USEC) { + tv->tv_sec += tv->tv_usec / USEC; + tv->tv_usec %= USEC; + } +} + /* * Debug the packet if requested. */ @@ -945,7 +971,7 @@ static void request_process_timer(REQUES #endif /* WITH_PROXY */ case REQUEST_RESPONSE_DELAY: - rad_assert(request->response_delay > 0); + rad_assert(request->response_delay.tv_sec > 0 && request->response_delay.tv_usec > 0); #ifdef WITH_COA rad_assert(!request->proxy || (request->packet->code == request->proxy->code)); #endif @@ -954,7 +980,7 @@ static void request_process_timer(REQUES when = request->reply->timestamp; - tv_add(&when, request->response_delay * USEC); + tv_inc(&when, &request->response_delay); if (timercmp(&when, &now, >)) { #ifdef DEBUG_STATE_MACHINE @@ -1401,7 +1437,8 @@ STATE_MACHINE_DECL(request_finish) * See if we need to delay an Access-Reject packet. */ if ((request->reply->code == PW_CODE_ACCESS_REJECT) && - (request->root->reject_delay > 0)) { + (request->root->reject_delay.tv_sec > 0 && + request->root->reject_delay.tv_usec > 0 )) { request->response_delay = request->root->reject_delay; #ifdef WITH_PROXY @@ -1410,7 +1447,8 @@ STATE_MACHINE_DECL(request_finish) * the reject any more. */ if (request->proxy && !request->proxy_reply) { - request->response_delay = 0; + request->response_delay.tv_sec = 0; + request->response_delay.tv_usec = 0; } #endif @@ -1419,7 +1457,8 @@ STATE_MACHINE_DECL(request_finish) /* * Send the reply. */ - if (!request->response_delay) { + if (request->response_delay.tv_sec == 0 && + request->response_delay.tv_usec == 0) { DEBUG_PACKET(request, request->reply, 1); request->listener->send(request->listener, request); @@ -1443,8 +1482,9 @@ STATE_MACHINE_DECL(request_finish) request->child_state = REQUEST_CLEANUP_DELAY; } } else { - RDEBUG2("Delaying response for %d seconds", - request->response_delay); + RDEBUG2("Delaying response for %d.%d seconds", + (int)request->response_delay.tv_sec, + (int)request->response_delay.tv_usec); NO_CHILD_THREAD; request->child_state = REQUEST_RESPONSE_DELAY; } --- src/modules/proto_dhcp/dhcpd.c.orig 2014-09-10 20:57:22.000000000 +0700 +++ src/modules/proto_dhcp/dhcpd.c 2014-10-29 01:23:47.000000000 +0700 @@ -424,10 +424,11 @@ static int dhcp_process(REQUEST *request if (request->reply->code == PW_DHCP_NAK) { vp = pairfind(request->reply->vps, PW_FREERADIUS_RESPONSE_DELAY, 0, TAG_ANY); if (vp) { - if (vp->vp_integer <= 10) { - request->response_delay = vp->vp_integer; + if (vp->vp_tv.tv_sec*1000000+vp->vp_tv.tv_usec <= 10000000) { + request->response_delay = vp->vp_tv; } else { - request->response_delay = 10; + request->response_delay.tv_sec = 10; + request->response_delay.tv_usec = 0; } } }
Eugene Grosbein wrote:
Well, here it is. Not sure about "#include <sys/time.h>" policy in the FreeRADIUS code, though it compiles and works for me.
It's not needed. Also, changing the definition of an existing attribute is a bad idea. I've re-worked the patch and pushed it to the v3.0.x branch on github. Please try that. You can set the response delay by doing: update reply { FreeRADIUS-Response-Delay-Usec = 1500000 } Which will be 1.5 seconds. Alan DeKok.
On 29.10.2014 04:04, Alan DeKok wrote:
Eugene Grosbein wrote:
Well, here it is. Not sure about "#include <sys/time.h>" policy in the FreeRADIUS code, though it compiles and works for me.
It's not needed. Also, changing the definition of an existing attribute is a bad idea.
I've re-worked the patch and pushed it to the v3.0.x branch on github. Please try that.
You can set the response delay by doing:
update reply { FreeRADIUS-Response-Delay-Usec = 1500000 }
Which will be 1.5 seconds.
Sadly, this variant is no use for me. I need to be able to return fractional seconds from the Perl code but your version does not allow me that. That is, DHCP NAK is not always a "reject". If legitimate user requests wrong IP address with its DHCPREQUEST, I should send back right IP address with DHCPNAK using zero delay. I should send DHCPNAK with non-zero delay for users with misconfigured hardware only (f.e. PPPoE user asking to DHCP; or if that's DHCP flooder). I have some logic in my perl code and in the database stored procedure it calls to decide that. Eugene Grosbein
Eugene Grosbein wrote:
Sadly, this variant is no use for me. I need to be able to return fractional seconds from the Perl code but your version does not allow me that.
Nonsense. An attribute is an attribute. The server doesn't care where it comes from.
That is, DHCP NAK is not always a "reject". If legitimate user requests wrong IP address with its DHCPREQUEST, I should send back right IP address with DHCPNAK using zero delay.
The code allows you to do that.
I should send DHCPNAK with non-zero delay for users with misconfigured hardware only (f.e. PPPoE user asking to DHCP; or if that's DHCP flooder).
I have some logic in my perl code and in the database stored procedure it calls to decide that.
That's fine. Does any *other* attribute fail to work when used from Perl? Does any *other* attribute require you to use an "update" section? No. Set "FreeRADIUS-Response-Delay-Usec = 1500000" in your Perl script. It will work. Alan DeKok.
On Wed, Oct 29, 2014 at 10:14:00AM -0400, Alan DeKok wrote:
Sadly, this variant is no use for me. I need to be able to return fractional seconds from the Perl code but your version does not allow me that.
Nonsense. An attribute is an attribute. The server doesn't care where it comes from.
Oh, now I see you have added PW_FREERADIUS_RESPONSE_DELAY_USEC support to modules/proto_dhcp/dhcpd.c with distinct commit. I missed it while reading github's commit history yesterday and got wrong impression it's not supported there. Nevermind. And thank you! Eugene Grosbein
On 29.10.2014 04:04, Alan DeKok wrote:
Eugene Grosbein wrote:
Well, here it is. Not sure about "#include <sys/time.h>" policy in the FreeRADIUS code, though it compiles and works for me.
It's not needed. Also, changing the definition of an existing attribute is a bad idea.
I've re-worked the patch and pushed it to the v3.0.x branch on github. Please try that.
You can set the response delay by doing:
update reply { FreeRADIUS-Response-Delay-Usec = 1500000 }
Which will be 1.5 seconds.
OTOH, why have you prohibited fraction delays in between 0 and 1? I can think of usefulness of 0.5 or 0.9 values too. Eugene Grosbein
Eugene Grosbein wrote:
OTOH, why have you prohibited fraction delays in between 0 and 1?
Because sub-second delays make much, much less sense. You have to draw a limit somewhere. I put it at one second, at least in part because that was the previous limit.
I can think of usefulness of 0.5 or 0.9 values too.
Which you don't explain here. Nice. "I can think of reasons why we can do X. But... I won't tell you" Alan DeKok.
On Wed, Oct 29, 2014 at 10:15:19AM -0400, Alan DeKok wrote:
OTOH, why have you prohibited fraction delays in between 0 and 1? Because sub-second delays make much, much less sense. You have to draw a limit somewhere. I put it at one second, at least in part because that was the previous limit.
I can think of usefulness of 0.5 or 0.9 values too.
Which you don't explain here. Nice.
"I can think of reasons why we can do X. But... I won't tell you"
I can explain but that will take more words and English is not my native language, but let's try. DHCP has some similarities with PPPoE. To get an IP address, a client sends a broadcast packet first: PADI or DHCPDISCOVER. Suppose, there is a farm of PPPoE or DHCP relays/servers in the vlan, so the client will get a bunch of replies: PADOs or DHCPOFFERs. Generally, the client chooses first (most quick) access server and may send a unicast request to it: PADR or DHCPREQUEST (I know that DHCPREQUEST may be broadcasted too, but leave this for now). It is possible to manage server's load with very small delays in a response to initial broadcast. For example, Cisco has a command "pado delay <miliseconds>" to conditionally insert such delay before PADO in sent (PPPoE Smart Server Selection feature, http://www.cisco.com/c/en/us/td/docs/ios/bbdsl/configuration/guide/bba_pppoe... ). So, we can preferably serve some users from one part of server farm and other users from another part but still have a backup in case of long failure of "first part" without noticable delay before answer. The same applies to the farm of FreeRAIDUS servers acting as DHCP servers. But artifical limitation for 1 second as minimum delay length would be noticable for no good reason. This is only one of use cases for small delays. It's local system administrator responsibility for selecting sane delay values and IMHO server software should not impose artifical limitations. You can never know in advance, what use cases may field practice present to an administrator, can you? Eugene Grosbein
On Wed, 29 Oct 2014 10:15:19 -0400, Alan DeKok wrote:
Which you don't explain here. Nice.
"I can think of reasons why we can do X. But... I won't tell you"
Alan DeKok. -
Hello Alan. Maybe I can provide another use case when a lower minimum reject delay than 1s could help. If you have a large setup with more that two or three Radius servers chained you have to hunt every millisecond when tuning timeouts. There may be backup servers and backup back-end LDAP/Database servers with their timeouts, etc. It is not unusual to have such setup among Telco operators. Access reject in such environment does not necessarily mean a password guessing. It might be even impossible to setup timeouts correctly when the you insert a fixed reject delay of 1s in a chain where you count tens of msec in order to fit into max overall request time. Responses after 1s can be considered late. Regards Ales
participants (3)
-
Alan DeKok -
Aleš Rygl -
Eugene Grosbein