Pull request 2245: 4923 gopacket DHCP vol.8

Updates #4923.

Squashed commit of the following:

commit 0bfccf8bc1e63c4f5a01ce7f268e767969b368a0
Merge: 305f9fe2f 0e5e8e4dd
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Jul 3 15:12:43 2024 +0300

    Merge branch 'master' into 4923-gopacket-dhcp-vol.8

commit 305f9fe2fe
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Jul 2 17:03:01 2024 +0300

    dhcpsvc: adjust interface

commit f05b9f42e2
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Jul 2 16:59:39 2024 +0300

    dhcpsvc: use logger

commit 4779f945ba
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Jul 2 14:59:22 2024 +0300

    dhcpsvc: add todo

commit ae1713e5f7
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Jul 1 15:49:22 2024 +0300

    dhcpsvc: use slog

internal/dhcpsvc/config.go

@@ -2,11 +2,12 @@ package dhcpsvc
 
 import (
 	"fmt"
-	"slices"
+	"log/slog"
 	"time"
 
+	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/mapsutil"
 	"github.com/AdguardTeam/golibs/netutil"
-	"golang.org/x/exp/maps"
 )
 
 // Config is the configuration for the DHCP service.
@@ -15,6 +16,9 @@ type Config struct {
 	// interface identified by its name.
 	Interfaces map[string]*InterfaceConfig
 
+	// Logger will be used to log the DHCP events.
+	Logger *slog.Logger
+
 	// LocalDomainName is the top-level domain name to use for resolving DHCP
 	// clients' hostnames.
 	LocalDomainName string
@@ -38,36 +42,44 @@ type InterfaceConfig struct {
 }
 
 // Validate returns an error in conf if any.
+//
+// TODO(e.burkov):  Unexport and rewrite the test.
 func (conf *Config) Validate() (err error) {
 	switch {
 	case conf == nil:
 		return errNilConfig
 	case !conf.Enabled:
 		return nil
-	case conf.ICMPTimeout < 0:
+	}
-		return newMustErr("icmp timeout", "be non-negative", conf.ICMPTimeout)
+
+	var errs []error
+	if conf.ICMPTimeout < 0 {
+		err = newMustErr("icmp timeout", "be non-negative", conf.ICMPTimeout)
+		errs = append(errs, err)
 	}
 
 	err = netutil.ValidateDomainName(conf.LocalDomainName)
 	if err != nil {
 		// Don't wrap the error since it's informative enough as is.
-		return err
+		errs = append(errs, err)
 	}
 
 	if len(conf.Interfaces) == 0 {
-		return errNoInterfaces
+		errs = append(errs, errNoInterfaces)
+
+		return errors.Join(errs...)
 	}
 
-	ifaces := maps.Keys(conf.Interfaces)
+	mapsutil.SortedRange(conf.Interfaces, func(iface string, ic *InterfaceConfig) (ok bool) {
-	slices.Sort(ifaces)
+		err = ic.validate()
-
+		if err != nil {
-	for _, iface := range ifaces {
+			errs = append(errs, fmt.Errorf("interface %q: %w", iface, err))
-		if err = conf.Interfaces[iface].validate(); err != nil {
-			return fmt.Errorf("interface %q: %w", iface, err)
 		}
-	}
 
-	return nil
+		return true
+	})
+
+	return errors.Join(errs...)
 }
 
 // validate returns an error in ic, if any.

internal/dhcpsvc/config_test.go

@@ -23,7 +23,8 @@ func TestConfig_Validate(t *testing.T) {
 	}, {
 		name: "empty",
 		conf: &dhcpsvc.Config{
-			Enabled: true,
+			Enabled:    true,
+			Interfaces: testInterfaceConf,
 		},
 		wantErrMsg: `bad domain name "": domain name is empty`,
 	}, {

internal/dhcpsvc/dhcpsvc.go

@@ -11,6 +11,14 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/next/agh"
 )
 
+const (
+	// keyInterface is the key for logging the network interface name.
+	keyInterface = "iface"
+
+	// keyFamily is the key for logging the handled address family.
+	keyFamily = "family"
+)
+
 // Interface is a DHCP service.
 //
 // TODO(e.burkov):  Separate HostByIP, MACByIP, IPByHost into a separate
@@ -50,21 +58,21 @@ type Interface interface {
 
 	// AddLease adds a new DHCP lease.  l must be valid.  It returns an error if
 	// l already exists.
-	AddLease(l *Lease) (err error)
+	AddLease(ctx context.Context, l *Lease) (err error)
 
 	// UpdateStaticLease replaces an existing static DHCP lease.  l must be
 	// valid.  It returns an error if the lease with the given hardware address
 	// doesn't exist or if other values match another existing lease.
-	UpdateStaticLease(l *Lease) (err error)
+	UpdateStaticLease(ctx context.Context, l *Lease) (err error)
 
 	// RemoveLease removes an existing DHCP lease.  l must be valid.  It returns
 	// an error if there is no lease equal to l.
-	RemoveLease(l *Lease) (err error)
+	RemoveLease(ctx context.Context, l *Lease) (err error)
 
 	// Reset removes all the DHCP leases.
 	//
 	// TODO(e.burkov):  If it's really needed?
-	Reset() (err error)
+	Reset(ctx context.Context) (err error)
 }
 
 // Empty is an [Interface] implementation that does nothing.
@@ -101,13 +109,13 @@ func (Empty) IPByHost(_ string) (ip netip.Addr) { return netip.Addr{} }
 func (Empty) Leases() (leases []*Lease) { return nil }
 
 // AddLease implements the [Interface] interface for Empty.
-func (Empty) AddLease(_ *Lease) (err error) { return nil }
+func (Empty) AddLease(_ context.Context, _ *Lease) (err error) { return nil }
 
 // UpdateStaticLease implements the [Interface] interface for Empty.
-func (Empty) UpdateStaticLease(_ *Lease) (err error) { return nil }
+func (Empty) UpdateStaticLease(_ context.Context, _ *Lease) (err error) { return nil }
 
 // RemoveLease implements the [Interface] interface for Empty.
-func (Empty) RemoveLease(_ *Lease) (err error) { return nil }
+func (Empty) RemoveLease(_ context.Context, _ *Lease) (err error) { return nil }
 
 // Reset implements the [Interface] interface for Empty.
-func (Empty) Reset() (err error) { return nil }
+func (Empty) Reset(_ context.Context) (err error) { return nil }

internal/dhcpsvc/interface.go

@@ -2,6 +2,7 @@ package dhcpsvc
 
 import (
 	"fmt"
+	"log/slog"
 	"slices"
 	"time"
 )
@@ -11,6 +12,9 @@ import (
 //
 // TODO(e.burkov):  Add other methods as [DHCPServer] evolves.
 type netInterface struct {
+	// logger logs the events related to the network interface.
+	logger *slog.Logger
+
 	// name is the name of the network interface.
 	name string
 

internal/dhcpsvc/server.go

@@ -1,16 +1,17 @@
 package dhcpsvc
 
 import (
+	"context"
 	"fmt"
+	"log/slog"
 	"net"
 	"net/netip"
-	"slices"
 	"sync"
 	"sync/atomic"
 	"time"
 
 	"github.com/AdguardTeam/golibs/errors"
-	"golang.org/x/exp/maps"
+	"github.com/AdguardTeam/golibs/mapsutil"
 )
 
 // DHCPServer is a DHCP server for both IPv4 and IPv6 address families.
@@ -19,6 +20,9 @@ type DHCPServer struct {
 	// information about its clients.
 	enabled *atomic.Bool
 
+	// logger logs common DHCP events.
+	logger *slog.Logger
+
 	// localTLD is the top-level domain name to use for resolving DHCP clients'
 	// hostnames.
 	localTLD string
@@ -43,8 +47,11 @@ type DHCPServer struct {
 // error if the given configuration can't be used.
 //
 // TODO(e.burkov):  Use.
-func New(conf *Config) (srv *DHCPServer, err error) {
+func New(ctx context.Context, conf *Config) (srv *DHCPServer, err error) {
+	l := conf.Logger
 	if !conf.Enabled {
+		l.DebugContext(ctx, "disabled")
+
 		// TODO(e.burkov):  Perhaps return [Empty]?
 		return nil, nil
 	}
@@ -52,27 +59,26 @@ func New(conf *Config) (srv *DHCPServer, err error) {
 	// TODO(e.burkov):  Add validations scoped to the network interfaces set.
 	ifaces4 := make(netInterfacesV4, 0, len(conf.Interfaces))
 	ifaces6 := make(netInterfacesV6, 0, len(conf.Interfaces))
+	var errs []error
 
-	ifaceNames := maps.Keys(conf.Interfaces)
+	mapsutil.SortedRange(conf.Interfaces, func(name string, iface *InterfaceConfig) (cont bool) {
-	slices.Sort(ifaceNames)
+		var i4 *netInterfaceV4
-
+		i4, err = newNetInterfaceV4(ctx, l, name, iface.IPv4)
-	var i4 *netInterfaceV4
-	var i6 *netInterfaceV6
-
-	for _, ifaceName := range ifaceNames {
-		iface := conf.Interfaces[ifaceName]
-
-		i4, err = newNetInterfaceV4(ifaceName, iface.IPv4)
 		if err != nil {
-			return nil, fmt.Errorf("interface %q: ipv4: %w", ifaceName, err)
+			errs = append(errs, fmt.Errorf("interface %q: ipv4: %w", name, err))
 		} else if i4 != nil {
 			ifaces4 = append(ifaces4, i4)
 		}
 
-		i6 = newNetInterfaceV6(ifaceName, iface.IPv6)
+		i6 := newNetInterfaceV6(ctx, l, name, iface.IPv6)
 		if i6 != nil {
 			ifaces6 = append(ifaces6, i6)
 		}
+
+		return true
+	})
+	if err = errors.Join(errs...); err != nil {
+		return nil, err
 	}
 
 	enabled := &atomic.Bool{}
@@ -80,6 +86,7 @@ func New(conf *Config) (srv *DHCPServer, err error) {
 
 	srv = &DHCPServer{
 		enabled:     enabled,
+		logger:      l,
 		localTLD:    conf.LocalDomainName,
 		leasesMu:    &sync.RWMutex{},
 		leases:      newLeaseIndex(),
@@ -159,7 +166,7 @@ func (srv *DHCPServer) IPByHost(host string) (ip netip.Addr) {
 }
 
 // Reset implements the [Interface] interface for *DHCPServer.
-func (srv *DHCPServer) Reset() (err error) {
+func (srv *DHCPServer) Reset(ctx context.Context) (err error) {
 	srv.leasesMu.Lock()
 	defer srv.leasesMu.Unlock()
 
@@ -171,11 +178,13 @@ func (srv *DHCPServer) Reset() (err error) {
 	}
 	srv.leases.clear()
 
+	srv.logger.DebugContext(ctx, "reset leases")
+
 	return nil
 }
 
 // AddLease implements the [Interface] interface for *DHCPServer.
-func (srv *DHCPServer) AddLease(l *Lease) (err error) {
+func (srv *DHCPServer) AddLease(ctx context.Context, l *Lease) (err error) {
 	defer func() { err = errors.Annotate(err, "adding lease: %w") }()
 
 	addr := l.IP
@@ -188,13 +197,27 @@ func (srv *DHCPServer) AddLease(l *Lease) (err error) {
 	srv.leasesMu.Lock()
 	defer srv.leasesMu.Unlock()
 
-	return srv.leases.add(l, iface)
+	err = srv.leases.add(l, iface)
+	if err != nil {
+		// Don't wrap the error since there is already an annotation deferred.
+		return err
+	}
+
+	iface.logger.DebugContext(
+		ctx, "added lease",
+		"hostname", l.Hostname,
+		"ip", l.IP,
+		"mac", l.HWAddr,
+		"static", l.IsStatic,
+	)
+
+	return nil
 }
 
 // UpdateStaticLease implements the [Interface] interface for *DHCPServer.
 //
 // TODO(e.burkov):  Support moving leases between interfaces.
-func (srv *DHCPServer) UpdateStaticLease(l *Lease) (err error) {
+func (srv *DHCPServer) UpdateStaticLease(ctx context.Context, l *Lease) (err error) {
 	defer func() { err = errors.Annotate(err, "updating static lease: %w") }()
 
 	addr := l.IP
@@ -207,11 +230,25 @@ func (srv *DHCPServer) UpdateStaticLease(l *Lease) (err error) {
 	srv.leasesMu.Lock()
 	defer srv.leasesMu.Unlock()
 
-	return srv.leases.update(l, iface)
+	err = srv.leases.update(l, iface)
+	if err != nil {
+		// Don't wrap the error since there is already an annotation deferred.
+		return err
+	}
+
+	iface.logger.DebugContext(
+		ctx, "updated lease",
+		"hostname", l.Hostname,
+		"ip", l.IP,
+		"mac", l.HWAddr,
+		"static", l.IsStatic,
+	)
+
+	return nil
 }
 
 // RemoveLease implements the [Interface] interface for *DHCPServer.
-func (srv *DHCPServer) RemoveLease(l *Lease) (err error) {
+func (srv *DHCPServer) RemoveLease(ctx context.Context, l *Lease) (err error) {
 	defer func() { err = errors.Annotate(err, "removing lease: %w") }()
 
 	addr := l.IP
@@ -224,7 +261,21 @@ func (srv *DHCPServer) RemoveLease(l *Lease) (err error) {
 	srv.leasesMu.Lock()
 	defer srv.leasesMu.Unlock()
 
-	return srv.leases.remove(l, iface)
+	err = srv.leases.remove(l, iface)
+	if err != nil {
+		// Don't wrap the error since there is already an annotation deferred.
+		return err
+	}
+
+	iface.logger.DebugContext(
+		ctx, "removed lease",
+		"hostname", l.Hostname,
+		"ip", l.IP,
+		"mac", l.HWAddr,
+		"static", l.IsStatic,
+	)
+
+	return nil
 }
 
 // ifaceForAddr returns the handled network interface for the given IP address,

internal/dhcpsvc/server_test.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -16,6 +17,12 @@ import (
 // testLocalTLD is a common local TLD for tests.
 const testLocalTLD = "local"
 
+// testTimeout is a common timeout for tests and contexts.
+const testTimeout time.Duration = 10 * time.Second
+
+// discardLog is a logger to discard test output.
+var discardLog = slogutil.NewDiscardLogger()
+
 // testInterfaceConf is a common set of interface configurations for tests.
 var testInterfaceConf = map[string]*dhcpsvc.InterfaceConfig{
 	"eth0": {
@@ -103,6 +110,7 @@ func TestNew(t *testing.T) {
 	}{{
 		conf: &dhcpsvc.Config{
 			Enabled:         true,
+			Logger:          discardLog,
 			LocalDomainName: testLocalTLD,
 			Interfaces: map[string]*dhcpsvc.InterfaceConfig{
 				"eth0": {
@@ -116,6 +124,7 @@ func TestNew(t *testing.T) {
 	}, {
 		conf: &dhcpsvc.Config{
 			Enabled:         true,
+			Logger:          discardLog,
 			LocalDomainName: testLocalTLD,
 			Interfaces: map[string]*dhcpsvc.InterfaceConfig{
 				"eth0": {
@@ -129,6 +138,7 @@ func TestNew(t *testing.T) {
 	}, {
 		conf: &dhcpsvc.Config{
 			Enabled:         true,
+			Logger:          discardLog,
 			LocalDomainName: testLocalTLD,
 			Interfaces: map[string]*dhcpsvc.InterfaceConfig{
 				"eth0": {
@@ -143,6 +153,7 @@ func TestNew(t *testing.T) {
 	}, {
 		conf: &dhcpsvc.Config{
 			Enabled:         true,
+			Logger:          discardLog,
 			LocalDomainName: testLocalTLD,
 			Interfaces: map[string]*dhcpsvc.InterfaceConfig{
 				"eth0": {
@@ -156,17 +167,22 @@ func TestNew(t *testing.T) {
 			`range start 127.0.0.1 is not within 192.168.0.1/24`,
 	}}
 
+	ctx := testutil.ContextWithTimeout(t, testTimeout)
+
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			_, err := dhcpsvc.New(tc.conf)
+			_, err := dhcpsvc.New(ctx, tc.conf)
 			testutil.AssertErrorMsg(t, tc.wantErrMsg, err)
 		})
 	}
 }
 
 func TestDHCPServer_AddLease(t *testing.T) {
-	srv, err := dhcpsvc.New(&dhcpsvc.Config{
+	ctx := testutil.ContextWithTimeout(t, testTimeout)
+
+	srv, err := dhcpsvc.New(ctx, &dhcpsvc.Config{
 		Enabled:         true,
+		Logger:          discardLog,
 		LocalDomainName: testLocalTLD,
 		Interfaces:      testInterfaceConf,
 	})
@@ -186,7 +202,7 @@ func TestDHCPServer_AddLease(t *testing.T) {
 	mac2 := mustParseMAC(t, "06:05:04:03:02:01")
 	mac3 := mustParseMAC(t, "02:03:04:05:06:07")
 
-	require.NoError(t, srv.AddLease(&dhcpsvc.Lease{
+	require.NoError(t, srv.AddLease(ctx, &dhcpsvc.Lease{
 		Hostname: host1,
 		IP:       ip1,
 		HWAddr:   mac1,
@@ -261,14 +277,17 @@ func TestDHCPServer_AddLease(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			testutil.AssertErrorMsg(t, tc.wantErrMsg, srv.AddLease(tc.lease))
+			testutil.AssertErrorMsg(t, tc.wantErrMsg, srv.AddLease(ctx, tc.lease))
 		})
 	}
 }
 
 func TestDHCPServer_index(t *testing.T) {
-	srv, err := dhcpsvc.New(&dhcpsvc.Config{
+	ctx := testutil.ContextWithTimeout(t, testTimeout)
+
+	srv, err := dhcpsvc.New(ctx, &dhcpsvc.Config{
 		Enabled:         true,
+		Logger:          discardLog,
 		LocalDomainName: testLocalTLD,
 		Interfaces:      testInterfaceConf,
 	})
@@ -313,7 +332,7 @@ func TestDHCPServer_index(t *testing.T) {
 		IsStatic: true,
 	}}
 	for _, l := range leases {
-		require.NoError(t, srv.AddLease(l))
+		require.NoError(t, srv.AddLease(ctx, l))
 	}
 
 	t.Run("ip_idx", func(t *testing.T) {
@@ -342,8 +361,11 @@ func TestDHCPServer_index(t *testing.T) {
 }
 
 func TestDHCPServer_UpdateStaticLease(t *testing.T) {
-	srv, err := dhcpsvc.New(&dhcpsvc.Config{
+	ctx := testutil.ContextWithTimeout(t, testTimeout)
+
+	srv, err := dhcpsvc.New(ctx, &dhcpsvc.Config{
 		Enabled:         true,
+		Logger:          discardLog,
 		LocalDomainName: testLocalTLD,
 		Interfaces:      testInterfaceConf,
 	})
@@ -386,7 +408,7 @@ func TestDHCPServer_UpdateStaticLease(t *testing.T) {
 		IsStatic: true,
 	}}
 	for _, l := range leases {
-		require.NoError(t, srv.AddLease(l))
+		require.NoError(t, srv.AddLease(ctx, l))
 	}
 
 	testCases := []struct {
@@ -456,14 +478,17 @@ func TestDHCPServer_UpdateStaticLease(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			testutil.AssertErrorMsg(t, tc.wantErrMsg, srv.UpdateStaticLease(tc.lease))
+			testutil.AssertErrorMsg(t, tc.wantErrMsg, srv.UpdateStaticLease(ctx, tc.lease))
 		})
 	}
 }
 
 func TestDHCPServer_RemoveLease(t *testing.T) {
-	srv, err := dhcpsvc.New(&dhcpsvc.Config{
+	ctx := testutil.ContextWithTimeout(t, testTimeout)
+
+	srv, err := dhcpsvc.New(ctx, &dhcpsvc.Config{
 		Enabled:         true,
+		Logger:          discardLog,
 		LocalDomainName: testLocalTLD,
 		Interfaces:      testInterfaceConf,
 	})
@@ -495,7 +520,7 @@ func TestDHCPServer_RemoveLease(t *testing.T) {
 		IsStatic: true,
 	}}
 	for _, l := range leases {
-		require.NoError(t, srv.AddLease(l))
+		require.NoError(t, srv.AddLease(ctx, l))
 	}
 
 	testCases := []struct {
@@ -546,7 +571,7 @@ func TestDHCPServer_RemoveLease(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			testutil.AssertErrorMsg(t, tc.wantErrMsg, srv.RemoveLease(tc.lease))
+			testutil.AssertErrorMsg(t, tc.wantErrMsg, srv.RemoveLease(ctx, tc.lease))
 		})
 	}
 
@@ -554,8 +579,11 @@ func TestDHCPServer_RemoveLease(t *testing.T) {
 }
 
 func TestDHCPServer_Reset(t *testing.T) {
-	srv, err := dhcpsvc.New(&dhcpsvc.Config{
+	ctx := testutil.ContextWithTimeout(t, testTimeout)
+
+	srv, err := dhcpsvc.New(ctx, &dhcpsvc.Config{
 		Enabled:         true,
+		Logger:          discardLog,
 		LocalDomainName: testLocalTLD,
 		Interfaces:      testInterfaceConf,
 	})
@@ -584,12 +612,12 @@ func TestDHCPServer_Reset(t *testing.T) {
 	}}
 
 	for _, l := range leases {
-		require.NoError(t, srv.AddLease(l))
+		require.NoError(t, srv.AddLease(ctx, l))
 	}
 
 	require.Len(t, srv.Leases(), len(leases))
 
-	require.NoError(t, srv.Reset())
+	require.NoError(t, srv.Reset(ctx))
 
 	assert.Empty(t, srv.Leases())
 }

internal/dhcpsvc/v4.go

@@ -1,13 +1,15 @@
 package dhcpsvc
 
 import (
+	"context"
 	"fmt"
+	"log/slog"
 	"net"
 	"net/netip"
 	"slices"
 	"time"
 
-	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/google/gopacket/layers"
 )
@@ -43,25 +45,133 @@ type IPv4Config struct {
 }
 
 // validate returns an error in conf if any.
-func (conf *IPv4Config) validate() (err error) {
+func (c *IPv4Config) validate() (err error) {
-	switch {
+	if c == nil {
-	case conf == nil:
 		return errNilConfig
-	case !conf.Enabled:
+	} else if !c.Enabled {
-		return nil
-	case !conf.GatewayIP.Is4():
-		return newMustErr("gateway ip", "be a valid ipv4", conf.GatewayIP)
-	case !conf.SubnetMask.Is4():
-		return newMustErr("subnet mask", "be a valid ipv4 cidr mask", conf.SubnetMask)
-	case !conf.RangeStart.Is4():
-		return newMustErr("range start", "be a valid ipv4", conf.RangeStart)
-	case !conf.RangeEnd.Is4():
-		return newMustErr("range end", "be a valid ipv4", conf.RangeEnd)
-	case conf.LeaseDuration <= 0:
-		return newMustErr("lease duration", "be less than %d", conf.LeaseDuration)
-	default:
 		return nil
 	}
+
+	var errs []error
+
+	if !c.GatewayIP.Is4() {
+		err = newMustErr("gateway ip", "be a valid ipv4", c.GatewayIP)
+		errs = append(errs, err)
+	}
+
+	if !c.SubnetMask.Is4() {
+		err = newMustErr("subnet mask", "be a valid ipv4 cidr mask", c.SubnetMask)
+		errs = append(errs, err)
+	}
+
+	if !c.RangeStart.Is4() {
+		err = newMustErr("range start", "be a valid ipv4", c.RangeStart)
+		errs = append(errs, err)
+	}
+
+	if !c.RangeEnd.Is4() {
+		err = newMustErr("range end", "be a valid ipv4", c.RangeEnd)
+		errs = append(errs, err)
+	}
+
+	if c.LeaseDuration <= 0 {
+		err = newMustErr("icmp timeout", "be positive", c.LeaseDuration)
+		errs = append(errs, err)
+	}
+
+	return errors.Join(errs...)
+}
+
+// netInterfaceV4 is a DHCP interface for IPv4 address family.
+type netInterfaceV4 struct {
+	// gateway is the IP address of the network gateway.
+	gateway netip.Addr
+
+	// subnet is the network subnet.
+	subnet netip.Prefix
+
+	// addrSpace is the IPv4 address space allocated for leasing.
+	addrSpace ipRange
+
+	// implicitOpts are the options listed in Appendix A of RFC 2131 and
+	// initialized with default values.  It must not have intersections with
+	// explicitOpts.
+	implicitOpts layers.DHCPOptions
+
+	// explicitOpts are the user-configured options.  It must not have
+	// intersections with implicitOpts.
+	explicitOpts layers.DHCPOptions
+
+	// netInterface is embedded here to provide some common network interface
+	// logic.
+	netInterface
+}
+
+// newNetInterfaceV4 creates a new DHCP interface for IPv4 address family with
+// the given configuration.  It returns an error if the given configuration
+// can't be used.
+func newNetInterfaceV4(
+	ctx context.Context,
+	l *slog.Logger,
+	name string,
+	conf *IPv4Config,
+) (i *netInterfaceV4, err error) {
+	l = l.With(
+		keyInterface, name,
+		keyFamily, netutil.AddrFamilyIPv4,
+	)
+	if !conf.Enabled {
+		l.DebugContext(ctx, "disabled")
+
+		return nil, nil
+	}
+
+	maskLen, _ := net.IPMask(conf.SubnetMask.AsSlice()).Size()
+	subnet := netip.PrefixFrom(conf.GatewayIP, maskLen)
+
+	switch {
+	case !subnet.Contains(conf.RangeStart):
+		return nil, fmt.Errorf("range start %s is not within %s", conf.RangeStart, subnet)
+	case !subnet.Contains(conf.RangeEnd):
+		return nil, fmt.Errorf("range end %s is not within %s", conf.RangeEnd, subnet)
+	}
+
+	addrSpace, err := newIPRange(conf.RangeStart, conf.RangeEnd)
+	if err != nil {
+		return nil, err
+	} else if addrSpace.contains(conf.GatewayIP) {
+		return nil, fmt.Errorf("gateway ip %s in the ip range %s", conf.GatewayIP, addrSpace)
+	}
+
+	i = &netInterfaceV4{
+		gateway:   conf.GatewayIP,
+		subnet:    subnet,
+		addrSpace: addrSpace,
+		netInterface: netInterface{
+			name:     name,
+			leaseTTL: conf.LeaseDuration,
+			logger:   l,
+		},
+	}
+	i.implicitOpts, i.explicitOpts = conf.options(ctx, l)
+
+	return i, nil
+}
+
+// netInterfacesV4 is a slice of network interfaces of IPv4 address family.
+type netInterfacesV4 []*netInterfaceV4
+
+// find returns the first network interface within ifaces containing ip.  It
+// returns false if there is no such interface.
+func (ifaces netInterfacesV4) find(ip netip.Addr) (iface4 *netInterface, ok bool) {
+	i := slices.IndexFunc(ifaces, func(iface *netInterfaceV4) (contains bool) {
+		return iface.subnet.Contains(ip)
+	})
+	if i < 0 {
+		return nil, false
+	}
+
+	return &ifaces[i].netInterface, true
 }
 
 // options returns the implicit and explicit options for the interface.  The two
@@ -69,14 +179,14 @@ func (conf *IPv4Config) validate() (err error) {
 // values.
 //
 // TODO(e.burkov):  DRY with the IPv6 version.
-func (conf *IPv4Config) options() (implicit, explicit layers.DHCPOptions) {
+func (c *IPv4Config) options(ctx context.Context, l *slog.Logger) (imp, exp layers.DHCPOptions) {
 	// Set default values of host configuration parameters listed in Appendix A
 	// of RFC-2131.
-	implicit = layers.DHCPOptions{
+	imp = layers.DHCPOptions{
 		// Values From Configuration
 
-		layers.NewDHCPOption(layers.DHCPOptSubnetMask, conf.SubnetMask.AsSlice()),
+		layers.NewDHCPOption(layers.DHCPOptSubnetMask, c.SubnetMask.AsSlice()),
-		layers.NewDHCPOption(layers.DHCPOptRouter, conf.GatewayIP.AsSlice()),
+		layers.NewDHCPOption(layers.DHCPOptRouter, c.GatewayIP.AsSlice()),
 
 		// IP-Layer Per Host
 
@@ -228,110 +338,29 @@ func (conf *IPv4Config) options() (implicit, explicit layers.DHCPOptions) {
 		// See https://datatracker.ietf.org/doc/html/rfc1122#section-4.2.3.6.
 		layers.NewDHCPOption(layers.DHCPOptTCPKeepAliveGarbage, []byte{0x1}),
 	}
-	slices.SortFunc(implicit, compareV4OptionCodes)
+	slices.SortFunc(imp, compareV4OptionCodes)
 
 	// Set values for explicitly configured options.
-	for _, exp := range conf.Options {
+	for _, o := range c.Options {
-		i, found := slices.BinarySearchFunc(implicit, exp, compareV4OptionCodes)
+		i, found := slices.BinarySearchFunc(imp, o, compareV4OptionCodes)
 		if found {
-			implicit = slices.Delete(implicit, i, i+1)
+			imp = slices.Delete(imp, i, i+1)
 		}
 
-		i, found = slices.BinarySearchFunc(explicit, exp, compareV4OptionCodes)
+		i, found = slices.BinarySearchFunc(exp, o, compareV4OptionCodes)
-		if exp.Length > 0 {
+		if o.Length > 0 {
-			explicit = slices.Insert(explicit, i, exp)
+			exp = slices.Insert(exp, i, o)
 		} else if found {
-			explicit = slices.Delete(explicit, i, i+1)
+			exp = slices.Delete(exp, i, i+1)
 		}
 	}
 
-	log.Debug("dhcpsvc: v4: implicit options: %s", implicit)
+	l.DebugContext(ctx, "options", "implicit", imp, "explicit", exp)
-	log.Debug("dhcpsvc: v4: explicit options: %s", explicit)
 
-	return implicit, explicit
+	return imp, exp
 }
 
 // compareV4OptionCodes compares option codes of a and b.
 func compareV4OptionCodes(a, b layers.DHCPOption) (res int) {
 	return int(a.Type) - int(b.Type)
 }
-
-// netInterfaceV4 is a DHCP interface for IPv4 address family.
-type netInterfaceV4 struct {
-	// gateway is the IP address of the network gateway.
-	gateway netip.Addr
-
-	// subnet is the network subnet.
-	subnet netip.Prefix
-
-	// addrSpace is the IPv4 address space allocated for leasing.
-	addrSpace ipRange
-
-	// implicitOpts are the options listed in Appendix A of RFC 2131 and
-	// initialized with default values.  It must not have intersections with
-	// explicitOpts.
-	implicitOpts layers.DHCPOptions
-
-	// explicitOpts are the user-configured options.  It must not have
-	// intersections with implicitOpts.
-	explicitOpts layers.DHCPOptions
-
-	// netInterface is embedded here to provide some common network interface
-	// logic.
-	netInterface
-}
-
-// newNetInterfaceV4 creates a new DHCP interface for IPv4 address family with
-// the given configuration.  It returns an error if the given configuration
-// can't be used.
-func newNetInterfaceV4(name string, conf *IPv4Config) (i *netInterfaceV4, err error) {
-	if !conf.Enabled {
-		return nil, nil
-	}
-
-	maskLen, _ := net.IPMask(conf.SubnetMask.AsSlice()).Size()
-	subnet := netip.PrefixFrom(conf.GatewayIP, maskLen)
-
-	switch {
-	case !subnet.Contains(conf.RangeStart):
-		return nil, fmt.Errorf("range start %s is not within %s", conf.RangeStart, subnet)
-	case !subnet.Contains(conf.RangeEnd):
-		return nil, fmt.Errorf("range end %s is not within %s", conf.RangeEnd, subnet)
-	}
-
-	addrSpace, err := newIPRange(conf.RangeStart, conf.RangeEnd)
-	if err != nil {
-		return nil, err
-	} else if addrSpace.contains(conf.GatewayIP) {
-		return nil, fmt.Errorf("gateway ip %s in the ip range %s", conf.GatewayIP, addrSpace)
-	}
-
-	i = &netInterfaceV4{
-		gateway:   conf.GatewayIP,
-		subnet:    subnet,
-		addrSpace: addrSpace,
-		netInterface: netInterface{
-			name:     name,
-			leaseTTL: conf.LeaseDuration,
-		},
-	}
-	i.implicitOpts, i.explicitOpts = conf.options()
-
-	return i, nil
-}
-
-// netInterfacesV4 is a slice of network interfaces of IPv4 address family.
-type netInterfacesV4 []*netInterfaceV4
-
-// find returns the first network interface within ifaces containing ip.  It
-// returns false if there is no such interface.
-func (ifaces netInterfacesV4) find(ip netip.Addr) (iface4 *netInterface, ok bool) {
-	i := slices.IndexFunc(ifaces, func(iface *netInterfaceV4) (contains bool) {
-		return iface.subnet.Contains(ip)
-	})
-	if i < 0 {
-		return nil, false
-	}
-
-	return &ifaces[i].netInterface, true
-}

internal/dhcpsvc/v4_internal_test.go

@@ -3,7 +3,10 @@ package dhcpsvc
 import (
 	"net/netip"
 	"testing"
+	"time"
 
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
+	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/google/gopacket/layers"
 	"github.com/stretchr/testify/assert"
 )
@@ -75,9 +78,12 @@ func TestIPv4Config_Options(t *testing.T) {
 		wantExplicit: layers.DHCPOptions{opt1},
 	}}
 
+	ctx := testutil.ContextWithTimeout(t, time.Second)
+	l := slogutil.NewDiscardLogger()
+
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			imp, exp := tc.conf.options()
+			imp, exp := tc.conf.options(ctx, l)
 			assert.Equal(t, tc.wantExplicit, exp)
 
 			for c := range exp {

internal/dhcpsvc/v6.go

@@ -1,12 +1,14 @@
 package dhcpsvc
 
 import (
+	"context"
 	"fmt"
+	"log/slog"
 	"net/netip"
 	"slices"
 	"time"
 
-	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/google/gopacket/layers"
 )
@@ -38,50 +40,26 @@ type IPv6Config struct {
 }
 
 // validate returns an error in conf if any.
-func (conf *IPv6Config) validate() (err error) {
+func (c *IPv6Config) validate() (err error) {
-	switch {
+	if c == nil {
-	case conf == nil:
 		return errNilConfig
-	case !conf.Enabled:
+	} else if !c.Enabled {
-		return nil
-	case !conf.RangeStart.Is6():
-		return fmt.Errorf("range start %s should be a valid ipv6", conf.RangeStart)
-	case conf.LeaseDuration <= 0:
-		return fmt.Errorf("lease duration %s must be positive", conf.LeaseDuration)
-	default:
 		return nil
 	}
-}
 
-// options returns the implicit and explicit options for the interface.  The two
+	var errs []error
-// lists are disjoint and the implicit options are initialized with default
-// values.
-//
-// TODO(e.burkov):  Add implicit options according to RFC.
-func (conf *IPv6Config) options() (implicit, explicit layers.DHCPv6Options) {
-	// Set default values of host configuration parameters listed in RFC 8415.
-	implicit = layers.DHCPv6Options{}
-	slices.SortFunc(implicit, compareV6OptionCodes)
 
-	// Set values for explicitly configured options.
+	if !c.RangeStart.Is6() {
-	for _, exp := range conf.Options {
+		err = fmt.Errorf("range start %s should be a valid ipv6", c.RangeStart)
-		i, found := slices.BinarySearchFunc(implicit, exp, compareV6OptionCodes)
+		errs = append(errs, err)
-		if found {
-			implicit = slices.Delete(implicit, i, i+1)
-		}
-
-		explicit = append(explicit, exp)
 	}
 
-	log.Debug("dhcpsvc: v6: implicit options: %s", implicit)
+	if c.LeaseDuration <= 0 {
-	log.Debug("dhcpsvc: v6: explicit options: %s", explicit)
+		err = fmt.Errorf("lease duration %s must be positive", c.LeaseDuration)
+		errs = append(errs, err)
+	}
 
-	return implicit, explicit
+	return errors.Join(errs...)
-}
-
-// compareV6OptionCodes compares option codes of a and b.
-func compareV6OptionCodes(a, b layers.DHCPv6Option) (res int) {
-	return int(a.Code) - int(b.Code)
 }
 
 // netInterfaceV6 is a DHCP interface for IPv6 address family.
@@ -116,8 +94,16 @@ type netInterfaceV6 struct {
 // the given configuration.
 //
 // TODO(e.burkov):  Validate properly.
-func newNetInterfaceV6(name string, conf *IPv6Config) (i *netInterfaceV6) {
+func newNetInterfaceV6(
+	ctx context.Context,
+	l *slog.Logger,
+	name string,
+	conf *IPv6Config,
+) (i *netInterfaceV6) {
+	l = l.With(keyInterface, name, keyFamily, netutil.AddrFamilyIPv6)
 	if !conf.Enabled {
+		l.DebugContext(ctx, "disabled")
+
 		return nil
 	}
 
@@ -126,11 +112,12 @@ func newNetInterfaceV6(name string, conf *IPv6Config) (i *netInterfaceV6) {
 		netInterface: netInterface{
 			name:     name,
 			leaseTTL: conf.LeaseDuration,
+			logger:   l,
 		},
 		raSLAACOnly:  conf.RASLAACOnly,
 		raAllowSLAAC: conf.RAAllowSLAAC,
 	}
-	i.implicitOpts, i.explicitOpts = conf.options()
+	i.implicitOpts, i.explicitOpts = conf.options(ctx, l)
 
 	return i
 }
@@ -159,3 +146,33 @@ func (ifaces netInterfacesV6) find(ip netip.Addr) (iface6 *netInterface, ok bool
 
 	return &ifaces[i].netInterface, true
 }
+
+// options returns the implicit and explicit options for the interface.  The two
+// lists are disjoint and the implicit options are initialized with default
+// values.
+//
+// TODO(e.burkov):  Add implicit options according to RFC.
+func (c *IPv6Config) options(ctx context.Context, l *slog.Logger) (imp, exp layers.DHCPv6Options) {
+	// Set default values of host configuration parameters listed in RFC 8415.
+	imp = layers.DHCPv6Options{}
+	slices.SortFunc(imp, compareV6OptionCodes)
+
+	// Set values for explicitly configured options.
+	for _, e := range c.Options {
+		i, found := slices.BinarySearchFunc(imp, e, compareV6OptionCodes)
+		if found {
+			imp = slices.Delete(imp, i, i+1)
+		}
+
+		exp = append(exp, e)
+	}
+
+	l.DebugContext(ctx, "options", "implicit", imp, "explicit", exp)
+
+	return imp, exp
+}
+
+// compareV6OptionCodes compares option codes of a and b.
+func compareV6OptionCodes(a, b layers.DHCPv6Option) (res int) {
+	return int(a.Code) - int(b.Code)
+}