[Nexthop][fboss2-dev] fboss2 bgp policy routing-policy term match commands by hillol-nexthop · Pull Request #1491 · meta/fboss

hillol-nexthop · 2026-08-07T19:30:38Z

Stacked on #1490 (bgp policy routing-policy term action). This PR's branch contains the whole stack below it, so the diff shows those commits too — please review only the bgp policy routing-policy term match commit(s) here. Merge the lower PRs first.

Stack (bottom → top): ~~#1477~~ → ~~#1487~~ → ~~#1488~~ (landed) → #1489 → #1490 → #1491 (this PR)

Adds the match level of a routing-policy term:

... term <seq-num> match from as-path-list <name>
... term <seq-num> match from origin <IGP|EGP|INCOMPLETE>
... term <seq-num> match from prefix-list <name>

Scoped to these three deliberately. The match conditions are written to policy_match_entries (the deprecated singular container) with the referenced list named in *_list_names, because that is what bgpd reads: the newer policy_matches field is not consumed anywhere in the daemon, so writing it would stage config that is silently ignored. The remaining documented match attributes (med, next-hop, local-preference) have no support in bgpd and are left out rather than staged as dead config.

Sample usage

$ fboss2-dev config protocol bgp policy routing-policy EXPORT-POLICY term 10 match from prefix-list PROD-PREFIXES
Successfully set prefix-list to: PROD-PREFIXES for routing-policy EXPORT-POLICY term 10 match

$ fboss2-dev config protocol bgp policy routing-policy EXPORT-POLICY term 10 match from origin IGP
Successfully set origin to: IGP for routing-policy EXPORT-POLICY term 10 match

$ fboss2-dev config protocol bgp policy routing-policy EXPORT-POLICY term 10 match from med 50
Error: unknown match attribute 'from med'. Valid attributes: from as-path-list, from origin, from prefix-list

Test plan

Unit — fboss2_cmd_config_test:

$ fboss2_cmd_config_test --gtest_filter=*TermMatch*
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermMatchTestFixture.argValidation (93 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermMatchTestFixture.unsupportedMatchesRejected (52 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermMatchTestFixture.fromAsPathList (224 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermMatchTestFixture.fromOrigin (150 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermMatchTestFixture.fromPrefixList (68 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermMatchTestFixture.matchKindsAreKeyedByType (158 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermMatchTestFixture.rejectedValueRollsBackPhantoms (85 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermMatchTestFixture.ancestorAttributeAlongsideMatchRejected (61 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermMatchTestFixture.fromUnknownAsPathListRejected (62 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermMatchTestFixture.fromAsPathListCopiesListOperator (85 ms)
[==========] 10 tests from 1 test suite ran. (1042 ms total)
[  PASSED  ] 10 tests.

Integration — fboss2_integration_test on a lab device with bgp_pp active:

$ fboss2_integration_test --gtest_filter='ConfigBgpPolicyRoutingPolicyTermMatchTest.*'
[       OK ] ConfigBgpPolicyRoutingPolicyTermMatchTest.FromUnknownAsPathListRejected (1052 ms)
[       OK ] ConfigBgpPolicyRoutingPolicyTermMatchTest.SetMatchesAndCommit (1345 ms)
[==========] 2 tests from 1 test suite ran.
[  PASSED  ] 2 tests.

Based-on: #1490

Review follow-up

from prefix-list <name> now refuses a name that does not exist, like from as-path-list already did: PrefixTreeMatch::PopulateReferences throws "Could not find PrefixList reference" at config load. No operator sync is needed for prefix-lists.

Summary: > **Rebased on `main`** now that #1475 has landed (cb12ffe39a1f); the diff is this PR's commits only. > > **Stack (bottom → top):** **#1477 (this PR)** → #1487 → #1488 → #1489 → #1490 → #1491 Adds the third BGP **policy** object-type command family to `fboss2-dev`: - `config protocol bgp policy prefix-list <name> [<attribute> <value>]` - `delete protocol bgp policy prefix-list <name>` The entry level ships in #1487 as `prefix-list <name> prefix <prefix/len>` for both config and delete: entries are keyed by their prefix, because bgpd rejects a `PrefixListEntry` carrying `seq_num`. Nothing in this PR writes one. ### Commands | Command | Thrift target | |---|---| | `prefix-list <name>` | `prefix_lists[]` (create/select) | | `prefix-list <name> description <string>` | `PrefixList.description` | | `prefix-list <name> boolean-operator OR` | `PrefixList.boolean_operator` (bgpd accepts only OR) | | `prefix-list <name> ip-version <v4\|v6>` | `PrefixList.version` (numeric 4/6) | | `delete ... prefix-list <name>` | removes the `prefix_lists[]` entry (refused while a term references it) | ### Design - Same dispatcher shape as the as-path-list and community-list families: value-shape factories from the shared `BgpCliAttrHandlers.h` (added in #1391), named setters, one registry line per attribute. Writes `bgp_policy.BgpPolicies.prefix_lists[]` through the typed `ConfigSession`. - **Only what bgpd accepts is offered.** `PrefixTreeMatch::validateAndCreatePrefixTree` (bgp++) throws on any list-level `compare_operator` and on any `boolean_operator` other than OR, but only once a routing-policy term references the list — so a bad list commits cleanly and fails the whole config later. The CLI therefore has no `compare-operator` attribute and accepts `boolean-operator OR` only; `bgp_json_to_cli.py` warns and skips instead of emitting the rejected values, warns with the count when it drops `prefixes[]`, and sanitizes every warning through `_printable()` so a control character in a JSON name cannot escape the `#` comment. - Lookup/create helpers for the list live in `BgpPrefixListCliUtils.h`, shared with the delete command; the entry helpers arrive with #1487. - A rejected value never leaves a phantom list behind: the dispatcher rolls back a list it implicitly created in the same invocation. **Delete is idempotent.** Deleting an absent target is a success with a `Warning:` (nothing is saved), so a replayed script never fails on something already gone; the reference guards still return `Error:`. ## Sample usage ``` $ fboss2-dev config protocol bgp policy prefix-list PROD-PREFIXES Successfully created BGP prefix-list PROD-PREFIXES Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy prefix-list PROD-PREFIXES description Production route prefixes Successfully set description to: Production route prefixes for prefix-list PROD-PREFIXES $ fboss2-dev config protocol bgp policy prefix-list PROD-PREFIXES boolean-operator OR Successfully set boolean-operator to: OR for prefix-list PROD-PREFIXES $ fboss2-dev config protocol bgp policy prefix-list PROD-PREFIXES ip-version v4 Successfully set ip-version to: v4 for prefix-list PROD-PREFIXES $ fboss2-dev delete protocol bgp policy prefix-list PROD-PREFIXES Successfully deleted BGP prefix-list PROD-PREFIXES ``` Idempotent delete, recorded on a lab device: ``` $ fboss2-dev delete protocol bgp policy prefix-list PROD-PREFIXES Warning: BGP prefix-list PROD-PREFIXES does not exist; nothing to delete ``` Pull Request resolved: #1477 Test Plan: **Unit (delete)** — `fboss2_cmd_config_test`: ``` $ fboss2_cmd_config_test --gtest_filter=*DeleteBgpPolicyPrefixList* [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.argValidation (81 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteExistingList (140 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteUnknownListWarns (68 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteTwiceIsIdempotent (118 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteUnknownLeavesOthersIntact (119 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteReferencedListRejected (113 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteReferencedListNamesEveryReferrer (164 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.referenceToOtherListDoesNotBlockDelete (282 ms) [==========] 14 tests from 1 test suites ran. [ PASSED ] 14 tests. ``` **Unit** — `fboss2_cmd_config_test`: ``` $ fboss2_cmd_config_test --gtest_filter=*BgpPolicyPrefixList* [ OK ] CmdConfigBgpPolicyPrefixListTestFixture.argValidation (115 ms) [ OK ] CmdConfigBgpPolicyPrefixListTestFixture.bareCreateList (166 ms) [ OK ] CmdConfigBgpPolicyPrefixListTestFixture.setListDescription (120 ms) [ OK ] CmdConfigBgpPolicyPrefixListTestFixture.setBooleanOperator (164 ms) [ OK ] CmdConfigBgpPolicyPrefixListTestFixture.booleanOperatorDefaultIsOr (126 ms) [ OK ] CmdConfigBgpPolicyPrefixListTestFixture.setCompareOperator (120 ms) [ OK ] CmdConfigBgpPolicyPrefixListTestFixture.setIpVersion (146 ms) [ OK ] CmdConfigBgpPolicyPrefixListTestFixture.namedListsAreDistinct (228 ms) [ OK ] CmdConfigBgpPolicyPrefixListTestFixture.reReferenceReportsExisting (140 ms) [ OK ] CmdConfigBgpPolicyPrefixListTestFixture.invalidBooleanOperatorRejected (85 ms) [ OK ] CmdConfigBgpPolicyPrefixListTestFixture.invalidIpVersionRejected (95 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.argValidation (86 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteExistingList (125 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteUnknownListRejected (55 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteUnknownLeavesOthersIntact (82 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteUnknownEntryRejected (117 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteEntryFromUnknownListRejected (81 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteReferencedListRejected (140 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.deleteReferencedListNamesEveryReferrer (173 ms) [ OK ] CmdDeleteBgpPolicyPrefixListTestFixture.referenceToOtherListDoesNotBlockDelete (272 ms) [==========] 23 tests from 2 test suites ran. (3099 ms total) [ PASSED ] 23 tests. ``` **Integration** — `fboss2_integration_test` on a lab device with `bgp_pp` active: ``` $ fboss2_integration_test --gtest_filter='ConfigBgpPolicyPrefixListTest.*' [ OK ] ConfigBgpPolicyPrefixListTest.SetListAttributesAndCommit (1320 ms) [ OK ] ConfigBgpPolicyPrefixListTest.DeleteListAndCommit (3355 ms) [==========] 2 tests from 1 test suite ran. [ PASSED ] 2 tests. ``` ## Community-list follow-ups (landed in #1475) Review on this PR flagged two gaps in the community-list commands that landed with #1475; the last commit here fixes them the way the as-path-list family already works: - bgpd resolves `communities_filter.community_list_names` (`CommunityMatch`) and `community_action_list_names` (`CommunityAction`) by name at config load and fails the load on a dangling one, and a community *action* additionally rejects a referenced list with no values (`"Missing communities in CommunityList"`). `delete ... community-list <name>` now refuses while any term references the list, and `delete ... community-list <name> community <value>` refuses to remove the last value while a community action references it. - A term's inline copy of the list must carry the same `boolean_operator` as the named list (`"Conflicting boolean_operator from the reference"`) and bgpd reads `exact_match` only from that inline copy, so setting either on the named list now writes through to every referencing match filter. Note: bgp++'s `CommunityMatch` constructor currently throws unless the inline `communities` is non-empty, so the CLI cannot yet create a by-name community-list *match*; the delete guard mainly protects action references and hand-written configs, and the write-through is forward-looking. Based-on: main Reviewed By: srikrishnagopu Differential Revision: D115057124 Pulled By: induvsuresh fbshipit-source-id: f59a7eb77184215c1e6e610c784e11e526095f32

Summary: > **Stacked on #1477 (#1477 is now rebased on `main`) (`config/delete protocol bgp policy prefix-list`). This PR's branch contains the whole stack below it, so the diff shows those commits too — please review only the `bgp policy prefix-list prefix` commit(s) here. Merge the lower PRs first. > > **Stack (bottom → top):** #1477 → **#1487 (this PR)** → #1488 → #1489 → #1490 → #1491 Adds the entry level of `config protocol bgp policy prefix-list` as its own CLI11 subcommand, `prefix-list <name> prefix <prefix/len> [<attribute> <value> ...]`, plus `delete ... prefix-list <name> prefix <prefix/len>`. **Entries are keyed by their prefix, not by a sequence number.** bgpd identifies a `PrefixListEntry` by `base_prefix`: it merges entries that share one, ignores order, and throws `"Unsupported Prefix configuration: seq_num"` on any entry carrying `seq_num`. A `seq-num` key therefore has nowhere to live that bgpd accepts, so the documented `entry <seq-num>` level is spelled `prefix <prefix/len>`: the prefix is the entry's identity and its `base_prefix`, the separate `base-prefix` attribute goes away, and nothing writes `seq_num`. One entry per prefix is the only shape the CLI needs; the documented grammar already exposes a single `prefix-len-range`, and bgpd merges the rest. Changing an entry's prefix is delete plus create, like every other keyed object. The list command owns only list attributes; this command owns the entry, including the composed `prefix-len-range` sub-attribute and the hand-written handlers whose value shapes have no factory (communities, prefix-len-range, regex). The list name reaches it through the ancestor-args tuple, and lookup/create is shared through `BgpPrefixListCliUtils.h`, which now keys entries by `base_prefix`. Only values bgpd accepts are offered: `match-logic EQUAL` only and the `EQ|GE|LE|NE|GT|LT` range operators (bgpd throws on `NOT_EQUAL` and `RG`); `max-allowed-subnet-count` is kept but documented as read only for golden-prefix policies. `bgp_json_to_cli.py` emits `prefix <base_prefix>` per entry, warns and skips the rejected values, drops a JSON `seq_num` with a warning, and skips an entry without `base_prefix`. A list attribute typed alongside `prefix` is rejected rather than silently dropped, because only the leaf command runs. ## Sample usage ``` $ fboss2-dev config protocol bgp policy prefix-list PL100 prefix 10.0.0.0/8 Successfully created BGP prefix-list PL100 prefix 10.0.0.0/8 Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy prefix-list PL100 prefix 10.0.0.0/8 prefix-len-range compare-operator GE Successfully set prefix-len-range compare-operator to: GE for prefix-list PL100 prefix 10.0.0.0/8 $ fboss2-dev config protocol bgp policy prefix-list PL100 prefix 10.0.0.0/8 prefix-len-range value 24 Successfully set prefix-len-range value to: 24 for prefix-list PL100 prefix 10.0.0.0/8 $ fboss2-dev config protocol bgp policy prefix-list PL100 prefix 10.0.0.0/8 communities 65000:100 Successfully added 65000:100 to communities for prefix-list PL100 prefix 10.0.0.0/8 $ fboss2-dev config protocol bgp policy prefix-list PL100 prefix 10.0.0.0/8 max-allowed-subnet-count 64 Successfully set max-allowed-subnet-count to: 64 for prefix-list PL100 prefix 10.0.0.0/8 $ fboss2-dev config protocol bgp policy prefix-list PL100 prefix 10.0.0.0 Error: Invalid prefix '10.0.0.0'; expected <prefix/len> $ fboss2-dev config protocol bgp policy prefix-list PL100 prefix 10.0.0.0/8 match-logic NOT_EQUAL Error: Invalid match-logic value 'NOT_EQUAL'; expected EQUAL $ fboss2-dev delete protocol bgp policy prefix-list PL100 prefix 10.0.0.0/8 Successfully deleted BGP prefix-list PL100 prefix 10.0.0.0/8 ``` Pull Request resolved: #1487 Test Plan: Bazel (`bazel.sh test //fboss/cli/fboss2/test/config:cmd_config_test`): full suite 1282/1282 pass, including the 15 `CmdConfigBgpPolicyPrefixListEntryTestFixture` and 8 `CmdDeleteBgpPolicyPrefixListTestFixture` cases; the converter suite (`test_bgp_json_to_cli.py`) passes. clang-tidy over the changed files reports only `modernize-type-traits` inside gtest's `EXPECT_THROW` expansion. Integration, `run_test.py fboss2_integration --filter="ConfigBgpPolicy*"` on a gold DUT prepared with the latest stable image (build 238327): ``` [ OK ] ConfigBgpPolicyPrefixListEntryTest.SetAttributesAndCommit [ OK ] ConfigBgpPolicyPrefixListEntryTest.DeleteEntryAndCommit PASSED : 12 FAILED : 0 ``` Based-on: #1477 ## Review follow-ups carried from the list-level PR - The entry-level delete selector, `prefixListEntryExists()` / `findOrCreatePrefixListEntry()` and their tests live here, so the entry subcommand and its delete side change together. - Entry identity is settled by keying on the prefix (see Summary); the earlier `seq-num` key was rejected by bgpd. - `match-logic` accepts only `EQUAL` and `prefix-len-range compare-operator` no longer offers `RG`: bgpd throws on both. `max-allowed-subnet-count` is kept but documented as read only for golden-prefix policies. j2c warns and skips the rejected values. Reviewed By: srikrishnagopu Differential Revision: D126102729 Pulled By: induvsuresh fbshipit-source-id: 295eaa6e7ded050c8aa4b9736bb6c6e4b22e59de

Summary: > **Stacked on #1487 (`bgp policy prefix-list entry`). This PR's branch contains the whole stack below it, so the diff shows those commits too — please review only the `config/delete protocol bgp policy routing-policy` commit(s) here. Merge the lower PRs first. > > **Stack (bottom → top):** #1477 → #1487 → **#1488 (this PR)** → #1489 → #1490 → #1491 Adds `config protocol bgp policy routing-policy <name> [description <string>]` and `delete protocol bgp policy routing-policy <name>`, staging a `BgpPolicyStatement` in the BGP config session. A bare `routing-policy <name>` creates (or selects) the statement; `description` is its only documented policy-level attribute. The nested term level is a separate stacked PR. The dispatcher follows the shape the other BGP policy families use: one factory per value shape in `BgpCliAttrHandlers.h`, named setters that do nothing but assign the thrift field, and a registry that is one line per attribute. Lookup/create helpers live in `BgpRoutingPolicyCliUtils.h` so the term subcommand and the delete command share them. A rejected value leaves nothing staged — a policy implicitly created for the failed command is rolled back. **Delete is idempotent.** Deleting an absent target is a success with a `Warning:` (nothing is saved), so a replayed script never fails on something already gone; the reference guards still return `Error:`. ## Sample usage ``` $ fboss2-dev config protocol bgp policy routing-policy EXPORT-POLICY Successfully created BGP routing-policy EXPORT-POLICY Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy routing-policy EXPORT-POLICY description Spine export policy Successfully set description to: Spine export policy for routing-policy EXPORT-POLICY Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev delete protocol bgp policy routing-policy EXPORT-POLICY Successfully deleted BGP routing-policy EXPORT-POLICY Config saved to: ~/.fboss2/bgp_config.json ``` Idempotent delete, recorded on a lab device: ``` $ fboss2-dev delete protocol bgp policy routing-policy NO-SUCH Warning: BGP routing-policy NO-SUCH does not exist; nothing to delete ``` Pull Request resolved: #1488 Test Plan: **Unit (delete)** — `fboss2_cmd_config_test`: ``` $ fboss2_cmd_config_test --gtest_filter=*DeleteBgpPolicyRoutingPolicy* [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.argValidation (108 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.deleteExistingPolicy (149 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.deleteUnknownPolicyWarns (87 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.deleteTwiceIsIdempotent (109 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.deleteUnknownLeavesOthersIntact (78 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.deleteReferencedPolicyRejected (165 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.deleteReferencedPolicyNamesEveryReferrer (188 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.referenceToOtherPolicyDoesNotBlockDelete (221 ms) [==========] 8 tests from 1 test suites ran. [ PASSED ] 8 tests. ``` **Unit** — `fboss2_cmd_config_test`: ``` $ fboss2_cmd_config_test --gtest_filter=*BgpPolicyRoutingPolicy* [ OK ] CmdConfigBgpPolicyRoutingPolicyTestFixture.argValidation (95 ms) [ OK ] CmdConfigBgpPolicyRoutingPolicyTestFixture.bareCreatePolicy (92 ms) [ OK ] CmdConfigBgpPolicyRoutingPolicyTestFixture.setDescription (89 ms) [ OK ] CmdConfigBgpPolicyRoutingPolicyTestFixture.namedPoliciesAreDistinct (208 ms) [ OK ] CmdConfigBgpPolicyRoutingPolicyTestFixture.reReferenceReportsExisting (85 ms) [ OK ] CmdConfigBgpPolicyRoutingPolicyTestFixture.emptyDescriptionRejected (54 ms) [ OK ] CmdConfigBgpPolicyRoutingPolicyTestFixture.rejectedValueOnExistingPolicyKeepsPolicy (73 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.argValidation (57 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.deleteExistingPolicy (112 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.deleteUnknownPolicyRejected (54 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.deleteUnknownLeavesOthersIntact (94 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.deleteReferencedPolicyRejected (101 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.deleteReferencedPolicyNamesEveryReferrer (151 ms) [ OK ] CmdDeleteBgpPolicyRoutingPolicyTestFixture.referenceToOtherPolicyDoesNotBlockDelete (257 ms) [==========] 14 tests from 2 test suites ran. (1530 ms total) [ PASSED ] 14 tests. ``` **Integration** — `fboss2_integration_test` on a lab device with `bgp_pp` active: ``` $ fboss2_integration_test --gtest_filter='ConfigBgpPolicyRoutingPolicyTest.*' [ OK ] ConfigBgpPolicyRoutingPolicyTest.SetDescriptionAndCommit (1277 ms) [ OK ] ConfigBgpPolicyRoutingPolicyTest.DeletePolicyAndCommit (3340 ms) [==========] 2 tests from 1 test suite ran. [ PASSED ] 2 tests. ``` Based-on: #1487 Reviewed By: joseph5wu Differential Revision: D126423390 Pulled By: induvsuresh fbshipit-source-id: 1f0100b5f6654d4656cbd485d616027a2c9f23ef

Adds `config protocol bgp policy routing-policy <name> term <seq-num> [description <string>]` and `delete protocol bgp policy routing-policy <name> term <seq-num>` — the seq-num-keyed term level of the routing-policy family. The term's action and match levels land as follow-up subcommands. Unlike the sibling policy families (which parse their nested level inside the parent's ObjectArgType), term is a real CLI11 subcommand with its own handler, mirroring the `config interface ... switchport` chain: the policy name arrives through the ancestor-args tuple. This required dropping positionals_at_end() from the routing-policy parent so CLI11 can classify `term` after the policy name; the trade-off (a policy-level attribute value spelling `term` is stolen by subcommand matching) is documented in the traits. The term subcommand keeps positionals_at_end() so its own value tokens are safe. A policy-level attribute mixed with a term command (`routing-policy X description y term 10`) is rejected instead of silently dropping the description, since only the leaf handler runs. - term dispatcher keyed by (policy name from parent) + seq-num, writing bgp_policy.BgpPolicyTerm in policy_entries[]. Term level: description term name from the seq-num (next_term_id references terms by name). - shared BgpRoutingPolicyCliUtils.h lookup/create helpers, used by the policy-level dispatcher, the term subcommand, and delete. - delete removes a single term by seq-num; the policy and other terms stay. - unit tests (15) + commit-path integration tests that exercise the CLI11 subcommand dispatch through the full parse and verify terms in bgpd's running config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Each policy term becomes `... routing-policy <name> term <seq-num>` lines (description), emitted inside the routing-policy generator; the nested action and match generators hook in here. A term without sequence_number cannot be addressed by the CLI and is emitted as a `# WARNING:` line. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Delete mirrors add: deleting a term from an absent policy, or an absent term, is a success with a `Warning:` and saves nothing, so a replayed script stays idempotent. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Use the IS_OSS include split for the bgp_config/bgp_policy thrift headers and ensure() the optional policies field in the term rollback, matching what landed for meta#1488. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ction commands Adds `config protocol bgp policy routing-policy <name> term <seq-num> action result <ACCEPT|REJECT|CONTINUE>` and `... action set <attribute> <value> ...` — the action level of a routing-policy term. `action` is a pure grouping node; `result` and `set` are its subcommands, each with its own handler (the policy and term args arrive through the ancestor-args tuple). The term traits drop positionals_at_end() so CLI11 can classify `action` (and later `match`) after the seq-num; ancestor attributes mixed with an action command are rejected since only the leaf runs. `set` MUST be a real subcommand rather than a parsed token: CLI11's _valid_subcommand walks the parent chain unconditionally, so a bare `set` arg token inside `action` was classified as the top-level `set` VERB and stolen ("The following arguments were not expected"). A local subcommand named `set` shadows the verb because _find_subcommand checks the local scope first. ACCEPT/DENY/NEXT_TERM in term_miss_action. GOTO-TERM stays deferred: no FlowControlAction arm, and the per-action-entry next_term_id target has no slot in the documented grammar. - `set <attr>` writes one bgp_policy.BgpPolicyAction entry per action kind in policy_entries[].policy_action_entries[] (keyed by which payload field is set; re-issuing a kind updates its entry): repeat_times}; the documented <asn-list> must be uniform since the thrift models one ASN repeated N times single-member CommunityList + route_action COMMUNITY_LIST_ADD/SET (bgpd rejects set_self) and `peer-address` (no thrift arm) are deferred - unit tests (14) + a commit-path integration test staging every action kind and verifying each entry in bgpd's running config. Also bumps CmdArgsLists MAX_DEPTH from 5 to 6: term action is the first six-level nested command (config protocol bgp policy routing-policy <n> term <s> action). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

A term's term_miss_action becomes `action result ACCEPT|REJECT|CONTINUE` (omitted for the NEXT_TERM default) and each policy_action_entries item one `action set` line, dispatched by BgpPolicyActionType: as-path prepend (asn repeated repeat_times), community [additive], local-pref, med (SET only), next-hop, origin and weight. Everything else (next-hop self, MED UPDATE/IGP, multi-community actions, GOTO and the other flow actions) is emitted as a `# WARNING:` line. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

`action result` wrote term_miss_action, which bgpd applies when the term does NOT match. The action on a match is the term's single flow-control entry in policy_action_entries (PERMIT/DENY/CONTINUE/GOTO), defaulting to permit. As a result, ACCEPT failed bgpd's config load (the FlowControlAction miss path has no ACCEPT case), REJECT accepted matching routes and denied everything else, and CONTINUE was a no-op. `result` now replaces the flow-control entry with PERMIT/DENY/CONTINUE (bgpd rejects more than one per term) and leaves term_miss_action alone. bgp_json_to_cli.py emits `result` from that entry and warns on a non-default term_miss_action. `action set community` now rejects anything bgpd's parseCommunityStr would not accept (two uint16 parts separated by one ':'). bgpd treats any other value as a regex, which it only allows for REMOVE, so a typo used to fail the daemon's config load.

Same IS_OSS include split and ensure() on the optional policies field as landed for meta#1488, applied to the term action files. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Adds `config protocol bgp policy routing-policy <name> term <seq-num> match from <attribute> <value>` — the match level of a routing-policy term, as a CLI11 subcommand with its own handler alongside `action`; the policy and term args arrive through the ancestor-args tuple, and ancestor attributes mixed with a match command are rejected since only the leaf runs. Scoped to the three match kinds bgpd actually implements. Reading private-BGP's PolicyTerm.cpp turned up two things the sheet's Thrift reference column does not convey, but its JSON field path column does: 1. bgpd reads BgpPolicyTerm.policy_match_entries — a single BgpPolicyMatch, marked @thrift.Deprecated. Its match loop is guarded by `if (term.policy_match_entries())`, and NOTHING in private-BGP reads the newer `policy_matches` list, so matches written there are silently ignored: the config round-trips through getRunningConfig while bgpd applies no match at all. Same for the atomic payloads — bgpd reads the deprecated as_path_filters/communities_filter/prefix_filters, not the newer as_path_list/community_list/prefix_list. 2. createPolicyAttributeMatchItem has no LOCAL_PREFERENCE, MED or NEXT_HOP case, so those atomic types hit `default:` and throw "BgpPolicyAtomicMatch Config input error for type", crash-looping the daemon. `from local-pref`, `from med` and `from next-hop` offered; they need a private-BGP change first, and a unit test pins that they stay rejected. CommunityMatch's constructor throws "The attribute \"communities\" is empty" unless the inline communities list is non-empty, and CommunityMatch::PopulateReferences -- the only thing that resolves community_list_names -- runs later, from PolicyManager, once every Policy is already constructed. The by-name reference path is therefore unreachable, so only inlined values would work, which is a snapshot rather than a reference. Same unit test pins it as rejected. - `from <attr>` writes one BgpPolicyAtomicMatch per match kind (keyed by BgpPolicyAtomicMatchType; re-issuing a kind updates its entry) into policy_match_entries, composing under its default AND — which is also the only operator bgpd accepts for more than one entry: The two list-typed matches take a full inline object with no name-only union arm, and bgpd treats that object's `name` as the reference: the match class keeps the name, then PolicyManager::PopulateReferences swaps in the real list from its by-name map. A name-only object is exactly how a by-name match is expressed. - setters are named functions, so the registry reads as a table: one line per attribute giving its dispatch key, value shape, and setter. - unit tests (8) + a commit-path integration test. The integration assertions are load-bearing here in a way staged-config ones are not: bgpd constructs a Policy from every statement, so a wrong encoding crash-loops it and fails the test rather than sitting inert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

A term's policy_match_entries (the field bgpd reads; policy_matches is not) become `match from as-path-list|origin|prefix-list <value>` lines. Match types the CLI cannot express, a non-AND match_logic_type, extra list names and policy_matches are emitted as `# WARNING:` lines. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…erator `match from as-path-list <name>` accepted any name, so a misspelled or forward reference committed a dangling as_path_list_names entry; bgpd fails the config load on it and crash-loops. This is the mirror of the as-path-list delete guard: the match now refuses a name that does not exist, before the policy or term is created, and copies the list's boolean_operator into the term's inline copy, which bgpd requires to agree with the list ("Conflicting boolean_operator"). The unit fixture seeds the list the way the as-path-list command stores it; the integration test already created its list before matching on it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

`regex <re>` writes the as_paths field bgpd actually matches on; there is no nested entry subcommand. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

PrefixTreeMatch::PopulateReferences resolves prefix_list_names by name at config load and throws "Could not find PrefixList reference" on a dangling one, failing the whole load. `from as-path-list` already refuses an unknown name for the same reason; do the same for prefix-lists. No operator sync is needed: bgpd compares none for a prefix-list reference. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Same IS_OSS include split and ensure() on the optional policies field as landed for meta#1488, applied to the term match files. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The action level now maps `action result` to policy_action_entries, so term_miss_action no longer produces that command. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

github-actions Bot added the nexthop label Aug 7, 2026

meta-cla Bot added the CLA Signed label Aug 7, 2026

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-match branch from 2fc0379 to e1f4a4e Compare August 10, 2026 04:44

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-match branch 3 times, most recently from 5b81d36 to d4d0b9b Compare August 10, 2026 05:52

hillol-nexthop marked this pull request as ready for review August 10, 2026 09:54

hillol-nexthop requested review from a team as code owners August 10, 2026 09:54

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-match branch 3 times, most recently from 8420b52 to 4dd273c Compare August 11, 2026 05:52

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-match branch 5 times, most recently from de4f1a9 to 8b34fc2 Compare August 27, 2026 04:19

github-actions Bot removed the nexthop label Oct 8, 2026

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-match branch 2 times, most recently from 69ed41a to 4f04d8b Compare October 9, 2026 18:02

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-match branch 2 times, most recently from 6b1610c to a7b944d Compare October 16, 2026 06:56

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-match branch from a7b944d to 95cb234 Compare October 21, 2026 11:14

hillol-nexthop and others added 18 commits October 23, 2026 18:34

Make BGP routing-policy term delete idempotent

7241ca2

Delete mirrors add: deleting a term from an absent policy, or an absent term, is a success with a `Warning:` and saves nothing, so a replayed script stays idempotent. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Use the canonical Meta copyright header in new files

596cb78

Use the canonical Meta copyright header in new files

5fc8143

Mirror the Meta import amendments from the routing-policy level

74a0b14

Same IS_OSS include split and ensure() on the optional policies field as landed for meta#1488, applied to the term action files. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Seed the term-match test's as-path-list with the flat regex attribute

f8d74e1

`regex <re>` writes the as_paths field bgpd actually matches on; there is no nested entry subcommand. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Use the canonical Meta copyright header in new files

b2a865d

Mirror the Meta import amendments from the routing-policy level

be7a00b

Same IS_OSS include split and ensure() on the optional policies field as landed for meta#1488, applied to the term match files. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Seed the term-match emitter test's result via a PERMIT action entry

134d9a9

The action level now maps `action result` to policy_action_entries, so term_miss_action no longer produces that command. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-match branch from 95cb234 to 134d9a9 Compare October 23, 2026 18:37

github-actions Bot added nexthop project:fboss2-dev FBOSS project: fboss2-dev labels Oct 23, 2026

Navigation Menu

Repository navigation

[Nexthop][fboss2-dev] fboss2 bgp policy routing-policy term match commands - #1491

[Nexthop][fboss2-dev] fboss2 bgp policy routing-policy term match commands#1491
hillol-nexthop wants to merge 18 commits into
meta:mainfrom
nexthop-ai:bgp-policy-routing-policy-term-match

hillol-nexthop commented Aug 7, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

hillol-nexthop commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sample usage

Test plan

Review follow-up

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

hillol-nexthop commented Aug 7, 2026 •

edited

Loading