[PATCH] udhcpd: account for script delay in lease

John Schroeder jschroed at gmail.com
Fri Dec 19 18:36:42 UTC 2014


Hello all,

I did not see any comments on my patch so I wanted to check in. Any 
thoughts on including it in BusyBox?

--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) {
>



More information about the busybox mailing list