fix(render): stop logging every route the export catch-all drops

CHANGELOG.md

@@ -43,6 +43,19 @@ follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - The peer form's role-specific checkbox group for the *other* role (the iBGP
   switches on an eBGP peer, and vice versa) was visible, greyed out, instead of
   hidden: the group's flex layout overrode the `hidden` attribute.
+- **Generated filters no longer log every route a catch-all drops.** BIRD logs a
+  reject's message once per route it drops. Export filters ended in
+  `reject "not permitted by any export policy"`, and import policies rejected
+  everything outside their allow-list, default-only rule, origin AS set or
+  origin-only check with a message too. On a full-table router such a catch-all
+  matches nearly the whole table on every session start and every UPDATE —
+  millions of syslog lines a day, enough for journald to rate-limit
+  `bird.service` and drop the session events worth reading. Catch-alls are now a
+  bare `reject;` with the reason kept as a config comment; vetoes (bogon, RPKI
+  invalid, AS-path, prefix-length and first-AS checks) still log why. A peer's
+  *Rejected on export* tab (`show route noexport`) still lists what was withheld.
+  The quieter filters take effect on the next apply, so after upgrading every
+  install with policies shows unapplied changes until then.
 
 ## [0.5.0] - 2026-07-22
 

internal/render/origin_test.go

@@ -32,7 +32,7 @@ func TestOriginPeerOnlyRendersOnTheSession(t *testing.T) {
 	in.PrefixSets, in.Policies, in.Peers = bogonSets(), []store.Policy{sanityPolicy()}, []store.Peer{p}
 	f := block(t, mustRender(t, in), "filter ebgp_in_edge_v4")
 
-	if !strings.Contains(f, `if bgp_path.last != 64600 then reject "prefix not originated by this peer";`) {
+	if !strings.Contains(f, "\tif bgp_path.last != 64600 then reject;\t# not originated by this peer\n") {
 		t.Errorf("origin-peer-only guard missing:\n%s", f)
 	}
 	// It must land on the peer's filter, not inside a shared policy function:
@@ -65,7 +65,7 @@ func TestOriginASSetRendersInThePolicy(t *testing.T) {
 	}
 	// It applies to both families: an ASN has no address family.
 	for _, fn := range []string{"function imp_IMPORT_SANITY_v4()", "function imp_IMPORT_SANITY_v6()"} {
-		if !strings.Contains(block(t, out, fn), `if ! (bgp_path.last ~ AS_CUSTOMER_A) then reject "origin AS not in AS_CUSTOMER_A";`) {
+		if !strings.Contains(block(t, out, fn), "\tif ! (bgp_path.last ~ AS_CUSTOMER_A) then reject;\t# origin AS not in AS_CUSTOMER_A\n") {
 			t.Errorf("%s should filter the origin AS", fn)
 		}
 	}

internal/render/prefixset_disable_test.go

@@ -93,7 +93,7 @@ func TestDisabledImportAllowListFailsClosed(t *testing.T) {
 	if strings.Contains(fn, "! (net ~ CUST_IN)") {
 		t.Errorf("a disabled allow-list must not render its membership check:\n%s", fn)
 	}
-	if !strings.Contains(fn, `reject "CUST_IN is disabled`) {
+	if !strings.Contains(fn, "CUST_IN is disabled, so this policy permits nothing here.\n\treject;") {
 		t.Errorf("a disabled allow-list must fail closed (reject all):\n%s", fn)
 	}
 }

internal/render/render.go

@@ -756,13 +756,13 @@ func writeImportBody(b *strings.Builder, in Input, pol store.Policy, fam family,
 			// permit nothing — rather than dropping the check, which would turn
 			// "accept only these prefixes" into "accept anything" and leak the table.
 			fmt.Fprintf(b, "\t# %s is disabled, so this policy permits nothing here.\n", ps.Name)
-			fmt.Fprintf(b, "\treject \"%s is disabled; no prefixes are permitted\";\n", ps.Name)
+			b.WriteString("\treject;\n")
 			return nil
 		case len(ps.Entries) == 0:
 			return fmt.Errorf("accept-only prefix set %q is empty", ps.Name)
 		case familyOf(ps) != fam.suffix:
 			fmt.Fprintf(b, "\t# %s is %s, so this policy permits nothing here.\n", ps.Name, ps.Family)
-			fmt.Fprintf(b, "\treject \"no %s prefixes are permitted by this policy\";\n", fam.channel)
+			b.WriteString("\treject;\n")
 			return nil
 		}
 	}
@@ -790,11 +790,16 @@ func writeImportBody(b *strings.Builder, in Input, pol store.Policy, fam family,
 		fmt.Fprintf(b, "\tif %s then {\n\t\tdest = RTD_BLACKHOLE;\n\t\taccept;\n\t}\n", cond)
 	}
 
+	// Rejects that drop everything not explicitly allowed stay bare, with the
+	// reason as a comment: BIRD logs a reject's message once per route, and a
+	// catch-all can match most of a full table on every session start and
+	// every UPDATE. Only vetoes — one specific thing wrong with a route — say
+	// why in the log.
 	switch pol.DefaultRoute {
 	case store.DefaultReject:
 		fmt.Fprintf(b, "\tif net = %s then reject \"default route not accepted\";\n", fam.anyRoute)
 	case store.DefaultOnly:
-		fmt.Fprintf(b, "\tif net != %s then reject \"only the default route is accepted\";\n", fam.anyRoute)
+		fmt.Fprintf(b, "\tif net != %s then reject;\t# only the default route is accepted\n", fam.anyRoute)
 	case store.DefaultAccept:
 		fmt.Fprintf(b, "\t# the default route is accepted like any other prefix\n")
 	}
@@ -833,7 +838,7 @@ func writeImportBody(b *strings.Builder, in Input, pol store.Policy, fam family,
 	}
 	if pol.AcceptOnlySetID.Valid {
 		name := sets[pol.AcceptOnlySetID.Int64].Name
-		fmt.Fprintf(b, "\tif ! (net ~ %s) then reject \"not in %s\";\n", name, name)
+		fmt.Fprintf(b, "\tif ! (net ~ %s) then reject;\t# not in %s\n", name, name)
 	}
 	// bgp_path.last is the AS that originated the route. Restricting it to the
 	// members of an expanded IRR AS-SET is how a transit provider says "announce
@@ -846,7 +851,7 @@ func writeImportBody(b *strings.Builder, in Input, pol store.Policy, fam family,
 		if len(as.Entries) == 0 {
 			return fmt.Errorf("origin AS set %q is empty", as.Name)
 		}
-		fmt.Fprintf(b, "\tif ! (bgp_path.last ~ %s) then reject \"origin AS not in %s\";\n", as.Name, as.Name)
+		fmt.Fprintf(b, "\tif ! (bgp_path.last ~ %s) then reject;\t# origin AS not in %s\n", as.Name, as.Name)
 	}
 	// RPKI route-origin validation. roa_check compares the route's origin AS
 	// against the ROAs its prefix holder published. Invalid means the origin is
@@ -1206,7 +1211,7 @@ func writePeerImportFilter(b *strings.Builder, in Input, p store.Peer, fam famil
 	// AS must be the peer itself. Prepending still works, because the origin is
 	// the last ASN in the path, not the first.
 	if p.OriginPeerOnly {
-		fmt.Fprintf(b, "\tif bgp_path.last != %d then reject \"prefix not originated by this peer\";\n", p.RemoteASN)
+		fmt.Fprintf(b, "\tif bgp_path.last != %d then reject;\t# not originated by this peer\n", p.RemoteASN)
 	}
 	if p.IsIBGP() {
 		// The opposite of the eBGP rule below, and the reason iBGP gets its own
@@ -1287,7 +1292,10 @@ func writePeerExportFilter(b *strings.Builder, in Input, p store.Peer, fam famil
 	for _, pol := range p.ExportPolicies {
 		fmt.Fprintf(b, "\t%s;\n", policyFunc(pol, fam))
 	}
-	b.WriteString("\treject \"not permitted by any export policy\";\n}\n\n")
+	// A bare reject: BIRD logs a reject's message for every route it drops, and
+	// this line drops nearly the whole table on every export to an upstream or
+	// iBGP peer. `show route noexport` still lists what was withheld.
+	b.WriteString("\treject;\t# not permitted by any export policy\n}\n\n")
 }
 
 // IsPrivateASN reports whether asn falls in one of the ranges the operator has

internal/render/render_test.go

@@ -2,6 +2,7 @@ package render
 
 import (
 	"database/sql"
+	"regexp"
 	"strings"
 	"testing"
 	"time"
@@ -155,7 +156,7 @@ func TestDefaultRouteModes(t *testing.T) {
 	in := baseInput()
 	in.PrefixSets, in.Policies = sets, []store.Policy{only}
 	fn := block(t, mustRender(t, in), "function imp_P_v4()")
-	if !strings.Contains(fn, `if net != 0.0.0.0/0 then reject "only the default route is accepted";`) {
+	if !strings.Contains(fn, "\tif net != 0.0.0.0/0 then reject;\t# only the default route is accepted") {
 		t.Errorf("accept-only-default must reject everything else:\n%s", fn)
 	}
 
@@ -336,8 +337,15 @@ func TestExportChainEndsInReject(t *testing.T) {
 	if first < 0 || second < 0 || first > second {
 		t.Errorf("export policies must be called in attachment order:\n%s", f)
 	}
-	if !strings.Contains(f, `reject "not permitted by any export policy";`) {
-		t.Error("the export filter must reject anything no policy accepted")
+	if !strings.HasSuffix(f, "\n\treject;\t# not permitted by any export policy") {
+		t.Errorf("the export filter must end by rejecting anything no policy accepted:\n%s", f)
+	}
+	// Without a message: BIRD logs a reject's text for every route it drops, and
+	// on a full-table router the catch-all drops nearly every route on every
+	// export — millions of syslog lines, enough for journald to start
+	// suppressing the ones that matter.
+	if strings.Contains(f, `reject "`) {
+		t.Errorf("the catch-all reject must not log a message per route:\n%s", f)
 	}
 }
 
@@ -383,11 +391,11 @@ func TestAcceptOnlySetRejectsTheOtherFamily(t *testing.T) {
 	in.Policies = []store.Policy{pol}
 	out := mustRender(t, in)
 
-	if v4 := block(t, out, "function imp_CUST_v4()"); !strings.Contains(v4, `if ! (net ~ CUST_A_V4) then reject "not in CUST_A_V4";`) {
+	if v4 := block(t, out, "function imp_CUST_v4()"); !strings.Contains(v4, "\tif ! (net ~ CUST_A_V4) then reject;\t# not in CUST_A_V4") {
 		t.Errorf("v4 function should permit only the set:\n%s", v4)
 	}
 	v6 := block(t, out, "function imp_CUST_v6()")
-	if !strings.Contains(v6, `reject "no ipv6 prefixes are permitted by this policy";`) {
+	if !strings.Contains(v6, "so this policy permits nothing here.\n\treject;") {
 		t.Errorf("v6 function must reject everything, not silently accept:\n%s", v6)
 	}
 }
@@ -1032,3 +1040,65 @@ func TestBlackholeKeepsOriginTag(t *testing.T) {
 		t.Errorf("origin tag must be added BEFORE the policy call so a blackhole accept keeps it:\n%s", f)
 	}
 }
+
+// BIRD logs a reject's message once for every route it drops. For a veto — one
+// specific thing wrong with this route — that is worth it: such routes are rare
+// and the line says why. A catch-all that drops everything not explicitly
+// allowed can match most of a full table on every session start and every
+// UPDATE, and its log lines crowd out the session events. Catch-alls stay
+// bare, with the reason as a config comment. A new messaged reject must be
+// added to the veto list here on purpose.
+func TestOnlyVetoRejectsCarryAMessage(t *testing.T) {
+	vetoes := map[string]bool{
+		"default route not accepted": true, "prefix length out of bounds": true, "bogon prefix": true,
+		"RPKI invalid": true, "AS path too long": true, "our own ASN in AS path": true,
+		"bogon ASN in AS path": true, "first AS is not the peer AS": true,
+	}
+
+	cust := store.PrefixSet{ID: 30, Name: "CUST_A_V4", Family: store.FamilyV4,
+		Entries: []store.PrefixEntry{{Prefix: "198.51.100.0/24", Modifier: "+"}}}
+	off := store.PrefixSet{ID: 31, Name: "OFF_V4", Family: store.FamilyV4, Disabled: true,
+		Entries: []store.PrefixEntry{{Prefix: "203.0.113.0/24"}}}
+	as := custASSet()
+	onlyDefault := store.Policy{ID: 10, Name: "ONLY_DEFAULT", Direction: store.DirImport,
+		DefaultRoute: store.DefaultOnly, BogonASNs: store.BogonASNsOff}
+	allow := store.Policy{ID: 11, Name: "CUST", Direction: store.DirImport, DefaultRoute: store.DefaultReject,
+		BogonASNs: store.BogonASNsOff, AcceptOnlySetID: sql.NullInt64{Int64: cust.ID, Valid: true},
+		OriginASSetID: sql.NullInt64{Int64: as.ID, Valid: true}}
+	disabled := store.Policy{ID: 12, Name: "OFF", Direction: store.DirImport, DefaultRoute: store.DefaultReject,
+		BogonASNs: store.BogonASNsOff, AcceptOnlySetID: sql.NullInt64{Int64: off.ID, Valid: true}}
+	export := store.Policy{ID: 13, Name: "EXPORT_CUST", Direction: store.DirExport, AnnounceFromCustomer: true}
+
+	p := ebgpPeer()
+	p.EnforceFirstAS, p.OriginPeerOnly = true, true
+	p.ImportPolicies = []store.Policy{sanityPolicy(), allow}
+	p.ExportPolicies = []store.Policy{export}
+
+	in := baseInput()
+	in.PrefixSets = append(bogonSets(), cust, off)
+	in.ASSets = []store.ASSet{as}
+	in.Policies = []store.Policy{sanityPolicy(), onlyDefault, allow, disabled, export}
+	in.Peers = []store.Peer{p}
+	out := mustRender(t, in)
+
+	msg := regexp.MustCompile(`reject "([^"]*)"`)
+	for _, m := range msg.FindAllStringSubmatch(out, -1) {
+		if !vetoes[m[1]] {
+			t.Errorf("reject %q logs once per route it drops; make it a bare reject with a comment, or add it to the vetoes if it is one", m[1])
+		}
+	}
+	// And the catch-alls are all there, bare — the test covers what it claims to.
+	for _, want := range []string{
+		"then reject;\t# only the default route is accepted\n",
+		"then reject;\t# not in CUST_A_V4\n",
+		"then reject;\t# origin AS not in AS_CUSTOMER_A\n",
+		"then reject;\t# not originated by this peer\n",
+		"\treject;\t# not permitted by any export policy\n",
+		"OFF_V4 is disabled, so this policy permits nothing here.\n\treject;\n",
+		"CUST_A_V4 is ipv4, so this policy permits nothing here.\n\treject;\n",
+	} {
+		if !strings.Contains(out, want) {
+			t.Errorf("expected the bare catch-all %q in:\n%s", want, out)
+		}
+	}
+}