Skip to content

Commit 2a311c2

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 2a311c2

7 files changed

Lines changed: 241 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: 96 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,36 @@ type netlinkFuncs struct {
741748
NeighList func(linkIndex, family int) ([]netlink.Neigh, error)
742749
}
743750

751+
// The EPERM latch is required: netlink orphans its socket when setsockopt
752+
// fails, so re-forcing the buffer on every restart would leak sockets. It is
753+
// shared by every subscription because CAP_NET_ADMIN is process-wide.
754+
func subscribeWithForcedBuffer[Options any](
755+
subscribe func(Options) error,
756+
options func(size int) Options,
757+
bufferSize int,
758+
forceFailed *atomic.Bool,
759+
log *slog.Logger,
760+
) error {
761+
if forceFailed.Load() {
762+
bufferSize = 0
763+
}
764+
765+
err := subscribe(options(bufferSize))
766+
if err == nil || bufferSize <= 0 || !errors.Is(err, unix.EPERM) {
767+
return err
768+
}
769+
770+
forceFailed.Store(true)
771+
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.",
772+
logfields.Error, err,
773+
logfields.BufferSize, bufferSize,
774+
)
775+
return subscribe(options(0))
776+
}
777+
744778
// makeNetlinkFuncs returns a *netlinkFuncs containing netlink accessors to the
745779
// network namespace of the calling goroutine's OS thread.
746-
func makeNetlinkFuncs() (*netlinkFuncs, error) {
780+
func makeNetlinkFuncs(log *slog.Logger, netlinkBufferSize int) (*netlinkFuncs, error) {
747781
netlinkHandle, err := safenetlink.NewHandle(&safenetlink.HandleConfig{NLFamilies: []int{unix.NETLINK_ROUTE}})
748782
if err != nil {
749783
return nil, fmt.Errorf("creating netlink handle: %w", err)
@@ -754,42 +788,78 @@ func makeNetlinkFuncs() (*netlinkFuncs, error) {
754788
return nil, fmt.Errorf("getting current netns: %w", err)
755789
}
756790

791+
netlinkBufferSize = max(netlinkBufferSize, 0)
792+
793+
var forceBufferFailed atomic.Bool
794+
757795
return &netlinkFuncs{
758796
RouteSubscribe: func(ch chan<- netlink.RouteUpdate, done <-chan struct{}, errorCallback func(error)) error {
759797
h := vns.NsHandle(cur.FD())
760-
return safenetlink.RouteSubscribeWithOptions(ch, done,
761-
netlink.RouteSubscribeOptions{
762-
ListExisting: false,
763-
ErrorCallback: errorCallback,
764-
Namespace: &h,
765-
})
798+
return subscribeWithForcedBuffer(
799+
func(opts netlink.RouteSubscribeOptions) error {
800+
return safenetlink.RouteSubscribeWithOptions(ch, done, opts)
801+
},
802+
func(size int) netlink.RouteSubscribeOptions {
803+
return netlink.RouteSubscribeOptions{
804+
ListExisting: false,
805+
ErrorCallback: errorCallback,
806+
Namespace: &h,
807+
ReceiveBufferSize: size,
808+
ReceiveBufferForceSize: size > 0,
809+
}
810+
},
811+
netlinkBufferSize, &forceBufferFailed, log)
766812
},
767813
AddrSubscribe: func(ch chan<- netlink.AddrUpdate, done <-chan struct{}, errorCallback func(error)) error {
768814
h := vns.NsHandle(cur.FD())
769-
return netlink.AddrSubscribeWithOptions(ch, done,
770-
netlink.AddrSubscribeOptions{
771-
ListExisting: false,
772-
ErrorCallback: errorCallback,
773-
Namespace: &h,
774-
})
815+
return subscribeWithForcedBuffer(
816+
func(opts netlink.AddrSubscribeOptions) error {
817+
return netlink.AddrSubscribeWithOptions(ch, done, opts)
818+
},
819+
func(size int) netlink.AddrSubscribeOptions {
820+
return netlink.AddrSubscribeOptions{
821+
ListExisting: false,
822+
ErrorCallback: errorCallback,
823+
Namespace: &h,
824+
ReceiveBufferSize: size,
825+
ReceiveBufferForceSize: size > 0,
826+
}
827+
},
828+
netlinkBufferSize, &forceBufferFailed, log)
775829
},
776830
LinkSubscribe: func(ch chan<- netlink.LinkUpdate, done <-chan struct{}, errorCallback func(error)) error {
777831
h := vns.NsHandle(cur.FD())
778-
return safenetlink.LinkSubscribeWithOptions(ch, done,
779-
netlink.LinkSubscribeOptions{
780-
ListExisting: false,
781-
ErrorCallback: errorCallback,
782-
Namespace: &h,
783-
})
832+
return subscribeWithForcedBuffer(
833+
func(opts netlink.LinkSubscribeOptions) error {
834+
return safenetlink.LinkSubscribeWithOptions(ch, done, opts)
835+
},
836+
func(size int) netlink.LinkSubscribeOptions {
837+
return netlink.LinkSubscribeOptions{
838+
ListExisting: false,
839+
ErrorCallback: errorCallback,
840+
Namespace: &h,
841+
ReceiveBufferSize: size,
842+
ReceiveBufferForceSize: size > 0,
843+
}
844+
},
845+
netlinkBufferSize, &forceBufferFailed, log)
784846
},
785847
NeighSubscribe: func(ch chan<- netlink.NeighUpdate, done <-chan struct{}, errorCallback func(error)) error {
786848
h := vns.NsHandle(cur.FD())
787-
return safenetlink.NeighSubscribeWithOptions(ch, done,
788-
netlink.NeighSubscribeOptions{
789-
ListExisting: false,
790-
ErrorCallback: errorCallback,
791-
Namespace: &h,
792-
})
849+
return subscribeWithForcedBuffer(
850+
func(opts netlink.NeighSubscribeOptions) error {
851+
return safenetlink.NeighSubscribeWithOptions(ch, done, opts)
852+
},
853+
func(size int) netlink.NeighSubscribeOptions {
854+
return netlink.NeighSubscribeOptions{
855+
ListExisting: false,
856+
ErrorCallback: errorCallback,
857+
Namespace: &h,
858+
ReceiveBufferSize: size,
859+
ReceiveBufferForceSize: size > 0,
860+
}
861+
},
862+
netlinkBufferSize, &forceBufferFailed, log)
793863
},
794864
LinkList: func() ([]netlink.Link, error) {
795865
return safenetlink.WithRetryResult(func() ([]netlink.Link, error) {

pkg/datapath/linux/devices_controller_test.go

Lines changed: 135 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,137 @@ func TestPrivilegedDevicesControllerScript(t *testing.T) {
9799
"testdata/device-*.txtar")
98100
}
99101

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