Skip to content

Commit b1ba451

Browse files
manoharan-nexthopbenoit-nexthop
authored andcommitted
Add buffer-pool configuration support
1. Some of the configuration commands like QoS buffer pool modification requires an agent restart and some are hitless. Inorder to take appropriate action during the commit, track the highest impact action by storing the same in `$HOME/.fboss2/action_level` at the end of each configuration change. The proposed change only prints a warning if the action needs to be taken. Later this can be modified to perform the action automatically 2. Add QoS buffer pool configuration commands ``` - fboss2 config qos buffer-pool <name> shared-bytes <value> - fboss2 config qos buffer-pool <name> headroom-bytes <value> - fboss2 config qos buffer-pool <name> reserved-bytes <value> ``` 1. Updated existing UT for action information 2. Added new tests for QoS buffer bool configurations. ``` [admin@fboss101 ~]$ ~/benoit/fboss2-dev config qos buffer-pool testpool headroom-bytes 1024 Successfully set headroom-bytes for buffer-pool 'testpool' to 1024 [admin@fboss101 ~]$ ~/benoit/fboss2-dev config session diff --- current live config +++ session config @@ -121,6 +121,12 @@ "arpAgerInterval": 5, "arpRefreshSeconds": 20, "arpTimeoutSeconds": 60, + "bufferPoolConfigs": { + "testpool": { + "headroomBytes": 1024, + "sharedBytes": 0 + } + }, "clientIdToAdminDistance": { "0": 20, "1": 1, [admin@fboss101 ~]$ ll ~/.fboss2 total 216 -rw-r--r-- 1 admin admin 213283 Jan 13 05:15 agent.conf -rw-r--r-- 1 admin admin 42 Jan 13 05:15 conf_metadata.json [admin@fboss101 ~]$ cat ~/.fboss2/conf_metadata.json { "action": { "WEDGE_AGENT": "AGENT_RESTART" } } ```
1 parent 30d3de1 commit b1ba451

23 files changed

+1394
-31
lines changed

cmake/CliFboss2.cmake

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,13 @@
33
# In general, libraries and binaries in fboss/foo/bar are built by
44
# cmake/FooBar.cmake
55

6+
add_fbthrift_cpp_library(
7+
cli_metadata
8+
fboss/cli/fboss2/cli_metadata.thrift
9+
OPTIONS
10+
json
11+
)
12+
613
add_fbthrift_cpp_library(
714
cli_model
815
fboss/cli/fboss2/cli.thrift
@@ -582,6 +589,14 @@ add_library(fboss2_config_lib
582589
fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceDescription.cpp
583590
fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceMtu.h
584591
fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceMtu.cpp
592+
fboss/cli/fboss2/commands/config/qos/CmdConfigQos.h
593+
fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPool.h
594+
fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPoolHeadroomBytes.h
595+
fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPoolHeadroomBytes.cpp
596+
fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPoolReservedBytes.h
597+
fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPoolReservedBytes.cpp
598+
fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPoolSharedBytes.h
599+
fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPoolSharedBytes.cpp
585600
fboss/cli/fboss2/commands/config/history/CmdConfigHistory.h
586601
fboss/cli/fboss2/commands/config/history/CmdConfigHistory.cpp
587602
fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.h
@@ -599,6 +614,7 @@ add_library(fboss2_config_lib
599614
)
600615

601616
target_link_libraries(fboss2_config_lib
617+
cli_metadata
602618
fboss2_lib
603619
agent_dir_util
604620
)

cmake/CliFboss2Test.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ add_executable(fboss2_cmd_test
3737
fboss/cli/fboss2/test/CmdConfigHistoryTest.cpp
3838
fboss/cli/fboss2/test/CmdConfigInterfaceDescriptionTest.cpp
3939
fboss/cli/fboss2/test/CmdConfigInterfaceMtuTest.cpp
40+
fboss/cli/fboss2/test/CmdConfigQosBufferPoolTest.cpp
4041
fboss/cli/fboss2/test/CmdConfigReloadTest.cpp
4142
fboss/cli/fboss2/test/CmdConfigSessionDiffTest.cpp
4243
fboss/cli/fboss2/test/CmdConfigSessionTest.cpp

fboss/cli/fboss2/BUCK

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@ thrift_library(
1515
thrift_srcs = {"cli.thrift": []},
1616
)
1717

18+
thrift_library(
19+
name = "cli_metadata",
20+
languages = [
21+
"cpp2",
22+
],
23+
thrift_cpp2_options = "json",
24+
thrift_srcs = {"cli_metadata.thrift": []},
25+
)
26+
1827
# NOTE: all of the actual command tree is managed inside CmdList.cpp
1928
# CmdList.h defines the data structure
2029
cpp_library(
@@ -785,6 +794,9 @@ cpp_library(
785794
"commands/config/history/CmdConfigHistory.cpp",
786795
"commands/config/interface/CmdConfigInterfaceDescription.cpp",
787796
"commands/config/interface/CmdConfigInterfaceMtu.cpp",
797+
"commands/config/qos/buffer_pool/CmdConfigQosBufferPoolHeadroomBytes.cpp",
798+
"commands/config/qos/buffer_pool/CmdConfigQosBufferPoolReservedBytes.cpp",
799+
"commands/config/qos/buffer_pool/CmdConfigQosBufferPoolSharedBytes.cpp",
788800
"commands/config/rollback/CmdConfigRollback.cpp",
789801
"commands/config/session/CmdConfigSessionCommit.cpp",
790802
"commands/config/session/CmdConfigSessionDiff.cpp",
@@ -798,6 +810,11 @@ cpp_library(
798810
"commands/config/interface/CmdConfigInterface.h",
799811
"commands/config/interface/CmdConfigInterfaceDescription.h",
800812
"commands/config/interface/CmdConfigInterfaceMtu.h",
813+
"commands/config/qos/CmdConfigQos.h",
814+
"commands/config/qos/buffer_pool/CmdConfigQosBufferPool.h",
815+
"commands/config/qos/buffer_pool/CmdConfigQosBufferPoolHeadroomBytes.h",
816+
"commands/config/qos/buffer_pool/CmdConfigQosBufferPoolReservedBytes.h",
817+
"commands/config/qos/buffer_pool/CmdConfigQosBufferPoolSharedBytes.h",
801818
"commands/config/rollback/CmdConfigRollback.h",
802819
"commands/config/session/CmdConfigSessionCommit.h",
803820
"commands/config/session/CmdConfigSessionDiff.h",
@@ -807,6 +824,7 @@ cpp_library(
807824
exported_deps = [
808825
"fbsource//third-party/fmt:fmt",
809826
"fbsource//third-party/re2:re2",
827+
":cli_metadata-cpp2-types",
810828
":cmd-common-utils",
811829
":cmd-handler",
812830
":fboss2-lib",

fboss/cli/fboss2/CmdHandlerImplConfig.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@
1919
#include "fboss/cli/fboss2/commands/config/interface/CmdConfigInterface.h"
2020
#include "fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceDescription.h"
2121
#include "fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceMtu.h"
22+
#include "fboss/cli/fboss2/commands/config/qos/CmdConfigQos.h"
23+
#include "fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPool.h"
24+
#include "fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPoolHeadroomBytes.h"
25+
#include "fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPoolReservedBytes.h"
26+
#include "fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPoolSharedBytes.h"
2227
#include "fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.h"
2328
#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.h"
2429
#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h"
@@ -40,5 +45,17 @@ template void
4045
CmdHandler<CmdConfigSessionCommit, CmdConfigSessionCommitTraits>::run();
4146
template void
4247
CmdHandler<CmdConfigSessionDiff, CmdConfigSessionDiffTraits>::run();
48+
template void CmdHandler<CmdConfigQos, CmdConfigQosTraits>::run();
49+
template void
50+
CmdHandler<CmdConfigQosBufferPool, CmdConfigQosBufferPoolTraits>::run();
51+
template void CmdHandler<
52+
CmdConfigQosBufferPoolSharedBytes,
53+
CmdConfigQosBufferPoolSharedBytesTraits>::run();
54+
template void CmdHandler<
55+
CmdConfigQosBufferPoolHeadroomBytes,
56+
CmdConfigQosBufferPoolHeadroomBytesTraits>::run();
57+
template void CmdHandler<
58+
CmdConfigQosBufferPoolReservedBytes,
59+
CmdConfigQosBufferPoolReservedBytesTraits>::run();
4360

4461
} // namespace facebook::fboss

fboss/cli/fboss2/CmdListConfig.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@
1717
#include "fboss/cli/fboss2/commands/config/interface/CmdConfigInterface.h"
1818
#include "fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceDescription.h"
1919
#include "fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceMtu.h"
20+
#include "fboss/cli/fboss2/commands/config/qos/CmdConfigQos.h"
21+
#include "fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPool.h"
22+
#include "fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPoolHeadroomBytes.h"
23+
#include "fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPoolReservedBytes.h"
24+
#include "fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPoolSharedBytes.h"
2025
#include "fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.h"
2126
#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.h"
2227
#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h"
@@ -57,6 +62,38 @@ const CommandTree& kConfigCommandTree() {
5762
}},
5863
},
5964

65+
{
66+
"config",
67+
"qos",
68+
"Configure QoS settings",
69+
commandHandler<CmdConfigQos>,
70+
argTypeHandler<CmdConfigQosTraits>,
71+
{{
72+
"buffer-pool",
73+
"Configure buffer pool settings",
74+
commandHandler<CmdConfigQosBufferPool>,
75+
argTypeHandler<CmdConfigQosBufferPoolTraits>,
76+
{{
77+
"shared-bytes",
78+
"Set buffer pool shared bytes",
79+
commandHandler<CmdConfigQosBufferPoolSharedBytes>,
80+
argTypeHandler<CmdConfigQosBufferPoolSharedBytesTraits>,
81+
},
82+
{
83+
"headroom-bytes",
84+
"Set buffer pool headroom bytes",
85+
commandHandler<CmdConfigQosBufferPoolHeadroomBytes>,
86+
argTypeHandler<CmdConfigQosBufferPoolHeadroomBytesTraits>,
87+
},
88+
{
89+
"reserved-bytes",
90+
"Set buffer pool reserved bytes",
91+
commandHandler<CmdConfigQosBufferPoolReservedBytes>,
92+
argTypeHandler<CmdConfigQosBufferPoolReservedBytesTraits>,
93+
}},
94+
}},
95+
},
96+
6097
{
6198
"config",
6299
"session",

fboss/cli/fboss2/CmdSubcommands.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,12 @@ CLI::App* CmdSubcommands::addCommand(
232232
subCmd->add_option(
233233
"revisions", args, "Revision(s) in the form 'rN' or 'current'");
234234
break;
235+
case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_BUFFER_POOL_NAME:
236+
subCmd->add_option("buffer_pool_name", args, "Buffer pool name");
237+
break;
238+
case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_BUFFER_BYTES:
239+
subCmd->add_option("bytes", args, "Buffer size in bytes");
240+
break;
235241
case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_UNINITIALIZE:
236242
case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_NONE:
237243
break;
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Copyright (c) 2004-present, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
*/
10+
11+
package "facebook.com/fboss/cli"
12+
13+
namespace cpp2 facebook.fboss.cli
14+
15+
// Action level required for config changes to take effect.
16+
// Used to track the highest impact action needed when committing config
17+
// changes.
18+
enum ConfigActionLevel {
19+
HITLESS = 0, // Can be applied with reloadConfig() - default
20+
AGENT_RESTART = 1, // Requires agent restart
21+
}
22+
23+
// Identifier for different agents that can be configured
24+
enum AgentType {
25+
WEDGE_AGENT = 1,
26+
}
27+
28+
// Metadata stored alongside the session configuration file.
29+
// This metadata tracks state that needs to persist across CLI invocations
30+
// within a single config session.
31+
struct ConfigSessionMetadata {
32+
// Maps each agent to the required action level for pending config changes.
33+
// Agents not in this map default to HITLESS.
34+
1: map<AgentType, ConfigActionLevel> action;
35+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Copyright (c) 2004-present, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
*/
10+
11+
#pragma once
12+
13+
#include "fboss/cli/fboss2/CmdHandler.h"
14+
#include "fboss/cli/fboss2/utils/CmdUtils.h"
15+
16+
namespace facebook::fboss {
17+
18+
struct CmdConfigQosTraits : public WriteCommandTraits {
19+
static constexpr utils::ObjectArgTypeId ObjectArgTypeId =
20+
utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_NONE;
21+
using ObjectArgType = std::monostate;
22+
using RetType = std::string;
23+
};
24+
25+
class CmdConfigQos : public CmdHandler<CmdConfigQos, CmdConfigQosTraits> {
26+
public:
27+
RetType queryClient(const HostInfo& /* hostInfo */) {
28+
throw std::runtime_error(
29+
"Incomplete command, please use one of the subcommands");
30+
}
31+
32+
void printOutput(const RetType& /* model */) {}
33+
};
34+
35+
} // namespace facebook::fboss
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Copyright (c) 2004-present, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
*/
10+
11+
#pragma once
12+
13+
#include <fmt/format.h>
14+
#include <functional>
15+
#include <map>
16+
#include <string>
17+
18+
#include "fboss/agent/gen-cpp2/switch_config_types.h"
19+
#include "fboss/cli/fboss2/session/ConfigSession.h"
20+
21+
namespace facebook::fboss {
22+
23+
/**
24+
* Helper function to set a buffer pool configuration field.
25+
*
26+
* This function handles the common logic for all buffer pool configuration
27+
* commands (shared-bytes, headroom-bytes, reserved-bytes):
28+
* - Gets or creates the buffer pool config map
29+
* - Gets or creates the specific buffer pool entry
30+
* - Calls the setter to update the specific field
31+
* - Saves the config
32+
* - Updates the required action level to AGENT_RESTART
33+
*
34+
* @param poolName The name of the buffer pool to configure
35+
* @param fieldName The name of the field being set (for the success message)
36+
* @param value The value to set
37+
* @param setter A lambda that sets the specific field on the BufferPoolConfig.
38+
* For new configs, it receives a reference to the new config.
39+
* For existing configs, it receives a reference to the existing
40+
* config.
41+
* @return A success message string
42+
*/
43+
template <typename SetterFn>
44+
std::string setBufferPoolConfigField(
45+
const std::string& poolName,
46+
const std::string& fieldName,
47+
int32_t value,
48+
SetterFn setter) {
49+
auto& session = ConfigSession::getInstance();
50+
auto& agentConfig = session.getAgentConfig();
51+
auto& switchConfig = *agentConfig.sw();
52+
53+
// Get or create the bufferPoolConfigs map
54+
if (!switchConfig.bufferPoolConfigs()) {
55+
switchConfig.bufferPoolConfigs() =
56+
std::map<std::string, cfg::BufferPoolConfig>{};
57+
}
58+
59+
auto& bufferPoolConfigs = *switchConfig.bufferPoolConfigs();
60+
61+
// Check if the buffer pool exists
62+
auto it = bufferPoolConfigs.find(poolName);
63+
if (it == bufferPoolConfigs.end()) {
64+
// Create a new buffer pool config
65+
// Note: sharedBytes is required, so we default it to 0
66+
cfg::BufferPoolConfig newConfig;
67+
newConfig.sharedBytes() = 0;
68+
setter(newConfig);
69+
bufferPoolConfigs[poolName] = std::move(newConfig);
70+
} else {
71+
// Update the existing buffer pool config
72+
setter(it->second);
73+
}
74+
75+
// Save the updated config and update the required action level
76+
// Buffer pool changes always require agent restart
77+
session.saveConfig(cli::ConfigActionLevel::AGENT_RESTART);
78+
79+
return fmt::format(
80+
"Successfully set {} for buffer-pool '{}' to {}",
81+
fieldName,
82+
poolName,
83+
value);
84+
}
85+
86+
} // namespace facebook::fboss

0 commit comments

Comments
 (0)