[Nexthop][fboss2-dev] fboss2 config/delete protocol bgp policy as-path-list commands by hillol-nexthop · Pull Request #1401 · facebook/fboss

hillol-nexthop · 2026-07-21T14:07:51Z

Stacked on #1395 (config 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 bgp policy as-path-list commit here. Merge the lower PRs first.

Stack (bottom → top): #1446 → #1345 → #1391 → #1395 → #1401 (this PR) → #1485 → #1475 → #1486 → #1477 → #1487 → #1488 → #1489 → #1490 → #1491

Summary

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>

The list's entry <seq-num> level is a CLI11 subcommand of its own and ships in #1485.

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 [Nexthop][fboss2-dev] fboss2 config/delete protocol bgp neighbor commands #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 (including the entry-level helpers) so the delete command and the entry subcommand in [Nexthop][fboss2-dev] fboss2 bgp policy as-path-list entry subcommand #1485 share them.
A rejected value leaves nothing staged: a list implicitly created for the failed command is rolled back.

Test Plan

Unit — CmdConfigBgpPolicyAsPathListTest (6) and CmdDeleteBgpPolicyAsPathListTest (4): arg validation, bare create, description round-trip, named lists staying distinct, re-reference reporting the existing list, unknown-attribute rejection, and delete of a staged vs. unknown list. All pass.

Integration on device (real bgpd daemon), ConfigBgpPolicyAsPathListTest (2): SetListDescriptionAndCommit and DeleteListAndCommit. Both stage and commit, then assert against bgpd's running config via the getRunningConfig RPC, confirming bgpd accepts and adopts the .policies blob end to end.

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

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

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

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

facebook-github-tools · 2026-08-03T03:16:25Z

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

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

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

facebook-github-tools · 2026-08-03T05:11:28Z

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

facebook-github-tools · 2026-08-03T06:13:31Z

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

facebook-github-tools · 2026-08-03T07:20:43Z

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

facebook-github-tools · 2026-08-03T08:09:32Z

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

facebook-github-tools · 2026-08-03T09:04:14Z

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

facebook-github-tools · 2026-08-03T09:25:10Z

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

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

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

facebook-github-tools · 2026-08-06T09:22:04Z

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

facebook-github-tools · 2026-08-07T19:30:23Z

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

Follow-up cleanups to the BGP-aware config session infra (facebook#1344): - Collapse the BGP_RESTART action level into AGENT_WARMBOOT (bgpd has no hitless reload; its restart already runs the agent-warmboot code path). - Introduce a single ConfigDomain descriptor + configDomains() and shared per-domain helpers so commit(), rollback() and `config session diff` handle the agent and BGP domains uniformly (private DiffDomain removed). - Make the agent skip-when-unchanged like BGP: a commit whose staged config equals what is already promoted is a true no-op (no git revision, no symlink churn, no reloadConfig()/bgpd restart). Change detection is semantic (compare the deserialized thrift structs), so formatting-only diffs don't count. - Consolidate `config session clear` onto a static stagedSessionFilePaths() and reuse ConfigSession::readStagedContent() in diff. - Make ConfigSession::saveConfig(service, level) generic over the service and reduce saveBgpConfig() to a thin wrapper. - Keep the heavy generated thrift headers out of ConfigSession.h: use the *_types_fwd.h forward-declaration headers, hold agentConfig_/bgpConfig_ by std::unique_ptr, and drop the configLoaded_/bgpConfigLoaded_ bools (null == not loaded). - clang-tidy: use auto for the SimpleJSONSerializer template-cast results. Built fboss2-dev + the config unit tests; config-session/commit/diff/BGP/clear tests pass. Verified the agent no-op behaviour live on test switches.

Removed comments about the destructor definition in ConfigSession.h.

…thInterface call main renamed findFirstEthInterface() to getRandomInterfacePortName() (virtual-management-port fix); convert the branch-added no-op-commit test to the new helper. Drop the three includes ConfigSession.cpp no longer uses directly (misc-include-cleaner runs as errors in CI).

facebook-github-tools · 2026-08-10T04:48:39Z

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

facebook-github-tools · 2026-08-10T05:43:58Z

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

Add `fboss2-dev config protocol bgp global <attr> <value>` on top of the BGP-aware ConfigSession (base PR). 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). - Collapse the 10 per-attribute global command classes into one dispatcher; reject cluster-id (no BgpConfig field) instead of writing dead config; bound switch-limit / max_golden_vips so out-of-range values aren't truncated. - Integration tests: ConfigBgpGlobalTest (each attr set+commit, verified in the promoted /etc/coop/bgpcpp/bgpcpp.conf) and ConfigBgpSessionTest (clear/diff/commit/rollback + a no-op-restart regression), sharing ConfigBgpTestBase. Test Plan: - bazel test //fboss/cli/fboss2/test/config:cmd_config_test - bazel build //fboss/cli/fboss2/test/integration_test:fboss2_integration_test - fboss2_integration_test on a DUT with bgp_pp active: ConfigBgpSessionTest 6/6 pass (clear, diff, commit-restarts-bgp_pp, rollback-restores-config, and the unchanged-config does-not-restart case); agent-session regression (ConfigInterfaceMtuTest) passes.

- Extract the value parsers (parseBool/parseInt/parseNonNegInt32, handler Result) into a shared BgpCliValueParsers.h so sibling BGP dispatchers can reuse them, and add a bounded parseAsn4Byte: local-asn/confed-asn accepted any uint64 and silently persisted out-of-range ASNs (>= 2^32 wrap the i64 field negative). - positionals_at_end() on the global command: CLI11's parent-chain subcommand fallthrough steals value tokens that match a sibling command name (e.g. a policy named "peer-group") and misparses the command. - Integration test base: probe the bgpd unit, and pass -c safe.directory=/etc/coop on the raw git invocations (gitHead / bgpTrackedAtRevision), mirroring the CLI's Git class -- /etc/coop is owned by another user (e.g. coop) on provisioned devices, which git otherwise rejects as dubious ownership and the helpers silently return empty results.

The commit-path global-attribute tests asserted on the promoted /etc/coop/bgpcpp/bgpcpp.conf, which only proves what the CLI wrote to disk. Route ConfigBgpTestBase::setAndCommit() through a new readRunningBgpConfigViaRpc() -- TBgpService::getRunningConfig against the local bgpd -- so every positive ConfigBgpGlobalTest asserts its attribute in the daemon's own view of its config, proving bgpd parsed and adopted the promoted file after the commit-triggered restart. The helper retries briefly on connection errors: systemd reports bgpd active as soon as the process starts, but its thrift server binds the port a few seconds later, so an RPC issued right after a restart races that window. Test Plan: - bazel build //fboss/cli/fboss2/test/integration_test:fboss2_integration_test - fboss2_integration_test on a DUT with bgpd active: ConfigBgpGlobalTest 7/7 pass (each attribute set+commit verified in bgpd's getRunningConfig view, plus the invalid-bool and negative-graceful-restart-time reject paths).

facebook-github-tools · 2026-08-10T10:14:31Z

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

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>

…ands 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. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…list Adds the first BGP policy object-type command family, 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> The list's `entry <seq-num>` level is a CLI11 subcommand of its own and lands in the next commit, so this one owns only the list level. - Follows the dispatcher shape the neighbor/peer-group families use: one factory per value shape from the shared BgpCliAttrHandlers.h, 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, since BgpConfig.policies already exists. - Lookup/create helpers live in BgpAsPathListCliUtils.h, including the entry-level helpers, so the delete command and the entry subcommand in the next commit share them rather than re-deriving the scan. - A rejected value leaves nothing staged: a list implicitly created for the failed command is rolled back before returning. - There is no per-entry delete; `delete ... as-path-list <name>` removes the whole list. - unit tests (10): CmdConfigBgpPolicyAsPathListTest (6) covering arg validation, bare create, the description round-trip, named lists staying distinct, re-reference reporting the existing list and unknown-attribute rejection, plus CmdDeleteBgpPolicyAsPathListTest (4). - integration tests (2): ConfigBgpPolicyAsPathListTest stages and commits, then asserts against bgpd's running config via the getRunningConfig RPC, confirming bgpd accepts and adopts the .policies blob end to end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

facebook-github-tools · 2026-08-10T13:59:57Z

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

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

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

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

hillol-nexthop force-pushed the bgp-policy-aspathlist branch 2 times, most recently from 85ccd7f to c5420a1 Compare July 21, 2026 15:14

hillol-nexthop force-pushed the bgp-policy-aspathlist branch 2 times, most recently from 72cdd81 to 0bf546a Compare July 31, 2026 16:14

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from 0bf546a to d88e6a7 Compare August 3, 2026 03:14

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from 40059ec to 2ad8bdc Compare August 3, 2026 05:11

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from 2ad8bdc to 5f67dd7 Compare August 3, 2026 06:13

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from 5f67dd7 to 924c188 Compare August 3, 2026 07:20

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from 924c188 to e071a4e Compare August 3, 2026 08:09

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from e071a4e to 2892023 Compare August 3, 2026 09:04

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from 2892023 to 0f5aef0 Compare August 3, 2026 09:25

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from 0f5aef0 to 95f1404 Compare August 4, 2026 07:24

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from 95f1404 to 3c14626 Compare August 6, 2026 09:21

hillol-nexthop mentioned this pull request Aug 6, 2026

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

Open

hillol-nexthop added 3 commits August 10, 2026 04:29

Remove comments regarding destructor in ConfigSession

8d74003

Removed comments about the destructor definition in ConfigSession.h.

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from 9c84065 to b43dc70 Compare August 10, 2026 04:44

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from b43dc70 to e64f7b7 Compare August 10, 2026 05:43

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

hillol-nexthop added 3 commits August 10, 2026 10:11

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from e64f7b7 to 1d1f51e Compare August 10, 2026 10:12

hillol-nexthop and others added 3 commits August 10, 2026 13:55

hillol-nexthop force-pushed the bgp-policy-aspathlist branch from 1d1f51e to 0b8188e Compare August 10, 2026 13:56

Uh oh!

Conversation

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

Uh oh!

Summary

Commands

Design

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 3, 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 3, 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 Aug 10, 2026

Uh oh!

facebook-github-tools Bot commented Aug 10, 2026

Uh oh!

facebook-github-tools Bot commented Aug 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

hillol-nexthop commented Jul 21, 2026 •

edited

Loading