Merge #683

683: Use renewal time from DHCP server ACK, if given r=Dirbaio a=JarredAllen

# Description

Per RFC 2132 section 9.11 the server can manually specify a renewal (T1) time different from the default value (half the lease time) through option code 58. This PR updates the behavior of the dhcp client to use that value, if provided, and only if not provided does it default to half of the lease duration.

Since smoltcp seems to ignore the REBINDING state, I also made it look for a provided rebinding (T2) time provided by the server (dhcp option 59) and made it use that value as the renewal time if no renewal time was provided and the rebinding time is less than the default. This behavior seems sensible to me, given that we're not following the REBINDING part of the spec, but I can change it to ignore option code 59, or any other handling, if that is preferred.

# Verification

I realized that this functionality was missing when I changed my configuration to set a 10 second renew time on a lease which lasts for a very long time, and observed that my devices (which use this library) weren't attempting to renew. To verify that this PR works, I ran it in my existing setup and confirmed that my devices now renew their leases at approximately 10 second intervals. I think this, alongside the tests in CI, should be good enough.

Co-authored-by: Jarred Allen <jarred@moveparallel.com>

Author: bors[bot]

src/socket/dhcpv4.rs

@@ -439,8 +439,23 @@ impl<'a> Socket<'a> {
             packet: None,
         };
 
-        // RFC 2131 indicates clients should renew a lease halfway through its expiration.
-        let renew_at = now + lease_duration / 2;
+        // Set renew time as per RFC 2131:
+        // The renew time (T1) can be specified by the server using option 58:
+        let renew_duration = dhcp_repr
+            .renew_duration
+            .map(|d| Duration::from_secs(d as u64))
+            // Since we don't follow the REBINDING part of the spec, when no
+            // explicit T1 time is given, we will also consider the rebinding
+            // time if it is given and less than the default.
+            .or_else(|| {
+                dhcp_repr
+                    .rebind_duration
+                    .map(|d| Duration::from_secs(d as u64).min(lease_duration / 2))
+            })
+            // Otherwise, we use the default T1 time, which is half the lease
+            // duration.
+            .unwrap_or(lease_duration / 2);
+        let renew_at = now + renew_duration;
         let expires_at = now + lease_duration;
 
         Some((config, renew_at, expires_at))
@@ -497,6 +512,8 @@ impl<'a> Socket<'a> {
             ),
             max_size: Some((cx.ip_mtu() - MAX_IPV4_HEADER_LEN - UDP_HEADER_LEN) as u16),
             lease_duration: None,
+            renew_duration: None,
+            rebind_duration: None,
             dns_servers: None,
             additional_options: self.outgoing_options,
         };
@@ -843,6 +860,8 @@ mod test {
         parameter_request_list: None,
         dns_servers: None,
         max_size: None,
+        renew_duration: None,
+        rebind_duration: None,
         lease_duration: None,
         additional_options: &[],
     };

src/wire/dhcpv4.rs

@@ -651,6 +651,10 @@ pub struct Repr<'a> {
     pub max_size: Option<u16>,
     /// The DHCP IP lease duration, specified in seconds.
     pub lease_duration: Option<u32>,
+    /// The DHCP IP renew duration (T1 interval), in seconds, if specified in the packet.
+    pub renew_duration: Option<u32>,
+    /// The DHCP IP rebind duration (T2 interval), in seconds, if specified in the packet.
+    pub rebind_duration: Option<u32>,
     /// When returned from [`Repr::parse`], this field will be `None`.
     /// However, when calling [`Repr::emit`], this field should contain only
     /// additional DHCP options not known to smoltcp.
@@ -735,6 +739,8 @@ impl<'a> Repr<'a> {
         let mut dns_servers = None;
         let mut max_size = None;
         let mut lease_duration = None;
+        let mut renew_duration = None;
+        let mut rebind_duration = None;
 
         for option in packet.options() {
             let data = option.data;
@@ -767,6 +773,12 @@ impl<'a> Repr<'a> {
                 (field::OPT_MAX_DHCP_MESSAGE_SIZE, 2) => {
                     max_size = Some(u16::from_be_bytes([data[0], data[1]]));
                 }
+                (field::OPT_RENEWAL_TIME_VALUE, 4) => {
+                    renew_duration = Some(u32::from_be_bytes([data[0], data[1], data[2], data[3]]))
+                }
+                (field::OPT_REBINDING_TIME_VALUE, 4) => {
+                    rebind_duration = Some(u32::from_be_bytes([data[0], data[1], data[2], data[3]]))
+                }
                 (field::OPT_IP_LEASE_TIME, 4) => {
                     lease_duration = Some(u32::from_be_bytes([data[0], data[1], data[2], data[3]]))
                 }
@@ -808,6 +820,8 @@ impl<'a> Repr<'a> {
             dns_servers,
             max_size,
             lease_duration,
+            renew_duration,
+            rebind_duration,
             message_type: message_type?,
             additional_options: &[],
         })
@@ -1148,6 +1162,8 @@ mod test {
             parameter_request_list: None,
             dns_servers: None,
             max_size: None,
+            renew_duration: None,
+            rebind_duration: None,
             lease_duration: Some(0xffff_ffff), // Infinite lease
             additional_options: &[],
         }
@@ -1167,6 +1183,8 @@ mod test {
             broadcast: false,
             secs: 0,
             max_size: Some(DHCP_SIZE),
+            renew_duration: None,
+            rebind_duration: None,
             lease_duration: None,
             requested_ip: Some(IP_NULL),
             client_identifier: Some(CLIENT_MAC),