dhcpsvc: finish discover test

internal/dhcpsvc/dhcpsvc_test.go

@@ -23,6 +23,9 @@ const testLocalTLD = "local"
 // testTimeout is a common timeout for tests and contexts.
 const testTimeout time.Duration = 10 * time.Second
 
+// testXid is a common transaction ID for DHCPv4 tests.
+const testXid = 1
+
 // testLogger is a common logger for tests.
 var testLogger = slogutil.NewDiscardLogger()
 

internal/dhcpsvc/handler4.go

@@ -159,15 +159,15 @@ func (iface *dhcpInterfaceV4) handleDiscover(
 			l.DebugContext(ctx, "different requested ip", "requested", reqIP, "lease", lease.IP)
 		}
 
+		lease.updateExpiry(iface.clock, iface.common.leaseTTL)
 		iface.respondOffer(ctx, req, fd, lease)
 
 		return
 	}
 
-	// TODO(e.burkov):  Allocate a new lease.
 	lease, err := iface.allocateLease(ctx, mac)
 	if err != nil {
-		l.ErrorContext(ctx, "allocating a lease", "error", err)
+		l.ErrorContext(ctx, "allocating a lease", slogutil.KeyError, err)
 
 		return
 	}

internal/dhcpsvc/handler4_test.go

@@ -2,6 +2,7 @@ package dhcpsvc_test
 
 import (
 	"context"
+	"encoding/binary"
 	"net"
 	"net/netip"
 	"testing"
@@ -9,49 +10,170 @@ import (
 
 	"github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc"
 	"github.com/AdguardTeam/golibs/testutil"
+	"github.com/AdguardTeam/golibs/testutil/faketime"
 	"github.com/AdguardTeam/golibs/testutil/servicetest"
-	"github.com/AdguardTeam/golibs/timeutil"
 	"github.com/google/gopacket"
 	"github.com/google/gopacket/layers"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestDHCPServer_ServeEther4_discover(t *testing.T) {
 	t.Parallel()
 
+	// ifaceName is the name of the test network interface.
+	const ifaceName = "iface0"
+
+	// leaseTTL is the lease duration used in this test.
+	const leaseTTL time.Duration = 24 * time.Hour
+
+	// NOTE: Keep in sync with testdata.
+	const (
+		// leaseHostnameStatic is the hostname for the static lease.
+		leaseHostnameStatic = "static4"
+
+		// leaseHostnameDynamic is the hostname for the dynamic lease.
+		leaseHostnameDynamic = "dynamic4"
+
+		// leaseHostnameExpired is the hostname for the expired lease.
+		leaseHostnameExpired = "expired4"
+	)
+
+	// NOTE: Keep in sync with testdata.
+	var (
+		// hwAddrUnknown is the MAC address for an unknown client.
+		hwAddrUnknown = net.HardwareAddr{0x0, 0x1, 0x2, 0x3, 0x4, 0x5}
+
+		// hwAddrStatic is the MAC address for a known static lease.
+		hwAddrStatic = net.HardwareAddr{0x1, 0x2, 0x3, 0x4, 0x5, 0x6}
+
+		// hwAddrDynamic is the MAC address for a known dynamic lease.
+		hwAddrDynamic = net.HardwareAddr{0x2, 0x3, 0x4, 0x5, 0x6, 0x7}
+
+		// hwAddrExpired is the MAC address for a known expired lease.
+		hwAddrExpired = net.HardwareAddr{0x3, 0x4, 0x5, 0x6, 0x7, 0x8}
+	)
+
+	currentTime := time.Date(2025, 1, 1, 1, 1, 1, 0, time.UTC)
+	testClock := &faketime.Clock{
+		OnNow: func() (now time.Time) {
+			return currentTime
+		},
+	}
+	dynamicLeaseExpiry := time.Date(2025, 1, 1, 10, 1, 1, 0, time.UTC).Sub(currentTime)
+
 	ipv4Conf := &dhcpsvc.IPv4Config{
-		Clock:         timeutil.SystemClock{},
+		Clock:         testClock,
 		SubnetMask:    netip.MustParseAddr("255.255.255.0"),
 		GatewayIP:     netip.MustParseAddr("192.168.0.1"),
 		RangeStart:    netip.MustParseAddr("192.168.0.100"),
 		RangeEnd:      netip.MustParseAddr("192.168.0.200"),
-		LeaseDuration: 24 * time.Hour,
+		LeaseDuration: leaseTTL,
 		Enabled:       true,
 	}
 	ifacesConfig := map[string]*dhcpsvc.InterfaceConfig{
-		"iface": {
+		ifaceName: {
 			IPv4: ipv4Conf,
 			IPv6: &dhcpsvc.IPv6Config{Enabled: false},
 		},
 	}
 
-	// TODO(e.burkov):  !! add cases for known lease and wrong packets.
+	// TODO(e.burkov):  Add cases for wrong packets.
 	testCases := []struct {
 		name string
 		in   gopacket.Packet
-		want []byte
+		want layers.DHCPOptions
 	}{{
 		name: "new",
-		in:   newDHCPDISCOVER(t),
-		want: nil,
+		in:   newDHCPDISCOVER(t, hwAddrUnknown),
+		want: layers.DHCPOptions{
+			layers.NewDHCPOption(
+				layers.DHCPOptMessageType,
+				[]byte{byte(layers.DHCPMsgTypeOffer)},
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptServerID,
+				ifacesConfig[ifaceName].IPv4.GatewayIP.AsSlice(),
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptLeaseTime,
+				binary.BigEndian.AppendUint32(nil, uint32(leaseTTL.Seconds())),
+			),
+		},
+	}, {
+		name: "existing_static",
+		in:   newDHCPDISCOVER(t, hwAddrStatic),
+		want: layers.DHCPOptions{
+			layers.NewDHCPOption(
+				layers.DHCPOptMessageType,
+				[]byte{byte(layers.DHCPMsgTypeOffer)},
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptServerID,
+				ifacesConfig[ifaceName].IPv4.GatewayIP.AsSlice(),
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptLeaseTime,
+				binary.BigEndian.AppendUint32(nil, uint32(leaseTTL.Seconds())),
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptHostname,
+				[]byte(leaseHostnameStatic),
+			),
+		},
+	}, {
+		name: "existing_dynamic",
+		in:   newDHCPDISCOVER(t, hwAddrDynamic),
+		want: layers.DHCPOptions{
+			layers.NewDHCPOption(
+				layers.DHCPOptMessageType,
+				[]byte{byte(layers.DHCPMsgTypeOffer)},
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptServerID,
+				ifacesConfig[ifaceName].IPv4.GatewayIP.AsSlice(),
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptLeaseTime,
+				binary.BigEndian.AppendUint32(nil, uint32((dynamicLeaseExpiry).Seconds())),
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptHostname,
+				[]byte(leaseHostnameDynamic),
+			),
+		},
+	}, {
+		name: "existing_dynamic_expired",
+		in:   newDHCPDISCOVER(t, hwAddrExpired),
+		want: layers.DHCPOptions{
+			layers.NewDHCPOption(
+				layers.DHCPOptMessageType,
+				[]byte{byte(layers.DHCPMsgTypeOffer)},
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptServerID,
+				ifacesConfig[ifaceName].IPv4.GatewayIP.AsSlice(),
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptLeaseTime,
+				binary.BigEndian.AppendUint32(nil, uint32(leaseTTL.Seconds())),
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptHostname,
+				[]byte(leaseHostnameExpired),
+			),
+		},
 	}}
 
 	for _, tc := range testCases {
-		ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, "iface")
+		req := testutil.RequireTypeAssert[*layers.DHCPv4](t, tc.in.Layer(layers.LayerTypeDHCPv4))
+
+		ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, ifaceName)
 
 		dhcpConf := &dhcpsvc.Config{
 			Interfaces:           ifacesConfig,
 			NetworkDeviceManager: ndMgr,
+			DBFilePath:           newTempDB(t),
 			Enabled:              true,
 		}
 		srv := newTestDHCPServer(t, dhcpConf)
@@ -72,11 +194,86 @@ func TestDHCPServer_ServeEther4_discover(t *testing.T) {
 				udp    = &layers.UDP{}
 				dhcpv4 = &layers.DHCPv4{}
 			)
-			requireEthernet(t, resp, eth, ip, udp, dhcpv4)
+			types := requireEthernet(t, resp, eth, ip, udp, dhcpv4)
+			require.Equal(t, []gopacket.LayerType{
+				eth.LayerType(),
+				ip.LayerType(),
+				udp.LayerType(),
+				dhcpv4.LayerType(),
+			}, types)
 
-			// TODO(e.burkov):  !! assert layers
+			assertDHCPv4Response(t, req, dhcpv4, tc.want)
 		})
 	}
+
+	t.Run("new_from_expired", func(t *testing.T) {
+		t.Parallel()
+
+		pkt := newDHCPDISCOVER(t, hwAddrUnknown)
+		req := testutil.RequireTypeAssert[*layers.DHCPv4](t, pkt.Layer(layers.LayerTypeDHCPv4))
+
+		ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, ifaceName)
+
+		narrowIPv4Conf := &dhcpsvc.IPv4Config{
+			Clock:         testClock,
+			SubnetMask:    netip.MustParseAddr("255.255.255.0"),
+			GatewayIP:     netip.MustParseAddr("192.168.0.1"),
+			RangeStart:    netip.MustParseAddr("192.168.0.100"),
+			RangeEnd:      netip.MustParseAddr("192.168.0.100"),
+			LeaseDuration: leaseTTL,
+			Enabled:       true,
+		}
+		narrowIfacesConfig := map[string]*dhcpsvc.InterfaceConfig{
+			ifaceName: {
+				IPv4: narrowIPv4Conf,
+				IPv6: &dhcpsvc.IPv6Config{Enabled: false},
+			},
+		}
+
+		dhcpConf := &dhcpsvc.Config{
+			Interfaces:           narrowIfacesConfig,
+			NetworkDeviceManager: ndMgr,
+			DBFilePath:           newTempDB(t),
+			Enabled:              true,
+		}
+		srv := newTestDHCPServer(t, dhcpConf)
+
+		servicetest.RequireRun(t, srv, testTimeout)
+
+		testutil.RequireSend(t, inCh, pkt, testTimeout)
+
+		resp, ok := testutil.RequireReceive(t, outCh, testTimeout)
+		require.True(t, ok)
+
+		var (
+			eth    = &layers.Ethernet{}
+			ip     = &layers.IPv4{}
+			udp    = &layers.UDP{}
+			dhcpv4 = &layers.DHCPv4{}
+		)
+		types := requireEthernet(t, resp, eth, ip, udp, dhcpv4)
+		require.Equal(t, []gopacket.LayerType{
+			eth.LayerType(),
+			ip.LayerType(),
+			udp.LayerType(),
+			dhcpv4.LayerType(),
+		}, types)
+
+		assertDHCPv4Response(t, req, dhcpv4, layers.DHCPOptions{
+			layers.NewDHCPOption(
+				layers.DHCPOptMessageType,
+				[]byte{byte(layers.DHCPMsgTypeOffer)},
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptServerID,
+				narrowIfacesConfig[ifaceName].IPv4.GatewayIP.AsSlice(),
+			),
+			layers.NewDHCPOption(
+				layers.DHCPOptLeaseTime,
+				binary.BigEndian.AppendUint32(nil, uint32(leaseTTL.Seconds())),
+			),
+		})
+	})
 }
 
 // newTestNetworkDeviceManager creates a network device manager for testing.  It
@@ -100,7 +297,6 @@ func newTestNetworkDeviceManager(
 			require.True(pt, ok)
 
 			data = pkt.Data()
-
 			ci = gopacket.CaptureInfo{
 				Length:        len(data),
 				CaptureLength: len(data),
@@ -134,35 +330,33 @@ func newTestNetworkDeviceManager(
 
 // newDHCPDISCOVER creates a new DHCPDISCOVER packet for testing.
 //
-// TODO(e.burkov):  !! add parameters.
-func newDHCPDISCOVER(tb testing.TB) (pkt gopacket.Packet) {
+// TODO(e.burkov):  Add parameters.
+func newDHCPDISCOVER(tb testing.TB, clientHWAddr net.HardwareAddr) (pkt gopacket.Packet) {
 	tb.Helper()
 
-	clientHWAddr := net.HardwareAddr{0x0, 0x1, 0x2, 0x3, 0x4, 0x5}
-
-	etherLayer := &layers.Ethernet{
+	eth := &layers.Ethernet{
 		SrcMAC:       clientHWAddr,
 		DstMAC:       net.HardwareAddr{0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
 		EthernetType: layers.EthernetTypeIPv4,
 	}
-	ipLayer := &layers.IPv4{
+	ip := &layers.IPv4{
 		Version:  4,
 		TTL:      dhcpsvc.IPv4DefaultTTL,
 		SrcIP:    net.IPv4zero.To4(),
 		DstIP:    net.IPv4bcast.To4(),
 		Protocol: layers.IPProtocolUDP,
 	}
-	udpLayer := &layers.UDP{
-		SrcPort: dhcpsvc.ClientPort,
-		DstPort: dhcpsvc.ServerPort,
+	udp := &layers.UDP{
+		SrcPort: dhcpsvc.ClientPortV4,
+		DstPort: dhcpsvc.ServerPortV4,
 	}
-	_ = udpLayer.SetNetworkLayerForChecksum(ipLayer)
+	_ = udp.SetNetworkLayerForChecksum(ip)
 
-	dhcpLayer := &layers.DHCPv4{
+	dhcp := &layers.DHCPv4{
 		Operation:    layers.DHCPOpRequest,
 		HardwareType: layers.LinkTypeEthernet,
 		HardwareLen:  dhcpsvc.EUI48AddrLen,
-		Xid:          1,
+		Xid:          testXid,
 		ClientHWAddr: clientHWAddr,
 		Options: layers.DHCPOptions{
 			layers.NewDHCPOption(layers.DHCPOptMessageType, []byte{
@@ -170,33 +364,57 @@ func newDHCPDISCOVER(tb testing.TB) (pkt gopacket.Packet) {
 			}),
 		},
 	}
+
+	return newTestPacket(tb, layers.LinkTypeEthernet, eth, ip, udp, dhcp)
+}
+
+// newTestPacket creates a valid packet from ls using first as first layer
+// decoder.
+func newTestPacket(
+	tb testing.TB,
+	first gopacket.Decoder,
+	ls ...gopacket.SerializableLayer,
+) (pkg gopacket.Packet) {
+	tb.Helper()
+
 	buf := gopacket.NewSerializeBuffer()
+
 	opts := gopacket.SerializeOptions{
 		FixLengths:       true,
 		ComputeChecksums: true,
 	}
-	err := gopacket.SerializeLayers(
-		buf,
-		opts,
-		etherLayer,
-		ipLayer,
-		udpLayer,
-		dhcpLayer,
-	)
+	err := gopacket.SerializeLayers(buf, opts, ls...)
 	require.NoError(tb, err)
 
-	return gopacket.NewPacket(buf.Bytes(), layers.LayerTypeEthernet, gopacket.Default)
+	return gopacket.NewPacket(buf.Bytes(), first, gopacket.Default)
 }
 
 // requireEthernet requires data to contain an Ethernet layer and all layers
-// from ls.
-func requireEthernet(tb testing.TB, data []byte, ls ...gopacket.DecodingLayer) {
+// from ls.  First of ls must be of type [layers.LayerTypeEthernet].
+func requireEthernet(
+	tb testing.TB,
+	data []byte,
+	ls ...gopacket.DecodingLayer,
+) (types []gopacket.LayerType) {
 	tb.Helper()
 
 	parser := gopacket.NewDecodingLayerParser(layers.LayerTypeEthernet, ls...)
 
-	layerTypes := make([]gopacket.LayerType, 0, len(ls))
-
-	err := parser.DecodeLayers(data, &layerTypes)
+	err := parser.DecodeLayers(data, &types)
 	require.NoError(tb, err)
+
+	return types
+}
+
+// assertDHCPv4Response asserts that the DHCPv4 response matches the expected
+// values.
+func assertDHCPv4Response(tb testing.TB, req, resp *layers.DHCPv4, wantOpts layers.DHCPOptions) {
+	tb.Helper()
+
+	assert.Equal(tb, layers.DHCPOpReply, resp.Operation, "operation")
+	assert.Equal(tb, req.HardwareType, resp.HardwareType, "hardware type")
+	assert.Equal(tb, req.HardwareLen, resp.HardwareLen, "hardware length")
+	assert.Equal(tb, req.Xid, resp.Xid, "xid")
+	assert.Equal(tb, req.ClientHWAddr, resp.ClientHWAddr, "client hardware address")
+	assert.Equal(tb, wantOpts, resp.Options, "options")
 }

internal/dhcpsvc/lease.go

@@ -6,6 +6,8 @@ import (
 	"net/netip"
 	"slices"
 	"time"
+
+	"github.com/AdguardTeam/golibs/timeutil"
 )
 
 // Lease is a DHCP lease.
@@ -57,3 +59,18 @@ var blockedHardwareAddr = make(net.HardwareAddr, EUI48AddrLen)
 func (l *Lease) IsBlocked() (blocked bool) {
 	return bytes.Equal(l.HWAddr, blockedHardwareAddr)
 }
+
+// updateExpiry updates the lease expiry time if the current time is past the
+// expiry.  For static leases, this operation is a no-op.
+func (l *Lease) updateExpiry(clock timeutil.Clock, ttl time.Duration) {
+	if l.IsStatic {
+		return
+	}
+
+	now := clock.Now()
+	if now.Before(l.Expiry) {
+		return
+	}
+
+	l.Expiry = now.Add(ttl)
+}

internal/dhcpsvc/options4.go

@@ -288,9 +288,17 @@ func (iface *dhcpInterfaceV4) updateOptions(req, resp *layers.DHCPv4) {
 	}
 }
 
-// appendLeaseTime appends the lease time option to the response.
-func appendLeaseTime(resp *layers.DHCPv4, leaseTime time.Duration) {
-	leaseTimeData := binary.BigEndian.AppendUint32(nil, uint32(leaseTime.Seconds()))
+// appendLeaseTime appends the lease time option to the response.  lease must
+// not be nil.
+func (iface *dhcpInterfaceV4) appendLeaseTime(resp *layers.DHCPv4, lease *Lease) {
+	var dur time.Duration
+	if lease.IsStatic {
+		dur = iface.common.leaseTTL
+	} else {
+		dur = lease.Expiry.Sub(iface.clock.Now())
+	}
+
+	leaseTimeData := binary.BigEndian.AppendUint32(nil, uint32(dur.Seconds()))
 
 	resp.Options = append(
 		resp.Options,

internal/dhcpsvc/testdata/TestDHCPServer_ServeEther4_discover/leases.json

@@ -0,0 +1,26 @@
+{
+    "leases": [
+        {
+            "expires": "2025-01-01T10:01:01Z",
+            "ip": "192.168.0.102",
+            "hostname": "dynamic4",
+            "mac": "02:03:04:05:06:07",
+            "static": false
+        },
+        {
+            "expires": "2025-01-01T01:01:01Z",
+            "ip": "192.168.0.103",
+            "hostname": "expired4",
+            "mac": "03:04:05:06:07:08",
+            "static": false
+        },
+        {
+            "expires": "",
+            "ip": "192.168.0.101",
+            "hostname": "static4",
+            "mac": "01:02:03:04:05:06",
+            "static": true
+        }
+    ],
+    "version": 1
+}

internal/dhcpsvc/testdata/TestDHCPServer_ServeEther4_discover/new_from_expired/leases.json

@@ -0,0 +1,12 @@
+{
+    "leases": [
+        {
+            "expires": "2025-01-01T01:01:00Z",
+            "ip": "192.168.0.100",
+            "hostname": "dynamic4",
+            "mac": "02:03:04:05:06:07",
+            "static": false
+        }
+    ],
+    "version": 1
+}

internal/dhcpsvc/v4.go

@@ -7,6 +7,7 @@ import (
 	"net"
 	"net/netip"
 	"slices"
+	"strings"
 	"time"
 
 	"github.com/AdguardTeam/golibs/errors"
@@ -293,10 +294,11 @@ func (iface *dhcpInterfaceV4) buildResponse(
 	resp.Options = append(
 		resp.Options,
 		layers.NewDHCPOption(layers.DHCPOptMessageType, []byte{byte(msgType)}),
+		// TODO(e.burkov):  Use network device address.
 		layers.NewDHCPOption(layers.DHCPOptServerID, iface.gateway.AsSlice()),
 	)
 
-	appendLeaseTime(resp, iface.common.leaseTTL)
+	iface.appendLeaseTime(resp, l)
 	iface.updateOptions(req, resp)
 
 	// Add hostname option if the lease has a hostname.
@@ -337,7 +339,10 @@ func (iface *dhcpInterfaceV4) allocateLease(
 	mac net.HardwareAddr,
 ) (l *Lease, err error) {
 	for {
-		l = iface.reserveLease(ctx, mac)
+		l, err = iface.reserveLease(ctx, mac)
+		if err != nil {
+			return nil, err
+		}
 
 		var ok bool
 		ok, err = iface.addrChecker.IsAvailable(l.IP)
@@ -356,59 +361,75 @@ func (iface *dhcpInterfaceV4) allocateLease(
 // reserveLease reserves a lease for a client by its MAC-address.  l is nil if a
 // new lease can't be allocated.  mac must be a valid according to
 // [netutil.ValidateMAC].  index mutex must be locked.
-//
-// TODO(e.burkov):  Use context.
-func (iface *dhcpInterfaceV4) reserveLease(_ context.Context, mac net.HardwareAddr) (l *Lease) {
+func (iface *dhcpInterfaceV4) reserveLease(
+	ctx context.Context,
+	mac net.HardwareAddr,
+) (l *Lease, err error) {
 	nextIP := iface.common.nextIP()
-	if nextIP == (netip.Addr{}) {
-		l = iface.common.findExpiredLease(iface.clock.Now())
-		if l == nil {
-			return nil
+	if nextIP != (netip.Addr{}) {
+		l = &Lease{
+			HWAddr: slices.Clone(mac),
+			IP:     nextIP,
+			Expiry: iface.clock.Now().Add(iface.common.leaseTTL),
 		}
 
-		// TODO(e.burkov):  Move validation from index methods into server's
-		// methods and use index here.
-		delete(iface.common.leases, macToKey(l.HWAddr))
-
-		l.HWAddr = slices.Clone(mac)
-		iface.common.leases[macToKey(mac)] = l
-
-		return l
+		return l, nil
 	}
 
-	l = &Lease{
-		HWAddr: slices.Clone(mac),
-		IP:     nextIP,
+	l = iface.common.findExpiredLease(iface.clock.Now())
+	if l == nil {
+		return nil, errors.Error("no addresses available to lease")
 	}
 
-	return l
+	// TODO(e.burkov):  Move validation from index methods into server's
+	// methods and use index here.
+	delete(iface.common.leases, macToKey(l.HWAddr))
+
+	l.HWAddr = slices.Clone(mac)
+	iface.common.leases[macToKey(mac)] = l
+
+	idx := iface.common.index
+
+	delete(idx.byAddr, l.IP)
+	delete(idx.byName, strings.ToLower(l.Hostname))
+
+	err = idx.dbStore(ctx, iface.common.logger)
+	if err != nil {
+		// Don't wrap the error since it's informative enough as is.
+		return nil, err
+	}
+
+	l.Hostname = ""
+	l.Expiry = iface.clock.Now().Add(iface.common.leaseTTL)
+
+	return l, nil
 }
 
-// IPv4DefaultTTL is the default Time to Live value in seconds as recommended by
-// RFC-1700.
-//
-// See https://datatracker.ietf.org/doc/html/rfc1700.
-const IPv4DefaultTTL = 64
-
 const (
-	// ServerPort is the standard DHCP server port.
-	//
-	// TODO(e.burkov):  !! reference RFC
-	ServerPort layers.UDPPort = 67
+	// IPv4DefaultTTL is the default Time to Live value in seconds as
+	// recommended by RFC 1700.
+	IPv4DefaultTTL = 64
 
-	// ClientPort is the standard DHCP client port.
-	//
-	// TODO(e.burkov):  !! reference RFC
-	ClientPort layers.UDPPort = 68
+	// IPProtoVersion is the IP internetwork general protocol version number as
+	// defined by RFC 1700.
+	IPProtoVersion = 4
+)
+
+// Port numbers for DHCPv4.
+//
+// See RFC 2131 Section 4.1.
+const (
+	// ServerPortV4 is the standard DHCPv4 server port.
+	ServerPortV4 layers.UDPPort = 67
+
+	// ClientPortV4 is the standard DHCPv4 client port.
+	ClientPortV4 layers.UDPPort = 68
 )
 
 // respond4 sends a DHCPv4 response.  fd and resp must not be nil.
 func respond4(fd *frameData, resp *layers.DHCPv4) (err error) {
+	// TODO(e.burkov):  Use pools for buffer and layers.
 	buf := gopacket.NewSerializeBuffer()
-	opts := gopacket.SerializeOptions{
-		FixLengths:       true,
-		ComputeChecksums: true,
-	}
 
 	eth := &layers.Ethernet{
 		SrcMAC:       fd.ether.SrcMAC,
@@ -416,24 +437,26 @@ func respond4(fd *frameData, resp *layers.DHCPv4) (err error) {
 		EthernetType: layers.EthernetTypeIPv4,
 	}
 	ip := &layers.IPv4{
-		Version:  4,
-		IHL:      5,
+		Version:  IPProtoVersion,
 		TTL:      IPv4DefaultTTL,
 		SrcIP:    net.IPv4zero.To4(),
 		DstIP:    net.IPv4bcast.To4(),
 		Protocol: layers.IPProtocolUDP,
 	}
 	udp := &layers.UDP{
-		SrcPort: ServerPort,
-		DstPort: ClientPort,
+		SrcPort: ServerPortV4,
+		DstPort: ClientPortV4,
 	}
 	_ = udp.SetNetworkLayerForChecksum(ip)
 
-	all := []gopacket.SerializableLayer{eth, ip, udp, resp}
+	opts := gopacket.SerializeOptions{
+		FixLengths:       true,
+		ComputeChecksums: true,
+	}
 
-	err = gopacket.SerializeLayers(buf, opts, all...)
+	err = gopacket.SerializeLayers(buf, opts, eth, ip, udp, resp)
 	if err != nil {
-		return err
+		return fmt.Errorf("constructing dhcp v4 response: %w", err)
 	}
 
 	return fd.device.WritePacketData(buf.Bytes())