[pull] master from AdguardTeam:master

go.mod

@@ -16,9 +16,11 @@ require (
 	// own code for that.  Perhaps, use gopacket.
 	github.com/go-ping/ping v1.2.0
 	github.com/google/go-cmp v0.7.0
+	// TODO(e.burkov): Remove this dependency along with the dhcpd package.
 	github.com/google/gopacket v1.1.19
 	github.com/google/renameio/v2 v2.0.2
 	github.com/google/uuid v1.6.0
+	github.com/gopacket/gopacket v1.6.1
 	github.com/insomniacslk/dhcp v0.0.0-20260407060928-11b94ed970f2
 	github.com/kardianos/service v1.2.4
 	github.com/mdlayher/ethernet v0.0.0-20220221185849-529eae5b6118

go.sum

@@ -91,6 +91,8 @@ github.com/gookit/assert v0.1.1 h1:lh3GcawXe/p+cU7ESTZ5Ui3Sm/x8JWpIis4/1aF0mY0=
 github.com/gookit/assert v0.1.1/go.mod h1:jS5bmIVQZTIwk42uXl4lyj4iaaxx32tqH16CFj0VX2E=
 github.com/gookit/color v1.6.0 h1:JjJXBTk1ETNyqyilJhkTXJYYigHG24TM9Xa2M1xAhRA=
 github.com/gookit/color v1.6.0/go.mod h1:9ACFc7/1IpHGBW8RwuDm/0YEnhg3dwwXpoMsmtyHfjs=
+github.com/gopacket/gopacket v1.6.1 h1:S19Ok/KVGDFNHVW2uCva5U0vZ+uHqiZQdxteL50v6Ak=
+github.com/gopacket/gopacket v1.6.1/go.mod h1:i3NaGaqfoWKAr1+g7qxEdWsmfT+MXuWkAe9+THv8LME=
 github.com/gordonklaus/ineffassign v0.2.0 h1:Uths4KnmwxNJNzq87fwQQDDnbNb7De00VOk9Nu0TySs=
 github.com/gordonklaus/ineffassign v0.2.0/go.mod h1:TIpymnagPSexySzs7F9FnO1XFTy8IT3a59vmZp5Y9Lw=
 github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg=

internal/dhcpsvc/dhcpsvc.go

@@ -8,7 +8,7 @@ import (
 	"net"
 	"net/netip"
 
-	"github.com/AdguardTeam/AdGuardHome/internal/next/agh"
+	"github.com/AdguardTeam/golibs/service"
 )
 
 const (
@@ -26,7 +26,7 @@ const (
 //
 // TODO(e.burkov):  Reconsider the requirements for the leases validity.
 type Interface interface {
-	agh.ServiceWithConfig[*Config]
+	service.Interface
 
 	// Enabled returns true if DHCP provides information about clients.
 	Enabled() (ok bool)
@@ -79,17 +79,14 @@ type Interface interface {
 type Empty struct{}
 
 // type check
-var _ agh.ServiceWithConfig[*Config] = Empty{}
+var _ service.Interface = Empty{}
 
 // Start implements the [Service] interface for Empty.
 func (Empty) Start(_ context.Context) (err error) { return nil }
 
 // Shutdown implements the [Service] interface for Empty.
 func (Empty) Shutdown(_ context.Context) (err error) { return nil }
 
-// Config implements the [ServiceWithConfig] interface for Empty.
-func (Empty) Config() (conf *Config) { return nil }
-
 // type check
 var _ Interface = Empty{}
 

internal/dhcpsvc/dhcpsvc_test.go

@@ -17,8 +17,9 @@ import (
 	"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/gopacket/gopacket"
+	"github.com/gopacket/gopacket/layers"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -296,17 +297,21 @@ func newTestPacket(
 	return gopacket.NewPacket(buf.Bytes(), first, gopacket.Default)
 }
 
-// assertNoResponse asserts that no response is received on the channel within
-// the timeout.
-//
-// TODO(e.burkov):  Improve the helper to not rely on timeout.
-func assertNoResponse(tb testing.TB, outCh <-chan []byte, timeout time.Duration) {
+// assertLeases asserts that the leases returned by srv are equal to orig if
+// wantChanged is false and not equal if wantChanged is true.  The assertion is
+// performed 10 times during half of [testTimeout].
+func assertLeases(tb testing.TB, orig []*dhcpsvc.Lease, srv dhcpsvc.Interface, wantChanged bool) {
 	tb.Helper()
 
-	var resp []byte
-	require.Panics(tb, func() {
-		resp, _ = testutil.RequireReceive(testutil.NewPanicT(tb), outCh, timeout)
-	})
+	cond := func() (ok bool) {
+		got := srv.Leases()
 
-	require.Nil(tb, resp)
+		return !assert.ObjectsAreEqual(orig, got)
+	}
+
+	if wantChanged {
+		assert.Eventually(tb, cond, testTimeout/2, testTimeout/20)
+	} else {
+		assert.Never(tb, cond, testTimeout/2, testTimeout/20)
+	}
 }

internal/dhcpsvc/handle.go

@@ -7,8 +7,8 @@ import (
 	"slices"
 
 	"github.com/AdguardTeam/golibs/logutil/slogutil"
-	"github.com/google/gopacket"
-	"github.com/google/gopacket/layers"
+	"github.com/gopacket/gopacket"
+	"github.com/gopacket/gopacket/layers"
 )
 
 // serveEther4 handles the incoming ethernet packets and dispatches them to the
@@ -37,8 +37,6 @@ func (srv *DHCPServer) serveEther4(ctx context.Context, iface *dhcpInterfaceV4,
 // appropriate handler.  It's used to run in a separate goroutine as it blocks
 // until packets channel is closed.  iface and nd must not be nil.  nd must have
 // at least a single address returned by its Addresses method.
-//
-//lint:ignore U1000 TODO(e.burkov): Use.
 func (srv *DHCPServer) serveEther6(ctx context.Context, iface *dhcpInterfaceV6, nd NetworkDevice) {
 	defer slogutil.RecoverAndLog(ctx, srv.logger)
 

internal/dhcpsvc/handler4.go

@@ -8,8 +8,8 @@ import (
 
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/logutil/slogutil"
-	"github.com/google/gopacket"
-	"github.com/google/gopacket/layers"
+	"github.com/gopacket/gopacket"
+	"github.com/gopacket/gopacket/layers"
 )
 
 // serveV4 handles the ethernet packet of IPv4 type.  iface must not be nil, fd

internal/dhcpsvc/handler4_test.go

@@ -4,16 +4,15 @@ import (
 	"cmp"
 	"net"
 	"net/netip"
-	"slices"
 	"testing"
 	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc"
 	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/AdguardTeam/golibs/testutil/servicetest"
-	"github.com/google/gopacket"
-	"github.com/google/gopacket/layers"
+	"github.com/gopacket/gopacket"
+	"github.com/gopacket/gopacket/layers"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -133,7 +132,7 @@ func TestDHCPServer_ServeEther4_discover(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, testIfaceName, testIfaceAddrV4)
+			ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, testIfaceAddrV4)
 			startTestDHCPServer(t, &dhcpsvc.Config{
 				Interfaces:           testIPv4InterfacesConf,
 				NetworkDeviceManager: ndMgr,
@@ -154,7 +153,7 @@ func TestDHCPServer_ServeEther4_discoverExpired(t *testing.T) {
 	pkt := newDHCPDISCOVER(t, testHWUnknown)
 	req := testutil.RequireTypeAssert[*layers.DHCPv4](t, pkt.Layer(layers.LayerTypeDHCPv4))
 
-	ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, testIfaceName, testIfaceAddrV4)
+	ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, testIfaceAddrV4)
 
 	startTestDHCPServer(t, &dhcpsvc.Config{
 		Interfaces:           testIPv4InterfacesConf,
@@ -205,7 +204,7 @@ func TestDHCPServer_ServeEther4_release(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			ndMgr, inCh, _ := newTestNetworkDeviceManager(t, testIfaceName, testIfaceAddrV4)
+			ndMgr, inCh, _ := newTestNetworkDeviceManager(t, testIfaceAddrV4)
 			srv := newTestDHCPServer(t, &dhcpsvc.Config{
 				Interfaces:           testIPv4InterfacesConf,
 				NetworkDeviceManager: ndMgr,
@@ -215,21 +214,10 @@ func TestDHCPServer_ServeEther4_release(t *testing.T) {
 			servicetest.RequireRun(t, srv, testTimeout)
 
 			leases := srv.Leases()
-			slices.SortStableFunc(leases, dhcpsvc.CompareLeases)
-			cond := func() (ok bool) {
-				got := srv.Leases()
-				slices.SortStableFunc(got, dhcpsvc.CompareLeases)
-
-				return !assert.ObjectsAreEqual(leases, got)
-			}
 
 			testutil.RequireSend(t, inCh, tc.req, testTimeout)
 
-			if tc.wantChange {
-				assert.Eventually(t, cond, testTimeout/2, testTimeout/20)
-			} else {
-				assert.Never(t, cond, testTimeout/2, testTimeout/20)
-			}
+			assertLeases(t, leases, srv, tc.wantChange)
 		})
 	}
 }
@@ -321,7 +309,7 @@ func TestDHCPServer_ServeEther4_requestSelecting(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, testIfaceName, testIfaceAddrV4)
+			ndMgr, dev, inCh, outCh := newTestNetworkDeviceAndManager(t, testIfaceAddrV4)
 			startTestDHCPServer(t, &dhcpsvc.Config{
 				Logger:               slogutil.NewDiscardLogger(),
 				Interfaces:           testIPv4InterfacesConf,
@@ -337,6 +325,10 @@ func TestDHCPServer_ServeEther4_requestSelecting(t *testing.T) {
 				require.True(t, ok)
 			}
 
+			if tc.wantOpts == nil {
+				dev.onWritePacketData = unexpectedWritePacketData
+			}
+
 			testutil.RequireSend(t, inCh, tc.request, testTimeout)
 
 			assertValidResponse4(t, dhcpv4FromPacket(t, tc.request), outCh, tc.wantOpts)
@@ -421,7 +413,11 @@ func TestDHCPServer_ServeEther4_requestInitReboot(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, testIfaceName, testIfaceAddrV4)
+			ndMgr, dev, inCh, outCh := newTestNetworkDeviceAndManager(t, testIfaceAddrV4)
+			if tc.wantOpts == nil {
+				dev.onWritePacketData = unexpectedWritePacketData
+			}
+
 			startTestDHCPServer(t, &dhcpsvc.Config{
 				Interfaces:           testIPv4InterfacesConf,
 				NetworkDeviceManager: ndMgr,
@@ -502,7 +498,7 @@ func TestDHCPServer_ServeEther4_requestRenewSuccess(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, testIfaceName, testIfaceAddrV4)
+			ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, testIfaceAddrV4)
 			startTestDHCPServer(t, &dhcpsvc.Config{
 				Interfaces:           testIPv4InterfacesConf,
 				NetworkDeviceManager: ndMgr,
@@ -559,7 +555,11 @@ func TestDHCPServer_ServeEther4_requestRenewFail(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, testIfaceName, testIfaceAddrV4)
+			ndMgr, dev, inCh, outCh := newTestNetworkDeviceAndManager(t, testIfaceAddrV4)
+			if tc.wantOpts == nil {
+				dev.onWritePacketData = unexpectedWritePacketData
+			}
+
 			startTestDHCPServer(t, &dhcpsvc.Config{
 				Interfaces:           testIPv4InterfacesConf,
 				NetworkDeviceManager: ndMgr,
@@ -609,7 +609,7 @@ func TestDHCPServer_ServeEther4_decline(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			ndMgr, inCh, _ := newTestNetworkDeviceManager(t, testIfaceName, testIfaceAddrV4)
+			ndMgr, inCh, _ := newTestNetworkDeviceManager(t, testIfaceAddrV4)
 			srv := newTestDHCPServer(t, &dhcpsvc.Config{
 				Interfaces:           testIPv4InterfacesConf,
 				NetworkDeviceManager: ndMgr,
@@ -619,21 +619,10 @@ func TestDHCPServer_ServeEther4_decline(t *testing.T) {
 			servicetest.RequireRun(t, srv, testTimeout)
 
 			leases := srv.Leases()
-			slices.SortStableFunc(leases, dhcpsvc.CompareLeases)
-			cond := func() (ok bool) {
-				got := srv.Leases()
-				slices.SortStableFunc(got, dhcpsvc.CompareLeases)
-
-				return !assert.ObjectsAreEqual(leases, got)
-			}
 
 			testutil.RequireSend(t, inCh, tc.req, testTimeout)
 
-			if tc.wantChange {
-				assert.Eventually(t, cond, testTimeout/2, testTimeout/20)
-			} else {
-				assert.Never(t, cond, testTimeout/2, testTimeout/20)
-			}
+			assertLeases(t, leases, srv, tc.wantChange)
 		})
 	}
 }
@@ -834,8 +823,8 @@ func requireEthernet(
 }
 
 // assertValidResponse4 asserts that recvCh eventually gets the response with
-// wantOpts for request.  If wantOpts is nil, asserts that no response is sent.
-// request and recvCh must not be nil.
+// wantOpts for request.  It does nothing if wantOpts is nil, which should be
+// used in case no response is expected.  request and recvCh must not be nil.
 func assertValidResponse4(
 	tb testing.TB,
 	request *layers.DHCPv4,
@@ -845,8 +834,6 @@ func assertValidResponse4(
 	tb.Helper()
 
 	if wantOpts == nil {
-		assertNoResponse(tb, recvCh, testTimeout/10)
-
 		return
 	}
 

internal/dhcpsvc/handler6.go

@@ -7,8 +7,8 @@ import (
 
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/logutil/slogutil"
-	"github.com/google/gopacket"
-	"github.com/google/gopacket/layers"
+	"github.com/gopacket/gopacket"
+	"github.com/gopacket/gopacket/layers"
 )
 
 // serveV6 handles the ethernet packet of IPv6 type. iface and pkt must not be
@@ -87,7 +87,7 @@ func (iface *dhcpInterfaceV6) handleSolicit(
 	lease, iaid := iface.allocateForSolicit(ctx, fd.ether.SrcMAC, req)
 
 	resp := &layers.DHCPv6{
-		MsgType:       layers.DHCPv6MsgTypeAdverstise,
+		MsgType:       layers.DHCPv6MsgTypeAdvertise,
 		TransactionID: req.TransactionID,
 	}
 

internal/dhcpsvc/handler6_test.go

@@ -9,8 +9,8 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc"
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/testutil"
-	"github.com/google/gopacket"
-	"github.com/google/gopacket/layers"
+	"github.com/gopacket/gopacket"
+	"github.com/gopacket/gopacket/layers"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -151,7 +151,7 @@ func TestDHCPServer_ServeEther6_solicit(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, testIfaceName, testIfaceAddrV6)
+			ndMgr, inCh, outCh := newTestNetworkDeviceManager(t, testIfaceAddrV6)
 			startTestDHCPServer(t, &dhcpsvc.Config{
 				Interfaces:           testIPv6InterfacesConf,
 				NetworkDeviceManager: ndMgr,
@@ -265,8 +265,9 @@ func newEthernetLayer(
 }
 
 // assertValidResponse6 asserts that the response received on recvCh is a valid
-// DHCPv6 response for the given request and contains the expected options.  If
-// wantOpts is nil, it asserts that no response is received.
+// DHCPv6 response for the given request and contains the expected options.  It
+// does nothing if wantOpts is nil, which should be used in case no response is
+// expected.  req and recvCh must not be nil.
 func assertValidResponse6(
 	tb testing.TB,
 	req *layers.DHCPv6,
@@ -276,8 +277,6 @@ func assertValidResponse6(
 	tb.Helper()
 
 	if wantOpts == nil {
-		assertNoResponse(tb, recvCh, testTimeout/10)
-
 		return
 	}
 
@@ -329,7 +328,7 @@ func assertValidDHCPv6(
 		if isRapidCommit {
 			assert.Equal(tb, layers.DHCPv6MsgTypeReply, resp.MsgType)
 		} else {
-			assert.Equal(tb, layers.DHCPv6MsgTypeAdverstise, resp.MsgType)
+			assert.Equal(tb, layers.DHCPv6MsgTypeAdvertise, resp.MsgType)
 		}
 	default:
 		tb.Errorf("request message type: %v: %s", errors.ErrUnexpectedValue, req.MsgType)

internal/dhcpsvc/lease.go

@@ -80,3 +80,9 @@ func (l *Lease) updateExpiry(clock timeutil.Clock, ttl time.Duration) {
 
 	l.Expiry = now.Add(ttl)
 }
+
+// compareLeases is a helper function that sorts a slice of leases according to
+// their IP.
+func compareLeases(a, b *Lease) (res int) {
+	return a.IP.Compare(b.IP)
+}

internal/dhcpsvc/lease_internal_test.go

@@ -3,9 +3,3 @@ package dhcpsvc
 // BlockedHardwareAddr is the hardware address used to mark a lease as blocked.
 // It's exported for testing purposes.
 var BlockedHardwareAddr = blockedHardwareAddr
-
-// CompareLeases is a helper function that sorts a slice of leases according to
-// their IP.
-func CompareLeases(a, b *Lease) (res int) {
-	return a.IP.Compare(b.IP)
-}