diff --git a/go.mod b/go.mod index 6b313f00e70..801292455d1 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index acc6965edf3..e7b4c31be76 100644 --- a/go.sum +++ b/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= diff --git a/internal/dhcpsvc/dhcpsvc.go b/internal/dhcpsvc/dhcpsvc.go index 0445966dcf7..3020c0b9506 100644 --- a/internal/dhcpsvc/dhcpsvc.go +++ b/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,7 +79,7 @@ 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 } @@ -87,9 +87,6 @@ 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{} diff --git a/internal/dhcpsvc/dhcpsvc_test.go b/internal/dhcpsvc/dhcpsvc_test.go index 7d0cf912bd3..fa93c5cff7e 100644 --- a/internal/dhcpsvc/dhcpsvc_test.go +++ b/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) + } } diff --git a/internal/dhcpsvc/handle.go b/internal/dhcpsvc/handle.go index 9d793e6ef1b..ef20e4f498b 100644 --- a/internal/dhcpsvc/handle.go +++ b/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) diff --git a/internal/dhcpsvc/handler4.go b/internal/dhcpsvc/handler4.go index f3e239f4530..6ee40e90795 100644 --- a/internal/dhcpsvc/handler4.go +++ b/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 diff --git a/internal/dhcpsvc/handler4_test.go b/internal/dhcpsvc/handler4_test.go index f4b40154737..095c6cc5efc 100644 --- a/internal/dhcpsvc/handler4_test.go +++ b/internal/dhcpsvc/handler4_test.go @@ -4,7 +4,6 @@ import ( "cmp" "net" "net/netip" - "slices" "testing" "time" @@ -12,8 +11,8 @@ import ( "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 } diff --git a/internal/dhcpsvc/handler6.go b/internal/dhcpsvc/handler6.go index 4c12b2f08f4..8e5b8c54e57 100644 --- a/internal/dhcpsvc/handler6.go +++ b/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, } diff --git a/internal/dhcpsvc/handler6_test.go b/internal/dhcpsvc/handler6_test.go index a491f9c9b9a..08f496a05a3 100644 --- a/internal/dhcpsvc/handler6_test.go +++ b/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) diff --git a/internal/dhcpsvc/lease.go b/internal/dhcpsvc/lease.go index cb2d87ec421..6be20c8cc70 100644 --- a/internal/dhcpsvc/lease.go +++ b/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) +} diff --git a/internal/dhcpsvc/lease_internal_test.go b/internal/dhcpsvc/lease_internal_test.go index 50976013281..2fe51753624 100644 --- a/internal/dhcpsvc/lease_internal_test.go +++ b/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) -} diff --git a/internal/dhcpsvc/networkdevice.go b/internal/dhcpsvc/networkdevice.go index 4ba89c49f8c..b38ebdeac3f 100644 --- a/internal/dhcpsvc/networkdevice.go +++ b/internal/dhcpsvc/networkdevice.go @@ -8,8 +8,8 @@ import ( "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/validate" - "github.com/google/gopacket" - "github.com/google/gopacket/layers" + "github.com/gopacket/gopacket" + "github.com/gopacket/gopacket/layers" ) // NetworkDeviceConfig is the configuration for a network device. diff --git a/internal/dhcpsvc/networkdevice_test.go b/internal/dhcpsvc/networkdevice_test.go index cf5dd5f5593..b200e206fdb 100644 --- a/internal/dhcpsvc/networkdevice_test.go +++ b/internal/dhcpsvc/networkdevice_test.go @@ -5,13 +5,12 @@ import ( "io" "net" "net/netip" - "sync/atomic" "testing" "github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc" "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/require" ) @@ -89,21 +88,23 @@ func (nd *testNetworkDevice) LinkType() (lt layers.LinkType) { return nd.onLinkType() } -// newTestNetworkDeviceManager creates a network device manager for testing. It -// requires that device opened have a deviceName. The device itself has a link -// type [layers.LinkTypeEthernet] and a hardware address [testIfaceHWAddr]. -// Incoming packets are received from inCh and outgoing packets are sent to -// outCh. -func newTestNetworkDeviceManager( +// newTestNetworkDeviceAndManager creates a network device manager for testing +// and returns it along with the device it opens. It requires that device +// opened have [testIfaceName] name. The device itself has a link type +// [layers.LinkTypeEthernet] and a hardware address [testIfaceHWAddr]. Incoming +// packets are received from inCh and outgoing packets are sent to outCh. +func newTestNetworkDeviceAndManager( tb testing.TB, - deviceName string, addr netip.Addr, -) (ndMgr dhcpsvc.NetworkDeviceManager, inCh chan<- gopacket.Packet, outCh <-chan []byte) { +) ( + ndMgr *testNetworkDeviceManager, + dev *testNetworkDevice, + inCh chan<- gopacket.Packet, + outCh <-chan []byte, +) { tb.Helper() - isOpened := &atomic.Bool{} - - dev, inCh, outCh := newTestNetworkDevice(tb, addr, isOpened) + dev, inCh, outCh = newTestNetworkDevice(tb, addr) pt := testutil.NewPanicT(tb) @@ -111,8 +112,7 @@ func newTestNetworkDeviceManager( _ context.Context, conf *dhcpsvc.NetworkDeviceConfig, ) (nd dhcpsvc.NetworkDevice, err error) { - isOpened.Store(true) - require.Equal(pt, deviceName, conf.Name) + require.Equal(pt, testIfaceName, conf.Name) return dev, nil } @@ -121,7 +121,7 @@ func newTestNetworkDeviceManager( onOpen: onOpen, } - return ndMgr, inCh, outCh + return ndMgr, dev, inCh, outCh } // newTestNetworkDevice creates a network device for testing. It has a link @@ -131,8 +131,7 @@ func newTestNetworkDeviceManager( func newTestNetworkDevice( tb testing.TB, addr netip.Addr, - isOpened *atomic.Bool, -) (nd dhcpsvc.NetworkDevice, inCh chan<- gopacket.Packet, outCh <-chan []byte) { +) (nd *testNetworkDevice, inCh chan<- gopacket.Packet, outCh <-chan []byte) { tb.Helper() in := make(chan gopacket.Packet) @@ -142,8 +141,6 @@ func newTestNetworkDevice( onReadPacketData := func() (data []byte, ci gopacket.CaptureInfo, err error) { pkt, ok := testutil.RequireReceive(pt, in, testTimeout) - require.Equalf(pt, isOpened.Load(), ok, "unexpected receive: %v", pkt) - if !ok { return nil, gopacket.CaptureInfo{}, io.EOF } @@ -158,8 +155,8 @@ func newTestNetworkDevice( } onClose := func() (err error) { - isOpened.Store(false) close(in) + close(out) return nil } @@ -191,3 +188,40 @@ func newTestNetworkDevice( onWritePacketData: onWritePacketData, }, in, out } + +// newTestNetworkDeviceAndManager creates a network device manager for testing +// and returns it. It requires that device opened have [testIfaceName] name. +// The device itself has a link type [layers.LinkTypeEthernet] and a hardware +// address [testIfaceHWAddr]. Incoming packets are received from inCh and +// outgoing packets are sent to outCh. +func newTestNetworkDeviceManager( + tb testing.TB, + addr netip.Addr, +) (ndMgr *testNetworkDeviceManager, inCh chan<- gopacket.Packet, outCh <-chan []byte) { + tb.Helper() + + dev, inCh, outCh := newTestNetworkDevice(tb, addr) + + pt := testutil.NewPanicT(tb) + + onOpen := func( + _ context.Context, + conf *dhcpsvc.NetworkDeviceConfig, + ) (nd dhcpsvc.NetworkDevice, err error) { + require.Equal(pt, testIfaceName, conf.Name) + + return dev, nil + } + + ndMgr = &testNetworkDeviceManager{ + onOpen: onOpen, + } + + return ndMgr, inCh, outCh +} + +// unexpectedWritePacketData is a helper function that panics if called, used to +// ensure that no packet data is written to the network device in tests. +func unexpectedWritePacketData(data []byte) (_ error) { + panic(testutil.UnexpectedCall(data)) +} diff --git a/internal/dhcpsvc/options4.go b/internal/dhcpsvc/options4.go index 3945c3133b5..da6e0f7e16a 100644 --- a/internal/dhcpsvc/options4.go +++ b/internal/dhcpsvc/options4.go @@ -10,7 +10,7 @@ import ( "time" "github.com/AdguardTeam/golibs/netutil" - "github.com/google/gopacket/layers" + "github.com/gopacket/gopacket/layers" ) // msgTypeOptions4 contains preallocated DHCPv4 message type options mapped by diff --git a/internal/dhcpsvc/options4_internal_test.go b/internal/dhcpsvc/options4_internal_test.go index ca13641267a..7659f2a0d89 100644 --- a/internal/dhcpsvc/options4_internal_test.go +++ b/internal/dhcpsvc/options4_internal_test.go @@ -7,7 +7,7 @@ import ( "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/testutil" - "github.com/google/gopacket/layers" + "github.com/gopacket/gopacket/layers" "github.com/stretchr/testify/assert" ) diff --git a/internal/dhcpsvc/options4_test.go b/internal/dhcpsvc/options4_test.go index 233a5106520..e89858b9fc4 100644 --- a/internal/dhcpsvc/options4_test.go +++ b/internal/dhcpsvc/options4_test.go @@ -6,7 +6,7 @@ import ( "testing" "time" - "github.com/google/gopacket/layers" + "github.com/gopacket/gopacket/layers" ) // newOptHostname creates a DHCP hostname (12) option. diff --git a/internal/dhcpsvc/options6.go b/internal/dhcpsvc/options6.go index c7cafdf83a5..3ffe14e7ff6 100644 --- a/internal/dhcpsvc/options6.go +++ b/internal/dhcpsvc/options6.go @@ -10,7 +10,7 @@ import ( "time" "github.com/AdguardTeam/golibs/validate" - "github.com/google/gopacket/layers" + "github.com/gopacket/gopacket/layers" ) // iaNAMinLen is the minimum length of an IA_NA option data field, in bytes. diff --git a/internal/dhcpsvc/options6_test.go b/internal/dhcpsvc/options6_test.go index 011102df6d3..3b22dc1c126 100644 --- a/internal/dhcpsvc/options6_test.go +++ b/internal/dhcpsvc/options6_test.go @@ -8,7 +8,7 @@ import ( "time" "github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc" - "github.com/google/gopacket/layers" + "github.com/gopacket/gopacket/layers" ) // newOptIANA creates a DHCPv6 Identity Association for Non-temporary Address diff --git a/internal/dhcpsvc/server.go b/internal/dhcpsvc/server.go index 00dfb36462a..c9fcf1ed077 100644 --- a/internal/dhcpsvc/server.go +++ b/internal/dhcpsvc/server.go @@ -15,8 +15,8 @@ import ( "github.com/AdguardTeam/golibs/container" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/netutil" - "github.com/google/gopacket" - "github.com/google/gopacket/layers" + "github.com/gopacket/gopacket" + "github.com/gopacket/gopacket/layers" ) // DHCPServer is a DHCP server for both IPv4 and IPv6 address families. @@ -43,6 +43,10 @@ type DHCPServer struct { // hostnames. localTLD string + // serveWG is used to wait for all serving goroutines to finish in + // [DHCPServer.Shutdown]. + serveWG *sync.WaitGroup + // leasesMu protects the leases index as well as leases in the interfaces. leasesMu *sync.RWMutex @@ -80,6 +84,7 @@ func New(ctx context.Context, conf *Config) (srv *DHCPServer, err error) { logger: l, deviceManager: conf.NetworkDeviceManager, localTLD: conf.LocalDomainName, + serveWG: &sync.WaitGroup{}, leasesMu: &sync.RWMutex{}, leases: newLeaseIndex(conf.DBFilePath), icmpTimeout: conf.ICMPTimeout, @@ -136,9 +141,7 @@ func (srv *DHCPServer) newInterfaces( } // type check -// -// TODO(e.burkov): Uncomment when the [Interface] interface is implemented. -// var _ Interface = (*DHCPServer)(nil) +var _ Interface = (*DHCPServer)(nil) // Start implements the [Interface] interface for *DHCPServer. func (srv *DHCPServer) Start(ctx context.Context) (err error) { @@ -166,7 +169,7 @@ func (srv *DHCPServer) Start(ctx context.Context) (err error) { Value: netDev, }) - go srv.serveEther4(context.WithoutCancel(ctx), iface, netDev) + srv.serveWG.Go(func() { srv.serveEther4(context.WithoutCancel(ctx), iface, netDev) }) } for _, iface := range srv.interfaces6 { @@ -187,7 +190,7 @@ func (srv *DHCPServer) Start(ctx context.Context) (err error) { Value: netDev, }) - go srv.serveEther6(context.WithoutCancel(ctx), iface, netDev) + srv.serveWG.Go(func() { srv.serveEther6(context.WithoutCancel(ctx), iface, netDev) }) } return errors.Join(errs...) @@ -207,6 +210,9 @@ func (srv *DHCPServer) Shutdown(ctx context.Context) (err error) { } } + // TODO(e.burkov): Respect the context cancellation. + srv.serveWG.Wait() + return errors.Join(errs...) } @@ -217,6 +223,14 @@ func (srv *DHCPServer) Enabled() (ok bool) { // Leases implements the [Interface] interface for *DHCPServer. func (srv *DHCPServer) Leases() (leases []*Lease) { + leases = srv.cloneLeases() + + slices.SortStableFunc(leases, compareLeases) + + return leases +} + +func (srv *DHCPServer) cloneLeases() (leases []*Lease) { srv.leasesMu.RLock() defer srv.leasesMu.RUnlock() diff --git a/internal/dhcpsvc/v4.go b/internal/dhcpsvc/v4.go index 65833d399d1..fd37a896079 100644 --- a/internal/dhcpsvc/v4.go +++ b/internal/dhcpsvc/v4.go @@ -14,7 +14,7 @@ import ( "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/timeutil" "github.com/AdguardTeam/golibs/validate" - "github.com/google/gopacket/layers" + "github.com/gopacket/gopacket/layers" ) // Port numbers for DHCPv4. diff --git a/internal/dhcpsvc/v6.go b/internal/dhcpsvc/v6.go index 2113cf22ce4..6353b7ee56d 100644 --- a/internal/dhcpsvc/v6.go +++ b/internal/dhcpsvc/v6.go @@ -16,7 +16,7 @@ import ( "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/timeutil" "github.com/AdguardTeam/golibs/validate" - "github.com/google/gopacket/layers" + "github.com/gopacket/gopacket/layers" ) // Port numbers for DHCPv6.