Skip to content

Commit c1ab18b

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 843c28f commit c1ab18b

35 files changed

Lines changed: 1011 additions & 1206 deletions

‎cmake/CliFboss2.cmake‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -836,26 +836,6 @@ add_library(fboss2_config_lib
836836
fboss/cli/fboss2/commands/config/protocol/bgp/CmdConfigProtocolBgp.h
837837
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.cpp
838838
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.h
839-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.cpp
840-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.h
841-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.cpp
842-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.h
843-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.cpp
844-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.h
845-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.cpp
846-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.h
847-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalNetwork6Add.cpp
848-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalNetwork6Add.h
849-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.cpp
850-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.h
851-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips.cpp
852-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips.h
853-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode.cpp
854-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode.h
855-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit.cpp
856-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit.h
857-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit.cpp
858-
fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit.h
859839
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.cpp
860840
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.h
861841
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
@@ -9,6 +9,8 @@ add_executable(fboss2_integration_test
99
fboss/cli/fboss2/oss/config/CmdListImpl.cpp
1010
fboss/cli/fboss2/test/integration_test/Fboss2IntegrationTest.cpp
1111
fboss/cli/fboss2/test/integration_test/ConfigArpTest.cpp
12+
fboss/cli/fboss2/test/integration_test/ConfigBgpGlobalTest.cpp
13+
fboss/cli/fboss2/test/integration_test/ConfigBgpSessionTest.cpp
1214
fboss/cli/fboss2/test/integration_test/ConfigConcurrentSessionsTest.cpp
1315
fboss/cli/fboss2/test/integration_test/ConfigCoppTest.cpp
1416
fboss/cli/fboss2/test/integration_test/ConfigInterfaceDescriptionTest.cpp

‎fboss/cli/fboss2/BUCK‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,16 +1064,6 @@ cpp_library(
10641064
"commands/config/protocol/bgp/BgpConfigSession.cpp",
10651065
"commands/config/protocol/bgp/CmdConfigProtocolBgp.cpp",
10661066
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.cpp",
1067-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.cpp",
1068-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.cpp",
1069-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.cpp",
1070-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.cpp",
1071-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalNetwork6Add.cpp",
1072-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.cpp",
1073-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips.cpp",
1074-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode.cpp",
1075-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit.cpp",
1076-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit.cpp",
10771067
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.cpp",
10781068
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.cpp",
10791069
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDescription.cpp",
@@ -1196,11 +1186,6 @@ cpp_library(
11961186
"commands/config/protocol/bgp/BgpConfigSession.h",
11971187
"commands/config/protocol/bgp/CmdConfigProtocolBgp.h",
11981188
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.h",
1199-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.h",
1200-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.h",
1201-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.h",
1202-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.h",
1203-
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.h",
12041189
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.h",
12051190
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.h",
12061191
"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
@@ -35,16 +35,6 @@
3535
#include "fboss/cli/fboss2/commands/config/protocol/CmdConfigProtocol.h"
3636
#include "fboss/cli/fboss2/commands/config/protocol/bgp/CmdConfigProtocolBgp.h"
3737
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.h"
38-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalClusterId.h"
39-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalConfedAsn.h"
40-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalHoldTime.h"
41-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalLocalAsn.h"
42-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalNetwork6Add.h"
43-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalRouterId.h"
44-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips.h"
45-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode.h"
46-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit.h"
47-
#include "fboss/cli/fboss2/commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit.h"
4838
#include "fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.h"
4939
#include "fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.h"
5040
#include "fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDescription.h"
@@ -306,91 +296,14 @@ const CommandTree& kConfigCommandTree() {
306296
{
307297
{
308298
"global",
309-
"Configure BGP global settings",
299+
"Configure BGP global settings: <attribute> <value> "
300+
"(router-id, local-asn, hold-time, confed-asn, "
301+
"count-confeds-in-as-path-len, "
302+
"graceful-restart-time, rib-allocated-path-ids, "
303+
"network6, switch-limit[-total-path|"
304+
"-max-golden-vips|-overload-protection-mode])",
310305
commandHandler<CmdConfigProtocolBgpGlobal>,
311306
argRegistrar<CmdConfigProtocolBgpGlobalTraits>,
312-
{
313-
{
314-
"router-id",
315-
"Set BGP router identifier",
316-
commandHandler<
317-
CmdConfigProtocolBgpGlobalRouterId>,
318-
argRegistrar<
319-
CmdConfigProtocolBgpGlobalRouterIdTraits>,
320-
},
321-
{
322-
"local-asn",
323-
"Set local AS number",
324-
commandHandler<
325-
CmdConfigProtocolBgpGlobalLocalAsn>,
326-
argRegistrar<
327-
CmdConfigProtocolBgpGlobalLocalAsnTraits>,
328-
},
329-
{
330-
"hold-time",
331-
"Set BGP hold time in seconds",
332-
commandHandler<
333-
CmdConfigProtocolBgpGlobalHoldTime>,
334-
argRegistrar<
335-
CmdConfigProtocolBgpGlobalHoldTimeTraits>,
336-
},
337-
{
338-
"confed-asn",
339-
"Set BGP confederation AS number",
340-
commandHandler<
341-
CmdConfigProtocolBgpGlobalConfedAsn>,
342-
argRegistrar<
343-
CmdConfigProtocolBgpGlobalConfedAsnTraits>,
344-
},
345-
{
346-
"cluster-id",
347-
"Set route reflector cluster ID",
348-
commandHandler<
349-
CmdConfigProtocolBgpGlobalClusterId>,
350-
argRegistrar<
351-
CmdConfigProtocolBgpGlobalClusterIdTraits>,
352-
},
353-
{
354-
"network6",
355-
"Add IPv6 network to advertise",
356-
commandHandler<
357-
CmdConfigProtocolBgpGlobalNetwork6Add>,
358-
argRegistrar<
359-
CmdConfigProtocolBgpGlobalNetwork6AddTraits>,
360-
},
361-
{
362-
"switch-limit",
363-
"Set switch limit prefix-limit",
364-
commandHandler<
365-
CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimit>,
366-
argRegistrar<
367-
CmdConfigProtocolBgpGlobalSwitchLimitPrefixLimitTraits>,
368-
},
369-
{
370-
"switch-limit-total-path",
371-
"Set switch limit total-path-limit",
372-
commandHandler<
373-
CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimit>,
374-
argRegistrar<
375-
CmdConfigProtocolBgpGlobalSwitchLimitTotalPathLimitTraits>,
376-
},
377-
{
378-
"switch-limit-max-golden-vips",
379-
"Set switch limit max-golden-vips",
380-
commandHandler<
381-
CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVips>,
382-
argRegistrar<
383-
CmdConfigProtocolBgpGlobalSwitchLimitMaxGoldenVipsTraits>,
384-
},
385-
{
386-
"switch-limit-overload-protection-mode",
387-
"Set switch limit overload-protection-mode",
388-
commandHandler<
389-
CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionMode>,
390-
argRegistrar<
391-
CmdConfigProtocolBgpGlobalSwitchLimitOverloadProtectionModeTraits>,
392-
},
393-
},
394307
},
395308
{
396309
"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)