Repository navigation
[Nexthop][fboss2-dev] fboss2 config/delete protocol bgp neighbor commands - #1391
Closed
hillol-nexthop wants to merge 5 commits into
Closed
hillol-nexthop wants to merge 5 commits into
hillol-nexthop wants to merge 5 commits into
Conversation
hillol-nexthop
force-pushed
the
bgpneighbor
branch
3 times, most recently
from
July 17, 2026 14:07
b3655bb to
66df523
Compare
This was referenced Jul 20, 2026
hillol-nexthop
force-pushed
the
bgpneighbor
branch
from
July 21, 2026 15:14
2c550b4 to
e7d07de
Compare
hillol-nexthop
force-pushed
the
bgpneighbor
branch
2 times, most recently
from
July 31, 2026 16:14
f3e5517 to
0077ebd
Compare
|
@hillol-nexthop has updated the pull request. You must reimport the pull request before landing. |
Contributor
|
This pull request has been imported. If you are a Meta employee, you can view this in D114376255. (Because this pull request was imported automatically, there will not be any future comments.) |
hillol-nexthop
force-pushed
the
bgpneighbor
branch
from
August 3, 2026 03:14
0077ebd to
4a98e2a
Compare
|
@hillol-nexthop has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
|
@hillol-nexthop has updated the pull request. You must reimport the pull request before landing. |
hillol-nexthop
force-pushed
the
bgpneighbor
branch
from
August 4, 2026 07:24
d0bc5e2 to
0c31624
Compare
|
@hillol-nexthop has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
|
@hillol-nexthop has updated the pull request. You must reimport the pull request before landing. |
This was referenced Aug 6, 2026
hillol-nexthop
marked this pull request as draft
August 7, 2026 18:14
hillol-nexthop
force-pushed
the
bgpneighbor
branch
from
August 7, 2026 19:29
01e8ce2 to
256b09f
Compare
|
@hillol-nexthop has updated the pull request. You must reimport the pull request before landing. |
hillol-nexthop
force-pushed
the
bgpneighbor
branch
from
August 10, 2026 04:44
256b09f to
1b66afd
Compare
|
@hillol-nexthop has updated the pull request. You must reimport the pull request before landing. |
hillol-nexthop
added a commit
to nexthop-ai/fboss
that referenced
this pull request
Sep 15, 2026
Review feedback on meta#1391: the generator quoted free-form strings but emitted numeric/boolean fields raw. The input JSON is not type-checked, so a string in an ASN or timer field reached the generated script unquoted. Quote every value uniformly; shlex.quote is a no-op for valid integers and booleans.
hillol-nexthop
force-pushed
the
bgpneighbor
branch
from
September 15, 2026 16:52
67cbfcf to
061acd9
Compare
|
@hillol-nexthop has updated the pull request. You must reimport the pull request before landing. |
hillol-nexthop
added a commit
to nexthop-ai/fboss
that referenced
this pull request
Sep 15, 2026
Review feedback on meta#1391: the generator quoted free-form strings but emitted numeric/boolean fields raw. The input JSON is not type-checked, so a string in an ASN or timer field reached the generated script unquoted. Quote every value uniformly; shlex.quote is a no-op for valid integers and booleans.
hillol-nexthop
force-pushed
the
bgpneighbor
branch
from
September 15, 2026 17:37
061acd9 to
2fdb667
Compare
|
@hillol-nexthop has updated the pull request. You must reimport the pull request before landing. |
rabbit-nexthop
pushed a commit
to nexthop-ai/fboss
that referenced
this pull request
Sep 16, 2026
Review feedback on meta#1391: the generator quoted free-form strings but emitted numeric/boolean fields raw. The input JSON is not type-checked, so a string in an ASN or timer field reached the generated script unquoted. Quote every value uniformly; shlex.quote is a no-op for valid integers and booleans.
hillol-nexthop
added a commit
to nexthop-ai/fboss
that referenced
this pull request
Sep 16, 2026
Review feedback on meta#1391: the generator quoted free-form strings but emitted numeric/boolean fields raw. The input JSON is not type-checked, so a string in an ASN or timer field reached the generated script unquoted. Quote every value uniformly; shlex.quote is a no-op for valid integers and booleans.
hillol-nexthop
force-pushed
the
bgpneighbor
branch
from
September 16, 2026 10:38
2fdb667 to
bd00fd0
Compare
|
@hillol-nexthop has updated the pull request. You must reimport the pull request before landing. |
meta-codesync Bot
pushed a commit
that referenced
this pull request
Sep 17, 2026
Summary: > **Stack (bottom → top):** **#1345 (this PR)** → #1391 → #1395 → #1401 → #1475 → #1477 → #1487 → #1488 → #1489 → #1490 → #1491 Add `fboss2-dev config protocol bgp global <attr> <value>` on top of the BGP-aware `ConfigSession`. It edits the typed `bgp::thrift::BgpConfig` via `ConfigSession::getBgpConfig()`/`saveBgpConfig()` — the whole-config, scope-agnostic typed API (no global/peer/peer-group special-casing in `ConfigSession`). - Collapses the 10 per-attribute global command classes into one dispatcher; rejects `cluster-id` (no `BgpConfig` field) instead of writing dead config; bounds `switch-limit` / `max_golden_vips` so out-of-range values aren't truncated. - Integration tests: `ConfigBgpGlobalTest` (each attribute set+commit, verified in the promoted `/etc/coop/bgpcpp/bgpcpp.conf`) and `ConfigBgpSessionTest` (clear/diff/commit/rollback + an unchanged-config no-op-restart regression), sharing `ConfigBgpTestBase`. ## Sample usage ``` $ fboss2-dev config protocol bgp global router-id 10.0.0.1 Successfully set BGP router-id to: 10.0.0.1 Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp global local-asn 65000 Successfully set BGP local-asn to: 65000 Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp global hold-time 90 Successfully set BGP hold-time to: 90 seconds Config saved to: ~/.fboss2/bgp_config.json $ fboss2-dev config protocol bgp global switch-limit-overload-protection-mode APPLY_GOLDEN_PREFIX_POLICY Successfully set switch_limit_config overload_protection_mode to: 2 Config saved to: ~/.fboss2/bgp_config.json # Rejected before anything is staged: $ fboss2-dev config protocol bgp global router-id not-an-ip Error: Invalid router-id address 'not-an-ip' $ fboss2-dev config protocol bgp global router-id 2001:db8::1 Error: router-id requires an IPv4 address, got '2001:db8::1' $ fboss2-dev config protocol bgp global switch-limit-overload-protection-mode 999 Error: switch-limit-overload-protection-mode value '999' is not a valid mode; expected one of: DROP_EXCESS_PREFIXES, APPLY_GOLDEN_PREFIX_POLICY ``` Pull Request resolved: #1345 Test Plan: **Unit** — `fboss2_cmd_config_test`: ``` $ fboss2_cmd_config_test --gtest_filter=CmdConfigBgpGlobal* [ OK ] CmdConfigBgpGlobalTestFixture.argValidation (101 ms) [ OK ] CmdConfigBgpGlobalTestFixture.routerIdAccepted (145 ms) [ OK ] CmdConfigBgpGlobalTestFixture.routerIdInvalidRejected (109 ms) [ OK ] CmdConfigBgpGlobalTestFixture.routerIdV6Rejected (90 ms) [ OK ] CmdConfigBgpGlobalTestFixture.routerIdArityRejected (97 ms) [ OK ] CmdConfigBgpGlobalTestFixture.rejectedRouterIdKeepsStagedValue (133 ms) [ OK ] CmdConfigBgpGlobalTestFixture.overloadProtectionModeIntegerAccepted (170 ms) [ OK ] CmdConfigBgpGlobalTestFixture.overloadProtectionModeNameAccepted (178 ms) [ OK ] CmdConfigBgpGlobalTestFixture.overloadProtectionModeInvalidRejected (71 ms) [ OK ] CmdConfigBgpGlobalTestFixture.rejectedOverloadProtectionModeKeepsStagedValue (100 ms) [==========] 10 tests from 1 test suite ran. (1199 ms total) [ PASSED ] 10 tests. ``` **Integration** — `fboss2_integration_test` on a lab device with `bgp_pp` active: ``` $ fboss2_integration_test --gtest_filter='ConfigBgpGlobalTest.*' [ OK ] ConfigBgpGlobalTest.SetCountConfedsInAsPathLenTrue (127 ms) [ OK ] ConfigBgpGlobalTest.SetCountConfedsInAsPathLenFalse (283 ms) [ OK ] ConfigBgpGlobalTest.SetGracefulRestartTime (2276 ms) [ OK ] ConfigBgpGlobalTest.SetRibAllocatedPathIdsTrue (2261 ms) [ OK ] ConfigBgpGlobalTest.SetRibAllocatedPathIdsFalse (2259 ms) [ OK ] ConfigBgpGlobalTest.InvalidBoolValueRejected (1052 ms) [ OK ] ConfigBgpGlobalTest.RouterIdInvalidRejected (46 ms) [ OK ] ConfigBgpGlobalTest.RouterIdV6Rejected (48 ms) [ OK ] ConfigBgpGlobalTest.OverloadProtectionModeOutOfRangeRejected (47 ms) [ OK ] ConfigBgpGlobalTest.SetOverloadProtectionModeAndCommit (106 ms) [ OK ] ConfigBgpGlobalTest.NegativeGracefulRestartTimeRejected (48 ms) [==========] 11 tests from 1 test suite ran. [ PASSED ] 11 tests. ``` Reviewed By: Anurag-Gumidelli Differential Revision: D113469141 Pulled By: hjkim417 fbshipit-source-id: 21f9cd0cea6eada213408e58a82392103af29ad8
rabbit-nexthop
pushed a commit
to nexthop-ai/fboss
that referenced
this pull request
Sep 17, 2026
Review feedback on meta#1391: the generator quoted free-form strings but emitted numeric/boolean fields raw. The input JSON is not type-checked, so a string in an ASN or timer field reached the generated script unquoted. Quote every value uniformly; shlex.quote is a no-op for valid integers and booleans.
mabel-bot Bot
pushed a commit
to nexthop-ai/fboss
that referenced
this pull request
Sep 17, 2026
Review feedback on meta#1391: the generator quoted free-form strings but emitted numeric/boolean fields raw. The input JSON is not type-checked, so a string in an ASN or timer field reached the generated script unquoted. Quote every value uniformly; shlex.quote is a no-op for valid integers and booleans.
rabbit-nexthop
pushed a commit
to nexthop-ai/fboss
that referenced
this pull request
Sep 17, 2026
Review feedback on meta#1391: the generator quoted free-form strings but emitted numeric/boolean fields raw. The input JSON is not type-checked, so a string in an ASN or timer field reached the generated script unquoted. Quote every value uniformly; shlex.quote is a no-op for valid integers and booleans.
Adds `config protocol bgp neighbor <ip-address> [<attribute> <value> ...]` and `delete protocol bgp neighbor <ip-address>` — the first per-peer BGP config family, beneath the `protocol bgp` grouping node. - neighbor dispatcher keyed by peer address, writing bgp_config.BgpPeer through the typed ConfigSession. Attributes cover the peer identity (remote/local ASN, description, peer-tag, peer-group), policy bindings (ingress/egress policy), session tunables (timers, route limits, next-hop, add-path, RR client, and the rest of the documented set), and the sheet-documented attributes bgpd does not support, which are rejected with an explanatory message rather than staged. - delete removes the peer by address. - ConfigBgpTestBase restores the committed BGP config after every test, so a failed run cannot leave a device carrying test config; commit-path integration tests verify each value through bgpd's own getRunningConfig RPC rather than only the staged file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review feedback on meta#1391: the generator quoted free-form strings but emitted numeric/boolean fields raw. The input JSON is not type-checked, so a string in an ASN or timer field reached the generated script unquoted. Quote every value uniformly; shlex.quote is a no-op for valid integers and booleans.
… flag unconverted j2c fields
Route limits: when neither the peer nor its group carries a RouteLimit, the
first warning-only/threshold edit seeded the thrift default (max_routes 12000,
a hard cap with session teardown) where bgpd enforced nothing. Seed
{max_routes 0, warning_only false, warning_limit 0} instead: bgpd reads 0 as
unlimited, so only the requested field changes.
Attributes: add remove-private-as, enforce-first-as, route-refresh and
ttl-security-hops (1..255, bgpd's kMin/kMaxTtlSecurityHops), the remaining
BgpPeer fields bgpd resolves peer > group. Document the fields deliberately
not exposed (enabled, router_port_id, graceful_restart_end_of_rib_seconds:
nothing in bgpd reads them).
bgp_json_to_cli.py: shared bool/int/link-bandwidth tables for the fields
BgpPeer and PeerGroup have in common; deprecated i32 ASNs map to the 4-byte
attribute; any JSON field with no CLI equivalent emits a `# WARNING` comment
line in the script and on stderr instead of being dropped silently.
Delete mirrors add: an absent neighbor is already the requested end state, so the delete succeeds with a `Warning:` instead of an `Error:`, and nothing is saved. A repeated delete and a delete of something that never existed behave the same. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
rabbit-nexthop
force-pushed
the
bgpneighbor
branch
from
September 17, 2026 08:15
bd00fd0 to
24afe41
Compare
|
@hillol-nexthop has updated the pull request. You must reimport the pull request before landing. |
|
@hillol-nexthop has updated the pull request. You must reimport the pull request before landing. |
Contributor
meta-codesync Bot
pushed a commit
that referenced
this pull request
Sep 21, 2026
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
meta-codesync Bot
pushed a commit
that referenced
this pull request
Sep 25, 2026
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
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 6, 2026
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
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 14, 2026
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
fboss2-dev config protocol bgp neighbor <ip-address> [<attribute> <value> ...]as a single dispatcher mirroring theglobalcommand: the neighbor address is the first positional token, one- or two-token attributes (timers hold-time,add-path send, ...) are matched longest-prefix-first, and handlers mutate the typedbgp::thrift::BgpPeerviaConfigSession::getBgpConfig()/saveBgpConfig(). Also addsfboss2-dev delete protocol bgp neighbor <ip-address>. The 26 old per-attributepeercommand classes and the now-dead per-peerfolly::dynamic BgpConfigSessionAPI are deleted.BgpPeerfieldremote-asn/local-asnremote_as_4_byte/local_as_4_byte(4-byte-bounded)description,peer-tag,peer-group,peer-id,typedescription,peer_tag,peer_group_name,peer_id,typeingress-policy/egress-policyingress_policy_name/egress_policy_namerr-client,confed-peer,redistribute-peer,enhanced-route-refresh,route-refresh,remove-private-as,enforce-first-as,next-hop-selfpassive <true|false>is_passive(true = PASSIVE_ONLY, false = PASSIVE_ACTIVE in bgpd)ttl-security-hops <1-255>ttl_security_hops(bgpd'skMin/kMaxTtlSecurityHops; it throws at load outside that range)afi disable-ipv4-afi/disable-ipv6-afi/ipv4-over-ipv6-nhdisable_ipv4_afi/disable_ipv6_afi/v4_over_v6_nexthopadd-path send|receive <bool>AddPathenum (RECEIVE/SEND/BOTH)bind-addr address,next-hop4,next-hop6local_addr,next_hop4,next_hop6(IP/family-validated)graceful-restart restart-time/stateful-habgp_peer_timers.graceful_restart_seconds/enable_stateful_hatimers hold-time/keepalive/out-delay/withdraw-unprog-delaybgp_peer_timers.*max-route pre-filter/post-filter/{pre,post}-warning-threshold/{pre,post}-warning-onlypre_filter/post_filterRouteLimit fieldslink-bandwidth,advertise-lbw,receive-lbwlink_bandwidth_bps,advertise_link_bandwidth,receive_link_bandwidth(enum-membership-validated)That is every
BgpPeerfield bgpd'sConfig.cppreads. Deliberately not exposed (documented in the dispatcher):enabled,router_port_idandbgp_peer_timers.graceful_restart_end_of_rib_seconds— nothing in bgpd reads them, so staging them would persist dead config.Bare
neighbor <ip>creates the peer; attribute sets find-or-create it, keyed by thefolly::IPAddress-normalizedpeer_addr. Prefix notation for passive sessions is accepted and host bits are masked (10.0.0.1/24→10.0.0.0/24), matching how bgpd keys dynamic peers, so equivalent spellings update/delete the same neighbor.delete protocol bgp neighbor <ip>removes the matching peer via the same shared normalize/find helpers (BgpPeerAddr.h); deleting an unknown neighbor errors without persisting anything.Behavior notes
bgp_peer_timers,pre_filterandpost_filterper struct against the peer group, andhold_time_seconds/keep_alive_secondsare non-optional. Before the first attribute of such a struct is set on a peer, the struct is seeded from what the peer inherits today — the group's copy, else the global hold time with hold/3 keepalive — and only the requested field changes. For route limits with nothing to inherit, the seed is{max_routes 0, warning_only false, warning_limit 0}: bgpd applies no limit to a peer without a struct and readsmax_routes == 0as unlimited, whereas the thrift default (12000) would have introduced a hard cap with session teardown that the operator never asked for.AddPathhas no disabled member and an unset peer field inherits the group, so it would otherwise be a no-op reported as success. Narrowing a group'sBOTHto one direction is allowed.0or3..65535, graceful-restart0..4095, TTL security hops1..255;link-bandwidthaccepts exactly what bgpd'sparseLinkBandwidthBpsaccepts (auto, decimals, case-insensitive K/M/G). New peers are seeded with parseable any-addresses (local_addr/next_hop4/next_hop6=0.0.0.0/::) because bgpd constructsfolly::IPAddressfrom these non-optional fields at load and aborts on empty strings.peer-port/bind-addr port(no per-peer port field; the daemon uses the globallisten_port),afi ipv4/ipv6-labeled-unicast(no thrift field).--helplists every attribute with its value shape, generated from the dispatch table (bgpNeighborAttributeHelp()is the subcommand's CLI11 footer); tests assert every registered attribute appears and parses. Tab completion cannot see free-form attributes; registering them as CLI11 subcommands is a larger change left for a follow-up.positionals_at_end()on the neighbor command stops parent-chain subcommand fallthrough from reclassifying attribute tokens that match sibling command names (neighbor <ip> peer-group Xmisparsed withXas the neighbor address).bgp_json_to_cli.pyemits the neighbor grammar with every JSON-sourced value passed throughshlex.quote; explicit zero timers/limits are kept; symbolicadd_pathenum names are handled; the deprecated i32remote_as/local_asmap to the 4-byte attribute; any JSON field with no CLI equivalent produces a# WARNING: neighbor <addr>: field <f> has no CLI equivalent, not convertedcomment line in the generated script (and on stderr) instead of being dropped silently. The shared bool/int/link-bandwidth/timer/route-limit tables are reused by the peer-group generator in [Nexthop][fboss2-dev] fboss2 config/delete protocol bgp peer-group commands #1395.TBgpService::getRunningConfigRPC, proving bgpd parsed and adopted the promoted config 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 returnError:.Sample usage
Idempotent delete, recorded on a lab device:
Test plan
Unit (delete) —
fboss2_cmd_config_test:Unit —
fboss2_cmd_config_test:Integration —
fboss2_integration_teston a lab device withbgp_ppactive:Based-on: #1345