Skip to content

Commit ec9f184

Browse files
Add fboss2-dev config and delete traffic-counter CLI
fboss2-dev config traffic-counter <name> <types> creates or updates a named entry in sw.trafficCounters, where types is a comma separated list of PACKETS,BYTES. The handler upserts by counter name: an existing counter's types are updated in place, otherwise a new TrafficCounter is appended. Every wanted type has to be named: a collapsed alias for "all of them" stops meaning that as soon as a cfg::CounterType is added, and an optional type argument lets a command line silently narrow a counter that is already collecting both. Types are stored in a fixed order so the config does not depend on the order they were typed in. fboss2-dev delete traffic-counter <name> removes a named counter from sw.trafficCounters. It refuses while a dataPlaneTrafficPolicy or cpuTrafficPolicy match action still references the counter, naming the referring matchers: deleting it would leave a dangling reference that ApplyThriftConfig rejects ("Invalid config: No counter named ..."). The deprecated globalEgressTrafficPolicy_DEPRECATED has no agent consumers and is not scanned. Both operations are hitless: trafficCounters is consumed at ACL apply-time in ApplyThriftConfig::updateAclsImpl() and does not require an agent restart on its own. ConfigTrafficCounterTest exercises the create+delete round trip against a real agent: config traffic-counter <name> PACKETS,BYTES -> hitless commit -> verify it appears in the running config with types [PACKETS, BYTES] -> delete traffic-counter <name> -> hitless commit -> verify the running config returns to baseline. The fixture deletes a leftover test counter before each case, so a run that died between create and delete leaves the next run a clean config instead of blocking it. The absent-counter refusal is left to the unit tests, which cover it directly. Unit tests cover arg validation, the create/update/no-op execution paths, unreferenced delete, cpu- and dataplane-policy referenced-counter refusal, and absent-counter refusal.
1 parent 64ea7d1 commit ec9f184

14 files changed

Lines changed: 863 additions & 0 deletions

cmake/CliFboss2.cmake

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,6 +1044,8 @@ add_library(fboss2_config_lib
10441044
fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.cpp
10451045
fboss/cli/fboss2/commands/config/session/CmdConfigSessionRebase.h
10461046
fboss/cli/fboss2/commands/config/session/CmdConfigSessionRebase.cpp
1047+
fboss/cli/fboss2/commands/config/traffic_counter/CmdConfigTrafficCounter.cpp
1048+
fboss/cli/fboss2/commands/config/traffic_counter/CmdConfigTrafficCounter.h
10471049
fboss/cli/fboss2/commands/config/tunnel/CmdConfigTunnel.cpp
10481050
fboss/cli/fboss2/commands/config/tunnel/CmdConfigTunnel.h
10491051
fboss/cli/fboss2/commands/config/tunnel/ip_in_ip/CmdConfigTunnelIpInIp.cpp
@@ -1100,6 +1102,8 @@ add_library(fboss2_config_lib
11001102
fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.h
11011103
fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.cpp
11021104
fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.h
1105+
fboss/cli/fboss2/commands/delete/traffic_counter/CmdDeleteTrafficCounter.cpp
1106+
fboss/cli/fboss2/commands/delete/traffic_counter/CmdDeleteTrafficCounter.h
11031107
fboss/cli/fboss2/commands/delete/tunnel/CmdDeleteTunnel.cpp
11041108
fboss/cli/fboss2/commands/delete/tunnel/CmdDeleteTunnel.h
11051109
fboss/cli/fboss2/commands/delete/tunnel/ip_in_ip/CmdDeleteTunnelIpInIp.cpp

cmake/CliFboss2TestConfig.cmake

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ add_executable(fboss2_cmd_config_test
3232
fboss/cli/fboss2/test/config/CmdConfigSessionDiffTest.cpp
3333
fboss/cli/fboss2/test/config/CmdConfigSessionTest.cpp
3434
fboss/cli/fboss2/test/config/CmdConfigTestBase.cpp
35+
fboss/cli/fboss2/test/config/CmdConfigTrafficCounterTest.cpp
3536
fboss/cli/fboss2/test/config/CmdConfigVlanDefaultTest.cpp
3637
fboss/cli/fboss2/test/config/CmdConfigTunnelIpInIpTest.cpp
3738
fboss/cli/fboss2/test/config/CmdConfigVlanManagerTest.cpp
@@ -47,6 +48,7 @@ add_executable(fboss2_cmd_config_test
4748
fboss/cli/fboss2/test/config/CmdDeleteQosPolicyTest.cpp
4849
fboss/cli/fboss2/test/config/CmdDeleteQosQueueConfigTest.cpp
4950
fboss/cli/fboss2/test/config/CmdDeleteTunnelIpInIpTest.cpp
51+
fboss/cli/fboss2/test/config/CmdDeleteTrafficCounterTest.cpp
5052
fboss/cli/fboss2/test/config/CmdDeleteVlanTest.cpp
5153
fboss/cli/fboss2/test/config/ConfigSessionSystemdTest.cpp
5254
fboss/cli/fboss2/test/config/ProfileValidationTest.cpp

cmake/CliFboss2TestIntegrationTest.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ add_executable(fboss2_integration_test
3636
fboss/cli/fboss2/test/integration_test/ConfigQosDefaultQueueConfigTest.cpp
3737
fboss/cli/fboss2/test/integration_test/ConfigQosPolicyMapTest.cpp
3838
fboss/cli/fboss2/test/integration_test/ConfigSessionClearTest.cpp
39+
fboss/cli/fboss2/test/integration_test/ConfigTrafficCounterTest.cpp
3940
fboss/cli/fboss2/test/integration_test/ConfigVlanCreateTest.cpp
4041
fboss/cli/fboss2/test/integration_test/ConfigInterfaceSwitchportTrunkAllowedVlanTest.cpp
4142
fboss/cli/fboss2/test/integration_test/ConfigVlanDefaultTest.cpp

fboss/cli/fboss2/BUCK

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,6 +1189,7 @@ cpp_library(
11891189
"commands/config/switch/admin_distance/CmdConfigAdminDistance.cpp",
11901190
"commands/config/switch/hostname/CmdConfigHostname.cpp",
11911191
"commands/config/switch/icmpv4_unavailable_src_addr/CmdConfigIcmpV4UnavailableSrcAddr.cpp",
1192+
"commands/config/traffic_counter/CmdConfigTrafficCounter.cpp",
11921193
"commands/config/tunnel/CmdConfigTunnel.cpp",
11931194
"commands/config/tunnel/ip_in_ip/CmdConfigTunnelIpInIp.cpp",
11941195
"commands/config/tunnel/ip_in_ip/TunnelIpInIpConfigUtils.cpp",
@@ -1220,6 +1221,7 @@ cpp_library(
12201221
"commands/delete/qos/policy/CmdDeleteQosPolicyMap.cpp",
12211222
"commands/delete/qos/queue_config/CmdDeleteQosQueueConfig.cpp",
12221223
"commands/delete/qos/queue_config/CmdDeleteQosQueueConfigQueueId.cpp",
1224+
"commands/delete/traffic_counter/CmdDeleteTrafficCounter.cpp",
12231225
"commands/delete/tunnel/CmdDeleteTunnel.cpp",
12241226
"commands/delete/tunnel/ip_in_ip/CmdDeleteTunnelIpInIp.cpp",
12251227
"commands/delete/tunnel/ip_in_ip/TunnelIpInIpDeleteUtils.cpp",
@@ -1334,6 +1336,7 @@ cpp_library(
13341336
"commands/config/switch/admin_distance/CmdConfigAdminDistance.h",
13351337
"commands/config/switch/hostname/CmdConfigHostname.h",
13361338
"commands/config/switch/icmpv4_unavailable_src_addr/CmdConfigIcmpV4UnavailableSrcAddr.h",
1339+
"commands/config/traffic_counter/CmdConfigTrafficCounter.h",
13371340
"commands/config/tunnel/CmdConfigTunnel.h",
13381341
"commands/config/tunnel/ip_in_ip/CmdConfigTunnelIpInIp.h",
13391342
"commands/config/tunnel/ip_in_ip/TunnelIpInIpConfigUtils.h",
@@ -1365,6 +1368,7 @@ cpp_library(
13651368
"commands/delete/qos/policy/CmdDeleteQosPolicyMap.h",
13661369
"commands/delete/qos/queue_config/CmdDeleteQosQueueConfig.h",
13671370
"commands/delete/qos/queue_config/CmdDeleteQosQueueConfigQueueId.h",
1371+
"commands/delete/traffic_counter/CmdDeleteTrafficCounter.h",
13681372
"commands/delete/tunnel/CmdDeleteTunnel.h",
13691373
"commands/delete/tunnel/ip_in_ip/CmdDeleteTunnelIpInIp.h",
13701374
"commands/delete/tunnel/ip_in_ip/TunnelIpInIpDeleteUtils.h",

fboss/cli/fboss2/CmdListConfig.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@
118118
#include "fboss/cli/fboss2/commands/config/switch/admin_distance/CmdConfigAdminDistance.h"
119119
#include "fboss/cli/fboss2/commands/config/switch/hostname/CmdConfigHostname.h"
120120
#include "fboss/cli/fboss2/commands/config/switch/icmpv4_unavailable_src_addr/CmdConfigIcmpV4UnavailableSrcAddr.h"
121+
#include "fboss/cli/fboss2/commands/config/traffic_counter/CmdConfigTrafficCounter.h"
121122
#include "fboss/cli/fboss2/commands/config/tunnel/CmdConfigTunnel.h"
122123
#include "fboss/cli/fboss2/commands/config/tunnel/ip_in_ip/CmdConfigTunnelIpInIp.h"
123124
#include "fboss/cli/fboss2/commands/config/tunnel/ip_in_ip/decap/CmdConfigTunnelIpInIpDecap.h"
@@ -148,6 +149,7 @@
148149
#include "fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.h"
149150
#include "fboss/cli/fboss2/commands/delete/qos/queue_config/CmdDeleteQosQueueConfig.h"
150151
#include "fboss/cli/fboss2/commands/delete/qos/queue_config/CmdDeleteQosQueueConfigQueueId.h"
152+
#include "fboss/cli/fboss2/commands/delete/traffic_counter/CmdDeleteTrafficCounter.h"
151153
#include "fboss/cli/fboss2/commands/delete/tunnel/CmdDeleteTunnel.h"
152154
#include "fboss/cli/fboss2/commands/delete/tunnel/ip_in_ip/CmdDeleteTunnelIpInIp.h"
153155
#include "fboss/cli/fboss2/commands/delete/tunnel/ip_in_ip/decap/CmdDeleteTunnelIpInIpDecap.h"
@@ -1020,6 +1022,12 @@ const CommandTree& kConfigCommandTree() {
10201022
}},
10211023
}}},
10221024

1025+
{"config",
1026+
"traffic-counter",
1027+
"Create or update a named traffic counter (PACKETS,BYTES)",
1028+
commandHandler<CmdConfigTrafficCounter>,
1029+
argRegistrar<CmdConfigTrafficCounterTraits>},
1030+
10231031
{
10241032
"config",
10251033
"vlan",
@@ -1238,6 +1246,13 @@ const CommandTree& kConfigCommandTree() {
12381246
}},
12391247
}}},
12401248

1249+
{"delete",
1250+
"traffic-counter",
1251+
"Delete a traffic counter (refuses while a traffic-policy match action "
1252+
"references it): <name>",
1253+
commandHandler<CmdDeleteTrafficCounter>,
1254+
argRegistrar<CmdDeleteTrafficCounterTraits>},
1255+
12411256
{"delete",
12421257
"vlan",
12431258
"Delete a VLAN and its interface (refuses while it is the default VLAN or a port's ingress VLAN)",
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
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+
#include "fboss/cli/fboss2/commands/config/traffic_counter/CmdConfigTrafficCounter.h"
12+
13+
#include "fboss/cli/fboss2/CmdHandler.cpp"
14+
15+
#include <fmt/format.h>
16+
#include <folly/String.h>
17+
#include <algorithm>
18+
#include <iostream>
19+
#include "fboss/cli/fboss2/session/ConfigSession.h"
20+
21+
namespace facebook::fboss {
22+
23+
namespace {
24+
// Accepted spellings of cfg::CounterType, matched case insensitively.
25+
constexpr std::string_view kTypePackets = "packets";
26+
constexpr std::string_view kTypeBytes = "bytes";
27+
constexpr std::string_view kTypeList = "PACKETS,BYTES";
28+
29+
std::string typesToString(const std::vector<cfg::CounterType>& types) {
30+
std::vector<std::string> names;
31+
names.reserve(types.size());
32+
for (auto type : types) {
33+
names.push_back(
34+
type == cfg::CounterType::BYTES ? std::string("BYTES")
35+
: std::string("PACKETS"));
36+
}
37+
return folly::join(",", names);
38+
}
39+
} // namespace
40+
41+
TrafficCounterArg::TrafficCounterArg(std::vector<std::string> v) {
42+
if (v.size() != 2) {
43+
throw std::invalid_argument(
44+
fmt::format(
45+
"Expected <name> <types>, where types is a comma separated list of {}",
46+
kTypeList));
47+
}
48+
49+
name_ = v[0];
50+
if (name_.empty()) {
51+
throw std::invalid_argument("Counter name must not be empty");
52+
}
53+
data_.push_back(v[0]);
54+
55+
std::vector<std::string> tokens;
56+
folly::split(',', v[1], tokens);
57+
bool hasPackets = false;
58+
bool hasBytes = false;
59+
for (auto token : tokens) {
60+
folly::toLowerAscii(token);
61+
if (token == kTypePackets) {
62+
hasPackets = true;
63+
} else if (token == kTypeBytes) {
64+
hasBytes = true;
65+
} else {
66+
throw std::invalid_argument(
67+
fmt::format(
68+
"Invalid counter type '{}'. Expected a comma separated list of {}",
69+
token,
70+
kTypeList));
71+
}
72+
}
73+
// Store in a fixed order so the config, and the already-configured
74+
// comparison below, do not depend on the order the types were typed in.
75+
if (hasPackets) {
76+
types_.push_back(cfg::CounterType::PACKETS);
77+
}
78+
if (hasBytes) {
79+
types_.push_back(cfg::CounterType::BYTES);
80+
}
81+
data_.push_back(v[1]);
82+
}
83+
84+
CmdConfigTrafficCounterTraits::RetType CmdConfigTrafficCounter::queryClient(
85+
const HostInfo& /* hostInfo */,
86+
const ObjectArgType& trafficCounter) {
87+
auto& session = ConfigSession::getInstance();
88+
auto& config = session.getAgentConfig();
89+
auto& swConfig = *config.sw();
90+
91+
const auto& name = trafficCounter.getName();
92+
const auto& newTypes = trafficCounter.getTypes();
93+
94+
auto& counters = *swConfig.trafficCounters();
95+
auto it = std::find_if(
96+
counters.begin(), counters.end(), [&](const cfg::TrafficCounter& c) {
97+
return *c.name() == name;
98+
});
99+
100+
if (it != counters.end()) {
101+
if (*it->types() == newTypes) {
102+
return fmt::format(
103+
"Traffic counter '{}' is already configured with type '{}'",
104+
name,
105+
typesToString(newTypes));
106+
}
107+
*it->types() = newTypes;
108+
session.saveConfig(
109+
cli::ServiceType::AGENT, cli::ConfigActionLevel::HITLESS);
110+
return fmt::format(
111+
"Successfully updated traffic counter '{}' to type '{}'",
112+
name,
113+
typesToString(newTypes));
114+
}
115+
116+
cfg::TrafficCounter newCounter;
117+
newCounter.name() = name;
118+
newCounter.types() = newTypes;
119+
counters.push_back(std::move(newCounter));
120+
121+
session.saveConfig(cli::ServiceType::AGENT, cli::ConfigActionLevel::HITLESS);
122+
123+
return fmt::format(
124+
"Successfully created traffic counter '{}' with type '{}'",
125+
name,
126+
typesToString(newTypes));
127+
}
128+
129+
void CmdConfigTrafficCounter::printOutput(const RetType& output) {
130+
std::cout << output << std::endl;
131+
}
132+
133+
// Explicit template instantiation
134+
template void
135+
CmdHandler<CmdConfigTrafficCounter, CmdConfigTrafficCounterTraits>::run();
136+
137+
} // namespace facebook::fboss
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
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 <vector>
14+
15+
#include "fboss/agent/gen-cpp2/switch_config_types.h"
16+
#include "fboss/cli/fboss2/CmdHandler.h"
17+
18+
namespace facebook::fboss {
19+
20+
// Args: <name> <types>, where types is a comma separated list of
21+
// PACKETS,BYTES
22+
class TrafficCounterArg : public utils::BaseObjectArgType<std::string> {
23+
public:
24+
/* implicit */ TrafficCounterArg( // NOLINT(google-explicit-constructor)
25+
std::vector<std::string> v);
26+
27+
const std::string& getName() const {
28+
return name_;
29+
}
30+
31+
const std::vector<cfg::CounterType>& getTypes() const {
32+
return types_;
33+
}
34+
35+
private:
36+
std::string name_;
37+
std::vector<cfg::CounterType> types_;
38+
};
39+
40+
struct CmdConfigTrafficCounterTraits : public WriteCommandTraits {
41+
static void addCliArg(CLI::App& cmd, std::vector<std::string>& args) {
42+
cmd.add_option(
43+
"name_and_types",
44+
args,
45+
"Counter name followed by a comma separated list of counter types (PACKETS,BYTES)")
46+
->expected(2);
47+
}
48+
using ObjectArgType = TrafficCounterArg;
49+
using RetType = std::string;
50+
};
51+
52+
class CmdConfigTrafficCounter : public CmdHandler<
53+
CmdConfigTrafficCounter,
54+
CmdConfigTrafficCounterTraits> {
55+
public:
56+
using ObjectArgType = CmdConfigTrafficCounterTraits::ObjectArgType;
57+
using RetType = CmdConfigTrafficCounterTraits::RetType;
58+
59+
RetType queryClient(
60+
const HostInfo& hostInfo,
61+
const ObjectArgType& trafficCounter);
62+
63+
void printOutput(const RetType& output);
64+
};
65+
66+
} // namespace facebook::fboss

0 commit comments

Comments
 (0)