| Message ID | 1430358118-5304-1-git-send-email-matt@thewebers.ws |
|---|---|
| State | Superseded |
| Headers | show |
>>>>> "Matt" == Matt Weber <matt@thewebers.ws> writes: > Signed-off-by: Matt Weber <matt@thewebers.ws> > --- > package/busybox/udhcpc.script | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > diff --git a/package/busybox/udhcpc.script b/package/busybox/udhcpc.script > index 50c52e6..d5d81bf 100755 > --- a/package/busybox/udhcpc.script > +++ b/package/busybox/udhcpc.script > @@ -61,6 +61,29 @@ case "$1" in > echo adding dns $i > echo "nameserver $i # $interface" >> $RESOLV_CONF > done > + > + # Based on http://sourceforge.net/p/kboot/mailman/message/1168535/ A more detailed commit message would be good. After looking at that URL I'm still not quite sure what this is needed for? It doesn't seem to be read again anywhere. > + } >> /var/lib/misc/udhcpc-$interface.leases Are you sure this should append the file and not overwrite it? Why would you want to keep older lease info around when you receive a new one? Perhaps this belongs in a hook in your rootfs overlay?
Peter, On Thu, Apr 30, 2015 at 9:31 AM, Peter Korsgaard <peter@korsgaard.com> wrote: >>>>>> "Matt" == Matt Weber <matt@thewebers.ws> writes: > > > Signed-off-by: Matt Weber <matt@thewebers.ws> > > --- > > package/busybox/udhcpc.script | 23 +++++++++++++++++++++++ > > 1 file changed, 23 insertions(+) > > > diff --git a/package/busybox/udhcpc.script b/package/busybox/udhcpc.script > > index 50c52e6..d5d81bf 100755 > > --- a/package/busybox/udhcpc.script > > +++ b/package/busybox/udhcpc.script > > @@ -61,6 +61,29 @@ case "$1" in > > echo adding dns $i > > echo "nameserver $i # $interface" >> $RESOLV_CONF > > done > > + > > + # Based on http://sourceforge.net/p/kboot/mailman/message/1168535/ > > A more detailed commit message would be good. After looking at that URL > I'm still not quite sure what this is needed for? It doesn't seem to be > read again anywhere. Sorry, I can add that. In general, it adds the leases functionality that mimics full dhclient behavior. This file can then be used at a system level to know least expiration and other server provided information. > >> + } >> /var/lib/misc/udhcpc-$interface.leases > > Are you sure this should append the file and not overwrite it? Why would > you want to keep older lease info around when you receive a new one? I just mimic'd dhclient but I could see in a embedded space you might only want to keep the latest and when a lease expires, removing this file. I think the full client usually used this file to set an interface after reboot to the previous lease if one was still valid. I wasn't going to keep this behavior since for embedded devices it doesn't make sense. > > Perhaps this belongs in a hook in your rootfs overlay? Right now there isn't any way to get dhcp lease information after a lease is provided. It seemed this should be a standard capability of the script. I do agree I should make it not append and clean up when there isnt' a lease. Thanks for the feedback!
>>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes: Hi, >> > + # Based on http://sourceforge.net/p/kboot/mailman/message/1168535/ >> >> A more detailed commit message would be good. After looking at that URL >> I'm still not quite sure what this is needed for? It doesn't seem to be >> read again anywhere. > Sorry, I can add that. In general, it adds the leases functionality > that mimics full dhclient behavior. This file can then be used at a > system level to know least expiration and other server provided > information. Ok, good. Is that really something we want to enforce for everyone using udhcpc with Buildroot? >>> + } >> /var/lib/misc/udhcpc-$interface.leases >> >> Are you sure this should append the file and not overwrite it? Why would >> you want to keep older lease info around when you receive a new one? > I just mimic'd dhclient but I could see in a embedded space you might > only want to keep the latest and when a lease expires, removing this > file. I think the full client usually used this file to set an > interface after reboot to the previous lease if one was still valid. > I wasn't going to keep this behavior since for embedded devices it > doesn't make sense. Ok. >> >> Perhaps this belongs in a hook in your rootfs overlay? > Right now there isn't any way to get dhcp lease information after a > lease is provided. It seemed this should be a standard capability of > the script. I do agree I should make it not append and clean up when > there isnt' a lease. I'm still not convinced this isn't something that belongs in a project specific rootfs overlay instead. That's part of the reason why we now support custom hooks in the udhcpc script.
Peter, On Thu, Apr 30, 2015 at 9:59 AM, Peter Korsgaard <peter@korsgaard.com> wrote: >>>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes: > > Hi, > > >> > + # Based on http://sourceforge.net/p/kboot/mailman/message/1168535/ > >> > >> A more detailed commit message would be good. After looking at that URL > >> I'm still not quite sure what this is needed for? It doesn't seem to be > >> read again anywhere. > > > Sorry, I can add that. In general, it adds the leases functionality > > that mimics full dhclient behavior. This file can then be used at a > > system level to know least expiration and other server provided > > information. > > Ok, good. Is that really something we want to enforce for everyone using > udhcpc with Buildroot? Possibly not. I'm good with doing a hook script for now. Half of my sending this patch in was to just publish the concept if anyone else wants to use/add to it. Thanks!
On 04/30/15 16:59, Peter Korsgaard wrote: >>>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes: > > Hi, > > >> > + # Based on http://sourceforge.net/p/kboot/mailman/message/1168535/ > >> > >> A more detailed commit message would be good. After looking at that URL > >> I'm still not quite sure what this is needed for? It doesn't seem to be > >> read again anywhere. > > > Sorry, I can add that. In general, it adds the leases functionality > > that mimics full dhclient behavior. This file can then be used at a > > system level to know least expiration and other server provided > > information. > > Ok, good. Is that really something we want to enforce for everyone using > udhcpc with Buildroot? I don't see why not. It's creating a small file in /tmp, that's all. Except for the appending, of course, otherwise a long-running system could just fill up /tmp with leases. Overwriting has the disadvantage that there's a race condition when the file is truncated at the same time that another process is reading from it, but I don't think the complexity of fixing that race is worth the bother. Regards, Arnout > > >>> + } >> /var/lib/misc/udhcpc-$interface.leases > >> > >> Are you sure this should append the file and not overwrite it? Why would > >> you want to keep older lease info around when you receive a new one? > > > I just mimic'd dhclient but I could see in a embedded space you might > > only want to keep the latest and when a lease expires, removing this > > file. I think the full client usually used this file to set an > > interface after reboot to the previous lease if one was still valid. > > I wasn't going to keep this behavior since for embedded devices it > > doesn't make sense. > > Ok. > > >> > >> Perhaps this belongs in a hook in your rootfs overlay? > > > Right now there isn't any way to get dhcp lease information after a > > lease is provided. It seemed this should be a standard capability of > > the script. I do agree I should make it not append and clean up when > > there isnt' a lease. > > I'm still not convinced this isn't something that belongs in a project > specific rootfs overlay instead. That's part of the reason why we now > support custom hooks in the udhcpc script. >
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes: Hi, >> > Sorry, I can add that. In general, it adds the leases functionality >> > that mimics full dhclient behavior. This file can then be used at a >> > system level to know least expiration and other server provided >> > information. >> >> Ok, good. Is that really something we want to enforce for everyone using >> udhcpc with Buildroot? > I don't see why not. It's creating a small file in /tmp, that's all. Except for > the appending, of course, otherwise a long-running system could just fill up > /tmp with leases. Overwriting has the disadvantage that there's a race condition > when the file is truncated at the same time that another process is reading from > it, but I don't think the complexity of fixing that race is worth the bother. True. I would like to see this (and other non-essential things like E.G. avahi-autoipd) move to hooks though. I wonder if the dhclient format is a sensible choice. Is that something any "standard" programs can work with? It doesn't seem very easy to parse in shell scripts. Maybe just shell compatible NAME="<val>" strings would be better?
diff --git a/package/busybox/udhcpc.script b/package/busybox/udhcpc.script index 50c52e6..d5d81bf 100755 --- a/package/busybox/udhcpc.script +++ b/package/busybox/udhcpc.script @@ -61,6 +61,29 @@ case "$1" in echo adding dns $i echo "nameserver $i # $interface" >> $RESOLV_CONF done + + # Based on http://sourceforge.net/p/kboot/mailman/message/1168535/ + { + echo "lease {" + echo " interface \"$interface\";" + echo " fixed-address $ip;" + [ -n "$boot_file" ] && echo " filename \"$boot_file\";" + [ -n "$subnet" ] && echo " option subnet-mask $subnet;" + [ -n "$router" ] && echo " option routers $router;" + echo " option dhcp-lease-time $lease;" + echo " option dhcp-message-type $dhcptype;" + [ -n "$dns" ] && echo " option domain-name-servers $dns;" + echo " option dhcp-server-identifier ${siaddr:-$serverid};" + [ -n "$domain" ] && echo " option domain-name \"$domain\";" + d=$(expr "$(date "+%Y.%m.%d-%H:%M:%S")" : "\(.*\):") + t=$(date -d "$d:$(expr $lease / 2)" "+%w %Y/%m/%d %H:%M:%S" 2>/dev/null) + [ -n "$t" ] && echo " renew $t;" + t=$(date -d "$d:$(expr 7 '*' $lease / 8)" "+%w %Y/%m/%d %H:%M:%S" 2>/dev/null) + [ -n "$t" ] && echo " rebind $t;" + t=$(date -d "$d:$lease" "+%w %Y/%m/%d %H:%M:%S" 2>/dev/null) + [ -n "$t" ] && echo " expire $t;" + echo "}" + } >> /var/lib/misc/udhcpc-$interface.leases ;; esac
Signed-off-by: Matt Weber <matt@thewebers.ws> --- package/busybox/udhcpc.script | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) -- 1.9.1