[PATCH] udhcpd: account for script delay in lease

[John Schroeder]

Hello all,

I ran into an issue where I was seeing a long delay in the scripts 
called in udhcp_run_script. I was using an old version of OpenWrt 
(kamikaze) and a satellite modem. An NTP script was being called and the 
modem would sometimes take a long time to respond to the DNS lookup when 
it was offline.

This delay started affecting my lease time. The lease that I would get 
from my satellite modem before it was online would be short: only 60 
seconds. The delay with NTP and the modem would typically be about 18 
seconds. This would cause the first DHCP renew request from dhcpc to be 
a little late. Under certain circumstances, I could even see the first 
DHCP renew to occur after the lease had expired!

I patched dhcpc.c in my build using the following patch below. I used 
the existing variable already_waited_sec to calculate the number of 
seconds it took to complete running the scripts. If the delay was not 
greater than the timeout (i.e. half the lease time), then the code would 
simply send the renew using the already_waited_sec (e.g. 30 sec - 18 sec 
= 12 sec to send the renew). If the delay was greater than the timeout 
but less than the lease time, the code subtracts the time out from the 
already_waited_sec, halves the time out, and continues as before. (E.g. 
already_waited_sec is 40 sec, then the timeout would be 15 sec and 
already_waited_sec would be changed to 10 sec. This would cause the 
renew to be sent out 5 seconds later at the 45 second mark.) This 
happens in a while loop while the timeout is greater than 0. (Note that 
if a renew event is missed, it is simply skipped. E.g. In a 60 sec 
lease, the 30 sec mark could be skipped and the next renew sent at 45 
seconds.) If the already_waited_sec ends up being greater than the 
timeout, the code will not send a renew and re-enter the requesting state.

Some improvements: If a renew event is missed, the code waits until the 
next timeout. An improvement could be to send a renew immediately. 
Another possible improvement would be to goto the renew event using a 
goto rather than continuing. Both of these issues I did not see as a big 
issue since I think both of these are unlikely events and are handled 
well enough. I mostly saw issues with the scripts taking a few seconds 
to run and wanted to account for the delay. I also did not want to send 
a renew after the lease had expired.

I also put in some comments but I am not sure if they are up to par for 
busybox. I would appreciate any feedback. Thank you.

--John Schroeder


--- a/networking/udhcp/dhcpc.c
+++ b/networking/udhcp/dhcpc.c
@@ -1752,6 +1752,7 @@
  #endif
                  /* enter bound state */
                  timeout = lease_seconds / 2;
+                timestamp_before_wait = (unsigned)monotonic_sec();
                  temp_addr.s_addr = packet.yiaddr;
                  bb_info_msg("Lease of %s obtained, lease time %u",
                      inet_ntoa(temp_addr), (unsigned)lease_seconds);
@@ -1774,7 +1775,21 @@
  #endif
                  /* make future renew packets use different xid */
                  /* xid = random_xid(); ...but why bother? */
-                already_waited_sec = 0;
+                already_waited_sec = (unsigned)monotonic_sec() - 
timestamp_before_wait;
+                /* If already beyond timeout, skip the next renew
+                 * event. Continue skipping renews and halving the
+                 * timeout until the next timeout is less that the
+                 * already waited time or the timeout is 0.
+                 */
+                while ((already_waited_sec > timeout) && (timeout > 0)) {
+                    already_waited_sec -= timeout;
+                    timeout >>= 1;
+                }
+                /* If timeout is 0, then the lease has expired. Set
+                 * already_waited_sec to 0.
+                 */
+                if (timeout == 0)
+                    already_waited_sec = 0;
                  continue; /* back to main loop */
              }
              if (*message == DHCPNAK) {