Skip to content

Commit 81dcf43

Browse files
committed
Pull request 2584: 4923-gopacket-dhcp-vol.20
Updates #4923. Squashed commit of the following: commit 6efeea0 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Fri Feb 13 16:09:10 2026 +0300 dhcpsvc: imp code commit 50486ed Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Thu Feb 12 17:56:34 2026 +0300 dhcpsvc: close devices
1 parent 6abc092 commit 81dcf43

3 files changed

Lines changed: 62 additions & 9 deletions

File tree

internal/dhcpsvc/networkdevice.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package dhcpsvc
22

33
import (
44
"context"
5+
"io"
56
"net/netip"
67

78
"github.com/AdguardTeam/golibs/errors"
@@ -28,10 +29,10 @@ func (conf *NetworkDeviceConfig) Validate() (err error) {
2829
}
2930

3031
// NetworkDeviceManager creates and manages network devices.
31-
//
32-
// TODO(e.burkov): Add device closing method.
3332
type NetworkDeviceManager interface {
3433
// Open opens a network device. conf must be valid.
34+
//
35+
// An attempt to open the same device multiple times may return an error.
3536
Open(ctx context.Context, conf *NetworkDeviceConfig) (dev NetworkDevice, err error)
3637
}
3738

@@ -59,6 +60,9 @@ func (EmptyNetworkDeviceManager) Open(
5960
type NetworkDevice interface {
6061
gopacket.PacketDataSource
6162

63+
// No methods of a device should be called after Close.
64+
io.Closer
65+
6266
// Addresses returns all IP addresses assigned to the device.
6367
Addresses() (ips []netip.Addr)
6468

@@ -82,6 +86,12 @@ func (EmptyNetworkDevice) ReadPacketData() (data []byte, ci gopacket.CaptureInfo
8286
return nil, gopacket.CaptureInfo{}, nil
8387
}
8488

89+
// Close implements the [io.Closer] interface for [EmptyNetworkDevice]. It
90+
// always returns nil.
91+
func (EmptyNetworkDevice) Close() (err error) {
92+
return nil
93+
}
94+
8595
// Addresses implements the [NetworkDevice] interface for [EmptyNetworkDevice].
8696
// It always returns nil.
8797
func (EmptyNetworkDevice) Addresses() (ips []netip.Addr) {

internal/dhcpsvc/networkdevice_test.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ package dhcpsvc_test
22

33
import (
44
"context"
5+
"io"
56
"net/netip"
7+
"sync/atomic"
68
"testing"
79

810
"github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc"
@@ -41,6 +43,7 @@ func (ndm *testNetworkDeviceManager) Open(
4143
// TODO(e.burkov): Move to aghtest.
4244
type testNetworkDevice struct {
4345
onReadPacketData func() (data []byte, ci gopacket.CaptureInfo, err error)
46+
onClose func() (err error)
4447
onAddresses func() (ips []netip.Addr)
4548
onLinkType func() (lt layers.LinkType)
4649
onWritePacketData func(data []byte) (err error)
@@ -55,6 +58,11 @@ func (nd *testNetworkDevice) ReadPacketData() (data []byte, ci gopacket.CaptureI
5558
return nd.onReadPacketData()
5659
}
5760

61+
// Close implements the [io.Closer] interface for *testNetworkDevice.
62+
func (nd *testNetworkDevice) Close() (err error) {
63+
return nd.onClose()
64+
}
65+
5866
// Addresses implements the [dhcpsvc.NetworkDevice] interface for
5967
// *testNetworkDevice.
6068
func (nd *testNetworkDevice) Addresses() (ips []netip.Addr) {
@@ -87,13 +95,19 @@ func newTestNetworkDeviceManager(
8795
inCh = make(chan gopacket.Packet)
8896
outCh = make(chan []byte)
8997

98+
isOpened := atomic.Bool{}
99+
90100
pt := testutil.PanicT{}
91101
addrs := []netip.Addr{addr}
92102

93103
dev := &testNetworkDevice{
94104
onReadPacketData: func() (data []byte, ci gopacket.CaptureInfo, err error) {
95105
pkt, ok := testutil.RequireReceive(pt, inCh, testTimeout)
96-
require.True(pt, ok)
106+
require.Equal(pt, isOpened.Load(), ok)
107+
108+
if !ok {
109+
return nil, gopacket.CaptureInfo{}, io.EOF
110+
}
97111

98112
data = pkt.Data()
99113
ci = gopacket.CaptureInfo{
@@ -103,6 +117,12 @@ func newTestNetworkDeviceManager(
103117

104118
return data, ci, nil
105119
},
120+
onClose: func() (err error) {
121+
isOpened.Store(false)
122+
close(inCh)
123+
124+
return nil
125+
},
106126
onAddresses: func() (ips []netip.Addr) {
107127
return addrs
108128
},
@@ -121,6 +141,7 @@ func newTestNetworkDeviceManager(
121141
_ context.Context,
122142
conf *dhcpsvc.NetworkDeviceConfig,
123143
) (nd dhcpsvc.NetworkDevice, err error) {
144+
isOpened.Store(true)
124145
require.Equal(pt, deviceName, conf.Name)
125146

126147
return dev, nil

internal/dhcpsvc/server.go

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"sync/atomic"
1313
"time"
1414

15+
"github.com/AdguardTeam/golibs/container"
1516
"github.com/AdguardTeam/golibs/errors"
1617
"github.com/AdguardTeam/golibs/netutil"
1718
)
@@ -30,6 +31,12 @@ type DHCPServer struct {
3031
// deviceManager is the manager of network devices.
3132
deviceManager NetworkDeviceManager
3233

34+
// devices are the network devices opened in [DHCPServer.Start], mapped to
35+
// their names. Those are closed in [DHCPServer.Shutdown].
36+
//
37+
// TODO(e.burkov): Consider storing those within interfaces.
38+
devices container.KeyValues[string, NetworkDevice]
39+
3340
// localTLD is the top-level domain name to use for resolving DHCP clients'
3441
// hostnames.
3542
localTLD string
@@ -137,17 +144,24 @@ func (srv *DHCPServer) Start(ctx context.Context) (err error) {
137144

138145
var errs []error
139146
for _, iface := range srv.interfaces4 {
140-
var nd NetworkDevice
141-
nd, err = srv.deviceManager.Open(ctx, &NetworkDeviceConfig{
142-
Name: iface.common.name,
147+
netDevName := iface.common.name
148+
149+
var netDev NetworkDevice
150+
netDev, err = srv.deviceManager.Open(ctx, &NetworkDeviceConfig{
151+
Name: netDevName,
143152
})
144153
if err != nil {
145154
errs = append(errs, err)
146155

147156
continue
148157
}
149158

150-
go srv.serveEther4(context.WithoutCancel(ctx), iface, nd)
159+
srv.devices = append(srv.devices, container.KeyValue[string, NetworkDevice]{
160+
Key: netDevName,
161+
Value: netDev,
162+
})
163+
164+
go srv.serveEther4(context.WithoutCancel(ctx), iface, netDev)
151165
}
152166

153167
// TODO(e.burkov): Serve EthernetTypeIPv6.
@@ -159,9 +173,17 @@ func (srv *DHCPServer) Start(ctx context.Context) (err error) {
159173
func (srv *DHCPServer) Shutdown(ctx context.Context) (err error) {
160174
srv.logger.DebugContext(ctx, "shutting down dhcp server")
161175

162-
// TODO(e.burkov): Close the packet source.
176+
var errs []error
177+
for _, kv := range srv.devices {
178+
netDevName, netDev := kv.Key, kv.Value
163179

164-
return nil
180+
err = netDev.Close()
181+
if err != nil {
182+
errs = append(errs, fmt.Errorf("closing device %q: %w", netDevName, err))
183+
}
184+
}
185+
186+
return errors.Join(errs...)
165187
}
166188

167189
// Enabled implements the [Interface] interface for *DHCPServer.

0 commit comments

Comments
 (0)