Skip to content

Commit 6507a36

Browse files
committed
datapath: Increase the devices controller's netlink receive buffer
The devices controller subscribes to address, route, link and neighbor updates on netlink sockets with the kernel default receive buffer. Under high churn that buffer overflows with ENOBUFS, and any one socket erroring restarts all four subscriptions. Size all four explicitly: 4MiB by default, configurable with the new --netlink-buffer-size flag. Signed-off-by: Jared Ledvina <jared.ledvina@datadoghq.com>
1 parent 67dfc5a commit 6507a36

7 files changed

Lines changed: 268 additions & 28 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: 108 additions & 26 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.NetlinkBufferSize, defaults.NetlinkBufferSize, "Size (in bytes) of the netlink socket receive buffer used by the devices controller's address, route, link and neighbor subscriptions; a larger buffer reduces the chance of ENOBUFS errors and subscription restarts under high churn")
6973
}
7074

7175
var (
@@ -93,6 +97,9 @@ type DevicesConfig struct {
9397
// ForceDeviceDetection forces the auto-detection of devices,
9498
// even if user-specific devices are explicitly listed.
9599
ForceDeviceDetection bool
100+
// NetlinkBufferSize is the receive buffer size in bytes requested for
101+
// the controller's netlink subscription sockets.
102+
NetlinkBufferSize int
96103
}
97104

98105
type devicesControllerParams struct {
@@ -141,7 +148,7 @@ func newDevicesController(lc cell.Lifecycle, p devicesControllerParams) (*device
141148
func (dc *devicesController) Start(startCtx cell.HookContext) error {
142149
if dc.params.NetlinkFuncs == nil {
143150
var err error
144-
dc.params.NetlinkFuncs, err = makeNetlinkFuncs()
151+
dc.params.NetlinkFuncs, err = makeNetlinkFuncs(dc.log, dc.params.Config.NetlinkBufferSize)
145152
if err != nil {
146153
return err
147154
}
@@ -741,9 +748,46 @@ type netlinkFuncs struct {
741748
NeighList func(linkIndex, family int) ([]netlink.Neigh, error)
742749
}
743750

751+
// receiveBufferSize normalizes negative sizes to 0: the netlink library skips
752+
// setsockopt only when the size is exactly 0, so a negative size would
753+
// otherwise reach setsockopt(SO_RCVBUF) and be silently clamped by the kernel.
754+
func receiveBufferSize(size int) (int, bool) {
755+
if size < 0 {
756+
size = 0
757+
}
758+
return size, size > 0
759+
}
760+
761+
// The EPERM latch is required: netlink orphans its socket when setsockopt
762+
// fails, so re-forcing the buffer on every restart would leak sockets. It is
763+
// shared by every subscription because CAP_NET_ADMIN is process-wide.
764+
func subscribeWithForcedBuffer[Options any](
765+
subscribe func(Options) error,
766+
options func(size int) Options,
767+
bufferSize int,
768+
forceFailed *atomic.Bool,
769+
log *slog.Logger,
770+
) error {
771+
if forceFailed.Load() {
772+
bufferSize = 0
773+
}
774+
775+
err := subscribe(options(bufferSize))
776+
if err == nil || bufferSize <= 0 || !errors.Is(err, unix.EPERM) {
777+
return err
778+
}
779+
780+
forceFailed.Store(true)
781+
log.Warn("Failed to force the netlink receive buffer (SO_RCVBUFFORCE requires CAP_NET_ADMIN); falling back to the kernel default. Netlink updates may be dropped under high churn.",
782+
logfields.Error, err,
783+
logfields.BufferSize, bufferSize,
784+
)
785+
return subscribe(options(0))
786+
}
787+
744788
// makeNetlinkFuncs returns a *netlinkFuncs containing netlink accessors to the
745789
// network namespace of the calling goroutine's OS thread.
746-
func makeNetlinkFuncs() (*netlinkFuncs, error) {
790+
func makeNetlinkFuncs(log *slog.Logger, netlinkBufferSize int) (*netlinkFuncs, error) {
747791
netlinkHandle, err := safenetlink.NewHandle(&safenetlink.HandleConfig{NLFamilies: []int{unix.NETLINK_ROUTE}})
748792
if err != nil {
749793
return nil, fmt.Errorf("creating netlink handle: %w", err)
@@ -754,42 +798,80 @@ func makeNetlinkFuncs() (*netlinkFuncs, error) {
754798
return nil, fmt.Errorf("getting current netns: %w", err)
755799
}
756800

801+
var forceBufferFailed atomic.Bool
802+
757803
return &netlinkFuncs{
758804
RouteSubscribe: func(ch chan<- netlink.RouteUpdate, done <-chan struct{}, errorCallback func(error)) error {
759805
h := vns.NsHandle(cur.FD())
760-
return safenetlink.RouteSubscribeWithOptions(ch, done,
761-
netlink.RouteSubscribeOptions{
762-
ListExisting: false,
763-
ErrorCallback: errorCallback,
764-
Namespace: &h,
765-
})
806+
return subscribeWithForcedBuffer(
807+
func(opts netlink.RouteSubscribeOptions) error {
808+
return safenetlink.RouteSubscribeWithOptions(ch, done, opts)
809+
},
810+
func(size int) netlink.RouteSubscribeOptions {
811+
size, force := receiveBufferSize(size)
812+
return netlink.RouteSubscribeOptions{
813+
ListExisting: false,
814+
ErrorCallback: errorCallback,
815+
Namespace: &h,
816+
ReceiveBufferSize: size,
817+
ReceiveBufferForceSize: force,
818+
}
819+
},
820+
netlinkBufferSize, &forceBufferFailed, log)
766821
},
767822
AddrSubscribe: func(ch chan<- netlink.AddrUpdate, done <-chan struct{}, errorCallback func(error)) error {
768823
h := vns.NsHandle(cur.FD())
769-
return netlink.AddrSubscribeWithOptions(ch, done,
770-
netlink.AddrSubscribeOptions{
771-
ListExisting: false,
772-
ErrorCallback: errorCallback,
773-
Namespace: &h,
774-
})
824+
return subscribeWithForcedBuffer(
825+
func(opts netlink.AddrSubscribeOptions) error {
826+
return netlink.AddrSubscribeWithOptions(ch, done, opts)
827+
},
828+
func(size int) netlink.AddrSubscribeOptions {
829+
size, force := receiveBufferSize(size)
830+
return netlink.AddrSubscribeOptions{
831+
ListExisting: false,
832+
ErrorCallback: errorCallback,
833+
Namespace: &h,
834+
ReceiveBufferSize: size,
835+
ReceiveBufferForceSize: force,
836+
}
837+
},
838+
netlinkBufferSize, &forceBufferFailed, log)
775839
},
776840
LinkSubscribe: func(ch chan<- netlink.LinkUpdate, done <-chan struct{}, errorCallback func(error)) error {
777841
h := vns.NsHandle(cur.FD())
778-
return safenetlink.LinkSubscribeWithOptions(ch, done,
779-
netlink.LinkSubscribeOptions{
780-
ListExisting: false,
781-
ErrorCallback: errorCallback,
782-
Namespace: &h,
783-
})
842+
return subscribeWithForcedBuffer(
843+
func(opts netlink.LinkSubscribeOptions) error {
844+
return safenetlink.LinkSubscribeWithOptions(ch, done, opts)
845+
},
846+
func(size int) netlink.LinkSubscribeOptions {
847+
size, force := receiveBufferSize(size)
848+
return netlink.LinkSubscribeOptions{
849+
ListExisting: false,
850+
ErrorCallback: errorCallback,
851+
Namespace: &h,
852+
ReceiveBufferSize: size,
853+
ReceiveBufferForceSize: force,
854+
}
855+
},
856+
netlinkBufferSize, &forceBufferFailed, log)
784857
},
785858
NeighSubscribe: func(ch chan<- netlink.NeighUpdate, done <-chan struct{}, errorCallback func(error)) error {
786859
h := vns.NsHandle(cur.FD())
787-
return safenetlink.NeighSubscribeWithOptions(ch, done,
788-
netlink.NeighSubscribeOptions{
789-
ListExisting: false,
790-
ErrorCallback: errorCallback,
791-
Namespace: &h,
792-
})
860+
return subscribeWithForcedBuffer(
861+
func(opts netlink.NeighSubscribeOptions) error {
862+
return safenetlink.NeighSubscribeWithOptions(ch, done, opts)
863+
},
864+
func(size int) netlink.NeighSubscribeOptions {
865+
size, force := receiveBufferSize(size)
866+
return netlink.NeighSubscribeOptions{
867+
ListExisting: false,
868+
ErrorCallback: errorCallback,
869+
Namespace: &h,
870+
ReceiveBufferSize: size,
871+
ReceiveBufferForceSize: force,
872+
}
873+
},
874+
netlinkBufferSize, &forceBufferFailed, log)
793875
},
794876
LinkList: func() ([]netlink.Link, error) {
795877
return safenetlink.WithRetryResult(func() ([]netlink.Link, error) {

pkg/datapath/linux/devices_controller_test.go

Lines changed: 150 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.NetlinkBufferSize)
6870
}),
6971
)
7072

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

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

0 commit comments

Comments
 (0)