cpupick: Keep single readers off tun.avoid_cpus without cpupick, warn and ignore bad lists

examples/config.yml

@@ -296,12 +296,12 @@ tun:
   #  - 4
 
   # Linux only. avoid_cpus lists CPU IDs the tun readers are never pinned to, exactly as listed (SMT siblings are not
-  # added for you). The default pin selection skips them, even with a single routine, and a cpu_affinity entry naming
-  # one is rejected like any other invalid entry. Other threads are not restricted, and the setting does nothing with
-  # pin_threads: false. Invalid IDs, or IDs covering every allowed CPU, fail startup. Not reloadable. To keep Nebula
-  # off the underlay NIC's receive IRQ CPUs, find them in /proc/interrupts and /proc/irq/<IRQ>/effective_affinity_list,
-  # add their SMT siblings from /sys/devices/system/cpu/cpu<N>/topology/thread_siblings_list if you want those kept
-  # clear too, and revisit the list when IRQ placement changes.
+  # added for you). The default pin selection skips them, and a cpu_affinity entry naming one is rejected like any
+  # other invalid entry. Other threads are not restricted, and the setting does nothing with pin_threads: false. An
+  # invalid list, or one covering every allowed CPU, is ignored with a warning. Not reloadable. To keep Nebula off the
+  # underlay NIC's receive IRQ CPUs, find them in /proc/interrupts and /proc/irq/<IRQ>/effective_affinity_list, add
+  # their SMT siblings from /sys/devices/system/cpu/cpu<N>/topology/thread_siblings_list if you want those kept clear
+  # too, and revisit the list when IRQ placement changes.
   #avoid_cpus: []
 
   # Route based MTU overrides, you have known vpn ip paths that can support larger MTUs you can increase/decrease them here

interface.go

@@ -60,6 +60,9 @@ type InterfaceConfig struct {
 	// shorter lists than `routines` cycle. Empty list keeps the default
 	// pin-to-(i % NumCPU) behavior. Only consulted when PinThreads is true.
 	CpuAffinity []int
+	// AvoidCPUs (tun.avoid_cpus) lists CPUs the default pin spread skips when
+	// CpuAffinity is empty. Only consulted when PinThreads is true.
+	AvoidCPUs []int
 	// PinThreads controls whether each TUN reader OS thread is pinned to a
 	// single CPU (via tun.pin_threads, default true). Pinning keeps each
 	// goroutine's sendmmsg on one XPS-selected NIC TX ring so per-flow
@@ -95,6 +98,8 @@ type Interface struct {
 	// Empty falls back to the default pin-to-(allowed CPU) behavior.
 	// Only consulted when pinThreads is true.
 	cpuAffinity []int
+	// avoidCPUs are removed from that fallback spread, unless they would empty it.
+	avoidCPUs []int
 	// pinThreads controls whether listenIn pins each TUN reader OS thread to
 	// a CPU at all (tun.pin_threads, default true). When false, threads are
 	// left free to migrate as on stock nebula.
@@ -247,6 +252,7 @@ func NewInterface(ctx context.Context, c *InterfaceConfig) (*Interface, error) {
 		connectionManager:     c.connectionManager,
 		conntrackCacheTimeout: c.ConntrackCacheTimeout,
 		cpuAffinity:           c.CpuAffinity,
+		avoidCPUs:             c.AvoidCPUs,
 		pinThreads:            c.PinThreads,
 		udpTxErrLimiter:       udpTxErrLimiter,
 		udpRelayTxErrLimiter:  udpRelayTxErrLimiter,
@@ -443,6 +449,8 @@ func (f *Interface) pinThisThread(i int) {
 		// Default: spread queues across the CPUs we're actually allowed to
 		// run on. Under a cpuset/taskset mask these aren't 0..NumCPU-1, so
 		// i % NumCPU would pick unrunnable IDs and every pin would fail.
+		// parseAvoidCPUs already dropped an avoid list that covers them all.
+		allowed = slices.DeleteFunc(allowed, func(c int) bool { return slices.Contains(f.avoidCPUs, c) })
 		cpu = allowed[i%len(allowed)]
 	} else {
 		cpu = i % runtime.NumCPU()

interface_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/slackhq/nebula/cert"
 	"github.com/slackhq/nebula/config"
 	"github.com/slackhq/nebula/test"
+	"github.com/slackhq/nebula/util"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -118,3 +119,22 @@ func TestReloadFirewall_NoChange(t *testing.T) {
 
 	assert.Same(t, fw, f.firewall, "firewall should not have been replaced")
 }
+
+// TestPinThisThreadFallbackSkipsAvoided covers the single reader, which never goes through cpupick: its fallback
+// spread over the allowed set must still skip tun.avoid_cpus.
+func TestPinThisThreadFallbackSkipsAvoided(t *testing.T) {
+	allowed, err := util.AllowedCPUs()
+	require.NoError(t, err)
+	if len(allowed) < 2 {
+		t.Skip("requires at least two allowed CPUs")
+	}
+	f := &Interface{l: test.NewLogger(), pinThreads: true, avoidCPUs: []int{allowed[0]}}
+	got := make(chan []int)
+	go func() {
+		// The pin leaves this goroutine locked to its thread, so the runtime discards the thread when it returns.
+		f.pinThisThread(0)
+		cpus, _ := util.AllowedCPUs()
+		got <- cpus
+	}()
+	assert.Equal(t, []int{allowed[1]}, <-got)
+}

main.go

@@ -243,10 +243,7 @@ func Main(c *config.C, configTest bool, buildVersion string, l *slog.Logger, dev
 	}
 
 	pinThreads := c.GetBool("tun.pin_threads", true)
-	avoidCPUs, err := parseAvoidCPUs(c)
-	if err != nil {
-		return nil, err
-	}
+	avoidCPUs := parseAvoidCPUs(c, l)
 	if len(avoidCPUs) > 0 && !pinThreads {
 		l.Warn("tun.avoid_cpus has no effect with tun.pin_threads disabled")
 	}
@@ -255,12 +252,10 @@ func Main(c *config.C, configTest bool, buildVersion string, l *slog.Logger, dev
 	//	return nil, fmt.Errorf("tun.avoid_cpus: %w", err)
 	//}
 	cpuAffinity := parseCpuAffinity(c, l, routines, avoidCPUs)
-	if pinThreads && (routines > 1 || len(avoidCPUs) > 0) && len(cpuAffinity) == 0 && !configTest {
+	if pinThreads && routines > 1 && len(cpuAffinity) == 0 && !configTest {
 		// The operator didn't choose pin CPUs, so pick a default set that
 		// prefers performance cores, skips tun.avoid_cpus and doesn't stack
-		// co-located instances onto allowed[0]. A single reader would
-		// otherwise just take allowed[0], so it only comes here to dodge
-		// tun.avoid_cpus.
+		// co-located instances onto allowed[0].
 
 		// key is used to seed the spreading of routines->cores.
 		// use PID if you want to ensure many different Nebulas in VMs or containers land on different cores
@@ -308,6 +303,7 @@ func Main(c *config.C, configTest bool, buildVersion string, l *slog.Logger, dev
 		punchy:                punchy,
 		ConntrackCacheTimeout: conntrackCacheTimeout,
 		CpuAffinity:           cpuAffinity,
+		AvoidCPUs:             avoidCPUs,
 		PinThreads:            pinThreads,
 		l:                     l,
 	}
@@ -437,30 +433,33 @@ func parseCpuAffinity(c *config.C, l *slog.Logger, routines int, avoid []int) []
 }
 
 // parseAvoidCPUs reads tun.avoid_cpus, a list of CPU IDs the tun readers must not be pinned to. IDs outside the
-// allowed set are accepted, since one config may serve differently shaped hosts. Unlike tun.cpu_affinity, a bad
-// entry, or a list covering every allowed CPU, fails startup rather than being ignored.
-func parseAvoidCPUs(c *config.C) ([]int, error) {
+// allowed set are accepted, since one config may serve differently shaped hosts. Like tun.cpu_affinity, an invalid
+// list is a warning and is ignored entirely, as is one covering every allowed CPU, which would leave nowhere to pin.
+func parseAvoidCPUs(c *config.C, l *slog.Logger) []int {
 	raw := c.Get("tun.avoid_cpus")
 	if raw == nil {
-		return nil, nil
+		return nil
 	}
 	values, ok := raw.([]any)
 	if !ok {
-		return nil, fmt.Errorf("tun.avoid_cpus must be a list of non-negative integers")
+		l.Warn("tun.avoid_cpus must be a list of integers; ignoring", "value", raw)
+		return nil
 	}
 	cpus := make([]int, 0, len(values))
-	for _, value := range values {
+	for i, value := range values {
 		cpu, ok := value.(int)
 		if !ok || cpu < 0 {
-			return nil, fmt.Errorf("tun.avoid_cpus must contain non-negative integers, got %v", value)
+			l.Warn("tun.avoid_cpus entry not a non-negative integer; ignoring avoid_cpus", "index", i, "value", value)
+			return nil
 		}
 		cpus = append(cpus, cpu)
 	}
 	if allowed, err := util.AllowedCPUs(); err == nil && len(allowed) > 0 &&
 		!slices.ContainsFunc(allowed, func(cpu int) bool { return !slices.Contains(cpus, cpu) }) {
-		return nil, fmt.Errorf("tun.avoid_cpus %v leaves none of the allowed CPUs %v", cpus, allowed)
+		l.Warn("tun.avoid_cpus covers every allowed CPU; ignoring avoid_cpus", "cpus", cpus, "allowed", allowed)
+		return nil
 	}
-	return cpus, nil
+	return cpus
 }
 
 func moduleVersion() string {

main_test.go

@@ -67,23 +67,15 @@ func TestParseAvoidCPUs(t *testing.T) {
 	for _, tc := range []struct {
 		value any
 		want  []int
-		valid bool
 	}{
-		{nil, nil, true}, {[]any{}, []int{}, true}, {[]any{1, 1 << 20}, []int{1, 1 << 20}, true},
-		{"auto", nil, false}, {"1,2", nil, false}, {[]any{-1}, nil, false}, {[]any{1.5}, nil, false},
-		{[]any{"2"}, nil, false},
+		{nil, nil}, {[]any{}, []int{}}, {[]any{1, 1 << 20}, []int{1, 1 << 20}},
+		{"auto", nil}, {"1,2", nil}, {[]any{-1}, nil}, {[]any{1.5}, nil}, {[]any{1, "2"}, nil},
 	} {
 		c := config.NewC(l)
 		if tc.value != nil {
 			c.Settings["tun"] = map[string]any{"avoid_cpus": tc.value}
 		}
-		got, err := parseAvoidCPUs(c)
-		if tc.valid {
-			require.NoError(t, err)
-			assert.Equal(t, tc.want, got)
-		} else {
-			require.Error(t, err, "%v", tc.value)
-		}
+		assert.Equal(t, tc.want, parseAvoidCPUs(c, l), "%v", tc.value)
 	}
 
 	// avoiding every allowed CPU leaves nowhere to pin; only checkable where the allowed set is known
@@ -94,8 +86,7 @@ func TestParseAvoidCPUs(t *testing.T) {
 		}
 		c := config.NewC(l)
 		c.Settings["tun"] = map[string]any{"avoid_cpus": all}
-		_, err := parseAvoidCPUs(c)
-		require.Error(t, err)
+		assert.Nil(t, parseAvoidCPUs(c, l))
 	}
 }