[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