From 778a79a6eacc46fc48ebd78554e32edef3dbb944 Mon Sep 17 00:00:00 2001 From: Nicolas Henneaux Date: Mon, 24 Nov 2025 18:03:30 +0100 Subject: [PATCH 1/2] fix(router): nft tables limit number of peers source batching them, failing at 3277 prefixes on nftables v1.0.9 with Ubuntu 24.04.3 LTS, 6.14.0-35-generic #35~24.04.1-Ubuntu --- .../firewall/nftables/manager_linux_test.go | 91 +++++++++++++++++++ client/firewall/nftables/router_linux.go | 61 +++++++++---- 2 files changed, 133 insertions(+), 19 deletions(-) diff --git a/client/firewall/nftables/manager_linux_test.go b/client/firewall/nftables/manager_linux_test.go index adec802c8a2..6b29c56069d 100644 --- a/client/firewall/nftables/manager_linux_test.go +++ b/client/firewall/nftables/manager_linux_test.go @@ -386,6 +386,97 @@ func TestNftablesManagerCompatibilityWithIptables(t *testing.T) { verifyIptablesOutput(t, stdout, stderr) } +func TestNftablesManagerCompatibilityWithIptablesFor6kPrefixes(t *testing.T) { + if check() != NFTABLES { + t.Skip("nftables not supported on this system") + } + + if _, err := exec.LookPath("iptables-save"); err != nil { + t.Skipf("iptables-save not available on this system: %v", err) + } + + // First ensure iptables-nft tables exist by running iptables-save + stdout, stderr := runIptablesSave(t) + verifyIptablesOutput(t, stdout, stderr) + + manager, err := Create(ifaceMock, iface.DefaultMTU) + require.NoError(t, err, "failed to create manager") + require.NoError(t, manager.Init(nil)) + + t.Cleanup(func() { + err := manager.Close(nil) + require.NoError(t, err, "failed to reset manager state") + + // Verify iptables output after reset + stdout, stderr := runIptablesSave(t) + verifyIptablesOutput(t, stdout, stderr) + }) + + const octet2Count = 25 + const octet3Count = 255 + prefixes := make([]netip.Prefix, 0, (octet2Count-1)*(octet3Count-1)) + for i := 1; i < octet2Count; i++ { + for j := 1; j < octet3Count; j++ { + addr := netip.AddrFrom4([4]byte{192, byte(j), byte(i), 0}) + prefixes = append(prefixes, netip.PrefixFrom(addr, 24)) + } + } + _, err = manager.AddRouteFiltering( + nil, + prefixes, + fw.Network{Prefix: netip.MustParsePrefix("10.2.0.0/24")}, + fw.ProtocolTCP, + nil, + &fw.Port{Values: []uint16{443}}, + fw.ActionAccept, + ) + require.NoError(t, err, "failed to add route filtering rule") + + stdout, stderr = runIptablesSave(t) + verifyIptablesOutput(t, stdout, stderr) +} + +func TestNftablesManagerCompatibilityWithIptablesForEmptyPrefixes(t *testing.T) { + if check() != NFTABLES { + t.Skip("nftables not supported on this system") + } + + if _, err := exec.LookPath("iptables-save"); err != nil { + t.Skipf("iptables-save not available on this system: %v", err) + } + + // First ensure iptables-nft tables exist by running iptables-save + stdout, stderr := runIptablesSave(t) + verifyIptablesOutput(t, stdout, stderr) + + manager, err := Create(ifaceMock, iface.DefaultMTU) + require.NoError(t, err, "failed to create manager") + require.NoError(t, manager.Init(nil)) + + t.Cleanup(func() { + err := manager.Close(nil) + require.NoError(t, err, "failed to reset manager state") + + // Verify iptables output after reset + stdout, stderr := runIptablesSave(t) + verifyIptablesOutput(t, stdout, stderr) + }) + + _, err = manager.AddRouteFiltering( + nil, + []netip.Prefix{}, + fw.Network{Prefix: netip.MustParsePrefix("10.2.0.0/24")}, + fw.ProtocolTCP, + nil, + &fw.Port{Values: []uint16{443}}, + fw.ActionAccept, + ) + require.NoError(t, err, "failed to add route filtering rule") + + stdout, stderr = runIptablesSave(t) + verifyIptablesOutput(t, stdout, stderr) +} + func compareExprsIgnoringCounters(t *testing.T, got, want []expr.Any) { t.Helper() require.Equal(t, len(got), len(want), "expression count mismatch") diff --git a/client/firewall/nftables/router_linux.go b/client/firewall/nftables/router_linux.go index e4debc1794f..cdb3cc8f490 100644 --- a/client/firewall/nftables/router_linux.go +++ b/client/firewall/nftables/router_linux.go @@ -44,9 +44,11 @@ const ( // ipTCPHeaderMinSize represents minimum IP (20) + TCP (20) header size for MSS calculation ipTCPHeaderMinSize = 40 -) -const refreshRulesMapError = "refresh rules map: %w" + // maxPrefixesSet 3277 prefixes start to fail, taking some margin + maxPrefixesSet = 3000 + refreshRulesMapError = "refresh rules map: %w" +) var ( errFilterTableNotFound = fmt.Errorf("'filter' table not found") @@ -346,26 +348,11 @@ func (r *router) AddRouteFiltering( } chain := r.chains[chainNameRoutingFw] - var exprs []expr.Any - - var source firewall.Network - switch { - case len(sources) == 1 && sources[0].Bits() == 0: - // If it's 0.0.0.0/0, we don't need to add any source matching - case len(sources) == 1: - // If there's only one source, we can use it directly - source.Prefix = sources[0] - default: - // If there are multiple sources, use a set - source.Set = firewall.NewPrefixSet(sources) - } - sourceExp, err := r.applyNetwork(source, sources, true) + exprs, err := r.applySources(sources) if err != nil { - return nil, fmt.Errorf("apply source: %w", err) + return nil, fmt.Errorf("apply sources (%d): %w", len(sources), err) } - exprs = append(exprs, sourceExp...) - destExp, err := r.applyNetwork(destination, nil, false) if err != nil { return nil, fmt.Errorf("apply destination: %w", err) @@ -425,6 +412,42 @@ func (r *router) AddRouteFiltering( return ruleKey, nil } +func (r *router) applySources(sources []netip.Prefix) ([]expr.Any, error) { + var source firewall.Network + if len(sources) == 0 { + return nil, nil + } + if len(sources) == 1 { + if sources[0].Bits() == 0 { + // If it's 0.0.0.0/0, we don't need to add any source matching + return nil, nil + } else { // If there's only one source, we can use it directly + source.Prefix = sources[0] + } + return r.applyNetwork(source, sources, true) + } + // If there are multiple sources, use a set + var exprs []expr.Any + + var subEnd int + for subStart := 0; subStart < len(sources); subStart += maxPrefixesSet { + subEnd += maxPrefixesSet + if subEnd > len(sources) { + subEnd = len(sources) + } + subSources := sources[subStart:subEnd] + source.Set = firewall.NewPrefixSet(subSources) + + sourceExp, err := r.applyNetwork(source, subSources, true) + if err != nil { + return nil, fmt.Errorf("apply source: %w (prefixes %d)", err, len(subSources)) + } + exprs = append(exprs, sourceExp...) + } + + return exprs, nil +} + func (r *router) getIpSet(set firewall.Set, prefixes []netip.Prefix, isSource bool) ([]expr.Any, error) { ref, err := r.ipsetCounter.Increment(set.HashedName(), setInput{ set: set, From b1243d1736362ee02556f49c0f2a2204d33c381d Mon Sep 17 00:00:00 2001 From: Nicolas Henneaux Date: Wed, 26 Nov 2025 14:25:48 +0100 Subject: [PATCH 2/2] fix(router): nft tables limit number of prefixes on ipSet creation --- client/firewall/nftables/router_linux.go | 86 ++++++++++++------------ 1 file changed, 42 insertions(+), 44 deletions(-) diff --git a/client/firewall/nftables/router_linux.go b/client/firewall/nftables/router_linux.go index cdb3cc8f490..ae29507ac11 100644 --- a/client/firewall/nftables/router_linux.go +++ b/client/firewall/nftables/router_linux.go @@ -45,8 +45,8 @@ const ( // ipTCPHeaderMinSize represents minimum IP (20) + TCP (20) header size for MSS calculation ipTCPHeaderMinSize = 40 - // maxPrefixesSet 3277 prefixes start to fail, taking some margin - maxPrefixesSet = 3000 + // maxPrefixesSet 1638 prefixes start to fail, taking some margin + maxPrefixesSet = 1500 refreshRulesMapError = "refresh rules map: %w" ) @@ -348,11 +348,26 @@ func (r *router) AddRouteFiltering( } chain := r.chains[chainNameRoutingFw] + var exprs []expr.Any + + var source firewall.Network + switch { + case len(sources) == 1 && sources[0].Bits() == 0: + // If it's 0.0.0.0/0, we don't need to add any source matching + case len(sources) == 1: + // If there's only one source, we can use it directly + source.Prefix = sources[0] + default: + // If there are multiple sources, use a set + source.Set = firewall.NewPrefixSet(sources) + } - exprs, err := r.applySources(sources) + sourceExp, err := r.applyNetwork(source, sources, true) if err != nil { - return nil, fmt.Errorf("apply sources (%d): %w", len(sources), err) + return nil, fmt.Errorf("apply source: %w", err) } + exprs = append(exprs, sourceExp...) + destExp, err := r.applyNetwork(destination, nil, false) if err != nil { return nil, fmt.Errorf("apply destination: %w", err) @@ -412,42 +427,6 @@ func (r *router) AddRouteFiltering( return ruleKey, nil } -func (r *router) applySources(sources []netip.Prefix) ([]expr.Any, error) { - var source firewall.Network - if len(sources) == 0 { - return nil, nil - } - if len(sources) == 1 { - if sources[0].Bits() == 0 { - // If it's 0.0.0.0/0, we don't need to add any source matching - return nil, nil - } else { // If there's only one source, we can use it directly - source.Prefix = sources[0] - } - return r.applyNetwork(source, sources, true) - } - // If there are multiple sources, use a set - var exprs []expr.Any - - var subEnd int - for subStart := 0; subStart < len(sources); subStart += maxPrefixesSet { - subEnd += maxPrefixesSet - if subEnd > len(sources) { - subEnd = len(sources) - } - subSources := sources[subStart:subEnd] - source.Set = firewall.NewPrefixSet(subSources) - - sourceExp, err := r.applyNetwork(source, subSources, true) - if err != nil { - return nil, fmt.Errorf("apply source: %w (prefixes %d)", err, len(subSources)) - } - exprs = append(exprs, sourceExp...) - } - - return exprs, nil -} - func (r *router) getIpSet(set firewall.Set, prefixes []netip.Prefix, isSource bool) ([]expr.Any, error) { ref, err := r.ipsetCounter.Increment(set.HashedName(), setInput{ set: set, @@ -505,16 +484,35 @@ func (r *router) createIpSet(setName string, input setInput) (*nftables.Set, err } elements := convertPrefixesToSet(prefixes) - if err := r.conn.AddSet(nfset, elements); err != nil { - return nil, fmt.Errorf("error adding elements to set %s: %w", setName, err) - } + nElements := len(elements) + maxElements := maxPrefixesSet * 2 + initialElements := elements[:min(maxElements, nElements)] + + if err := r.conn.AddSet(nfset, initialElements); err != nil { + return nil, fmt.Errorf("error adding set %s: %w", setName, err) + } if err := r.conn.Flush(); err != nil { return nil, fmt.Errorf("flush error: %w", err) } + log.Debugf("Created new ipset: %s with %d initial prefixes (total prefixes %d)", setName, len(initialElements)/2, len(prefixes)) - log.Printf("Created new ipset: %s with %d elements", setName, len(elements)/2) + var subEnd int + for subStart := maxElements; subStart < nElements; subStart += maxElements { + subEnd = min(subStart+maxElements, nElements) + subElement := elements[subStart:subEnd] + nSubPrefixes := len(subElement) / 2 + log.Tracef("Adding new prefixes (%d) in ipset: %s", nSubPrefixes, setName) + if err := r.conn.SetAddElements(nfset, subElement); err != nil { + return nil, fmt.Errorf("error adding prefixes (%d) to set %s: %w", nSubPrefixes, setName, err) + } + if err := r.conn.Flush(); err != nil { + return nil, fmt.Errorf("flush error: %w", err) + } + log.Debugf("Added new prefixes (%d) in ipset: %s", nSubPrefixes, setName) + } + log.Infof("Created new ipset: %s with %d prefixes", setName, len(prefixes)) return nfset, nil }