[Nexthop][fboss2-dev] fboss2 config/delete protocol bgp peer-group commands by hillol-nexthop · Pull Request #1395 · meta/fboss

hillol-nexthop · 2026-07-20T07:01:43Z

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

Summary

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

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

facebook-github-tools · 2026-07-31T16:15:05Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

meta-codesync · 2026-07-31T16:20:10Z

This pull request has been imported. If you are a Meta employee, you can view this in D114376290. (Because this pull request was imported automatically, there will not be any future comments.)

facebook-github-tools · 2026-08-03T03:15:58Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

facebook-github-tools · 2026-08-03T04:14:49Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

facebook-github-tools · 2026-08-04T07:24:57Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

facebook-github-tools · 2026-08-06T09:23:03Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

facebook-github-tools · 2026-08-07T19:29:35Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

facebook-github-tools · 2026-08-10T04:44:50Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

facebook-github-tools · 2026-09-17T08:15:32Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

facebook-github-tools · 2026-09-17T08:43:11Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

Summary: > **Stacked on #1345 (`config protocol bgp global`). 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 neighbor` commit(s) here. Merge the lower PRs first. > > **Stack (bottom → top):** #1345 → **#1391 (this PR)** → #1395 → #1401 → #1475 → #1477 → #1487 → #1488 → #1489 → #1490 → #1491 Adds `fboss2-dev config protocol bgp neighbor <ip-address> [<attribute> <value> ...]` as a single dispatcher mirroring the `global` command: 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 typed `bgp::thrift::BgpPeer` via `ConfigSession::getBgpConfig()` / `saveBgpConfig()`. Also adds `fboss2-dev delete protocol bgp neighbor <ip-address>`. The 26 old per-attribute `peer` command classes and the now-dead per-peer `folly::dynamic BgpConfigSession` API are deleted. | Attribute | `BgpPeer` field | |---|---| | `remote-asn` / `local-asn` | `remote_as_4_byte` / `local_as_4_byte` (4-byte-bounded) | | `description`, `peer-tag`, `peer-group`, `peer-id`, `type` | `description`, `peer_tag`, `peer_group_name`, `peer_id`, `type` | | `ingress-policy` / `egress-policy` | `ingress_policy_name` / `egress_policy_name` | | `rr-client`, `confed-peer`, `redistribute-peer`, `enhanced-route-refresh`, `route-refresh`, `remove-private-as`, `enforce-first-as`, `next-hop-self` | matching bools | | `passive <true\|false>` | `is_passive` (true = PASSIVE_ONLY, false = PASSIVE_ACTIVE in bgpd) | | `ttl-security-hops <1-255>` | `ttl_security_hops` (bgpd's `kMin/kMaxTtlSecurityHops`; it throws at load outside that range) | | `afi disable-ipv4-afi` / `disable-ipv6-afi` / `ipv4-over-ipv6-nh` | `disable_ipv4_afi` / `disable_ipv6_afi` / `v4_over_v6_nexthop` | | `add-path send\|receive <bool>` | merged into the `AddPath` enum (RECEIVE/SEND/BOTH) | | `bind-addr address`, `next-hop4`, `next-hop6` | `local_addr`, `next_hop4`, `next_hop6` (IP/family-validated) | | `graceful-restart restart-time` / `stateful-ha` | `bgp_peer_timers.graceful_restart_seconds` / `enable_stateful_ha` | | `timers hold-time` / `keepalive` / `out-delay` / `withdraw-unprog-delay` | `bgp_peer_timers.*` | | `max-route pre-filter` / `post-filter` / `{pre,post}-warning-threshold` / `{pre,post}-warning-only` | `pre_filter` / `post_filter` RouteLimit fields | | `link-bandwidth`, `advertise-lbw`, `receive-lbw` | `link_bandwidth_bps`, `advertise_link_bandwidth`, `receive_link_bandwidth` (enum-membership-validated) | That is every `BgpPeer` field bgpd's `Config.cpp` reads. Deliberately not exposed (documented in the dispatcher): `enabled`, `router_port_id` and `bgp_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 the `folly::IPAddress`-normalized `peer_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 - **Inherited values are preserved.** bgpd resolves `bgp_peer_timers`, `pre_filter` and `post_filter` per *struct* against the peer group, and `hold_time_seconds` / `keep_alive_seconds` are 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 reads `max_routes == 0` as unlimited, whereas the thrift default (12000) would have introduced a hard cap with session teardown that the operator never asked for. - **Handlers edit a copy** of the peer and commit on success, so a rejected value never leaves a half-edited peer or a seeded struct behind. - **add-path** merges into the *effective* value (the peer's, else the group's). Disabling a direction the group still enables is rejected with a message naming the group: `AddPath` has 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's `BOTH` to one direction is allowed. - **Validation matches bgpd**: hold time `0` or `3..65535`, graceful-restart `0..4095`, TTL security hops `1..255`; `link-bandwidth` accepts exactly what bgpd's `parseLinkBandwidthBps` accepts (`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 constructs `folly::IPAddress` from these non-optional fields at load and aborts on empty strings. - **Rejected instead of persisting dead config**: `peer-port` / `bind-addr port` (no per-peer port field; the daemon uses the global `listen_port`), `afi ipv4/ipv6-labeled-unicast` (no thrift field). - **`--help`** lists 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. - **CLI11 token stealing fixed**: `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 X` misparsed with `X` as the neighbor address). - **`bgp_json_to_cli.py`** emits the neighbor grammar with every JSON-sourced value passed through `shlex.quote`; explicit zero timers/limits are kept; symbolic `add_path` enum names are handled; the deprecated i32 `remote_as`/`local_as` map to the 4-byte attribute; any JSON field with no CLI equivalent produces a `# WARNING: neighbor <addr>: field <f> has no CLI equivalent, not converted` comment 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 #1395. - Integration tests verify the committed neighbor config through the daemon's `TBgpService::getRunningConfig` RPC, 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 return `Error:`. ## Sample usage ``` $ fboss2-dev config protocol bgp neighbor 2001:db8::99 remote-asn 65510 Successfully set remote-asn to: 65510 for neighbor 2001:db8::99 $ fboss2-dev config protocol bgp neighbor 2001:db8::99 timers hold-time 90 Successfully set timers hold-time to: 90 seconds for neighbor 2001:db8::99 $ fboss2-dev config protocol bgp neighbor 2001:db8::99 passive true Successfully enabled passive for neighbor 2001:db8::99 $ fboss2-dev config protocol bgp neighbor 2001:db8::99 ttl-security-hops 300 Error: Invalid ttl-security-hops value '300'; expected 1-255 $ fboss2-dev config session commit Config session committed successfully as d99c4cd and bgpd (restart) restarted. $ fboss2-dev delete protocol bgp neighbor 2001:db8::99 Successfully deleted BGP neighbor 2001:db8::99 ``` Idempotent delete, recorded on a lab device: ``` $ fboss2-dev delete protocol bgp neighbor 192.0.2.99 Warning: BGP neighbor 192.0.2.99 does not exist; nothing to delete ``` Pull Request resolved: #1391 Test Plan: **Unit (delete)** — `fboss2_cmd_config_test`: ``` $ fboss2_cmd_config_test --gtest_filter=*DeleteBgpNeighbor* [ OK ] CmdDeleteBgpNeighborTestFixture.argValidation (52 ms) [ OK ] CmdDeleteBgpNeighborTestFixture.deleteExistingNeighbor (165 ms) [ OK ] CmdDeleteBgpNeighborTestFixture.deleteByAnySpelling (123 ms) [ OK ] CmdDeleteBgpNeighborTestFixture.deleteUnknownNeighborWarns (68 ms) [ OK ] CmdDeleteBgpNeighborTestFixture.deleteTwiceIsIdempotent (588 ms) [ OK ] CmdDeleteBgpNeighborTestFixture.deleteUnknownLeavesOthersIntact (119 ms) [==========] 6 tests from 1 test suites ran. [ PASSED ] 6 tests. ``` **Unit** — `fboss2_cmd_config_test`: ``` $ fboss2_cmd_config_test --gtest_filter=*BgpNeighbor* [ OK ] CmdConfigBgpNeighborTestFixture.argValidation (95 ms) [ OK ] CmdConfigBgpNeighborTestFixture.bareCreate (110 ms) [ OK ] CmdConfigBgpNeighborTestFixture.setRemoteAsn (116 ms) [ OK ] CmdConfigBgpNeighborTestFixture.timersAccumulate (151 ms) [ OK ] CmdConfigBgpNeighborTestFixture.addPathMerge (230 ms) [ OK ] CmdConfigBgpNeighborTestFixture.passive (185 ms) [ OK ] CmdConfigBgpNeighborTestFixture.boolAndStringAttributes (1007 ms) [ OK ] CmdConfigBgpNeighborTestFixture.parityAttributes (873 ms) [ OK ] CmdConfigBgpNeighborTestFixture.valueValidation (133 ms) [ OK ] CmdConfigBgpNeighborTestFixture.routeLimits (140 ms) [ OK ] CmdConfigBgpNeighborTestFixture.normalizedAddressesShareOnePeer (114 ms) [ OK ] CmdConfigBgpNeighborTestFixture.prefixSpellingsShareOnePeer (90 ms) [ OK ] CmdConfigBgpNeighborTestFixture.timersSeededFromPeerGroup (111 ms) [ OK ] CmdConfigBgpNeighborTestFixture.timersSeededFromGlobalWithoutGroup (75 ms) [ OK ] CmdConfigBgpNeighborTestFixture.timersNotReseededOncePresent (140 ms) [ OK ] CmdConfigBgpNeighborTestFixture.routeLimitSeededFromPeerGroup (88 ms) [ OK ] CmdConfigBgpNeighborTestFixture.routeLimitSeededUnlimitedWithoutGroup (127 ms) [ OK ] CmdConfigBgpNeighborTestFixture.routeLimitNotReseededOncePresent (126 ms) [ OK ] CmdConfigBgpNeighborTestFixture.ttlSecurityHopsRange (171 ms) [ OK ] CmdConfigBgpNeighborTestFixture.addPathMergesWithPeerGroupValue (149 ms) [ OK ] CmdConfigBgpNeighborTestFixture.addPathCannotDisableGroupDirection (116 ms) [ OK ] CmdConfigBgpNeighborTestFixture.addPathNarrowsGroupBothToOneDirection (119 ms) [ OK ] CmdConfigBgpNeighborTestFixture.timerRanges (164 ms) [ OK ] CmdConfigBgpNeighborTestFixture.rejectedValueLeavesPeerUntouched (162 ms) [ OK ] CmdConfigBgpNeighborTestFixture.distinctAddressesCreateDistinctPeers (109 ms) [ OK ] CmdConfigBgpNeighborTestFixture.attributeHelpListsEveryAttribute (50 ms) [ OK ] CmdConfigBgpNeighborTestFixture.cliHelpFooterCarriesAttributeList (53 ms) [ OK ] CmdConfigBgpNeighborTestFixture.invalidValuesRejected (66 ms) [ OK ] CmdConfigBgpNeighborTestFixture.unsupportedAttributesRejected (70 ms) [ OK ] CmdDeleteBgpNeighborTestFixture.argValidation (53 ms) [ OK ] CmdDeleteBgpNeighborTestFixture.deleteExistingNeighbor (106 ms) [ OK ] CmdDeleteBgpNeighborTestFixture.deleteByAnySpelling (160 ms) [ OK ] CmdDeleteBgpNeighborTestFixture.deleteByEquivalentPrefixSpelling (115 ms) [ OK ] CmdDeleteBgpNeighborTestFixture.deleteUnknownNeighborRejected (57 ms) [ OK ] CmdDeleteBgpNeighborTestFixture.deleteUnknownLeavesOthersIntact (180 ms) [==========] 35 tests from 2 test suites ran. (5828 ms total) [ PASSED ] 35 tests. ``` **Integration** — `fboss2_integration_test` on a lab device with `bgp_pp` active: ``` $ fboss2_integration_test --gtest_filter='ConfigBgpNeighborTest.*' [ OK ] ConfigBgpNeighborTest.SetRemoteAsnAndCommit (13722 ms) [ OK ] ConfigBgpNeighborTest.SetTimersHoldTimeAndCommit (12148 ms) [ OK ] ConfigBgpNeighborTest.GracefulRestartKeepsInheritedHoldTime (11413 ms) [ OK ] ConfigBgpNeighborTest.SessionKnobsAndCommit (11168 ms) [ OK ] ConfigBgpNeighborTest.RouteLimitWarningKeepsUnlimitedCap (12787 ms) [ OK ] ConfigBgpNeighborTest.DeleteNeighborAndCommit (20256 ms) [ PASSED ] 6 tests. ``` Based-on: #1345 Reviewed By: jaiharil Differential Revision: D114376255 Pulled By: hjkim417 fbshipit-source-id: 3fe82432503ececae3ff911da5e07cf00ee2f7e7

facebook-github-tools · 2026-09-18T10:23:38Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

facebook-github-tools · 2026-09-18T10:27:12Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

facebook-github-tools · 2026-09-18T16:53:26Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

hjkim417

@hillol-nexthop, left few comments. Please take a look.

hjkim417 · 2026-09-19T14:12:33Z

+void seedRouteLimit(PeerGroup& group, bool pre) {
+  auto field = pre ? group.pre_filter() : group.post_filter();
+  if (field) {
+    return;
+  }
+  bgp::thrift::RouteLimit unlimited;
+  unlimited.max_routes() = 0;
+  unlimited.warning_only() = false;
+  unlimited.warning_limit() = 0;
+  field = std::move(unlimited);
+}


seedRouteLimit writes the seeded RouteLimit to a local auto field variable, not to the group. group.pre_filter() / group.post_filter() return an optional_field_ref by value, so field = std::move(unlimited) mutates a detached copy and the group's field stays unset. Compare with seedTimers, which correctly writes group.bgp_peer_timers() = std::move(timers). Fix: replace the last line with if (pre) { group.pre_filter() = std::move(unlimited); } else { group.post_filter() = std::move(unlimited); }, or assign through the ref directly (e.g. (pre ? group.pre_filter() : group.post_filter()) = ... if the ref supports it).

apache::thrift::optional_field_ref<bgp::thrift::RouteLimit&> is the return type of group.pre_filter() and group.post_filter() and this is a ref. In fact UT CmdConfigBgpPeerGroupTest.cpp covers this.

facebook-github-tools · 2026-09-21T04:14:15Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

facebook-github-tools · 2026-09-21T04:48:25Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

Collapses the 18 per-attribute `config protocol bgp peer-group` command classes (built on the deprecated `folly::dynamic` BgpConfigSession) into a single typed dispatcher, mirroring CmdConfigProtocolBgpNeighbor. Also adds `delete protocol bgp peer-group <name>`. 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 through ConfigSession::getBgpConfig() / saveBgpConfig(), so adding a tunable is a one-entry change in the dispatch table rather than a new command class. 36 command files are deleted. - Covers the 29 dispatch keys that map to a PeerGroup thrift field: remote-asn / local-asn (4-byte-bounded), description, peer-tag, ingress-policy / egress-policy, rr-client, confed-peer, redistribute-peer, enhanced-route-refresh, connect-mode, 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 (plus the warning-threshold / warning-only knobs), timers hold-time|keepalive|out-delay|withdraw-unprog-delay, and next-hop-self. - Rejected rather than persisted as dead config (precedent: cluster-id in the global command, connect-mode BOTH in the neighbor command): connect-mode BOTH, since thrift only models is_passive. add-path send|receive merges into the AddPath enum bitmask, and clearing the last direction unsets the field. - Attributes with no per-peer-group thrift field (afi ipv4-labeled-unicast, afi ipv6-labeled-unicast, peer-port) are absent from the dispatch table, so they are refused at parse time as unknown attributes; a rejected value never lands on disk. - 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 (same fix as the neighbor command). - unit tests (16): CmdConfigBgpPeerGroupTest (12) covering arg validation, longest-prefix match, the add-path merge matrix, connect-mode, the bool/string/route-limit attributes, value validation and unknown-attribute rejection, plus CmdDeleteBgpPeerGroupTest (4). - integration tests (3): ConfigBgpPeerGroupTest asserts the committed peer_groups through bgpd's getRunningConfig RPC, proving the daemon parsed and adopted the promoted config rather than only checking the file the CLI wrote.

…UT gaps delete protocol bgp peer-group now refuses to delete a group that a neighbor's peer_group_name still references, naming the referencing neighbors, instead of committing a dangling reference bgpd has no defined behavior for. New/extended unit tests close the coverage gaps found in review: - timers out-delay and withdraw-unprog-delay (previously untested; withdraw-unprog-delay is a parity attribute tested for neighbor) - valueValidation now asserts the half-created group is rolled back (state assertion, not just the error string) - wrongValueCountRejected: zero/too-many value token counts - overwriteSameAttributeLastWins: re-setting an attribute updates in place - namedGroupsAreDistinct now asserts field preservation across mutations and sibling isolation - delete: referenced-group rejected (all referencing neighbors named), unreferenced delete unaffected by other references, delete-then- recreate starts clean Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Two pre-existing CI failures surfaced on every CLI-touching PR: - ConfigBgp*Test: the SetUp gate sampled systemctl is-active once. On fboss-sim bgpd flaps activating->active->failed from boot, so one unlucky sample lands in the brief active window, runs the test, and the bgpd thrift RPC then times out after 90s (seen as SetGracefulRestartTime / SetCountConfedsInAsPathLenFalse failures; green runs are the ones where every sample landed on inactive and skipped). The gate now also requires bgpd to answer its getRunningConfig RPC within 10s, else the test skips like the rest of the flap cycle. - ConfigVlanDefaultTest.ChangeDefaultVlanWithPortInNonDefaultVlan vacated only the test port before changing the default VLAN, but the change-default guard refuses while ANY port still uses the old default VLAN without an interface. On baselines generated before the default VLAN, so the guard always fired. Vacate every eth port from the old default VLAN first and restore them after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The suite is ~60 files of ConfigSession-backed tests, each doing several fsync-backed writeFileAtomic calls per case; with this PR's added peer-group tests the full run takes ~530s on a dev box and timed out at the CI job's default moderate test timeout (300s) — killed mid-fsync in ConfigSessionTestFixture.commandTrackingMultipleCommands, 50/51 remaining tests green. size=large raises the test timeout to 900s (same precedent as thrift_packet_stream/tests). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…rammar Signed-off-by: hillol-nexthop <hillol@nexthop.ai>

…gap, flag unconverted j2c fields Route limits: a group with no RouteLimit imposes none on its members (bgpd skips the check when the struct is absent), yet the first warning-only or threshold edit created the struct with the thrift default max_routes 12000, a hard cap with session teardown the operator never asked for. The struct is now seeded {max_routes 0, warning_only false, warning_limit 0} (0 = unlimited in bgpd) and only the requested field changes; a rejected value leaves no struct behind. Attributes: add every remaining PeerGroup field bgpd resolves peer > group: remove-private-as, enforce-first-as, route-refresh, ttl-security-hops (1..255), link-bandwidth, advertise-lbw, receive-lbw -- same spellings and validation as the neighbor command. Document the fields deliberately not exposed because bgpd never reads them from a group (local_addr, next_hop4/6, enabled, router_port_id, graceful_restart_end_of_rib_seconds). bgp_json_to_cli.py: the peer-group generator covers the new attributes via the shared tables, maps the deprecated i32 ASNs to the 4-byte attribute, and emits a `# WARNING` line (script + stderr) for any field with no CLI equivalent instead of dropping it. test_complete_peer_group_loses_no_field now asserts 36 commands.

Delete mirrors add: an absent group is already the requested end state, so the delete succeeds with a `Warning:` instead of an `Error:`, and nothing is saved. The referenced-by-neighbors guard still errors. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

A bare `config protocol bgp neighbor <addr>` or `peer-group <name>` on an object that already exists said "Successfully created" and re-saved the unchanged config. Return "already exists" before touching the session instead, matching the VLAN and ACL config commands. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…d is generated `name` is the only required PeerGroup field and bgpd accepts a group carrying nothing else, so JSON-to-CLI replay must still create it; otherwise a neighbor referring to it names a group that never existed and the replayed config fails to load. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

facebook-github-tools · 2026-09-21T05:07:00Z

@hillol-nexthop has updated the pull request. You must reimport the pull request before landing.

meta-codesync · 2026-09-21T19:36:30Z

@hjkim417 merged this pull request in 01e49e1.

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

hillol-nexthop requested review from a team as code owners July 20, 2026 07:01

meta-cla Bot added the CLA Signed label Jul 20, 2026

github-actions Bot added the nexthop label Jul 20, 2026

hillol-nexthop force-pushed the bgp-peer-group branch from 208842a to 7699625 Compare July 20, 2026 07:08

hillol-nexthop mentioned this pull request Jul 21, 2026

[Nexthop][fboss2-dev] fboss2 config/delete protocol bgp policy as-path-list commands #1401

Closed

hillol-nexthop force-pushed the bgp-peer-group branch from 856a6d5 to 4f223c9 Compare July 21, 2026 15:14

hillol-nexthop force-pushed the bgp-peer-group branch 2 times, most recently from 71ada08 to 92a6082 Compare July 31, 2026 16:14

hillol-nexthop force-pushed the bgp-peer-group branch from 92a6082 to 84951af Compare August 3, 2026 03:14

hillol-nexthop force-pushed the bgp-peer-group branch from a7d3e63 to 690f8a8 Compare August 4, 2026 07:24

hillol-nexthop force-pushed the bgp-peer-group branch from 690f8a8 to 84a208a Compare August 6, 2026 09:21

This was referenced Aug 6, 2026

[Nexthop][fboss2-dev] fboss2 config/delete protocol bgp policy community-list commands #1475

Closed

[Nexthop][fboss2-dev] fboss2 config/delete protocol bgp policy prefix-list commands #1477

Closed

hillol-nexthop marked this pull request as draft August 7, 2026 18:15

hillol-nexthop force-pushed the bgp-peer-group branch from 84a208a to c868417 Compare August 7, 2026 19:29

hillol-nexthop force-pushed the bgp-peer-group branch from c868417 to f3d968b Compare August 10, 2026 04:44

rabbit-nexthop force-pushed the bgp-peer-group branch from fae2453 to 1798656 Compare September 17, 2026 08:15

rabbit-nexthop force-pushed the bgp-peer-group branch from 1663687 to 79c309c Compare September 18, 2026 10:23

hillol-nexthop force-pushed the bgp-peer-group branch from c092230 to 46fdd12 Compare September 18, 2026 16:52

hjkim417 reviewed Sep 19, 2026

View reviewed changes

hillol-nexthop and others added 9 commits September 21, 2026 05:04

[Nexthop] Seed inherited peer-group timers, expose passive, fix j2c g…

4c366a0

…rammar Signed-off-by: hillol-nexthop <hillol@nexthop.ai>

hillol-nexthop force-pushed the bgp-peer-group branch from 64ec83e to a3572fc Compare September 21, 2026 05:06

meta-codesync Bot closed this in 01e49e1 Sep 21, 2026

meta-codesync Bot added the Merged label Sep 21, 2026

rabbit-nexthop deleted the bgp-peer-group branch October 18, 2026 18:31

Conversation

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

Uh oh!

Summary

Behavior notes

Sample usage

Test plan

Uh oh!

facebook-github-tools Bot commented Jul 31, 2026

Uh oh!

meta-codesync Bot commented Jul 31, 2026

Uh oh!

facebook-github-tools Bot commented Aug 3, 2026

Uh oh!

facebook-github-tools Bot commented Aug 3, 2026

Uh oh!

facebook-github-tools Bot commented Aug 4, 2026

Uh oh!

facebook-github-tools Bot commented Aug 6, 2026

Uh oh!

facebook-github-tools Bot commented Aug 7, 2026

Uh oh!

facebook-github-tools Bot commented Aug 10, 2026

Uh oh!

facebook-github-tools Bot commented Sep 17, 2026

Uh oh!

facebook-github-tools Bot commented Sep 17, 2026

Uh oh!

facebook-github-tools Bot commented Sep 18, 2026

Uh oh!

facebook-github-tools Bot commented Sep 18, 2026

Uh oh!

facebook-github-tools Bot commented Sep 18, 2026

Uh oh!

hjkim417 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hjkim417 Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

hillol-nexthop Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

facebook-github-tools Bot commented Sep 21, 2026

Uh oh!

facebook-github-tools Bot commented Sep 21, 2026

Uh oh!

facebook-github-tools Bot commented Sep 21, 2026

Uh oh!

meta-codesync Bot commented Sep 21, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hillol-nexthop commented Jul 20, 2026 •

edited

Loading