Re: dhcpd uses bad current time

2013-05-16 Thread Otto Moerbeek 
On Wed, May 15, 2013 at 10:29:22AM +0200, Gerhard Roth wrote:

 On Wed, 15 May 2013 10:15:54 +0200
 Gerhard Roth gr...@genua.de wrote:
 
  In dhcpd, variable cur_time is set only once per dispatch loop.
  Unfortunately, this is done before the poll(2) call. Since poll(2)
  may sleep for an arbitrary amount of time, the value of cur_time
  might refer to some long ago point in time. When message dispatching
  is done, timeouts and lease ends are calculated based on this
  outdated cur_time value that was set before the call to poll(2).
  
  It's fine to set cur_time only once per loop, but we should do it after
  the blocking poll(2) syscall so that its value is more accurate.
  
  ok?
  
  Gerhard
  
 
 
 Sorry, the previous version will break timeout handling when there
 are no incoming packets. I notice the moment that I sent the mail.
 
 So let's keep all the old code and just add another update of cur_time
 after poll(2).

ok with me,

-Otto

 
 Gerhard
 
 
 Index: usr.sbin/dhcpd/dispatch.c
 ===
 RCS file: /cvs/src/usr.sbin/dhcpd/dispatch.c,v
 retrieving revision 1.30
 diff -u -p -r1.30 dispatch.c
 --- usr.sbin/dhcpd/dispatch.c 19 Apr 2013 21:25:39 -  1.30
 +++ usr.sbin/dhcpd/dispatch.c 15 May 2013 08:26:49 -
 @@ -360,6 +360,7 @@ another:
   case 0:
   continue;   /* no packets */
   }
 + time(cur_time);
  
   for (i = 0, l = protocols; l; l = l-next) {
   struct interface_info *ip = l-local;

Re: dhcpd uses bad current time

2013-05-15 Thread Otto Moerbeek 
On Wed, May 15, 2013 at 10:15:54AM +0200, Gerhard Roth wrote:

 In dhcpd, variable cur_time is set only once per dispatch loop.
 Unfortunately, this is done before the poll(2) call. Since poll(2)
 may sleep for an arbitrary amount of time, the value of cur_time
 might refer to some long ago point in time. When message dispatching
 is done, timeouts and lease ends are calculated based on this
 outdated cur_time value that was set before the call to poll(2).
 
 It's fine to set cur_time only once per loop, but we should do it after
 the blocking poll(2) syscall so that its value is more accurate.
 
 ok?
 
 Gerhard

I don't think the first call to time() should be left out. cur_time is
potentially used in the first iteration of the loop. 

-Otto
 
 
 
 
 Index: usr.sbin/dhcpd/dispatch.c
 ===
 RCS file: /cvs/src/usr.sbin/dhcpd/dispatch.c,v
 retrieving revision 1.30
 diff -u -p -r1.30 dispatch.c
 --- usr.sbin/dhcpd/dispatch.c 19 Apr 2013 21:25:39 -  1.30
 +++ usr.sbin/dhcpd/dispatch.c 15 May 2013 08:09:17 -
 @@ -306,8 +306,6 @@ dispatch(void)
* still a timeout registered, time out the poll
* call then.
*/
 - time(cur_time);
 -another:
   if (timeouts) {
   if (timeouts-when = cur_time) {
   struct dhcpd_timeout *t = timeouts;
 @@ -315,7 +313,7 @@ another:
   (*(t-func))(t-what);
   t-next = free_timeouts;
   free_timeouts = t;
 - goto another;
 + continue;
   }
  
   /*
 @@ -360,6 +358,7 @@ another:
   case 0:
   continue;   /* no packets */
   }
 + time(cur_time);
  
   for (i = 0, l = protocols; l; l = l-next) {
   struct interface_info *ip = l-local;

Re: dhcpd uses bad current time

2013-05-15 Thread Gerhard Roth 
On Wed, 15 May 2013 10:15:54 +0200
Gerhard Roth gr...@genua.de wrote:

 In dhcpd, variable cur_time is set only once per dispatch loop.
 Unfortunately, this is done before the poll(2) call. Since poll(2)
 may sleep for an arbitrary amount of time, the value of cur_time
 might refer to some long ago point in time. When message dispatching
 is done, timeouts and lease ends are calculated based on this
 outdated cur_time value that was set before the call to poll(2).
 
 It's fine to set cur_time only once per loop, but we should do it after
 the blocking poll(2) syscall so that its value is more accurate.
 
 ok?
 
 Gerhard
 


Sorry, the previous version will break timeout handling when there
are no incoming packets. I notice the moment that I sent the mail.

So let's keep all the old code and just add another update of cur_time
after poll(2).

Gerhard


Index: usr.sbin/dhcpd/dispatch.c
===
RCS file: /cvs/src/usr.sbin/dhcpd/dispatch.c,v
retrieving revision 1.30
diff -u -p -r1.30 dispatch.c
--- usr.sbin/dhcpd/dispatch.c   19 Apr 2013 21:25:39 -  1.30
+++ usr.sbin/dhcpd/dispatch.c   15 May 2013 08:26:49 -
@@ -360,6 +360,7 @@ another:
case 0:
continue;   /* no packets */
}
+   time(cur_time);
 
for (i = 0, l = protocols; l; l = l-next) {
struct interface_info *ip = l-local;

Re: dhcpd uses bad current time

2013-05-15 Thread Gerhard Roth 
On Wed, 15 May 2013 10:24:22 +0200 Otto Moerbeek o...@drijf.net wrote:
 On Wed, May 15, 2013 at 10:15:54AM +0200, Gerhard Roth wrote:
 
  In dhcpd, variable cur_time is set only once per dispatch loop.
  Unfortunately, this is done before the poll(2) call. Since poll(2)
  may sleep for an arbitrary amount of time, the value of cur_time
  might refer to some long ago point in time. When message dispatching
  is done, timeouts and lease ends are calculated based on this
  outdated cur_time value that was set before the call to poll(2).
  
  It's fine to set cur_time only once per loop, but we should do it after
  the blocking poll(2) syscall so that its value is more accurate.
  
  ok?
  
  Gerhard
 
 I don't think the first call to time() should be left out. cur_time is
 potentially used in the first iteration of the loop. 
 

Well, it is set in main(), so it is initialized. But nevertheless you're
that we should keep it because it is required for correct timeout
handling if there are no incoming packets.

Just sent out a second diff that keeps the time(2) call at the top
of the loop.

Gerhard