Skip to content

Commit 00c725d

Browse files
hillol-nexthopclaude
hillol-nexthop
and
claude
committed
fboss2 config protocol bgp global command
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. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent dff0880 commit 00c725d

35 files changed

Lines changed: 1027 additions & 1206 deletions

‎cmake/CliFboss2.cmake‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -850,26 +850,6 @@ add_library(fboss2_config_lib
850850
fboss/cli/fboss2/commands/config/protocol/bgp/CmdConfigProtocolBgp.h
851851
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.cpp
852852
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.h
853-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.cpp
854-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.h
855-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.cpp
856-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.h
857-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.cpp
858-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.h
859-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.cpp
860-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.h
861-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalNetwork6Add.cpp
862-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalNetwork6Add.h
863-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.cpp
864-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.h
865-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips.cpp
866-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips.h
867-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode.cpp
868-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode.h
869-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit.cpp
870-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit.h
871-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit.cpp
872-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit.h
873853
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.cpp
874854
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.h
875855
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.cpp

‎cmake/CliFboss2TestIntegrationTest.cmake‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ add_executable(fboss2_integration_test
1010
fboss/cli/fboss2/test/integration_test/Fboss2IntegrationTest.cpp
1111
fboss/cli/fboss2/test/integration_test/ConfigAdminDistanceTest.cpp
1212
fboss/cli/fboss2/test/integration_test/ConfigArpTest.cpp
13+
fboss/cli/fboss2/test/integration_test/ConfigBgpGlobalTest.cpp
14+
fboss/cli/fboss2/test/integration_test/ConfigBgpSessionTest.cpp
1315
fboss/cli/fboss2/test/integration_test/ConfigConcurrentSessionsTest.cpp
1416
fboss/cli/fboss2/test/integration_test/ConfigHostnameTest.cpp
1517
fboss/cli/fboss2/test/integration_test/ConfigIcmpV4UnavailableSrcAddrTest.cpp

‎fboss/cli/fboss2/BUCK‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,16 +1071,6 @@ cpp_library(
10711071
"commands/config/protocol/bgp/BgpConfigSession.cpp",
10721072
"commands/config/protocol/bgp/CmdConfigProtocolBgp.cpp",
10731073
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.cpp",
1074-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.cpp",
1075-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.cpp",
1076-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.cpp",
1077-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.cpp",
1078-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalNetwork6Add.cpp",
1079-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.cpp",
1080-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips.cpp",
1081-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode.cpp",
1082-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit.cpp",
1083-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit.cpp",
10841074
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.cpp",
10851075
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.cpp",
10861076
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDescription.cpp",
@@ -1210,11 +1200,6 @@ cpp_library(
12101200
"commands/config/protocol/bgp/BgpConfigSession.h",
12111201
"commands/config/protocol/bgp/CmdConfigProtocolBgp.h",
12121202
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.h",
1213-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.h",
1214-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.h",
1215-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.h",
1216-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.h",
1217-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.h",
12181203
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.h",
12191204
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.h",
12201205
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDescription.h",

‎fboss/cli/fboss2/CmdListConfig.cpp‎

Lines changed: 6 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,6 @@
3737
#include "fboss/cli/fboss2/commands/config/protocol/CmdConfigProtocol.h"
3838
#include "fboss/cli/fboss2/commands/config/protocol/bgp/CmdConfigProtocolBgp.h"
3939
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.h"
40-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.h"
41-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.h"
42-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.h"
43-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.h"
44-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalNetwork6Add.h"
45-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.h"
46-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips.h"
47-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode.h"
48-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit.h"
49-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit.h"
5040
#include "fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.h"
5141
#include "fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.h"
5242
#include "fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDescription.h"
@@ -358,91 +348,14 @@ const CommandTree& kConfigCommandTree() {
358348
{
359349
{
360350
"global",
361-
"Configure BGP global settings",
351+
"Configure BGP global settings: <attribute> <value> "
352+
"(router-id, local-asn, hold-time, confed-asn, "
353+
"count-confeds-in-as-path-len, "
354+
"graceful-restart-time, rib-allocated-path-ids, "
355+
"network6, switch-limit[-total-path|"
356+
"-max-golden-vips|-overload-protection-mode])",
362357
commandHandler<CmdConfigProtocolBgpGlobal>,
363358
argRegistrar<CmdConfigProtocolBgpGlobalTraits>,
364-
{
365-
{
366-
"router-id",
367-
"Set BGP router identifier",
368-
commandHandler<
369-
CmdConfigProtocolBgpGlobalRouterId>,
370-
argRegistrar<
371-
CmdConfigProtocolBgpGlobalRouterIdTraits>,
372-
},
373-
{
374-
"local-asn",
375-
"Set local AS number",
376-
commandHandler<
377-
CmdConfigProtocolBgpGlobalLocalAsn>,
378-
argRegistrar<
379-
CmdConfigProtocolBgpGlobalLocalAsnTraits>,
380-
},
381-
{
382-
"hold-time",
383-
"Set BGP hold time in seconds",
384-
commandHandler<
385-
CmdConfigProtocolBgpGlobalHoldTime>,
386-
argRegistrar<
387-
CmdConfigProtocolBgpGlobalHoldTimeTraits>,
388-
},
389-
{
390-
"confed-asn",
391-
"Set BGP confederation AS number",
392-
commandHandler<
393-
CmdConfigProtocolBgpGlobalConfedAsn>,
394-
argRegistrar<
395-
CmdConfigProtocolBgpGlobalConfedAsnTraits>,
396-
},
397-
{
398-
"cluster-id",
399-
"Set route reflector cluster ID",
400-
commandHandler<
401-
CmdConfigProtocolBgpGlobalClusterId>,
402-
argRegistrar<
403-
CmdConfigProtocolBgpGlobalClusterIdTraits>,
404-
},
405-
{
406-
"network6",
407-
"Add IPv6 network to advertise",
408-
commandHandler<
409-
CmdConfigProtocolBgpGlobalNetwork6Add>,
410-
argRegistrar<
411-
CmdConfigProtocolBgpGlobalNetwork6AddTraits>,
412-
},
413-
{
414-
"switch-limit",
415-
"Set switch limit prefix-limit",
416-
commandHandler<
417-
CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit>,
418-
argRegistrar<
419-
CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimitTraits>,
420-
},
421-
{
422-
"switch-limit-total-path",
423-
"Set switch limit total-path-limit",
424-
commandHandler<
425-
CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit>,
426-
argRegistrar<
427-
CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimitTraits>,
428-
},
429-
{
430-
"switch-limit-max-golden-vips",
431-
"Set switch limit max-golden-vips",
432-
commandHandler<
433-
CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips>,
434-
argRegistrar<
435-
CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVipsTraits>,
436-
},
437-
{
438-
"switch-limit-overload-protection-mode",
439-
"Set switch limit overload-protection-mode",
440-
commandHandler<
441-
CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode>,
442-
argRegistrar<
443-
CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionModeTraits>,
444-
},
445-
},
446359
},
447360
{
448361
"peer-group",

‎fboss/cli/fboss2/commands/config/protocol/bgp/BgpConfigSession.cpp‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -208,20 +208,6 @@ std::optional<uint64_t> BgpConfigSession::getConfedAsn() const {
208208
return std::nullopt;
209209
}
210210

211-
void BgpConfigSession::setClusterId(const std::string& clusterId) {
212-
ensureConfigLoaded();
213-
bgpConfig_["cluster_id"] = clusterId;
214-
}
215-
216-
std::optional<std::string> BgpConfigSession::getClusterId() const {
217-
const_cast<BgpConfigSession*>(this)->ensureConfigLoaded();
218-
if (bgpConfig_.count("cluster_id") &&
219-
!bgpConfig_["cluster_id"].asString().empty()) {
220-
return bgpConfig_["cluster_id"].asString();
221-
}
222-
return std::nullopt;
223-
}
224-
225211
void BgpConfigSession::setListenAddress(const std::string& listenAddr) {
226212
ensureConfigLoaded();
227213
bgpConfig_["listen_addr"] = listenAddr;

‎fboss/cli/fboss2/commands/config/protocol/bgp/BgpConfigSession.h‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ namespace facebook::fboss {
4242
* - local_as_4_byte: i64 - Local AS number (RFC 6793)
4343
* - hold_time: i32 - Hold time in seconds (default 30)
4444
* - local_confed_as_4_byte: i64 - Confederation AS number
45-
* - cluster_id: string - Route reflector cluster ID
4645
* - peers: list<BgpPeer> - List of BGP peers
4746
* - peer_groups: list<PeerGroup> - List of peer groups
4847
*
@@ -103,10 +102,6 @@ class BgpConfigSession {
103102
void setConfedAsn(uint64_t asn);
104103
std::optional<uint64_t> getConfedAsn() const;
105104

106-
// cluster_id: string - Route reflector cluster ID
107-
void setClusterId(const std::string& clusterId);
108-
std::optional<std::string> getClusterId() const;
109-
110105
// listen_addr: string - Listen address for BGP sessions
111106
void setListenAddress(const std::string& listenAddr);
112107
std::optional<std::string> getListenAddress() const;

0 commit comments

Comments
 (0)