From 5f390d4845dfdf469e49109bb0801d757ea18adf Mon Sep 17 00:00:00 2001 From: David Bond Date: Thu, 28 May 2026 13:22:46 +0100 Subject: [PATCH] cmd/containerboot: clamp MSS to PMTU for proxy group pods (#19686) (#19900) Single-pod ingress/egress proxies already called ClampMSSToPMTU when setting up forwarding rules, but the proxy group (HA) code paths in egressservices.go and ingressservices.go did not. This caused TCP connections through proxy group pods to suffer from MSS/MTU mismatch issues in environments where path MTU discovery is not working. Add ClampMSSToPMTU calls in the egress sync loop (alongside the existing EnsureSNATForDst call) and in addDNATRuleForSvc (alongside the existing EnsureDNATRuleForSvc call), mirroring what the single-pod forwarding rules already do. Also add MSS clamping assertions to TestSyncIngressConfigs and track ClampMSSToPMTU calls in FakeNetfilterRunner. Fixes issue #19812 https://github.com/tailscale/tailscale/issues/19812. Tracking internal ticket TSS-86326. (cherry picked from commit 4b8115bb2c1c20688ec0aa3e6d1856983df741f0) Signed-off-by: Jay Tung Co-authored-by: dragondscv Co-authored-by: Jay Tung Co-authored-by: Claude Sonnet 4.6 --- cmd/containerboot/egressservices.go | 3 ++ cmd/containerboot/ingressservices.go | 8 ++++- cmd/containerboot/ingressservices_test.go | 42 +++++++++++++++++++++-- util/linuxfw/fake_netfilter.go | 12 ++++++- 4 files changed, 60 insertions(+), 5 deletions(-) diff --git a/cmd/containerboot/egressservices.go b/cmd/containerboot/egressservices.go index abde12523..79fb77c85 100644 --- a/cmd/containerboot/egressservices.go +++ b/cmd/containerboot/egressservices.go @@ -250,6 +250,9 @@ func (ep *egressProxy) syncEgressConfigs(cfgs egressservices.Configs, status *eg if err := ep.nfr.EnsureSNATForDst(local, t); err != nil { return nil, fmt.Errorf("error setting up SNAT rule: %w", err) } + if err := ep.nfr.ClampMSSToPMTU(tailscaleTunInterface, t); err != nil { + return nil, fmt.Errorf("error clamping MSS to PMTU: %w", err) + } } } // Update the status. Status will be written back to the state Secret by the caller. diff --git a/cmd/containerboot/ingressservices.go b/cmd/containerboot/ingressservices.go index d76bf86e0..d8ad01717 100644 --- a/cmd/containerboot/ingressservices.go +++ b/cmd/containerboot/ingressservices.go @@ -265,7 +265,13 @@ func ensureIngressRulesAdded(cfgs map[string]ingressservices.Config, nfr linuxfw func addDNATRuleForSvc(nfr linuxfw.NetfilterRunner, serviceName string, tsIP, clusterIP netip.Addr) error { log.Printf("adding DNAT rule for Tailscale Service %s with IP %s to Kubernetes Service IP %s", serviceName, tsIP, clusterIP) - return nfr.EnsureDNATRuleForSvc(serviceName, tsIP, clusterIP) + if err := nfr.EnsureDNATRuleForSvc(serviceName, tsIP, clusterIP); err != nil { + return err + } + if err := nfr.ClampMSSToPMTU(tailscaleTunInterface, clusterIP); err != nil { + return fmt.Errorf("error clamping MSS to PMTU: %w", err) + } + return nil } // ensureIngressRulesDeleted takes a map of Tailscale Services and rules and ensures that the firewall rules are deleted. diff --git a/cmd/containerboot/ingressservices_test.go b/cmd/containerboot/ingressservices_test.go index 46330103e..1643bb11c 100644 --- a/cmd/containerboot/ingressservices_test.go +++ b/cmd/containerboot/ingressservices_test.go @@ -7,6 +7,7 @@ import ( "net/netip" + "slices" "testing" "tailscale.com/kube/ingressservices" @@ -22,6 +23,7 @@ func TestSyncIngressConfigs(t *testing.T) { TailscaleServiceIP netip.Addr ClusterIP netip.Addr } + wantClampedAddrs []netip.Addr // cluster IPs that should have MSS clamping applied }{ { name: "add_new_rules_when_no_existing_config", @@ -35,6 +37,7 @@ func TestSyncIngressConfigs(t *testing.T) { }{ "svc:foo": makeWantService("100.64.0.1", "10.0.0.1"), }, + wantClampedAddrs: []netip.Addr{netip.MustParseAddr("10.0.0.1")}, }, { name: "add_multiple_services", @@ -52,6 +55,11 @@ func TestSyncIngressConfigs(t *testing.T) { "svc:bar": makeWantService("100.64.0.2", "10.0.0.2"), "svc:baz": makeWantService("100.64.0.3", "10.0.0.3"), }, + wantClampedAddrs: []netip.Addr{ + netip.MustParseAddr("10.0.0.1"), + netip.MustParseAddr("10.0.0.2"), + netip.MustParseAddr("10.0.0.3"), + }, }, { name: "add_both_ipv4_and_ipv6_rules", @@ -65,6 +73,10 @@ func TestSyncIngressConfigs(t *testing.T) { }{ "svc:foo": makeWantService("2001:db8::1", "2001:db8::2"), }, + wantClampedAddrs: []netip.Addr{ + netip.MustParseAddr("10.0.0.1"), + netip.MustParseAddr("2001:db8::2"), + }, }, { name: "add_ipv6_only_rules", @@ -78,6 +90,7 @@ func TestSyncIngressConfigs(t *testing.T) { }{ "svc:ipv6": makeWantService("2001:db8::10", "2001:db8::20"), }, + wantClampedAddrs: []netip.Addr{netip.MustParseAddr("2001:db8::20")}, }, { name: "delete_all_rules_when_config_removed", @@ -94,6 +107,7 @@ func TestSyncIngressConfigs(t *testing.T) { TailscaleServiceIP netip.Addr ClusterIP netip.Addr }{}, + wantClampedAddrs: nil, // no rules added, no clamping }, { name: "add_remove_modify", @@ -117,6 +131,10 @@ func TestSyncIngressConfigs(t *testing.T) { "svc:foo": makeWantService("100.64.0.1", "10.0.0.2"), "svc:new": makeWantService("100.64.0.4", "10.0.0.4"), }, + wantClampedAddrs: []netip.Addr{ + netip.MustParseAddr("10.0.0.2"), + netip.MustParseAddr("10.0.0.4"), + }, }, { name: "update_with_outdated_status", @@ -152,12 +170,17 @@ func TestSyncIngressConfigs(t *testing.T) { "svc:web-ipv6": makeWantService("2001:db8::10", "2001:db8::20"), "svc:api": makeWantService("100.64.0.20", "10.0.0.20"), }, + wantClampedAddrs: []netip.Addr{ + netip.MustParseAddr("10.0.0.10"), + netip.MustParseAddr("10.0.0.20"), + netip.MustParseAddr("2001:db8::20"), + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var nfr linuxfw.NetfilterRunner = linuxfw.NewFakeNetfilterRunner() + nfr := linuxfw.NewFakeNetfilterRunner() ep := &ingressProxy{ nfr: nfr, @@ -170,8 +193,7 @@ func TestSyncIngressConfigs(t *testing.T) { t.Fatalf("syncIngressConfigs failed: %v", err) } - fake := nfr.(*linuxfw.FakeNetfilterRunner) - gotServices := fake.GetServiceState() + gotServices := nfr.GetServiceState() if len(gotServices) != len(tt.wantServices) { t.Errorf("got %d services, want %d", len(gotServices), len(tt.wantServices)) } @@ -188,6 +210,20 @@ func TestSyncIngressConfigs(t *testing.T) { t.Errorf("service %s: got ClusterIP %v, want %v", svc, got.ClusterIP, want.ClusterIP) } } + + gotClamped := nfr.GetClampedAddrs() + slices.SortFunc(gotClamped, func(a, b netip.Addr) int { return a.Compare(b) }) + slices.SortFunc(tt.wantClampedAddrs, func(a, b netip.Addr) int { return a.Compare(b) }) + if len(gotClamped) != len(tt.wantClampedAddrs) { + t.Errorf("ClampMSSToPMTU: got %v, want %v", gotClamped, tt.wantClampedAddrs) + } else { + for i := range gotClamped { + if gotClamped[i] != tt.wantClampedAddrs[i] { + t.Errorf("ClampMSSToPMTU: got %v, want %v", gotClamped, tt.wantClampedAddrs) + break + } + } + } }) } } diff --git a/util/linuxfw/fake_netfilter.go b/util/linuxfw/fake_netfilter.go index e9d853508..1ecfc1c39 100644 --- a/util/linuxfw/fake_netfilter.go +++ b/util/linuxfw/fake_netfilter.go @@ -19,6 +19,8 @@ type FakeNetfilterRunner struct { TailscaleServiceIP netip.Addr ClusterIP netip.Addr } + // clampedAddrs tracks addresses passed to ClampMSSToPMTU. + clampedAddrs []netip.Addr } // NewFakeNetfilterRunner creates a new FakeNetfilterRunner. @@ -83,7 +85,15 @@ func (f *FakeNetfilterRunner) DNATWithLoadBalancer(origDst netip.Addr, dsts []ne } func (f *FakeNetfilterRunner) EnsureSNATForDst(src, dst netip.Addr) error { return nil } func (f *FakeNetfilterRunner) DNATNonTailscaleTraffic(tun string, dst netip.Addr) error { return nil } -func (f *FakeNetfilterRunner) ClampMSSToPMTU(tun string, addr netip.Addr) error { return nil } +func (f *FakeNetfilterRunner) ClampMSSToPMTU(tun string, addr netip.Addr) error { + f.clampedAddrs = append(f.clampedAddrs, addr) + return nil +} + +// GetClampedAddrs returns the addresses passed to ClampMSSToPMTU. +func (f *FakeNetfilterRunner) GetClampedAddrs() []netip.Addr { + return f.clampedAddrs +} func (f *FakeNetfilterRunner) AddMagicsockPortRule(port uint16, network string) error { return nil } func (f *FakeNetfilterRunner) DelMagicsockPortRule(port uint16, network string) error { return nil } func (f *FakeNetfilterRunner) DeletePortMapRuleForSvc(svc, tun string, targetIP netip.Addr, pm PortMap) error {