feat(parse): add Cisco sh ip bgp text dump support#143
Conversation
- Auto-detect Cisco fixed-width BGP table dumps via preamble - Parse prefix, next-hop, metric, local-pref, weight, AS path, origin - Handle multi-path continuation lines and wrapped-prefix layouts - Support all parse filters, output formats, and MRT export (--mrt-type) - Whitespace-split header column detection (replaces fixed find()) - MrtType::infer() auto-selects Rib for text dumps, Updates for MRT
digizeph
left a comment
digizeph
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approve — solid implementation, well-tested, clean architecture.
✅ Looks Good
- Whitespace-split header detection with multi-word merging (
"Next"+"Hop") is robust and avoids hardcoded positions — correctly handles PCH format variations - Byte-position slicing for route-line columns keeps numeric attributes (metric, local-pref) distinct from AS-path ASNs even when columns are blank — the right call vs. whitespace split
ParseFilters::filter_specs()refactor extracts shared filter logic into one method that bothto_parser()andto_filters()consume — clean DRY, no duplicationMrtTypeenum withinfer()is forward-looking — RIB for text dumps, Updates for MRT, with explicit--mrt-typeoverride- Auto-detection peeks 256 bytes then re-opens for full parse — clean separation, no MRT parser breakage
- 13 focused unit tests cover detection, headers, continuations, wrapped prefixes, all origin codes, empty numeric attrs, round-trip BgpElem construction
- Streaming fast path preserved for ordinary MRT parse when no buffering/export needed — no perf regression
- CI:
cargo fmt --check✓,cargo clippy --all-features -- -D warnings✓,cargo test --all-features— 304 passed ✓
💡 Suggestions (non-blocking)
-
text_dump.rs:200— Document wrapped-prefix IPv4-only limitation.parse_wrapped_prefix_linechecksline.len() > next_hop_startto detect prefix-only lines. This works for IPv4 prefixes (< 21 chars at standard PCH column offset) but would miss longer IPv6 wrapped prefixes. Not a blocker since the primary target is PCH IPv4 dumps, but worth a doc comment. -
text_dump.rs:364— Hardcodedtimestamp = 0.0. A real timestamp (extracted from the PCH URL date component, e.g.2026/07/01→ Unix) would make the exported MRT data more useful for time-range queries. Consider a follow-up PR. -
text_dump.rs:264— AS-set flattening.{/}tokens are silently dropped, converting AS-sets into flat AS-sequences. This is reasonable for most use cases, but a one-line doc comment onas_path_from_tokenswould make the trade-off explicit.
🔍 Architecture note
The text dump path always materializes into Vec<BgpElem> (RIBs are inherently full-snapshot). For the 97k-prefix PCH dumps tested, this is fine. If much larger dumps appear in the future, a streaming adapter that yields BgpElem from parse_text_dump would be the natural refactor — the internal structure already supports it.
Reviewed by Scouty (Hermes Agent)
| )) | ||
| } | ||
|
|
||
| fn parse_wrapped_prefix_line(line: &str, next_hop_start: usize) -> Option<String> { |
There was a problem hiding this comment.
💡 Suggestion: This detection (line.len() > next_hop_start) works for IPv4 wrapped prefixes (< 21 chars at standard PCH column offset) but would miss longer IPv6 prefixes. A doc comment noting this limitation would help future readers. Not a blocker — the primary target is PCH IPv4 dumps.
| fn as_path_from_tokens(tokens: &[String]) -> AsPath { | ||
| let mut asns: Vec<Asn> = Vec::new(); | ||
| for token in tokens { | ||
| if token == "{" || token == "}" { |
There was a problem hiding this comment.
💡 Suggestion: { / } AS-set delimiters are silently dropped, flattening sets into sequences. A short doc comment would make this deliberate trade-off explicit for future readers.
| } | ||
| }; | ||
|
|
||
| let timestamp = 0.0_f64; |
There was a problem hiding this comment.
💡 Suggestion: Hardcoded 0.0 timestamp. Consider extracting a date from the PCH URL path (e.g. 2026/07/01 → Unix) so exported MRT data is usable in time-range queries. Fine for the initial merge.
- Fix parse_wrapped_prefix_line to handle IPv6 and long prefixes by
checking whether route data exists after the next-hop column instead
of using a line-length heuristic; swap loop order so route-line
parsing is tried before wrapped-prefix fallback
- Add infer_timestamp_from_path() that extracts YYYY.MM.DD from
PCH-style filenames/URLs and converts to Unix timestamp
- Add parse_text_dump_with_timestamp() to accept an explicit timestamp;
CLI passes inferred timestamp, falls back to 0.0 when undetectable
- Document AS-set {/} flattening in as_path_from_tokens
1 participant
Summary
Adds auto-detection and parsing of Cisco
sh ip bgpfixed-width text dumps to themonocle parsecommand. The parser extracts all standard BGP attributes, supports multipath continuations, and outputs via all existing formats (JSON, table, MRT).Usage
Changes
New file
src/lens/parse/text_dump.rs— full parser: format detection, header parsing, route-line parsing, BgpElem construction, 13 unit testsModified files
src/lens/parse/mod.rs— addedMrtTypeenum (Rib/Updates) withinfer()method; wiredtext_dumpmodulesrc/bin/commands/parse.rs— format auto-detection branch (peek 256 bytes),--mrt-typeflag, text dump → parse_text_dump pathKey design decisions
find()substring matchingribfor text dumps (RIB snapshots) andupdatesfor MRT filesTest plan
cargo fmt --check✓cargo clippy --all-features -- -D warnings✓cargo test --all-features— 302 passed, 0 failed ✓-o 13335) → 5,092 correct results ✓-f prefix,next_hop,as_path) ✓Closes #142