ateomnet: enable IPv6 forwarding in worker pod netns

[SURAJ KUMAR (krsnaSuraj)]

ateomnet: enable IPv6 forwarding in the worker pod netns

Fixes: #945

Problem

EnableForwarding (formerly EnableIPv4Forwarding) only wrote /proc/sys/net/ipv4/ip_forward. It had no
IPv6 counterpart, so net.ipv6.conf.all.forwarding stayed at 0 in the worker
pod network namespace. On dual-stack / IPv6-only clusters, every IPv6 packet
an actor sends — including its own DNS queries — is dropped by the kernel's
ip6_forward(), because the packet enters on the actor veth and must be
forwarded out through eth0.

Reproduced as TestActorEgress returning 504 on IPv6-only clusters (4/4 runs).

Fix

The helper is renamed to EnableForwarding since it now covers both address
families, and it writes /proc/sys/net/ipv6/conf/all/forwarding. conf.all.forwarding=1 sets the
forwarding state for all existing interfaces (and is the default for new
ones), so a single write covers both the veth and eth0.

The write is factored into a writeSysctlIfUnset helper that preserves the
original behavior:

• if the sysctl already reads 1, return early (no remount, no write);
• otherwise clear the read-only bind-mount on /proc/sys (the worker holds
  CAP_SYS_ADMIN and uses no user namespace), write 1\n, and restore ro.

Test

internal/ateomnet/write_sysctl_test.go covers the helper's fast paths
(already-set is untouched, unset/zero is written) with a temp file, no root
required. The privileged remount path is exercised by the existing netns
integration tests (withTestNetNS).

Compatibility

• IPv4-only clusters are unaffected (IPv4 path unchanged; writing the IPv6
  sysctl is a no-op where IPv6 is disabled or the file is absent — the helper
  returns nil on missing path).
• No change to the nftables rules; the existing
  InstallActorNftablesRules IPv4-only TODO remains accurate — this PR only
  restores the kernel forwarding path so IPv6 packets reach the pod's eth0.

Verification

• gofmt -l clean.
• go build ./internal/ateomnet/ passes.
• go test ./internal/ateomnet/ -run TestWriteSysctlIfUnset passes.
• Full go test ./internal/ateomnet/ passes.

Merge order

#1057 gives the actor veth an IPv6 address and a default route; without this
PR's forwarding write that route is a black hole. This PR must merge before
#1057. No current CI lane catches the ordering (default e2e clusters are
IPv4-only); tracked in #1094 (opt-in dual-stack kind e2e).

[SURAJ KUMAR (krsnaSuraj)]

Any chance of a review on this? It's now part of the IPv6 e2e chain — #1059 (the signal-only draft) rides on the worker-pod forwarding + writeSysctlIfUnset fix from this branch, and the hack/create-kind-cluster.sh head it depends on. Landing #979 lets #1059 drop it from its branch instead of carrying it.

CI is green (check-changes + CLA pass) and the branch is mergeable. Happy to adjust anything from review.

[Yuan Gao (ygao-g)]

Two asks before merge. The premise checks out first: writing net.ipv6.conf.all.forwarding does propagate to interfaces that already exist and to conf.default, and it does not purge a statically installed default route — so the single write covers the veth and eth0 whatever the ordering.

1. Commit 2's branch is never executed. A coverage run puts the os.Stat/IsNotExist lines at 0. unset_written looks like it covers the case, but its temp path can be created, so it returns at the os.WriteFile fast path. Procfs cannot create a missing node, so production always reaches os.Stat — the opposite of what the test asserts. Pointing a subtest at a path under a directory that does not exist closes it; with the one below, both lines go covered.

2. EnableIPv4Forwarding now enables both families. One call site, so the rename is contained.

Illustrative diff — builds, vet clean, passes

diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go
index a2a95017..1f0fbc92 100644
--- a/internal/ateomnet/net.go
+++ b/internal/ateomnet/net.go
@@ -192,11 +192,11 @@ func PodIPv4() (net.IP, error) {
 	return nil, fmt.Errorf("pod eth0 has no IPv4 address")
 }
 
-// EnableIPv4Forwarding enables IPv4 forwarding in the current network namespace.
-// It also enables IPv6 forwarding so actor IPv6 traffic (including DNS queries
-// on IPv6-capable clusters) is routed between the veth and eth0 instead of
-// being dropped by ip6_forward().
-func EnableIPv4Forwarding() error {
+// 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
 	// kernel would not route traffic between those interfaces even though both
@@ -595,7 +595,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
index b4cb1db1..d687932d 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 {

Rebased on current main here: go test ./... and the root-gated suite both pass.

[Yuan Gao (ygao-g)]

LGTM.

Confirmed by measurement rather than by reading: in a netns replica of the worker topology, actor IPv6 egress across the veth is 100% packet loss with net.ipv6.conf.all.forwarding=0 and 0% with it set. On a running dual-stack kind cluster the worker pod netns reads ipv6 all.forwarding=0 and ipv4 ip_forward=1, so this is the write that actually changes behaviour — the existing IPv4 one is already a no-op in a pod.

One sequencing note for whoever merges: #1057 gives the actor veth an IPv6 address and a default route, and without this PR that route is a black hole. Neither PR declares the dependency, and no current CI lane would catch it — the e2e clusters are IPv4-only by default, so the IPv6 path never runs.

[SURAJ KUMAR (krsnaSuraj)]

Both asks are addressed; the delta since your approval is exactly two commits:

• 63af4fe — EnableIPv4Forwarding renamed to EnableForwarding, doc comment updated, single call site in SetupActorNetwork adjusted. Matches your illustrative diff.
• c1809e4 — new missing_path_is_noop subtest pointing at a node under a nonexistent directory, so the os.Stat/IsNotExist branch is the one that executes, as procfs always does in production.

Local verification: go test -count=1 ./internal/ateomnet/ passes, and the coverage profile now shows the IsNotExist return (net.go:235-238) executed — previously 0.

On the sequencing note: agreed, the dependency should be declared. GitHub has no native PR-to-PR dependency, so the practical options are a merge-order note in both PR bodies — I'm adding one here stating #979 must merge before #1057; worth mirroring in #1057 — or a - [ ] task-list item if you prefer something merge automation can read. Longer term, the CI gap you flagged looks like what #1094 (opt-in dual-stack kind e2e) is aimed at.

CI is green on the updated head (check-changes + CLA). Happy to re-request review if you want a fresh look at the two commits; otherwise I'll take your approval as covering them.