Skip to content

tools: Normalize aggregate-address command when doing frr-reload#22284

Open
ton31337 wants to merge 1 commit into
FRRouting:masterfrom
opensourcerouting:fix/aggregate-address_normalize
Open

tools: Normalize aggregate-address command when doing frr-reload#22284
ton31337 wants to merge 1 commit into
FRRouting:masterfrom
opensourcerouting:fix/aggregate-address_normalize

Conversation

@ton31337

@ton31337 ton31337 commented Jun 9, 2026

Copy link
Copy Markdown
Member

No description provided.

Without this if we swap the parameters in frr.conf and show running shows the
order differently, the route is re-added which means a short hole.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
@frrbot frrbot Bot added the tools label Jun 9, 2026
@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a get_normalized_aggregate_address_line function to frr-reload.py that canonicalizes the keyword ordering of aggregate-address commands. Because FRR accepts keywords in any order but emits them in a fixed order, a config file with keywords in a non-canonical order caused frr-reload to always see a diff and re-apply the aggregate — briefly withdrawing the prefix on every reload.

  • Parses and reorders the six supported keywords (as-set, summary-only, route-map, origin, matching-MED-only, suppress-map) into the exact order FRR's bgp_config_write_network emits them, confirmed in bgpd/bgp_route.c:18926–18948.
  • Handles both CIDR (A.B.C.D/M) and old-style (A.B.C.D A.B.C.D) prefix notation; any line containing an unrecognized token is returned untouched to avoid silent drops.

Confidence Score: 4/5

Safe to merge; the normalization is additive and falls back to returning the original line for any unrecognized input.

The canonical keyword order produced by the new function matches exactly what bgpd's bgp_config_write_network outputs (confirmed in bgpd/bgp_route.c). The regex correctly handles both CIDR and legacy A.B.C.D A.B.C.D prefix notation, all six valid keywords are covered, and the else-return-line guard means any future keyword addition degrades gracefully.

tools/frr-reload.py — specifically the prefix-capture regex in get_normalized_aggregate_address_line; verified correct but slightly subtle.

Important Files Changed

Filename Overview
tools/frr-reload.py Adds aggregate-address keyword normalization; canonical order confirmed against bgpd config-write output; safe fall-through for unknown tokens.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[line starts with 'aggregate-address '] --> B[regex match prefix + rest]
    B -- no match --> C[return line unchanged]
    B -- match --> D[tokenize rest into list]
    D --> E{for each token}
    E -- as-set --> F[as_set = True]
    E -- summary-only --> G[summary_only = True]
    E -- matching-MED-only --> H[match_med = True]
    E -- route-map --> I[consume next token as route_map]
    E -- origin --> J[consume next token as origin]
    E -- suppress-map --> K[consume next token as suppress_map]
    E -- unknown token --> C
    F & G & H & I & J & K --> E
    E -- done --> L[build canonical string]
    L --> M[return normalized line]
Loading

Reviews (1): Last reviewed commit: "tools: Normalize aggregate-address comma..." | Re-trigger Greptile

@riw777 riw777 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@ton31337 ton31337 added this to the 10.7 milestone Jun 11, 2026
@ton31337

Copy link
Copy Markdown
Member Author

@Mergifyio backport stable/10.7

@mergify

mergify Bot commented Jun 11, 2026

Copy link
Copy Markdown

backport stable/10.7

🟠 Waiting for conditions to match

Details
  • merged [📌 backport requirement]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants