Skip to content

Commit 998a774

Browse files
hillol-nexthophillol-nexthop
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.
1 parent 0a68317 commit 998a774

35 files changed

Lines changed: 1027 additions & 1208 deletions

‎cmake/CliFboss2.cmake‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -875,26 +875,6 @@ add_library(fboss2_config_lib
875875
fboss/cli/fboss2/commands/config/protocol/bgp/CmdConfigProtocolBgp.h
876876
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.cpp
877877
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.h
878-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.cpp
879-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.h
880-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.cpp
881-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.h
882-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.cpp
883-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.h
884-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.cpp
885-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.h
886-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalNetwork6Add.cpp
887-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalNetwork6Add.h
888-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.cpp
889-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.h
890-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips.cpp
891-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips.h
892-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode.cpp
893-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode.h
894-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit.cpp
895-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit.h
896-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit.cpp
897-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit.h
898878
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.cpp
899879
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.h
900880
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
@@ -11,6 +11,8 @@ add_executable(fboss2_integration_test
1111
fboss/cli/fboss2/test/integration_test/ConfigAdminDistanceTest.cpp
1212
fboss/cli/fboss2/test/integration_test/ConfigAclRuleTest.cpp
1313
fboss/cli/fboss2/test/integration_test/ConfigArpTest.cpp
14+
fboss/cli/fboss2/test/integration_test/ConfigBgpGlobalTest.cpp
15+
fboss/cli/fboss2/test/integration_test/ConfigBgpSessionTest.cpp
1416
fboss/cli/fboss2/test/integration_test/ConfigConcurrentSessionsTest.cpp
1517
fboss/cli/fboss2/test/integration_test/ConfigHostnameTest.cpp
1618
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
@@ -1099,16 +1099,6 @@ cpp_library(
10991099
"commands/config/protocol/bgp/BgpConfigSession.cpp",
11001100
"commands/config/protocol/bgp/CmdConfigProtocolBgp.cpp",
11011101
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.cpp",
1102-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.cpp",
1103-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.cpp",
1104-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.cpp",
1105-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.cpp",
1106-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalNetwork6Add.cpp",
1107-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.cpp",
1108-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips.cpp",
1109-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode.cpp",
1110-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit.cpp",
1111-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit.cpp",
11121102
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.cpp",
11131103
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.cpp",
11141104
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDescription.cpp",
@@ -1246,11 +1236,6 @@ cpp_library(
12461236
"commands/config/protocol/bgp/BgpConfigSession.h",
12471237
"commands/config/protocol/bgp/CmdConfigProtocolBgp.h",
12481238
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.h",
1249-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.h",
1250-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.h",
1251-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.h",
1252-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.h",
1253-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.h",
12541239
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.h",
12551240
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.h",
12561241
"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
@@ -39,16 +39,6 @@
3939
#include "fboss/cli/fboss2/commands/config/protocol/CmdConfigProtocol.h"
4040
#include "fboss/cli/fboss2/commands/config/protocol/bgp/CmdConfigProtocolBgp.h"
4141
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.h"
42-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.h"
43-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.h"
44-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.h"
45-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.h"
46-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalNetwork6Add.h"
47-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.h"
48-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips.h"
49-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode.h"
50-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit.h"
51-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit.h"
5242
#include "fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.h"
5343
#include "fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.h"
5444
#include "fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDescription.h"
@@ -379,91 +369,14 @@ const CommandTree& kConfigCommandTree() {
379369
{
380370
{
381371
"global",
382-
"Configure BGP global settings",
372+
"Configure BGP global settings: <attribute> <value> "
373+
"(router-id, local-asn, hold-time, confed-asn, "
374+
"count-confeds-in-as-path-len, "
375+
"graceful-restart-time, rib-allocated-path-ids, "
376+
"network6, switch-limit[-total-path|"
377+
"-max-golden-vips|-overload-protection-mode])",
383378
commandHandler<CmdConfigProtocolBgpGlobal>,
384379
argRegistrar<CmdConfigProtocolBgpGlobalTraits>,
385-
{
386-
{
387-
"router-id",
388-
"Set BGP router identifier",
389-
commandHandler<
390-
CmdConfigProtocolBgpGlobalRouterId>,
391-
argRegistrar<
392-
CmdConfigProtocolBgpGlobalRouterIdTraits>,
393-
},
394-
{
395-
"local-asn",
396-
"Set local AS number",
397-
commandHandler<
398-
CmdConfigProtocolBgpGlobalLocalAsn>,
399-
argRegistrar<
400-
CmdConfigProtocolBgpGlobalLocalAsnTraits>,
401-
},
402-
{
403-
"hold-time",
404-
"Set BGP hold time in seconds",
405-
commandHandler<
406-
CmdConfigProtocolBgpGlobalHoldTime>,
407-
argRegistrar<
408-
CmdConfigProtocolBgpGlobalHoldTimeTraits>,
409-
},
410-
{
411-
"confed-asn",
412-
"Set BGP confederation AS number",
413-
commandHandler<
414-
CmdConfigProtocolBgpGlobalConfedAsn>,
415-
argRegistrar<
416-
CmdConfigProtocolBgpGlobalConfedAsnTraits>,
417-
},
418-
{
419-
"cluster-id",
420-
"Set route reflector cluster ID",
421-
commandHandler<
422-
CmdConfigProtocolBgpGlobalClusterId>,
423-
argRegistrar<
424-
CmdConfigProtocolBgpGlobalClusterIdTraits>,
425-
},
426-
{
427-
"network6",
428-
"Add IPv6 network to advertise",
429-
commandHandler<
430-
CmdConfigProtocolBgpGlobalNetwork6Add>,
431-
argRegistrar<
432-
CmdConfigProtocolBgpGlobalNetwork6AddTraits>,
433-
},
434-
{
435-
"switch-limit",
436-
"Set switch limit prefix-limit",
437-
commandHandler<
438-
CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit>,
439-
argRegistrar<
440-
CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimitTraits>,
441-
},
442-
{
443-
"switch-limit-total-path",
444-
"Set switch limit total-path-limit",
445-
commandHandler<
446-
CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit>,
447-
argRegistrar<
448-
CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimitTraits>,
449-
},
450-
{
451-
"switch-limit-max-golden-vips",
452-
"Set switch limit max-golden-vips",
453-
commandHandler<
454-
CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips>,
455-
argRegistrar<
456-
CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVipsTraits>,
457-
},
458-
{
459-
"switch-limit-overload-protection-mode",
460-
"Set switch limit overload-protection-mode",
461-
commandHandler<
462-
CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode>,
463-
argRegistrar<
464-
CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionModeTraits>,
465-
},
466-
},
467380
},
468381
{
469382
"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)