From a203ccb67ef274d3589b5794db26c41099d269ee Mon Sep 17 00:00:00 2001 From: Suraj Kumar Date: Wed, 2 Sep 2026 05:03:51 +0000 Subject: [PATCH 1/2] ateomnet: extract the forwarding sysctl write into a helper EnableIPv4Forwarding inlined the sysctl write: read, fast-path write, bind-remount /proc/sys read-write, write again. Extract that sequence into writeSysctlIfUnset so callers state intent ("ensure this sysctl reads 1") instead of the remount dance, and so the IPv6 forwarding write in the follow-up change is one more call rather than a copy. The extraction is behaviour-preserving: the same sysctl is read, the same fast-path write is attempted, and the same remount fallback runs when the write fails. Rename EnableIPv4Forwarding to EnableForwarding since the helper is family-agnostic and the next change enables both families from this function; the single call site in SetupActorNetwork is updated. Add unit tests for the helper's fast paths (already set, unset, zero) against temp files standing in for /proc/sys nodes. The privileged bind-remount path stays covered by the netns integration tests, which require root. --- internal/ateomnet/net.go | 31 +++++++--- internal/ateomnet/write_sysctl_test.go | 84 ++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 9 deletions(-) create mode 100644 internal/ateomnet/write_sysctl_test.go diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 91203a8e04..c3ef73af75 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -192,31 +192,44 @@ func PodIPv4() (net.IP, error) { return nil, fmt.Errorf("pod eth0 has no IPv4 address") } -// EnableIPv4Forwarding enables IPv4 forwarding in the current network namespace. -func EnableIPv4Forwarding() error { +// EnableForwarding enables IPv4 forwarding in the current network namespace. +func EnableForwarding() error { // Forwarding is required because actor packets now enter the worker pod via // the host-side veth and then leave through the pod's eth0. Without this, the // kernel would not route traffic between those interfaces even though both // live in the worker pod network namespace. - // - // Without privileged, the container runtime bind-mounts /proc/sys read-only. - // The worker holds CAP_SYS_ADMIN and uses no user namespace, so the ro flag - // is not locked: clear it, write the sysctl, restore ro. const path = "/proc/sys/net/ipv4/ip_forward" + if err := writeSysctlIfUnset(path); err != nil { + return fmt.Errorf("while enabling IPv4 forwarding in worker pod netns: %w", err) + } + return nil +} + +// writeSysctlIfUnset writes "1\n" to a sysctl path unless it already reads "1". +// If the path does not exist, it returns nil — the sysctl is simply unavailable, +// not an error. +func writeSysctlIfUnset(path string) error { if b, err := os.ReadFile(path); err == nil && len(b) > 0 && b[0] == '1' { return nil } if err := os.WriteFile(path, []byte("1\n"), 0o644); err == nil { return nil } + if _, err := os.Stat(path); os.IsNotExist(err) { + // Path absent (e.g. IPv6 disabled in kernel): nothing to enable. + return nil + } + // Without privileged, the container runtime bind-mounts /proc/sys read-only. + // The worker holds CAP_SYS_ADMIN and uses no user namespace, so the ro flag + // is not locked: clear it, write the sysctl, restore ro. if err := unix.Mount("none", "/proc/sys", "", unix.MS_BIND|unix.MS_REMOUNT, ""); err != nil { - return fmt.Errorf("while remounting /proc/sys read-write to enable IPv4 forwarding: %w", err) + return fmt.Errorf("while remounting /proc/sys read-write to enable forwarding: %w", err) } defer func() { _ = unix.Mount("none", "/proc/sys", "", unix.MS_BIND|unix.MS_REMOUNT|unix.MS_RDONLY, "") }() if err := os.WriteFile(path, []byte("1\n"), 0o644); err != nil { - return fmt.Errorf("while enabling IPv4 forwarding in worker pod netns: %w", err) + return fmt.Errorf("while writing %s: %w", path, err) } return nil } @@ -565,7 +578,7 @@ func SetupActorNetwork(ctx context.Context, cfg NetworkConfig) (retErr error) { return fmt.Errorf("while configuring actor veth in interior netns: %w", err) } - if err := EnableIPv4Forwarding(); err != nil { + if err := EnableForwarding(); err != nil { return err } if err := InstallActorNftablesRules(cfg.EgressRedirectPort); err != nil { diff --git a/internal/ateomnet/write_sysctl_test.go b/internal/ateomnet/write_sysctl_test.go new file mode 100644 index 0000000000..b4cb1db169 --- /dev/null +++ b/internal/ateomnet/write_sysctl_test.go @@ -0,0 +1,84 @@ +//go:build linux + +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ateomnet + +import ( + "os" + "path/filepath" + "testing" +) + +// TestWriteSysctlIfUnset verifies writeSysctlIfUnset's fast paths against a +// temp file standing in for a /proc/sys node: it must not rewrite a value +// that already reads "1", and it must write "1\n" when the value is missing +// or unset. The privileged bind-remount path is covered by the netns +// integration tests (withTestNetNS), which require root. +func TestWriteSysctlIfUnset(t *testing.T) { + dir := t.TempDir() + + t.Run("already_set", func(t *testing.T) { + p := filepath.Join(dir, "already") + // Sentinel content: if writeSysctlIfUnset rewrote the file, the value + // would change to "1\n" and this assertion would fail. Keeping the + // file larger than the helper's output makes a silent rewrite + // detectable. + if err := os.WriteFile(p, []byte("1 other-content\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := writeSysctlIfUnset(p); err != nil { + t.Fatalf("writeSysctlIfUnset: %v", err) + } + b, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + if string(b) != "1 other-content\n" { + t.Fatalf("already-set file was rewritten: %q", b) + } + }) + + t.Run("unset_written", func(t *testing.T) { + p := filepath.Join(dir, "unset") + if err := writeSysctlIfUnset(p); err != nil { + t.Fatalf("writeSysctlIfUnset: %v", err) + } + b, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + if len(b) < 1 || b[0] != '1' { + t.Fatalf("expected '1' written, got %q", b) + } + }) + + t.Run("zero_is_rewritten", func(t *testing.T) { + p := filepath.Join(dir, "zero") + if err := os.WriteFile(p, []byte("0\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := writeSysctlIfUnset(p); err != nil { + t.Fatalf("writeSysctlIfUnset: %v", err) + } + b, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + if len(b) < 1 || b[0] != '1' { + t.Fatalf("expected '1' written, got %q", b) + } + }) +} From 9925d0fc7d0c96920cec2240c162a1c5867b8086 Mon Sep 17 00:00:00 2001 From: Suraj Kumar Date: Wed, 2 Sep 2026 05:04:08 +0000 Subject: [PATCH 2/2] ateomnet: enable IPv6 forwarding in worker pod netns Actor packets that arrive on the host-side veth and leave via the pod's eth0 are IPv6 on dual-stack / IPv6-only clusters. Without net.ipv6.conf.all.forwarding the kernel drops every IPv6 packet in ip6_forward(), including the actor's DNS queries. conf.all.forwarding implies the per-interface default, so a single write covers the veth and eth0 whatever the ordering. Also cover writeSysctlIfUnset's missing-path branch: procfs cannot create a missing node, so production always reaches the os.Stat path (e.g. IPv6 sysctls on a kernel with IPv6 disabled). Point a subtest at a node under a directory that does not exist so the IsNotExist return executes instead of returning at the os.WriteFile fast path. --- internal/ateomnet/net.go | 19 ++++++++++++++++--- internal/ateomnet/write_sysctl_test.go | 15 +++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index c3ef73af75..e7ec2446dc 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -192,7 +192,10 @@ func PodIPv4() (net.IP, error) { return nil, fmt.Errorf("pod eth0 has no IPv4 address") } -// EnableForwarding enables IPv4 forwarding in the current network namespace. +// EnableForwarding enables IPv4 and IPv6 forwarding in the current network +// namespace, so actor traffic (including DNS queries on IPv6-capable clusters) +// is routed between the veth and eth0 instead of being dropped by ip_forward() +// or ip6_forward(). func EnableForwarding() error { // Forwarding is required because actor packets now enter the worker pod via // the host-side veth and then leave through the pod's eth0. Without this, the @@ -202,12 +205,22 @@ func EnableForwarding() error { if err := writeSysctlIfUnset(path); err != nil { return fmt.Errorf("while enabling IPv4 forwarding in worker pod netns: %w", err) } + // IPv6 forwarding: actor packets that arrive on the veth and leave via eth0 + // are IPv6 on dual-stack / IPv6-only clusters. Without + // net.ipv6.conf.all.forwarding the kernel drops every IPv6 packet in + // ip6_forward(), including the actor's DNS queries. conf.all.forwarding=1 + // also implies the per-interface default, so a single write covers the veth + // and eth0. + const v6path = "/proc/sys/net/ipv6/conf/all/forwarding" + if err := writeSysctlIfUnset(v6path); err != nil { + return fmt.Errorf("while enabling IPv6 forwarding in worker pod netns: %w", err) + } return nil } // writeSysctlIfUnset writes "1\n" to a sysctl path unless it already reads "1". -// If the path does not exist, it returns nil — the sysctl is simply unavailable, -// not an error. +// If the path does not exist (e.g. IPv6 sysctls on a kernel with IPv6 disabled), +// it returns nil — IPv6 forwarding is simply unavailable, not an error. func writeSysctlIfUnset(path string) error { if b, err := os.ReadFile(path); err == nil && len(b) > 0 && b[0] == '1' { return nil diff --git a/internal/ateomnet/write_sysctl_test.go b/internal/ateomnet/write_sysctl_test.go index b4cb1db169..d687932d8c 100644 --- a/internal/ateomnet/write_sysctl_test.go +++ b/internal/ateomnet/write_sysctl_test.go @@ -65,6 +65,21 @@ func TestWriteSysctlIfUnset(t *testing.T) { } }) + t.Run("missing_path_is_noop", func(t *testing.T) { + // A node under a directory that does not exist stands in for + // /proc/sys/net/ipv6/... on a kernel with IPv6 disabled. The other + // subtests' paths can be created, so they return at the os.WriteFile + // fast path; this is the only one that reaches the os.Stat branch, + // which is what procfs always does in production. + p := filepath.Join(dir, "no-such-dir", "forwarding") + if err := writeSysctlIfUnset(p); err != nil { + t.Fatalf("writeSysctlIfUnset on a missing path: %v", err) + } + if _, err := os.Stat(p); !os.IsNotExist(err) { + t.Fatalf("expected %s to stay absent, stat err = %v", p, err) + } + }) + t.Run("zero_is_rewritten", func(t *testing.T) { p := filepath.Join(dir, "zero") if err := os.WriteFile(p, []byte("0\n"), 0o644); err != nil {