[PATCH] udhcpd: account for script delay in lease

Denys Vlasenko vda.linux at googlemail.com
Sun Dec 21 15:07:58 UTC 2014


On Mon, Dec 1, 2014 at 4:45 AM, John Schroeder <jschroed at gmail.com> wrote:
> 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.

What would happen if script never returns?

> 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.

The above is complicated. Why do you want to do that?

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


How about a straightforward code which sets
already_waited_sec to script duration? -


@@ -1756,7 +1757,10 @@ int udhcpc_main(int argc UNUSED_PARAM, c
                                bb_info_msg("Lease of %s obtained,
lease time %u",
                                        inet_ntoa(temp_addr),
(unsigned)lease_seconds);
                                requested_ip = packet.yiaddr;
+
+                               start = monotonic_sec();
                                udhcp_run_script(&packet, state ==
REQUESTING ? "bound" : "renew");
+                               already_waited_sec =
(unsigned)monotonic_sec() - start;

                                state = BOUND;
                                change_listen_mode(LISTEN_NONE);
@@ -1774,7 +1778,7 @@ int udhcpc_main(int argc UNUSED_PARAM, c
 #endif
                                /* make future renew packets use
different xid */
                                /* xid = random_xid(); ...but why bother? */
-                               already_waited_sec = 0;


If  already_waited_sec > timeout,  then existing code will immediately
go into renew (or rebind) state and start sending packets immediately.
What's wrong with that?


More information about the busybox mailing list