Hi Karel, nice patch. There are two different standpoints here; one is to do like you suggests. The other is to pass what the server actually provides to the script.
My personal opinion is to fix-up this in the script itself if necessary, else it will introduce an unexpected behavior. BR, Markus On 2020-07-15 18:15, Karel Dolezal wrote: > Hi, > > I've noticed udhcpc was not attempting to renew the address in time, > in case the server offered > only 30s lease time. The reason ended up being the "--script" which I > use to install the address. > It was still getting the 30s value, whereas udhcpc internally corrects > the 30s value into 122s. > > Please see the proposed patch below. > > Best regards > Karel > > > > Fixes DHCP lease time passed from udhcpc into the "--script" > > For unusually short leases, udhcpc enforces a minimum lease time to avoid > excessive network traffic. > > The original, unadjusted value, was however still being passed to the dhcp > event script, which was making it install the obtained IP address with a > lifetime shorter than the udhcpc's renewal period. As a result, the address > aged out and was removed without udhcpc attempting a renewal for some time. > > This patch writes the corrected value back to the packet variable, from > which > the script's environment is populated. > > Signed-off-by: Karel Dolezal <[email protected]> > > --- a/networking/udhcp/dhcpc.c 2020-07-10 11:11:43.229706806 +0200 > +++ b/networking/udhcp/dhcpc.c 2020-07-10 11:17:30.366254882 +0200 > @@ -1867,16 +1867,21 @@ int udhcpc_main(int argc UNUSED_PARAM, c > lease_seconds = 60 * 60; > } else { > /* it IS unaligned sometimes, don't > "optimize" */ > move_from_unaligned32(lease_seconds, > temp); > lease_seconds = ntohl(lease_seconds); > /* paranoia: must not be too small and > not prone to overflows */ > /* timeout > 60 - ensures at least one > unicast renew attempt */ > - if (lease_seconds < 2 * 61) > + if (lease_seconds < 2 * 61) { > lease_seconds = 2 * 61; > + /* The --script reads from the > &packet variable itself. */ > + /* Write the adjusted value > back there, to give script */ > + /* the same lease time as we'll > use internally. */ > + move_to_unaligned32(temp, > htonl(lease_seconds)); > + } > //if (lease_seconds > 0x7fffffff) > // lease_seconds = 0x7fffffff; > //^^^not necessary since "timeout = > lease_seconds / 2" > //does not overflow even for 0xffffffff. > } > #if ENABLE_FEATURE_UDHCPC_ARPING > if (opt & OPT_a) { > _______________________________________________ > busybox mailing list > [email protected] > http://lists.busybox.net/mailman/listinfo/busybox _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
