Merge pull request #505 from theli-ua/v0.7.x

Emit dns servers in DHCPv4 repr. Fixes #504

.github/workflows/clippy.yml

@@ -1,6 +1,6 @@
 on:
   push:
-    branches: [ staging, trying, master ]
+    branches: [ staging, trying, master, v0.* ]
   pull_request_target:
 
 name: Clippy check

.github/workflows/fuzz.yml

@@ -1,6 +1,6 @@
 on:
   push:
-    branches: [ staging, trying, master ]
+    branches: [ staging, trying, master, v0.* ]
   pull_request:
 
 name: Fuzz

.github/workflows/test.yml

@@ -1,6 +1,6 @@
 on:
   push:
-    branches: [ staging, trying, master ]
+    branches: [ staging, trying, master, v0.* ]
   pull_request:
 
 name: Test

CHANGELOG.md

@@ -6,8 +6,37 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-### New features
-* dhcp: Updated DHCP client to respect lease times provided by the server.
+- Update `managed` from 0.7 to 0.8 ([442](https://github.com/smoltcp-rs/smoltcp/pull/442))
+
+## [0.7.4] - 2021-06-11
+
+- tcp: fix "subtract sequence numbers with underflow" on remote window shrink. (#490)
+- tcp: fix substract with overflow when receiving a SYNACK with unincremented ACK number. (#491)
+- tcp: use nonzero initial sequence number to workaround misbehaving servers. (#492)
+
+## [0.7.3] - 2021-05-29
+
+- Fix "unused attribute" error in recent nightlies.
+
+## [0.7.2] - 2021-05-29
+
+- iface: check for ipv4 subnet broadcast addrs everywhere (#462)
+- dhcp: always send parameter_request_list. (#456)
+- dhcp: Clear expiration time on reset. (#456)
+- phy: fix FaultInjector returning a too big buffer when simulating a drop on tx (#463)
+- tcp rtte: fix "attempt to multiply with overflow". (#476)
+- tcp: LastAck should only change to Closed on ack of fin. (#477)
+- wire/dhcpv4: account for lease time, router and subnet options in DhcpRepr::buffer_len (#478)
+
+## [0.7.1] - 2021-03-27
+
+- ndisc: Fix NeighborSolicit incorrectly asking for src addr instead of dst addr ([419](https://github.com/smoltcp-rs/smoltcp/pull/419))
+- dhcpv4: respect lease time from the server instead of renewing every 60 seconds. ([437](https://github.com/smoltcp-rs/smoltcp/pull/437))
+- Fix build errors due to invalid combinations of features ([416](https://github.com/smoltcp-rs/smoltcp/pull/416), [447](https://github.com/smoltcp-rs/smoltcp/pull/447))
+- wire/ipv4: make some functions const ([420](https://github.com/smoltcp-rs/smoltcp/pull/420))
+- phy: fix BPF on OpenBSD ([421](https://github.com/smoltcp-rs/smoltcp/pull/421), [427](https://github.com/smoltcp-rs/smoltcp/pull/427))
+- phy: enable RawSocket, TapInterface on Android ([435](https://github.com/smoltcp-rs/smoltcp/pull/435))
+- phy: fix phy_wait for waits longer than 1 second ([449](https://github.com/smoltcp-rs/smoltcp/pull/449))
 
 ## [0.7.0] - 2021-01-20
 
@@ -48,4 +77,8 @@ only processed when directed to the 255.255.255.255 address. ([377](https://gith
 - Simplify lifetime parameters of sockets, SocketSet, EthernetInterface ([410](https://github.com/smoltcp-rs/smoltcp/pull/410), [413](https://github.com/smoltcp-rs/smoltcp/pull/413))
 
 [Unreleased]: https://github.com/smoltcp-rs/smoltcp/compare/v0.7.0...HEAD
+[0.7.4]: https://github.com/smoltcp-rs/smoltcp/compare/v0.7.3...v0.7.4
+[0.7.3]: https://github.com/smoltcp-rs/smoltcp/compare/v0.7.2...v0.7.3
+[0.7.2]: https://github.com/smoltcp-rs/smoltcp/compare/v0.7.1...v0.7.2
+[0.7.1]: https://github.com/smoltcp-rs/smoltcp/compare/v0.7.0...v0.7.1
 [0.7.0]: https://github.com/smoltcp-rs/smoltcp/compare/v0.6.0...v0.7.0

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "smoltcp"
-version = "0.7.0"
+version = "0.7.4"
 edition = "2018"
 authors = ["whitequark <whitequark@whitequark.org>"]
 description = "A TCP/IP stack designed for bare-metal, real-time systems without a heap."
@@ -32,12 +32,12 @@ url = "1.0"
 std = ["managed/std"]
 alloc = ["managed/alloc"]
 verbose = []
-ethernet = []
-"phy-raw_socket" = ["std", "libc"]
-"phy-tap_interface" = ["std", "libc"]
+ethernet = ["socket"]
+"phy-raw_socket" = ["std", "libc", "ethernet"]
+"phy-tap_interface" = ["std", "libc", "ethernet"]
 "proto-ipv4" = []
 "proto-igmp" = ["proto-ipv4"]
-"proto-dhcpv4" = ["proto-ipv4", "socket-raw"]
+"proto-dhcpv4" = ["proto-ipv4", "socket-raw", "ethernet"]
 "proto-ipv6" = []
 "socket" = []
 "socket-raw" = ["socket"]

src/dhcp/clientv4.rs

@@ -288,7 +288,7 @@ impl Client {
             requested_ip: None,
             client_identifier: Some(mac),
             server_identifier: None,
-            parameter_request_list: None,
+            parameter_request_list: Some(PARAMETER_REQUEST_LIST),
             max_size: Some(raw_socket.payload_recv_capacity() as u16),
             lease_duration: None,
             dns_servers: None,
@@ -321,7 +321,6 @@ impl Client {
                 dhcp_repr.broadcast = false;
                 dhcp_repr.requested_ip = Some(r_state.requested_ip);
                 dhcp_repr.server_identifier = Some(r_state.server_identifier);
-                dhcp_repr.parameter_request_list = Some(PARAMETER_REQUEST_LIST);
                 net_trace!("DHCP send request to {} = {:?}", endpoint, dhcp_repr);
                 send_packet(iface, endpoint, dhcp_repr)
             }
@@ -353,6 +352,7 @@ impl Client {
         net_trace!("DHCP reset");
         self.state = ClientState::Discovering;
         self.next_egress = now;
+        self.lease_expiration = None;
     }
 }
 

src/iface/ethernet.rs

@@ -989,7 +989,7 @@ impl<'a> InterfaceInner<'a> {
         let checksum_caps = self.device_capabilities.checksum.clone();
         let ipv4_repr = Ipv4Repr::parse(&ipv4_packet, &checksum_caps)?;
 
-        if !ipv4_repr.src_addr.is_unicast() {
+        if !self.is_unicast_v4(ipv4_repr.src_addr) {
             // Discard packets with non-unicast source addresses.
             net_debug!("non-unicast source address");
             return Err(Error::Malformed)
@@ -1012,9 +1012,8 @@ impl<'a> InterfaceInner<'a> {
         let handled_by_raw_socket = false;
 
         if !self.has_ip_addr(ipv4_repr.dst_addr) &&
-           !ipv4_repr.dst_addr.is_broadcast() &&
            !self.has_multicast_group(ipv4_repr.dst_addr) &&
-           !self.is_subnet_broadcast(ipv4_repr.dst_addr) {
+           !self.is_broadcast_v4(ipv4_repr.dst_addr) {
 
             // Ignore IP packets not directed at us, or broadcast, or any of the multicast groups.
             // If AnyIP is enabled, also check if the packet is routed locally.
@@ -1070,6 +1069,18 @@ impl<'a> InterfaceInner<'a> {
             })
             .any(|broadcast_address| address == broadcast_address)
     } 
+    
+    /// Checks if an ipv4 address is broadcast, taking into account subnet broadcast addresses
+    #[cfg(feature = "proto-ipv4")]
+    fn is_broadcast_v4(&self, address: Ipv4Address) -> bool {
+        address.is_broadcast() || self.is_subnet_broadcast(address)
+    }
+
+    /// Checks if an ipv4 address is unicast, taking into account subnet broadcast addresses
+    #[cfg(feature = "proto-ipv4")]
+    fn is_unicast_v4(&self, address: Ipv4Address) -> bool {
+        address.is_unicast() && !self.is_subnet_broadcast(address)
+    }
 
     /// Host duties of the **IGMPv2** protocol.
     ///
@@ -1312,10 +1323,10 @@ impl<'a> InterfaceInner<'a> {
                    (&self, ipv4_repr: Ipv4Repr, icmp_repr: Icmpv4Repr<'icmp>) ->
                    Option<IpPacket<'frame>>
     {
-        if !ipv4_repr.src_addr.is_unicast() {
+        if !self.is_unicast_v4(ipv4_repr.src_addr) {
             // Do not send ICMP replies to non-unicast sources
             None
-        } else if ipv4_repr.dst_addr.is_unicast() {
+        } else if self.is_unicast_v4(ipv4_repr.dst_addr) {
             // Reply as normal when src_addr and dst_addr are both unicast
             let ipv4_reply_repr = Ipv4Repr {
                 src_addr:    ipv4_repr.dst_addr,
@@ -1325,7 +1336,7 @@ impl<'a> InterfaceInner<'a> {
                 hop_limit:   64
             };
             Some(IpPacket::Icmpv4((ipv4_reply_repr, icmp_repr)))
-        } else if ipv4_repr.dst_addr.is_broadcast() {
+        } else if self.is_broadcast_v4(ipv4_repr.dst_addr) {
             // Only reply to broadcasts for echo replies and not other ICMP messages
             match icmp_repr {
                 Icmpv4Repr::EchoReply {..} => match self.ipv4_address() {

src/lib.rs

@@ -64,19 +64,6 @@
 //! feature ever defined, to ensure that, when the representation layer is unable to make sense
 //! of a packet, it is still logged correctly and in full.
 //!
-//! ## Packet and representation layer support
-//! | Protocol | Packet | Representation |
-//! |----------|--------|----------------|
-//! | Ethernet | Yes    | Yes            |
-//! | ARP      | Yes    | Yes            |
-//! | IPv4     | Yes    | Yes            |
-//! | ICMPv4   | Yes    | Yes            |
-//! | IGMPv1/2 | Yes    | Yes            |
-//! | IPv6     | Yes    | Yes            |
-//! | ICMPv6   | Yes    | Yes            |
-//! | TCP      | Yes    | Yes            |
-//! | UDP      | Yes    | Yes            |
-//!
 //! [wire]: wire/index.html
 //! [osi]: https://en.wikipedia.org/wiki/OSI_model
 //! [berk]: https://en.wikipedia.org/wiki/Berkeley_sockets
@@ -96,6 +83,20 @@ compile_error!("at least one socket needs to be enabled"); */
 #[cfg(feature = "alloc")]
 extern crate alloc;
 
+#[cfg(not(any(feature = "proto-ipv4", feature = "proto-ipv6")))]
+compile_error!("You must enable at least one of the following features: proto-ipv4, proto-ipv6");
+
+#[cfg(all(
+    feature = "socket",
+    not(any(
+        feature = "socket-raw",
+        feature = "socket-udp",
+        feature = "socket-tcp",
+        feature = "socket-icmp",
+    ))
+))]
+compile_error!("If you enable the socket feature, you must enable at least one of the following features: socket-raw, socket-udp, socket-tcp, socket-icmp");
+
 use core::fmt;
 
 #[macro_use]

src/macros.rs

@@ -6,7 +6,6 @@ macro_rules! net_log {
 }
 
 #[cfg(not(feature = "log"))]
-#[macro_use]
 macro_rules! net_log {
     ($level:ident, $($arg:expr),*) => { $( let _ = $arg; )* }
 }
@@ -25,8 +24,8 @@ macro_rules! enum_with_unknown {
         pub enum $name:ident($ty:ty) {
             $(
               $( #[$variant_attr:meta] )*
-              $variant:ident = $value:expr $(,)*
-            ),+
+              $variant:ident = $value:expr
+            ),+ $(,)?
         }
     ) => {
         #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)]

src/phy/fault_injector.rs

@@ -295,7 +295,7 @@ impl<'a, Tx: phy::TxToken> phy::TxToken for TxToken<'a, Tx> {
         };
 
         if drop {
-            return f(&mut self.junk);
+            return f(&mut self.junk[..len]);
         }
 
         let Self { token, state, config, .. } = self;

src/phy/sys/mod.rs

@@ -47,7 +47,8 @@ pub fn wait(fd: RawFd, duration: Option<Duration>) -> io::Result<()> {
         let mut timeout = libc::timeval { tv_sec: 0, tv_usec: 0 };
         let timeout_ptr =
             if let Some(duration) = duration {
-                timeout.tv_usec = (duration.total_millis() * 1_000) as libc::suseconds_t;
+                timeout.tv_sec = duration.secs() as libc::time_t;
+                timeout.tv_usec = (duration.millis() * 1_000) as libc::suseconds_t;
                 &mut timeout as *mut _
             } else {
                 ptr::null_mut()

src/socket/tcp.rs

@@ -54,6 +54,11 @@ impl fmt::Display for State {
     }
 }
 
+/// Initial sequence number. This used to be 0, but some servers don't behave correctly
+/// with that, so we use a non-zero starting sequence number. TODO: randomize instead.
+/// https://github.com/smoltcp-rs/smoltcp/issues/489
+const INITIAL_SEQ_NO: TcpSeqNumber = TcpSeqNumber(42);
+
 // Conservative initial RTT estimate.
 const RTTE_INITIAL_RTT: u32 = 300;
 const RTTE_INITIAL_DEV: u32 = 100;
@@ -140,7 +145,7 @@ impl RttEstimator {
             // all packets sent would incur a retransmit. To avoid this, force an estimate
             // increase if we see 3 consecutive retransmissions without any successful sample.
             self.rto_count = 0;
-            self.rtt *= 2;
+            self.rtt = RTTE_MAX_RTO.min(self.rtt*2);
             let rto = self.retransmission_timeout().millis();
             net_trace!("rtte: too many retransmissions, increasing: rtt={:?} dev={:?} rto={:?}", self.rtt, self.deviation, rto);
         }
@@ -390,7 +395,7 @@ impl<'a> TcpSocket<'a> {
             listen_address:  IpAddress::default(),
             local_endpoint:  IpEndpoint::default(),
             remote_endpoint: IpEndpoint::default(),
-            local_seq_no:    TcpSeqNumber::default(),
+            local_seq_no:    INITIAL_SEQ_NO,
             remote_seq_no:   TcpSeqNumber::default(),
             remote_last_seq: TcpSeqNumber::default(),
             remote_last_ack: None,
@@ -591,7 +596,7 @@ impl<'a> TcpSocket<'a> {
         self.listen_address  = IpAddress::default();
         self.local_endpoint  = IpEndpoint::default();
         self.remote_endpoint = IpEndpoint::default();
-        self.local_seq_no    = TcpSeqNumber::default();
+        self.local_seq_no    = INITIAL_SEQ_NO;
         self.remote_seq_no   = TcpSeqNumber::default();
         self.remote_last_seq = TcpSeqNumber::default();
         self.remote_last_ack = None;
@@ -1184,6 +1189,16 @@ impl<'a> TcpSocket<'a> {
                 self.abort();
                 return Err(Error::Dropped)
             }
+            // SYN|ACK in the SYN-SENT state must have the exact ACK number.
+            (State::SynSent, &TcpRepr {
+                control: TcpControl::Syn, ack_number: Some(ack_number), ..
+            }) => {
+                if ack_number != self.local_seq_no + 1 {
+                    net_debug!("{}:{}:{}: unacceptable SYN|ACK in response to initial SYN",
+                               self.meta.handle, self.local_endpoint, self.remote_endpoint);
+                    return Err(Error::Dropped)
+                }
+            }
             // Every acknowledgement must be for transmitted but unacknowledged data.
             (_, &TcpRepr { ack_number: Some(ack_number), .. }) => {
                 let unacknowledged = self.tx_buffer.len() + control_len;
@@ -1438,10 +1453,14 @@ impl<'a> TcpSocket<'a> {
 
             // ACK packets in LAST-ACK state change it to CLOSED.
             (State::LastAck, TcpControl::None) => {
-                // Clear the remote endpoint, or we'll send an RST there.
-                self.set_state(State::Closed);
-                self.local_endpoint  = IpEndpoint::default();
-                self.remote_endpoint = IpEndpoint::default();
+                if ack_of_fin {
+                    // Clear the remote endpoint, or we'll send an RST there.
+                    self.set_state(State::Closed);
+                    self.local_endpoint  = IpEndpoint::default();
+                    self.remote_endpoint = IpEndpoint::default();
+                } else {
+                    self.timer.set_for_idle(timestamp, self.keep_alive);
+                }
             }
 
             _ => {
@@ -1786,11 +1805,33 @@ impl<'a> TcpSocket<'a> {
             State::Established | State::FinWait1 | State::Closing | State::CloseWait | State::LastAck => {
                 // Extract as much data as the remote side can receive in this packet
                 // from the transmit buffer.
+
+                // Right edge of window, ie the max sequence number we're allowed to send.
+                let win_right_edge = self.local_seq_no + self.remote_win_len;
+
+                // Max amount of octets we're allowed to send according to the remote window.
+                let win_limit = if win_right_edge >= self.remote_last_seq {
+                    win_right_edge - self.remote_last_seq
+                } else {
+                    // This can happen if we've sent some data and later the remote side
+                    // has shrunk its window so that data is no longer inside the window.
+                    // This should be very rare and is strongly discouraged by the RFCs,
+                    // but it does happen in practice.
+                    // http://www.tcpipguide.com/free/t_TCPWindowManagementIssues.htm
+                    0
+                };
+
+                // Maximum size we're allowed to send. This can be limited by 3 factors:
+                // 1. remote window
+                // 2. MSS the remote is willing to accept, probably determined by their MTU
+                // 3. MSS we can send, determined by our MTU.
+                let size = win_limit
+                    .min(self.remote_mss)
+                    .min(caps.max_transmission_unit - ip_repr.buffer_len() - repr.mss_header_len());
+
                 let offset = self.remote_last_seq - self.local_seq_no;
-                let win_limit = self.local_seq_no + self.remote_win_len - self.remote_last_seq;
-                let size = cmp::min(cmp::min(win_limit, self.remote_mss),
-                     caps.max_transmission_unit - ip_repr.buffer_len() - repr.mss_header_len());
                 repr.payload = self.tx_buffer.get_allocated(offset, size);
+
                 // If we've sent everything we had in the buffer, follow it with the PSH or FIN
                 // flags, depending on whether the transmit half of the connection is open.
                 if offset + repr.payload.len() == self.tx_buffer.len() {
@@ -2712,6 +2753,29 @@ mod test {
         sanity!(s, socket_established());
     }
 
+    #[test]
+    fn test_syn_sent_syn_ack_not_incremented() {
+        let mut s = socket_syn_sent();
+        recv!(s, [TcpRepr {
+            control:    TcpControl::Syn,
+            seq_number: LOCAL_SEQ,
+            ack_number: None,
+            max_seg_size: Some(BASE_MSS),
+            window_scale: Some(0),
+            sack_permitted: true,
+            ..RECV_TEMPL
+        }]);
+        send!(s, TcpRepr {
+            control:    TcpControl::Syn,
+            seq_number: REMOTE_SEQ,
+            ack_number: Some(LOCAL_SEQ), // WRONG
+            max_seg_size: Some(BASE_MSS - 80),
+            window_scale: Some(0),
+            ..SEND_TEMPL
+        }, Err(Error::Dropped));
+        assert_eq!(s.state, State::SynSent);
+    }
+
     #[test]
     fn test_syn_sent_rst() {
         let mut s = socket_syn_sent();
@@ -2783,10 +2847,12 @@ mod test {
             (1048576, 5),
         ] {
             let mut s = socket_with_buffer_sizes(64, *buffer_size);
+            s.local_seq_no = LOCAL_SEQ;
             assert_eq!(s.remote_win_shift, *shift_amt);
             s.connect(REMOTE_END, LOCAL_END).unwrap();
             recv!(s, [TcpRepr {
                 control: TcpControl::Syn,
+                seq_number: LOCAL_SEQ,
                 ack_number: None,
                 max_seg_size: Some(BASE_MSS),
                 window_scale: Some(*shift_amt),
@@ -3005,6 +3071,48 @@ mod test {
         }]);
     }
 
+    #[test]
+    fn test_established_send_window_shrink() {
+        let mut s = socket_established();
+
+        // 6 octets fit on the remote side's window, so we send them.
+        s.send_slice(b"abcdef").unwrap();
+        recv!(s, [TcpRepr {
+            seq_number: LOCAL_SEQ + 1,
+            ack_number: Some(REMOTE_SEQ + 1),
+            payload: &b"abcdef"[..],
+            ..RECV_TEMPL
+        }]);
+        assert_eq!(s.tx_buffer.len(), 6);
+
+        println!("local_seq_no={} remote_win_len={} remote_last_seq={}", s.local_seq_no, s.remote_win_len, s.remote_last_seq);
+
+        // - Peer doesn't ack them yet
+        // - Sends data so we need to reply with an ACK
+        // - ...AND and sends a window announcement that SHRINKS the window, so data we've
+        //   previously sent is now outside the window. Yes, this is allowed by TCP.
+        send!(s, TcpRepr {
+            seq_number: REMOTE_SEQ + 1,
+            ack_number: Some(LOCAL_SEQ + 1),
+            window_len: 3,
+            payload: &b"xyzxyz"[..],
+            ..SEND_TEMPL
+        });
+        assert_eq!(s.tx_buffer.len(), 6);
+
+        println!("local_seq_no={} remote_win_len={} remote_last_seq={}", s.local_seq_no, s.remote_win_len, s.remote_last_seq);
+
+        // More data should not get sent since it doesn't fit in the window
+        s.send_slice(b"foobar").unwrap();
+        recv!(s, [TcpRepr {
+            seq_number: LOCAL_SEQ + 1 + 6,
+            ack_number: Some(REMOTE_SEQ + 1 + 6),
+            window_len: 64 - 6,
+            ..RECV_TEMPL
+        }]);
+    }
+
+
     #[test]
     fn test_established_send_wrap() {
         let mut s = socket_established();
@@ -3489,6 +3597,34 @@ mod test {
         assert_eq!(s.state, State::Closed);
     }
 
+    #[test]
+    fn test_last_ack_ack_not_of_fin() {
+        let mut s = socket_last_ack();
+        recv!(s, [TcpRepr {
+            control: TcpControl::Fin,
+            seq_number: LOCAL_SEQ + 1,
+            ack_number: Some(REMOTE_SEQ + 1 + 1),
+            ..RECV_TEMPL
+        }]);
+        assert_eq!(s.state, State::LastAck);
+
+        // ACK received that doesn't ack the FIN: socket should stay in LastAck.
+        send!(s, TcpRepr {
+            seq_number: REMOTE_SEQ + 1 + 1,
+            ack_number: Some(LOCAL_SEQ + 1),
+            ..SEND_TEMPL
+        });
+        assert_eq!(s.state, State::LastAck);
+
+        // ACK received of fin: socket should change to Closed.
+        send!(s, TcpRepr {
+            seq_number: REMOTE_SEQ + 1 + 1,
+            ack_number: Some(LOCAL_SEQ + 1 + 1),
+            ..SEND_TEMPL
+        });
+        assert_eq!(s.state, State::Closed);
+    }
+
     #[test]
     fn test_last_ack_close() {
         let mut s = socket_last_ack();

src/wire/dhcpv4.rs

@@ -8,6 +8,8 @@ use crate::wire::arp::Hardware;
 
 const DHCP_MAGIC_NUMBER: u32 = 0x63825363;
 
+pub const MAX_DNS_SERVERS: usize = 3;
+
 enum_with_unknown! {
     /// The possible opcodes of a DHCP packet.
     pub enum OpCode(u8) {
@@ -680,7 +682,7 @@ pub struct Repr<'a> {
     /// the client is interested in.
     pub parameter_request_list: Option<&'a [u8]>,
     /// DNS servers
-    pub dns_servers: Option<[Option<Ipv4Address>; 3]>,
+    pub dns_servers: Option<[Option<Ipv4Address>; MAX_DNS_SERVERS]>,
     /// The maximum size dhcp packet the interface can receive
     pub max_size: Option<u16>,
     /// The DHCP IP lease duration, specified in seconds.
@@ -697,6 +699,13 @@ impl<'a> Repr<'a> {
         if self.client_identifier.is_some() { len += 9; }
         if self.server_identifier.is_some() { len += 6; }
         if self.max_size.is_some() { len += 4; }
+        if self.router.is_some() { len += 6; }
+        if self.subnet_mask.is_some() { len += 6; }
+        if self.lease_duration.is_some() { len += 6; }
+        if let Some(dns_servers) = self.dns_servers {
+            len += 2;
+            len += dns_servers.iter().flatten().count() * core::mem::size_of::<u32>();
+        }
         if let Some(list) = self.parameter_request_list { len += list.len() + 2; }
 
         len
@@ -774,7 +783,7 @@ impl<'a> Repr<'a> {
                     parameter_request_list = Some(data);
                 }
                 DhcpOption::Other {kind: field::OPT_DOMAIN_NAME_SERVER, data} => {
-                    let mut servers = [None; 3];
+                    let mut servers = [None; MAX_DNS_SERVERS];
                     for (server, chunk) in servers.iter_mut().zip(data.chunks(4)) {
                         *server = Some(Ipv4Address::from_bytes(chunk));
                     }
@@ -839,6 +848,19 @@ impl<'a> Repr<'a> {
             if let Some(duration) = self.lease_duration {
                 let tmp = options; options = DhcpOption::IpLeaseTime(duration).emit(tmp);
             }
+            if let Some(dns_servers) = self.dns_servers {
+                const IP_SIZE: usize = core::mem::size_of::<u32>();
+                let mut servers = [0; MAX_DNS_SERVERS * IP_SIZE];
+
+                let data_len = dns_servers.iter().flatten()
+                    .enumerate()
+                    .inspect(|(i, ip)| {
+                        servers[(i * IP_SIZE)..((i + 1) * IP_SIZE)]
+                            .copy_from_slice(ip.as_bytes());
+                    }).count() * IP_SIZE;
+                let option = DhcpOption::Other{ kind: field::OPT_DOMAIN_NAME_SERVER, data: &servers[..data_len] };
+                let tmp = options; options = option.emit(tmp);
+            }
             if let Some(list) = self.parameter_request_list {
                 let option = DhcpOption::Other{ kind: field::OPT_PARAMETER_REQUEST_LIST, data: list };
                 let tmp = options; options = option.emit(tmp);
@@ -1012,6 +1034,28 @@ mod test {
         assert_eq!(packet, DISCOVER_BYTES);
     }
 
+    fn offer_repr() -> Repr<'static> {
+        Repr {
+            message_type: MessageType::Offer,
+            transaction_id: 0x3d1d,
+            client_hardware_address: CLIENT_MAC,
+            client_ip: IP_NULL,
+            your_ip: IP_NULL,
+            server_ip: IP_NULL,
+            router: Some(IP_NULL),
+            subnet_mask: Some(IP_NULL),
+            relay_agent_ip: IP_NULL,
+            broadcast: false,
+            requested_ip: None,
+            client_identifier: Some(CLIENT_MAC),
+            server_identifier: None,
+            parameter_request_list: None,
+            dns_servers: None,
+            max_size: None,
+            lease_duration: Some(0xffff_ffff), // Infinite lease
+        }
+    }
+
     fn discover_repr() -> Repr<'static> {
         Repr {
             message_type: MessageType::Discover,
@@ -1055,6 +1099,37 @@ mod test {
         }
     }
 
+    #[test]
+    fn test_emit_offer() {
+        let repr = offer_repr();
+        let mut bytes = vec![0xa5; repr.buffer_len()];
+        let mut packet = Packet::new_unchecked(&mut bytes);
+        repr.emit(&mut packet).unwrap();
+    }
+
+    #[test]
+    fn test_emit_offer_dns() {
+        let repr = {
+            let mut repr = offer_repr();
+            repr.dns_servers = Some([
+            Some(Ipv4Address([163, 1, 74, 6])),
+            Some(Ipv4Address([163, 1, 74, 7])),
+            Some(Ipv4Address([163, 1, 74, 3]))]);
+            repr
+        };
+        let mut bytes = vec![0xa5; repr.buffer_len()];
+        let mut packet = Packet::new_unchecked(&mut bytes);
+        repr.emit(&mut packet).unwrap();
+
+        let packet = Packet::new_unchecked(&bytes);
+        let repr_parsed = Repr::parse(&packet).unwrap();
+
+        assert_eq!(repr_parsed.dns_servers,  Some([
+            Some(Ipv4Address([163, 1, 74, 6])),
+            Some(Ipv4Address([163, 1, 74, 7])),
+            Some(Ipv4Address([163, 1, 74, 3]))]));
+    }
+
     #[test]
     fn test_emit_dhcp_option() {
         static DATA: &[u8] = &[1, 3, 6];