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

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

Stacked on #1489 (bgp policy routing-policy term). 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 action commit(s) here. Merge the lower PRs first.

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

Adds the action level of a routing-policy term:

... term <seq-num> action result <ACCEPT|REJECT|NEXT_TERM|NEXT_POLICY>
... term <seq-num> action set as-path-prepend <asn> [<asn> ...]
... term <seq-num> action set community <value> [additive]
... term <seq-num> action set local-pref <0-4294967295>
... term <seq-num> action set med <0-4294967295>
... term <seq-num> action set next-hop <ip-address>
... term <seq-num> action set origin <IGP|EGP|INCOMPLETE>
... term <seq-num> action set weight <0-65535>

action is a grouping node with result and set leaves. set has to be a local subcommand of action rather than a bare keyword: CLI11 walks the whole parent chain when validating a subcommand name, so an unqualified set matched the root set verb and the leaf received empty args.

Two daemon constraints shape what is written and what is refused. Each entry stamps the deprecated BgpPolicyAction.type field, because that is the field bgpd dispatches on — leaving it unset makes bgpd reject the config (type: 0) and crash-loop. Communities are written to the deprecated community_action for the same reason. next-hop self is refused up front, since bgpd rejects that spelling.

Sample usage

$ fboss2-dev config protocol bgp policy routing-policy EXPORT-POLICY term 10 action result CONTINUE
Successfully set result to: CONTINUE for routing-policy EXPORT-POLICY term 10

$ fboss2-dev config protocol bgp policy routing-policy EXPORT-POLICY term 10 action set as-path prepend 65000 65000
Successfully set as-path prepend to: 65000 x2 for routing-policy EXPORT-POLICY term 10

$ fboss2-dev config protocol bgp policy routing-policy EXPORT-POLICY term 10 action set community 65000:100 additive
Successfully set community to: 65000:100 (additive) for routing-policy EXPORT-POLICY term 10

$ fboss2-dev config protocol bgp policy routing-policy EXPORT-POLICY term 10 action set next-hop 2001:db8::99
Successfully set next-hop to: 2001:db8::99 for routing-policy EXPORT-POLICY term 10

Test plan

Unit — fboss2_cmd_config_test:

$ fboss2_cmd_config_test --gtest_filter=*TermAction*
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.argValidation (90 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.setResult (129 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.gotoTermRejected (56 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.setAsPathPrepend (69 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.mixedAsnPrependRejected (67 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.setCommunity (99 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.setLocalPref (77 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.setMed (100 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.setNextHop (120 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.setOrigin (75 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.setWeight (82 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.actionKindsGetTheirOwnEntries (137 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.rejectedValueRollsBackPhantoms (70 ms)
[       OK ] CmdConfigBgpPolicyRoutingPolicyTermActionTestFixture.ancestorAttributeAlongsideActionRejected (77 ms)
[==========] 14 tests from 1 test suite ran. (1255 ms total)
[  PASSED  ] 14 tests.

Integration — fboss2_integration_test on a lab device with bgp_pp active:

$ fboss2_integration_test --gtest_filter='ConfigBgpPolicyRoutingPolicyTermActionTest.*'
[       OK ] ConfigBgpPolicyRoutingPolicyTermActionTest.SetActionsAndCommit (2460 ms)
[==========] 1 test from 1 test suite ran.
[  PASSED  ] 1 test.

Based-on: #1489

Summary: > **Stacked on #1391 (`config/delete protocol bgp neighbor`). 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 peer-group` commit(s) here. Merge the lower PRs first. > > **Stack (bottom → top):** #1345 → #1391 → **#1395 (this PR)** → #1401 → #1475 → #1477 → #1487 → #1488 → #1489 → #1490 → #1491 Collapses the ~20 legacy per-attribute `config protocol bgp peer-group` command classes (built on the deprecated `folly::dynamic` `BgpConfigSession`) into a single typed dispatcher, mirroring `CmdConfigProtocolBgpNeighbor` (#1391). The group name is the first positional token; the next one or two tokens name the attribute (matched longest-prefix-first, so `timers hold-time` wins over any `timers` prefix); the rest are its value(s). Handlers mutate the typed `bgp::thrift::PeerGroup` directly via `ConfigSession::getBgpConfig()` / `saveBgpConfig()`, so adding a tunable is a one-entry change in the dispatch table rather than a new command class. Also adds `delete protocol bgp peer-group <name>`. Deleting a group that a neighbor's `peer_group_name` still references is refused (the error names the referencing neighbors), so a commit can never carry a dangling group reference. Covers the 36 dispatch keys that map to a `PeerGroup` field bgpd reads from a group: `remote-asn`/`local-asn` (4-byte-bounded), `description`, `peer-tag`, `ingress-policy`/`egress-policy`, `rr-client`, `confed-peer`, `redistribute-peer`, `enhanced-route-refresh`, `route-refresh`, `remove-private-as`, `enforce-first-as`, `passive`, `ttl-security-hops`, `add-path send|receive`, `afi disable-ipv4-afi|disable-ipv6-afi|ipv4-over-ipv6-nh`, `graceful-restart restart-time|stateful-ha`, `max-route pre-filter|post-filter` (+ the warning-threshold/warning-only knobs), `timers hold-time|keepalive|out-delay|withdraw-unprog-delay`, `next-hop-self`, `link-bandwidth`, `advertise-lbw`, `receive-lbw`. Spellings and validation are identical to the neighbor command, so the two share one attribute grammar. Deliberately not exposed (documented in the dispatcher): `local_addr`, `next_hop4`, `next_hop6` (bgpd reads them per peer only, with no group fallback), `enabled`, `router_port_id` and `bgp_peer_timers.graceful_restart_end_of_rib_seconds` (read nowhere in bgpd). ### Behavior notes - **`passive true|false`** exposes `is_passive` directly: bgpd reads true as PASSIVE_ONLY and false as PASSIVE_ACTIVE (listen and connect); the config model has no way to request active-only. `add-path send|receive` merges into the `AddPath` enum bitmask (RECEIVE/SEND/BOTH), and clearing the last direction unsets the field. - **Inherited timers are preserved.** bgpd applies a group's `bgp_peer_timers` to its members as a whole struct, and hold/keepalive are non-optional i32 (default 0 = no keepalives). The first timer attribute set on a group seeds the struct from the global hold time (keepalive = hold/3, bgpd's own fallback) and then changes the one field. - **Route limits are seeded unlimited, not 12000.** A group with no `pre_filter`/`post_filter` imposes no limit on its members (`AdjRib::capRoutesPerPeer` returns early on an absent struct), and bgpd reads `max_routes == 0` as unlimited. So the first `max-route ...-warning-threshold` / `...-warning-only` edit seeds `{max_routes 0, warning_only false, warning_limit 0}` and changes only the requested field, rather than materialising the thrift default `max_routes = 12000` — a hard cap with session teardown the operator never asked for. - **Handlers edit a copy** of the group, so a rejected value leaves nothing seeded behind (and a rejected first attribute leaves no half-created group). - **Validation matches bgpd**: hold time 0 or 3..65535 (bgpd refuses 1-2, the OPEN field is 16 bits), graceful-restart 0..4095 (RFC 4724), TTL security hops 1..255 (`kMin/kMaxTtlSecurityHops`; bgpd throws at load otherwise), `link-bandwidth` accepts what `parseLinkBandwidthBps` accepts (`auto`, decimals, case-insensitive K/M/G). - **`bgp_json_to_cli.py`** generates the peer-group grammar above from the per-shape tables shared with the neighbor generator (only the scalar field list differs), so a fully populated `PeerGroup` converts with nothing dropped: `test_complete_peer_group_loses_no_field` asserts one command per leaf field (36). The deprecated i32 `remote_as`/`local_as` map to the 4-byte attribute. Fields bgpd never reads from a group are reported as `# WARNING: peer-group <name>: field <f> has no CLI equivalent, not converted` comment lines in the generated script (and on stderr) instead of being dropped silently. - **Attributes with no per-peer-group thrift field** — `afi ipv4-labeled-unicast`, `afi ipv6-labeled-unicast`, `peer-port` — are not part of the dispatch table, so they are refused at parse time as unknown attributes; a rejected value never lands on disk. - **CLI11 token stealing fixed** (same fix as the neighbor command): `positionals_at_end()` stops parent-chain subcommand fallthrough from reclassifying an attribute token that matches a sibling command name once the group name has been consumed. - Integration tests verify the committed peer-group config through the daemon's `getRunningConfig` RPC — proving `bgpd` parsed and adopted the promoted `peer_groups` rather than only checking the file the CLI wrote. **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 peer-group SPINE remote-asn 65510 Successfully set remote-asn to: 65510 for peer-group SPINE $ fboss2-dev config protocol bgp peer-group SPINE timers hold-time 90 Successfully set timers hold-time to: 90 seconds for peer-group SPINE $ fboss2-dev config protocol bgp peer-group SPINE passive true Successfully enabled passive for peer-group SPINE $ fboss2-dev config protocol bgp peer-group SPINE max-route pre-warning-threshold 30000 Successfully set max-route pre-warning-threshold to: 30000 for peer-group SPINE $ fboss2-dev config protocol bgp peer-group SPINE timers hold-time 2 Error: timers hold-time must be 0 or between 3 and 65535 seconds, got 2 $ fboss2-dev delete protocol bgp peer-group SPINE Error: BGP peer-group SPINE is still referenced by neighbor(s): 192.0.2.1; delete or re-point those neighbors first ``` Idempotent delete, recorded on a lab device: ``` $ fboss2-dev delete protocol bgp peer-group NO-SUCH-GROUP Warning: BGP peer-group NO-SUCH-GROUP does not exist; nothing to delete ``` Pull Request resolved: #1395 Test Plan: **Unit (delete)** — `fboss2_cmd_config_test`: ``` $ fboss2_cmd_config_test --gtest_filter=*DeleteBgpPeerGroup* [ OK ] CmdDeleteBgpPeerGroupTestFixture.argValidation (91 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteExistingGroup (195 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteUnknownGroupWarns (84 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteTwiceIsIdempotent (149 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteUnknownLeavesOthersIntact (102 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteReferencedGroupRejected (92 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteUnreferencedGroupSucceeds (124 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteThenRecreateStartsClean (174 ms) [==========] 8 tests from 1 test suites ran. [ PASSED ] 8 tests. ``` **Unit** — `fboss2_cmd_config_test`: ``` $ fboss2_cmd_config_test --gtest_filter=*BgpPeerGroup* [ OK ] CmdConfigBgpPeerGroupTestFixture.argValidation (94 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.bareCreate (106 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.setRemoteAsn (79 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.timersAccumulate (187 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.addPathMerge (152 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.passive (97 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.boolAndStringAttributes (765 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.routeLimits (369 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.linkBandwidthAttributes (211 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.ttlSecurityHopsRange (108 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.valueValidation (82 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.wrongValueCountRejected (76 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.overwriteSameAttributeLastWins (137 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.namedGroupsAreDistinct (128 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.timersSeededFromGlobal (102 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.timersNotReseededOncePresent (161 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.routeLimitSeededUnlimited (122 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.routeLimitNotReseededOncePresent (115 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.timerRanges (193 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.rejectedValueLeavesGroupUntouched (114 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.invalidValuesRejected (66 ms) [ OK ] CmdConfigBgpPeerGroupTestFixture.unsupportedAttributesRejected (65 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.argValidation (69 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteExistingGroup (118 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteUnknownGroupRejected (59 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteUnknownLeavesOthersIntact (95 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteReferencedGroupRejected (103 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteUnreferencedGroupSucceeds (148 ms) [ OK ] CmdDeleteBgpPeerGroupTestFixture.deleteThenRecreateStartsClean (128 ms) [==========] 29 tests from 2 test suites ran. (4261 ms total) [ PASSED ] 29 tests. ``` **Integration** — `fboss2_integration_test` on a lab device with `bgp_pp` active: ``` $ fboss2_integration_test --gtest_filter='ConfigBgpPeerGroupTest.*' [ OK ] ConfigBgpPeerGroupTest.SetRemoteAsnAndCommit (11580 ms) [ OK ] ConfigBgpPeerGroupTest.SetTimersHoldTimeAndCommit (14385 ms) [ OK ] ConfigBgpPeerGroupTest.SetPassiveAndCommit (11728 ms) [ OK ] ConfigBgpPeerGroupTest.SessionKnobsAndCommit (12531 ms) [ OK ] ConfigBgpPeerGroupTest.RouteLimitWarningKeepsUnlimitedCap (11697 ms) [ OK ] ConfigBgpPeerGroupTest.DeleteGroupAndCommit (19856 ms) [ PASSED ] 6 tests. ``` Based-on: #1391 Reviewed By: induvsuresh Differential Revision: D114376290 Pulled By: hjkim417 fbshipit-source-id: d643ae0ecd569b12abfbb42d79a08151282349ad

Summary: > **Stacked on #1395 (`config/delete protocol bgp peer-group`). 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 as-path-list` commit(s) here. Merge the lower PRs first. > > **Stack (bottom → top):** #1345 → #1391 → #1395 → **#1401 (this PR)** → #1475 → #1477 → #1487 → #1488 → #1489 → #1490 → #1491 Adds the first BGP **policy** object-type command family to `fboss2-dev`, under a new `policy` grouping node beneath `protocol bgp` (sibling to `global`/`neighbor`/`peer-group`): - `config protocol bgp policy as-path-list <name> [description <string>]` - `delete protocol bgp policy as-path-list <name>` Patterns are flat `regex <regex>` attributes (one per invocation): bgpd matches on the flat `as_paths` string list and never reads the structured `as_path_list[]` entries, so the list has no nested entry level. ### Commands | Command | Thrift target | |---|---| | `as-path-list <name>` | `aspath_lists[]` (create/select) | | `as-path-list <name> description <string>` | `AsPathList.description` | | `delete ... as-path-list <name>` | removes the `aspath_lists[]` entry | ### Design - Follows the dispatcher shape the neighbor/peer-group families use: one factory per value shape from the shared `BgpCliAttrHandlers.h` (added in #1391), named setters that do nothing but assign the thrift field, and a registry that is one line per attribute. Writes `bgp_policy.BgpPolicies.aspath_lists[]` through the typed `ConfigSession` — no new session plumbing, `BgpConfig.policies` already exists. - Lookup/create helpers live in `BgpAsPathListCliUtils.h`, shared with the delete command. - A rejected value leaves nothing staged: a list implicitly created for the failed command is rolled back. ## Sample usage Recorded on a lab device. ``` $ fboss2-dev config protocol bgp policy as-path-list AS65000 Successfully created BGP as-path-list AS65000 Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy as-path-list AS65000 regex ^65000_ Successfully added regex ^65000_ for as-path-list AS65000 Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy as-path-list AS65000 regex ^65000_ regex ^65000_ already present for as-path-list AS65000 Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy as-path-list AS65000 regex ^65000 65001$ Error: regex '^65000 65001$' must not contain whitespace; bgpd renders the AS path with `_` between AS numbers $ fboss2-dev config protocol bgp policy as-path-list AS65000 regex [ Error: Malformed regex '[': Character set declaration starting with [ terminated prematurely - either no ] was found or the set had no content. The error occurred while parsing the regular expression: '[>>>HERE>>>'. $ fboss2-dev config protocol bgp policy as-path-list AS65000 boolean-operator AND Successfully set boolean-operator to: AND for as-path-list AS65000 Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy as-path-list AS65000 boolean-operator NOT Error: Invalid boolean-operator value 'NOT'; expected AND|OR $ fboss2-dev delete protocol bgp policy as-path-list AS65000 regex ^65000_ Successfully deleted BGP as-path-list AS65000 regex ^65000_ Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev delete protocol bgp policy as-path-list AS65000 regex ^65000_ Warning: BGP as-path-list AS65000 has no regex ^65000_; nothing to delete $ fboss2-dev delete protocol bgp policy as-path-list NO-SUCH Warning: BGP as-path-list NO-SUCH does not exist; nothing to delete ``` Reference guard: ``` $ fboss2-dev delete protocol bgp policy as-path-list AS65000 Error: BGP as-path-list AS65000 is still referenced by policy RM100 term 10; remove those matches first ``` <details><summary>Previous hand-written sample</summary> ``` $ fboss2-dev config protocol bgp policy as-path-list AS65000 Successfully created BGP as-path-list AS65000 Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy as-path-list AS65000 description Upstream transit paths Successfully set description to: Upstream transit paths for as-path-list AS65000 Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy as-path-list AS65000 regex ^65000_ Successfully added regex ^65000_ for as-path-list AS65000 Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy as-path-list AS65000 regex ^65000_ regex ^65000_ already present $ fboss2-dev config protocol bgp policy as-path-list AS65000 boolean-operator AND Successfully set boolean-operator to: AND for as-path-list AS65000 Config saved to: ~/.fboss2/bgp_config.json # A malformed regex is rejected before anything is staged (bgpd compiles these # with boost::regex at load, so a bad one would fail the daemon after commit): $ fboss2-dev config protocol bgp policy as-path-list AS65000 regex [ Error: Malformed regex '[': Invalid range end # Removing one expression, and the reference guard on the whole list: $ fboss2-dev delete protocol bgp policy as-path-list AS65000 regex ^65000_ Successfully deleted BGP as-path-list AS65000 regex ^65000_ $ fboss2-dev delete protocol bgp policy as-path-list AS65000 Error: BGP as-path-list AS65000 is still referenced by policy EXPORT-POLICY term 10; remove those matches first ``` </details> Pull Request resolved: #1401 Test Plan: **Unit** — `fboss2_cmd_config_test`: ``` $ fboss2_cmd_config_test --gtest_filter=*BgpPolicyAsPathList* [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.argValidation (86 ms) [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.bareCreateList (207 ms) [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.setListDescription (145 ms) [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.namedListsAreDistinct (222 ms) [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.reReferenceReportsExisting (164 ms) [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.unknownAttributeRejected (89 ms) [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.regexAppends (151 ms) [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.regexAddAgainIsIdempotent (165 ms) [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.malformedRegexRejected (89 ms) [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.regexWithWhitespaceRejected (123 ms) [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.booleanOperatorAccepted (193 ms) [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.booleanOperatorNotRejected (73 ms) [ OK ] CmdConfigBgpPolicyAsPathListTestFixture.booleanOperatorPropagatesToReferencingTerms (199 ms) [ OK ] CmdDeleteBgpPolicyAsPathListTestFixture.argValidation (77 ms) [ OK ] CmdDeleteBgpPolicyAsPathListTestFixture.deleteExistingList (126 ms) [ OK ] CmdDeleteBgpPolicyAsPathListTestFixture.deleteUnknownListWarns (62 ms) [ OK ] CmdDeleteBgpPolicyAsPathListTestFixture.deleteTwiceIsIdempotent (116 ms) [ OK ] CmdDeleteBgpPolicyAsPathListTestFixture.deleteUnknownLeavesOthersIntact (96 ms) [ OK ] CmdDeleteBgpPolicyAsPathListTestFixture.deleteReferencedListRejected (125 ms) [ OK ] CmdDeleteBgpPolicyAsPathListTestFixture.deleteReferencedListNamesEveryReferrer (139 ms) [ OK ] CmdDeleteBgpPolicyAsPathListTestFixture.referenceToOtherListDoesNotBlockDelete (145 ms) [ OK ] CmdDeleteBgpPolicyAsPathListTestFixture.rejectedDeleteDoesNotStageWhenNothingStaged (68 ms) [ OK ] CmdDeleteBgpPolicyAsPathListTestFixture.deleteOneRegex (211 ms) [ OK ] CmdDeleteBgpPolicyAsPathListTestFixture.deleteUnknownRegexWarns (90 ms) [ OK ] CmdDeleteBgpPolicyAsPathListTestFixture.deleteRegexTwiceIsIdempotent (167 ms) [==========] 25 tests from 2 test suites ran. [ PASSED ] 25 tests. ``` **Integration** — `fboss2_integration_test` on a lab device with `bgp_pp` active: ``` $ fboss2_integration_test --gtest_filter='ConfigBgpPolicyAsPathListTest.*' [ OK ] ConfigBgpPolicyAsPathListTest.SetListDescriptionAndCommit (1275 ms) [ OK ] ConfigBgpPolicyAsPathListTest.SetRegexAndOperatorAndCommit (2316 ms) [ OK ] ConfigBgpPolicyAsPathListTest.MalformedRegexRejected (1054 ms) [ OK ] ConfigBgpPolicyAsPathListTest.DeleteListAndCommit (2357 ms) [==========] 4 tests from 1 test suite ran. [ PASSED ] 4 tests. ``` Based-on: #1395 Reviewed By: KevinYakar Differential Revision: D114376328 Pulled By: induvsuresh fbshipit-source-id: 4de3901a3de1ad7fd46403e4ba1bee754c8a3df4

Summary: > **Stacked on #1401 (`config/delete protocol bgp policy as-path-list`). 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 community-list` commit(s) here. Merge the lower PRs first. > > **Stack (bottom → top):** #1345 → #1391 → #1395 → #1401 → **#1475 (this PR)** → #1477 → #1487 → #1488 → #1489 → #1490 → #1491 Adds the second BGP **policy** object-type command family to `fboss2-dev`: - `config protocol bgp policy community-list <name> [<attribute> <value>]` - `delete protocol bgp policy community-list <name> [community <community>]` Community values are **flat** `community <community>` attributes, one per invocation. bgpd matches on the flat `communities` string list (a literal `<asn>:<value>` or a regex, compiled at load) and never reads the structured `members[]`, so a nested member level would write a field the daemon ignores. The same rule already shapes as-path-list (#1401): flat repetition where the daemon stores a flat string list, nested entries only where it stores objects (prefix-list, routing-policy). ### Commands | Command | Thrift target | |---|---| | `community-list <name>` | `community_lists[]` (create/select) | | `community-list <name> description <string>` | `CommunityList.description` | | `community-list <name> community <community>` | appends to `CommunityList.communities` (idempotent) | | `community-list <name> boolean-operator <AND\|OR>` | `CommunityList.boolean_operator` | | `community-list <name> exact-match <bool>` | `CommunityList.exact_match` | | `delete ... community-list <name>` | removes the `community_lists[]` entry | | `delete ... community-list <name> community <community>` | removes one value (list survives) | ### Design - Same dispatcher shape as as-path-list: value-shape factories from the shared `BgpCliAttrHandlers.h` (#1391), named setters, one registry line per attribute. Writes `bgp_policy.BgpPolicies.community_lists[]` through the typed `ConfigSession`. - `community <community>` is a hand-written handler like as-path-list's `regex`: one token, no whitespace, compiled with `boost::regex` so a value bgpd would reject at load never reaches the session; append-if-absent, `already present` on repeat. - `boolean-operator` offers only `AND|OR`: bgpd treats every operator other than OR as "all must match", so `NOT` would silently behave as AND. - Delete is idempotent: an absent list or value is a success with a `Warning:` and saves nothing. Deleting the last value resets `communities` so a later add starts from the same shape. - A rejected value never leaves a phantom list behind: the dispatcher rolls back a list it implicitly created in the same invocation. - `bgp_json_to_cli.py` emits one `community` line per value and warns on `NOT`, `members` and `community_list_names` instead of dropping them. ## Sample usage Recorded on a lab device. ``` $ fboss2-dev config protocol bgp policy community-list PROD Successfully created BGP community-list PROD Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy community-list PROD community 65000:100 Successfully added community 65000:100 for community-list PROD Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy community-list PROD community 65000:100 community 65000:100 already present for community-list PROD Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy community-list PROD community ^65000:.*$ Successfully added community ^65000:.*$ for community-list PROD Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy community-list PROD community 65000:100 65000:200 Error: community '65000:100 65000:200' must not contain whitespace; repeat the command once per community $ fboss2-dev config protocol bgp policy community-list PROD community 65000:( Error: Malformed community '65000:(': Unmatched marking parenthesis ( or \(. The error occurred while parsing the regular expression: '65000:(>>>HERE>>>'. $ fboss2-dev config protocol bgp policy community-list PROD exact-match true Successfully enabled exact-match for community-list PROD Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy community-list PROD boolean-operator AND Successfully set boolean-operator to: AND for community-list PROD Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp policy community-list PROD boolean-operator NOT Error: Invalid boolean-operator value 'NOT'; expected AND|OR $ fboss2-dev delete protocol bgp policy community-list PROD community 65000:100 Successfully deleted BGP community-list PROD community 65000:100 Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev delete protocol bgp policy community-list PROD community 65000:100 Warning: BGP community-list PROD has no community 65000:100; nothing to delete $ fboss2-dev delete protocol bgp policy community-list NO-SUCH Warning: BGP community-list NO-SUCH does not exist; nothing to delete ``` After `config session commit`, bgpd's running config carries the value where it is matched: ``` "community_lists": [ { "boolean_operator": 2, "communities": [ "65000:100" ], "description": "", "name": "FBOSS2-TX-CL" } ], ``` Pull Request resolved: #1475 Test Plan: **Unit** — `fboss2_cmd_config_test`: ``` $ fboss2_cmd_config_test --gtest_filter=*BgpPolicyCommunityList* [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.argValidation (62 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.bareCreateList (204 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.setListDescription (105 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.setBooleanOperator (121 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.booleanOperatorNotRejected (66 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.booleanOperatorDefaultIsOr (160 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.setExactMatch (178 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.namedListsAreDistinct (249 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.reReferenceReportsExisting (194 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.invalidBooleanOperatorRejected (94 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.invalidExactMatchRejected (82 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.communityAppends (284 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.communityAcceptsRegex (149 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.communityAddAgainIsIdempotent (187 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.malformedCommunityRejected (103 ms) [ OK ] CmdConfigBgpPolicyCommunityListTestFixture.communityWithWhitespaceRejected (158 ms) [ OK ] CmdDeleteBgpPolicyCommunityListTestFixture.argValidation (74 ms) [ OK ] CmdDeleteBgpPolicyCommunityListTestFixture.deleteExistingList (157 ms) [ OK ] CmdDeleteBgpPolicyCommunityListTestFixture.deleteUnknownListWarns (69 ms) [ OK ] CmdDeleteBgpPolicyCommunityListTestFixture.deleteTwiceIsIdempotent (94 ms) [ OK ] CmdDeleteBgpPolicyCommunityListTestFixture.deleteUnknownLeavesOthersIntact (85 ms) [ OK ] CmdDeleteBgpPolicyCommunityListTestFixture.deleteOneCommunity (226 ms) [ OK ] CmdDeleteBgpPolicyCommunityListTestFixture.deleteUnknownCommunityWarns (118 ms) [ OK ] CmdDeleteBgpPolicyCommunityListTestFixture.deleteCommunityTwiceIsIdempotent (115 ms) [ OK ] CmdDeleteBgpPolicyCommunityListTestFixture.deleteCommunityFromValuelessListWarns (108 ms) [ OK ] CmdDeleteBgpPolicyCommunityListTestFixture.deleteCommunityFromUnknownListWarns (58 ms) [==========] 26 tests from 2 test suites ran. [ PASSED ] 26 tests. ``` **j2c** — `python3 -m unittest fboss.cli.fboss2.tools.tests.test_bgp_json_to_cli`: OK. **Integration** — `fboss2_integration_test` on a lab device with `bgp_pp` active: ``` $ fboss2_integration_test --gtest_filter='ConfigBgpPolicyCommunityListTest.*' [ OK ] ConfigBgpPolicyCommunityListTest.SetListAttributesAndCommit (311 ms) [ OK ] ConfigBgpPolicyCommunityListTest.SetCommunityAndOperatorAndCommit (2329 ms) [ OK ] ConfigBgpPolicyCommunityListTest.MalformedCommunityRejected (1052 ms) [ OK ] ConfigBgpPolicyCommunityListTest.DeleteListAndCommit (2372 ms) [==========] 4 tests from 1 test suite ran. [ PASSED ] 4 tests. ``` Based-on: #1401 Reviewed By: srikrishnagopu Differential Revision: D115034419 Pulled By: induvsuresh fbshipit-source-id: 2d7171534bbc37147494c020481dbaba3c72caa2

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>

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-action branch from cf0eb36 to 6c87bbb Compare August 10, 2026 04:44

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-action branch 3 times, most recently from 8ff343a to 8c19b12 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-action branch 3 times, most recently from 2f9758c to 0cac475 Compare August 11, 2026 05:52

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-action branch 5 times, most recently from edd76d7 to 66b2011 Compare August 27, 2026 04:19

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-action branch 2 times, most recently from 27fc0f2 to 86f90e0 Compare September 21, 2026 05:07

rabbit-nexthop force-pushed the bgp-policy-routing-policy-term-action branch from 86f90e0 to 11b31ef Compare September 22, 2026 05:23

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-action branch from 06e54ad to 0af6daa Compare September 30, 2026 20:00

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-action branch from 0af6daa to 979d372 Compare October 8, 2026 11:31

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

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-action branch 2 times, most recently from a6ed36b to ddc9d4c Compare October 9, 2026 18:02

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-action branch 2 times, most recently from 4a438c8 to 6325956 Compare October 16, 2026 06:56

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-action branch from 4d03175 to 1d7c78a Compare October 21, 2026 11:14

hillol-nexthop and others added 10 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>

hillol-nexthop force-pushed the bgp-policy-routing-policy-term-action branch from 1d7c78a to 74a0b14 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 action commands - #1490

[Nexthop][fboss2-dev] fboss2 bgp policy routing-policy term action commands#1490
hillol-nexthop wants to merge 10 commits into
meta:mainfrom
nexthop-ai:bgp-policy-routing-policy-term-action

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

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