Skip to content

Commit 04fcb95

Browse files
committed
[datapath] Increase netlink receive buffer for neighbor subscription
The devices controller subscribes to neighbor updates on a netlink socket with the kernel default receive buffer, which overflows with ENOBUFS under high neighbor churn and restarts every one of the controller's netlink subscriptions. Size that buffer explicitly: 4MiB by default, configurable with the new --neighbor-netlink-buffer-size flag. Signed-off-by: Jared Ledvina <jared.ledvina@datadoghq.com> (cherry picked from commit 500b455)
1 parent e98edf6 commit 04fcb95

7 files changed

Lines changed: 217 additions & 11 deletions

File tree

Documentation/cmdref/cilium-agent.md

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Documentation/cmdref/cilium-agent_hive.md

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Documentation/cmdref/cilium-agent_hive_dot-graph.md

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/datapath/linux/devices_controller.go

Lines changed: 56 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ package linux
77

88
import (
99
"context"
10+
"errors"
1011
"fmt"
1112
"log/slog"
1213
"net"
1314
"net/netip"
1415
"slices"
1516
"strings"
17+
"sync/atomic"
1618

1719
"github.com/cilium/hive/cell"
1820
"github.com/cilium/statedb"
@@ -66,6 +68,8 @@ func (c DevicesConfig) Flags(flags *pflag.FlagSet) {
6668
flags.StringSlice(option.Devices, []string{}, "List of devices facing cluster/external network (used for BPF NodePort, BPF masquerading and host firewall); supports '+' as wildcard in device name, e.g. 'eth+'; support '!' to exclude devices, e.g. '!eth+' excludes any device with prefix 'eth'. Note '!' says nothing about which ones to include. A device must match other criteria to be selected; The filters are matched in order and whatever matched first wins.")
6769

6870
flags.Bool(option.ForceDeviceDetection, false, "Forces the auto-detection of devices, even if specific devices are explicitly listed")
71+
72+
flags.Int(option.NeighborNetlinkBufferSize, defaults.NeighborNetlinkBufferSize, "Size (in bytes) of the netlink socket receive buffer used to subscribe to neighbor updates; a larger buffer reduces the chance of ENOBUFS errors and subscription restarts under high neighbor churn")
6973
}
7074

7175
var (
@@ -92,7 +96,8 @@ type DevicesConfig struct {
9296
Devices []string
9397
// ForceDeviceDetection forces the auto-detection of devices,
9498
// even if user-specific devices are explicitly listed.
95-
ForceDeviceDetection bool
99+
ForceDeviceDetection bool
100+
NeighborNetlinkBufferSize int
96101
}
97102

98103
type devicesControllerParams struct {
@@ -141,7 +146,7 @@ func newDevicesController(lc cell.Lifecycle, p devicesControllerParams) (*device
141146
func (dc *devicesController) Start(startCtx cell.HookContext) error {
142147
if dc.params.NetlinkFuncs == nil {
143148
var err error
144-
dc.params.NetlinkFuncs, err = makeNetlinkFuncs()
149+
dc.params.NetlinkFuncs, err = makeNetlinkFuncs(dc.log, dc.params.Config.NeighborNetlinkBufferSize)
145150
if err != nil {
146151
return err
147152
}
@@ -715,9 +720,50 @@ type netlinkFuncs struct {
715720
NeighList func(linkIndex, family int) ([]netlink.Neigh, error)
716721
}
717722

723+
func neighSubscribeOptions(receiveBufferSize int, ns *vns.NsHandle, errorCallback func(error)) netlink.NeighSubscribeOptions {
724+
if receiveBufferSize < 0 {
725+
receiveBufferSize = 0
726+
}
727+
return netlink.NeighSubscribeOptions{
728+
ListExisting: false,
729+
ErrorCallback: errorCallback,
730+
Namespace: ns,
731+
ReceiveBufferSize: receiveBufferSize,
732+
ReceiveBufferForceSize: receiveBufferSize > 0,
733+
}
734+
}
735+
736+
// The EPERM latch is required: netlink orphans its socket when setsockopt
737+
// fails, so re-forcing the buffer on every restart would leak sockets.
738+
func subscribeNeighWithBufferFallback(
739+
subscribe func(netlink.NeighSubscribeOptions) error,
740+
bufferSize int,
741+
ns *vns.NsHandle,
742+
errorCallback func(error),
743+
forceFailed *atomic.Bool,
744+
log *slog.Logger,
745+
) error {
746+
forceBufferSize := bufferSize
747+
if forceFailed.Load() {
748+
forceBufferSize = 0
749+
}
750+
751+
err := subscribe(neighSubscribeOptions(forceBufferSize, ns, errorCallback))
752+
if err == nil || forceBufferSize <= 0 || !errors.Is(err, unix.EPERM) {
753+
return err
754+
}
755+
756+
forceFailed.Store(true)
757+
log.Warn("Failed to force the netlink receive buffer for neighbor updates (SO_RCVBUFFORCE requires CAP_NET_ADMIN); falling back to the kernel default. Neighbor updates may be dropped under high neighbor churn.",
758+
logfields.Error, err,
759+
logfields.BufferSize, bufferSize,
760+
)
761+
return subscribe(neighSubscribeOptions(0, ns, errorCallback))
762+
}
763+
718764
// makeNetlinkFuncs returns a *netlinkFuncs containing netlink accessors to the
719765
// network namespace of the calling goroutine's OS thread.
720-
func makeNetlinkFuncs() (*netlinkFuncs, error) {
766+
func makeNetlinkFuncs(log *slog.Logger, neighborReceiveBufferSize int) (*netlinkFuncs, error) {
721767
netlinkHandle, err := safenetlink.NewHandle(&safenetlink.HandleConfig{NLFamilies: []int{unix.NETLINK_ROUTE}})
722768
if err != nil {
723769
return nil, fmt.Errorf("creating netlink handle: %w", err)
@@ -728,6 +774,8 @@ func makeNetlinkFuncs() (*netlinkFuncs, error) {
728774
return nil, fmt.Errorf("getting current netns: %w", err)
729775
}
730776

777+
var neighForceBufferFailed atomic.Bool
778+
731779
return &netlinkFuncs{
732780
RouteSubscribe: func(ch chan<- netlink.RouteUpdate, done <-chan struct{}, errorCallback func(error)) error {
733781
h := vns.NsHandle(cur.FD())
@@ -758,12 +806,11 @@ func makeNetlinkFuncs() (*netlinkFuncs, error) {
758806
},
759807
NeighSubscribe: func(ch chan<- netlink.NeighUpdate, done <-chan struct{}, errorCallback func(error)) error {
760808
h := vns.NsHandle(cur.FD())
761-
return safenetlink.NeighSubscribeWithOptions(ch, done,
762-
netlink.NeighSubscribeOptions{
763-
ListExisting: false,
764-
ErrorCallback: errorCallback,
765-
Namespace: &h,
766-
})
809+
return subscribeNeighWithBufferFallback(
810+
func(opts netlink.NeighSubscribeOptions) error {
811+
return safenetlink.NeighSubscribeWithOptions(ch, done, opts)
812+
},
813+
neighborReceiveBufferSize, &h, errorCallback, &neighForceBufferFailed, log)
767814
},
768815
LinkList: func() ([]netlink.Link, error) {
769816
return safenetlink.WithRetryResult(func() ([]netlink.Link, error) {

pkg/datapath/linux/devices_controller_test.go

Lines changed: 151 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ import (
3131
"golang.org/x/sys/unix"
3232

3333
"github.com/cilium/cilium/pkg/datapath/tables"
34+
"github.com/cilium/cilium/pkg/defaults"
3435
"github.com/cilium/cilium/pkg/hive"
36+
"github.com/cilium/cilium/pkg/option"
3537
"github.com/cilium/cilium/pkg/testutils"
3638
)
3739

@@ -62,9 +64,9 @@ func TestPrivilegedDevicesControllerScript(t *testing.T) {
6264

6365
h := hive.New(
6466
DevicesControllerCell,
65-
cell.Provide(func() (*netlinkFuncs, error) {
67+
cell.Provide(func(log *slog.Logger) (*netlinkFuncs, error) {
6668
// Provide the normal netlink interface, restricted to the test network namespace.
67-
return makeNetlinkFuncs()
69+
return makeNetlinkFuncs(log, defaults.NeighborNetlinkBufferSize)
6870
}),
6971
)
7072

@@ -97,6 +99,153 @@ func TestPrivilegedDevicesControllerScript(t *testing.T) {
9799
"testdata/device-*.txtar")
98100
}
99101

102+
func TestNeighSubscribeOptions(t *testing.T) {
103+
errorCallback := func(error) {}
104+
ns := netns.None()
105+
106+
t.Run("configured size is applied and forced", func(t *testing.T) {
107+
opts := neighSubscribeOptions(defaults.NeighborNetlinkBufferSize, &ns, errorCallback)
108+
assert.Equal(t, defaults.NeighborNetlinkBufferSize, opts.ReceiveBufferSize)
109+
assert.True(t, opts.ReceiveBufferForceSize, "buffer size should be forced past net.core.rmem_max")
110+
assert.Same(t, &ns, opts.Namespace)
111+
assert.False(t, opts.ListExisting)
112+
})
113+
114+
t.Run("zero size leaves kernel default in place", func(t *testing.T) {
115+
opts := neighSubscribeOptions(0, &ns, errorCallback)
116+
assert.Equal(t, 0, opts.ReceiveBufferSize)
117+
assert.False(t, opts.ReceiveBufferForceSize)
118+
})
119+
120+
t.Run("negative size is normalized to kernel default", func(t *testing.T) {
121+
opts := neighSubscribeOptions(-1, &ns, errorCallback)
122+
assert.Equal(t, 0, opts.ReceiveBufferSize)
123+
assert.False(t, opts.ReceiveBufferForceSize)
124+
})
125+
}
126+
127+
func TestSubscribeNeighWithBufferFallback(t *testing.T) {
128+
log := hivetest.Logger(t)
129+
ns := netns.None()
130+
cb := func(error) {}
131+
132+
type call struct {
133+
size int
134+
force bool
135+
}
136+
137+
newFake := func(errs ...error) (*[]call, func(netlink.NeighSubscribeOptions) error) {
138+
calls := &[]call{}
139+
i := 0
140+
return calls, func(opts netlink.NeighSubscribeOptions) error {
141+
*calls = append(*calls, call{opts.ReceiveBufferSize, opts.ReceiveBufferForceSize})
142+
var err error
143+
if i < len(errs) {
144+
err = errs[i]
145+
}
146+
i++
147+
return err
148+
}
149+
}
150+
151+
t.Run("success on forced attempt does not latch", func(t *testing.T) {
152+
var flag atomic.Bool
153+
calls, sub := newFake(nil)
154+
require.NoError(t, subscribeNeighWithBufferFallback(sub, 4096, &ns, cb, &flag, log))
155+
require.Len(t, *calls, 1)
156+
assert.Equal(t, call{4096, true}, (*calls)[0])
157+
assert.False(t, flag.Load())
158+
})
159+
160+
t.Run("EPERM falls back to kernel default and latches", func(t *testing.T) {
161+
var flag atomic.Bool
162+
calls, sub := newFake(unix.EPERM, nil)
163+
require.NoError(t, subscribeNeighWithBufferFallback(sub, 4096, &ns, cb, &flag, log))
164+
require.Len(t, *calls, 2)
165+
assert.Equal(t, call{4096, true}, (*calls)[0])
166+
assert.Equal(t, call{0, false}, (*calls)[1], "fallback must not force the buffer")
167+
assert.True(t, flag.Load(), "EPERM should latch so future restarts skip forcing")
168+
})
169+
170+
t.Run("latched flag skips forcing without falling back", func(t *testing.T) {
171+
var flag atomic.Bool
172+
flag.Store(true)
173+
calls, sub := newFake(nil)
174+
require.NoError(t, subscribeNeighWithBufferFallback(sub, 4096, &ns, cb, &flag, log))
175+
require.Len(t, *calls, 1)
176+
assert.Equal(t, call{0, false}, (*calls)[0])
177+
})
178+
179+
t.Run("non-EPERM error propagates without fallback or latch", func(t *testing.T) {
180+
var flag atomic.Bool
181+
wantErr := errors.New("socket setup failed")
182+
calls, sub := newFake(wantErr)
183+
err := subscribeNeighWithBufferFallback(sub, 4096, &ns, cb, &flag, log)
184+
require.ErrorIs(t, err, wantErr)
185+
require.Len(t, *calls, 1, "must not fall back on a non-EPERM error")
186+
assert.False(t, flag.Load(), "must not latch on a transient/unrelated error")
187+
})
188+
189+
t.Run("fallback error propagates and still latches", func(t *testing.T) {
190+
var flag atomic.Bool
191+
fallbackErr := errors.New("still broken")
192+
calls, sub := newFake(unix.EPERM, fallbackErr)
193+
err := subscribeNeighWithBufferFallback(sub, 4096, &ns, cb, &flag, log)
194+
require.ErrorIs(t, err, fallbackErr)
195+
require.Len(t, *calls, 2)
196+
assert.True(t, flag.Load())
197+
})
198+
199+
t.Run("latch persists across restarts: force once, then never again", func(t *testing.T) {
200+
var flag atomic.Bool
201+
calls, sub := newFake(unix.EPERM, nil, nil)
202+
203+
require.NoError(t, subscribeNeighWithBufferFallback(sub, 4096, &ns, cb, &flag, log))
204+
require.NoError(t, subscribeNeighWithBufferFallback(sub, 4096, &ns, cb, &flag, log))
205+
206+
require.Len(t, *calls, 3, "first call forces+falls back (2), second skips forcing (1)")
207+
assert.Equal(t, call{4096, true}, (*calls)[0])
208+
assert.Equal(t, call{0, false}, (*calls)[1])
209+
assert.Equal(t, call{0, false}, (*calls)[2], "second restart must not re-attempt forcing")
210+
})
211+
212+
t.Run("disabled buffer returns the error without forcing or latching", func(t *testing.T) {
213+
var flag atomic.Bool
214+
wantErr := unix.EPERM
215+
calls, sub := newFake(wantErr)
216+
err := subscribeNeighWithBufferFallback(sub, 0, &ns, cb, &flag, log)
217+
require.ErrorIs(t, err, wantErr)
218+
require.Len(t, *calls, 1)
219+
assert.Equal(t, call{0, false}, (*calls)[0])
220+
assert.False(t, flag.Load())
221+
})
222+
}
223+
224+
func TestNeighborNetlinkBufferSize_Config(t *testing.T) {
225+
newConfig := func(t *testing.T, args ...string) DevicesConfig {
226+
var got DevicesConfig
227+
h := hive.New(
228+
cell.Config(DevicesConfig{}),
229+
cell.Invoke(func(cfg DevicesConfig) { got = cfg }),
230+
)
231+
flags := pflag.NewFlagSet("", pflag.ContinueOnError)
232+
h.RegisterFlags(flags)
233+
require.NoError(t, flags.Parse(args))
234+
require.NoError(t, h.Populate(hivetest.Logger(t)))
235+
return got
236+
}
237+
238+
t.Run("default is applied", func(t *testing.T) {
239+
cfg := newConfig(t)
240+
assert.Equal(t, defaults.NeighborNetlinkBufferSize, cfg.NeighborNetlinkBufferSize)
241+
})
242+
243+
t.Run("flag overrides default", func(t *testing.T) {
244+
cfg := newConfig(t, "--"+option.NeighborNetlinkBufferSize+"=1024")
245+
assert.Equal(t, 1024, cfg.NeighborNetlinkBufferSize)
246+
})
247+
}
248+
100249
func TestDevicesController_Restarts(t *testing.T) {
101250
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
102251
defer cancel()

pkg/defaults/defaults.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ const (
2020
)
2121

2222
const (
23+
// NeighborNetlinkBufferSize is the default value for option.NeighborNetlinkBufferSize
24+
NeighborNetlinkBufferSize = 4 << 20 // 4MiB
25+
2326
// ClusterHealthPort is the default value for option.ClusterHealthPort
2427
ClusterHealthPort = 4240
2528

pkg/option/config.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ const (
112112
// Forces the auto-detection of devices, even if specific devices are explicitly listed
113113
ForceDeviceDetection = "force-device-detection"
114114

115+
// NeighborNetlinkBufferSize sets the size in bytes of the netlink socket
116+
// receive buffer used by the devices controller's neighbor subscription
117+
NeighborNetlinkBufferSize = "neighbor-netlink-buffer-size"
118+
115119
// DirectRoutingDevice is the name of a device used to connect nodes in
116120
// direct routing mode (only required by BPF NodePort)
117121
DirectRoutingDevice = "direct-routing-device"

0 commit comments

Comments
 (0)