Skip to content

Commit 89fd9c1

Browse files
Add fboss2-dev config and delete traffic-counter CLI
fboss2-dev config traffic-counter <name> [packets|bytes|both] creates or updates a named entry in sw.trafficCounters. The handler upserts by counter name: an existing counter's types are updated in place, otherwise a new TrafficCounter is appended. The type argument is optional and defaults to packets; both maps to the (PACKETS, BYTES) pair. 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> both -> 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. A second case asserts deleting a nonexistent counter exits non-zero. 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 855a784 commit 89fd9c1

14 files changed

Lines changed: 829 additions & 0 deletions

cmake/CliFboss2.cmake

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,6 +1042,8 @@ add_library(fboss2_config_lib
10421042
fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.cpp
10431043
fboss/cli/fboss2/commands/config/session/CmdConfigSessionRebase.h
10441044
fboss/cli/fboss2/commands/config/session/CmdConfigSessionRebase.cpp
1045+
fboss/cli/fboss2/commands/config/traffic_counter/CmdConfigTrafficCounter.cpp
1046+
fboss/cli/fboss2/commands/config/traffic_counter/CmdConfigTrafficCounter.h
10451047
fboss/cli/fboss2/commands/config/tunnel/CmdConfigTunnel.cpp
10461048
fboss/cli/fboss2/commands/config/tunnel/CmdConfigTunnel.h
10471049
fboss/cli/fboss2/commands/config/tunnel/ip_in_ip/CmdConfigTunnelIpInIp.cpp
@@ -1092,6 +1094,8 @@ add_library(fboss2_config_lib
10921094
fboss/cli/fboss2/commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h
10931095
fboss/cli/fboss2/commands/delete/arp/CmdDeleteArp.cpp
10941096
fboss/cli/fboss2/commands/delete/arp/CmdDeleteArp.h
1097+
fboss/cli/fboss2/commands/delete/traffic_counter/CmdDeleteTrafficCounter.cpp
1098+
fboss/cli/fboss2/commands/delete/traffic_counter/CmdDeleteTrafficCounter.h
10951099
fboss/cli/fboss2/commands/delete/tunnel/CmdDeleteTunnel.cpp
10961100
fboss/cli/fboss2/commands/delete/tunnel/CmdDeleteTunnel.h
10971101
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
@@ -45,6 +46,7 @@ add_executable(fboss2_cmd_config_test
4546
fboss/cli/fboss2/test/config/CmdDeleteInterfaceTest.cpp
4647
fboss/cli/fboss2/test/config/CmdDeleteQosQueueConfigTest.cpp
4748
fboss/cli/fboss2/test/config/CmdDeleteTunnelIpInIpTest.cpp
49+
fboss/cli/fboss2/test/config/CmdDeleteTrafficCounterTest.cpp
4850
fboss/cli/fboss2/test/config/CmdDeleteVlanTest.cpp
4951
fboss/cli/fboss2/test/config/ConfigSessionSystemdTest.cpp
5052
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
@@ -1188,6 +1188,7 @@ cpp_library(
11881188
"commands/config/switch/admin_distance/CmdConfigAdminDistance.cpp",
11891189
"commands/config/switch/hostname/CmdConfigHostname.cpp",
11901190
"commands/config/switch/icmpv4_unavailable_src_addr/CmdConfigIcmpV4UnavailableSrcAddr.cpp",
1191+
"commands/config/traffic_counter/CmdConfigTrafficCounter.cpp",
11911192
"commands/config/tunnel/CmdConfigTunnel.cpp",
11921193
"commands/config/tunnel/ip_in_ip/CmdConfigTunnelIpInIp.cpp",
11931194
"commands/config/tunnel/ip_in_ip/TunnelIpInIpConfigUtils.cpp",
@@ -1217,6 +1218,7 @@ cpp_library(
12171218
"commands/delete/qos/default_policy/CmdDeleteQosDefaultPolicy.cpp",
12181219
"commands/delete/qos/queue_config/CmdDeleteQosQueueConfig.cpp",
12191220
"commands/delete/qos/queue_config/CmdDeleteQosQueueConfigQueueId.cpp",
1221+
"commands/delete/traffic_counter/CmdDeleteTrafficCounter.cpp",
12201222
"commands/delete/tunnel/CmdDeleteTunnel.cpp",
12211223
"commands/delete/tunnel/ip_in_ip/CmdDeleteTunnelIpInIp.cpp",
12221224
"commands/delete/tunnel/ip_in_ip/TunnelIpInIpDeleteUtils.cpp",
@@ -1330,6 +1332,7 @@ cpp_library(
13301332
"commands/config/switch/admin_distance/CmdConfigAdminDistance.h",
13311333
"commands/config/switch/hostname/CmdConfigHostname.h",
13321334
"commands/config/switch/icmpv4_unavailable_src_addr/CmdConfigIcmpV4UnavailableSrcAddr.h",
1335+
"commands/config/traffic_counter/CmdConfigTrafficCounter.h",
13331336
"commands/config/tunnel/CmdConfigTunnel.h",
13341337
"commands/config/tunnel/ip_in_ip/CmdConfigTunnelIpInIp.h",
13351338
"commands/config/tunnel/ip_in_ip/TunnelIpInIpConfigUtils.h",
@@ -1359,6 +1362,7 @@ cpp_library(
13591362
"commands/delete/qos/default_policy/CmdDeleteQosDefaultPolicy.h",
13601363
"commands/delete/qos/queue_config/CmdDeleteQosQueueConfig.h",
13611364
"commands/delete/qos/queue_config/CmdDeleteQosQueueConfigQueueId.h",
1365+
"commands/delete/traffic_counter/CmdDeleteTrafficCounter.h",
13621366
"commands/delete/tunnel/CmdDeleteTunnel.h",
13631367
"commands/delete/tunnel/ip_in_ip/CmdDeleteTunnelIpInIp.h",
13641368
"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"
@@ -146,6 +147,7 @@
146147
#include "fboss/cli/fboss2/commands/delete/qos/default_policy/CmdDeleteQosDefaultPolicy.h"
147148
#include "fboss/cli/fboss2/commands/delete/qos/queue_config/CmdDeleteQosQueueConfig.h"
148149
#include "fboss/cli/fboss2/commands/delete/qos/queue_config/CmdDeleteQosQueueConfigQueueId.h"
150+
#include "fboss/cli/fboss2/commands/delete/traffic_counter/CmdDeleteTrafficCounter.h"
149151
#include "fboss/cli/fboss2/commands/delete/tunnel/CmdDeleteTunnel.h"
150152
#include "fboss/cli/fboss2/commands/delete/tunnel/ip_in_ip/CmdDeleteTunnelIpInIp.h"
151153
#include "fboss/cli/fboss2/commands/delete/tunnel/ip_in_ip/decap/CmdDeleteTunnelIpInIpDecap.h"
@@ -1018,6 +1020,12 @@ const CommandTree& kConfigCommandTree() {
10181020
}},
10191021
}}},
10201022

1023+
{"config",
1024+
"traffic-counter",
1025+
"Create or update a named traffic counter (packets|bytes|both)",
1026+
commandHandler<CmdConfigTrafficCounter>,
1027+
argRegistrar<CmdConfigTrafficCounterTraits>},
1028+
10211029
{
10221030
"config",
10231031
"vlan",
@@ -1224,6 +1232,13 @@ const CommandTree& kConfigCommandTree() {
12241232
}},
12251233
}}},
12261234

1235+
{"delete",
1236+
"traffic-counter",
1237+
"Delete a traffic counter (refuses while a traffic-policy match action "
1238+
"references it): <name>",
1239+
commandHandler<CmdDeleteTrafficCounter>,
1240+
argRegistrar<CmdDeleteTrafficCounterTraits>},
1241+
12271242
{"delete",
12281243
"vlan",
12291244
"Delete a VLAN and its interface (refuses while it is the default VLAN or a port's ingress VLAN)",
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
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+
constexpr std::string_view kTypePackets = "packets";
25+
constexpr std::string_view kTypeBytes = "bytes";
26+
constexpr std::string_view kTypeBoth = "both";
27+
28+
std::string typesToString(const std::vector<cfg::CounterType>& types) {
29+
bool hasPackets =
30+
std::find(types.begin(), types.end(), cfg::CounterType::PACKETS) !=
31+
types.end();
32+
bool hasBytes =
33+
std::find(types.begin(), types.end(), cfg::CounterType::BYTES) !=
34+
types.end();
35+
if (hasPackets && hasBytes) {
36+
return std::string(kTypeBoth);
37+
}
38+
if (hasBytes) {
39+
return std::string(kTypeBytes);
40+
}
41+
return std::string(kTypePackets);
42+
}
43+
} // namespace
44+
45+
TrafficCounterArg::TrafficCounterArg(std::vector<std::string> v) {
46+
if (v.empty()) {
47+
throw std::invalid_argument(
48+
"Counter name is required, followed by an optional type (packets|bytes|both)");
49+
}
50+
if (v.size() > 2) {
51+
throw std::invalid_argument(
52+
"Expected <name> [packets|bytes|both]; got too many arguments");
53+
}
54+
55+
name_ = v[0];
56+
if (name_.empty()) {
57+
throw std::invalid_argument("Counter name must not be empty");
58+
}
59+
data_.push_back(v[0]);
60+
61+
if (v.size() == 2) {
62+
std::string typeStr = v[1];
63+
folly::toLowerAscii(typeStr);
64+
if (typeStr == kTypePackets) {
65+
types_ = {cfg::CounterType::PACKETS};
66+
} else if (typeStr == kTypeBytes) {
67+
types_ = {cfg::CounterType::BYTES};
68+
} else if (typeStr == kTypeBoth) {
69+
types_ = {cfg::CounterType::PACKETS, cfg::CounterType::BYTES};
70+
} else {
71+
throw std::invalid_argument(
72+
"Invalid counter type '" + v[1] +
73+
"'. Expected 'packets', 'bytes', or 'both'");
74+
}
75+
data_.push_back(v[1]);
76+
}
77+
}
78+
79+
CmdConfigTrafficCounterTraits::RetType CmdConfigTrafficCounter::queryClient(
80+
const HostInfo& /* hostInfo */,
81+
const ObjectArgType& trafficCounter) {
82+
auto& session = ConfigSession::getInstance();
83+
auto& config = session.getAgentConfig();
84+
auto& swConfig = *config.sw();
85+
86+
const auto& name = trafficCounter.getName();
87+
const auto& newTypes = trafficCounter.getTypes();
88+
89+
auto& counters = *swConfig.trafficCounters();
90+
auto it = std::find_if(
91+
counters.begin(), counters.end(), [&](const cfg::TrafficCounter& c) {
92+
return *c.name() == name;
93+
});
94+
95+
if (it != counters.end()) {
96+
if (*it->types() == newTypes) {
97+
return fmt::format(
98+
"Traffic counter '{}' is already configured with type '{}'",
99+
name,
100+
typesToString(newTypes));
101+
}
102+
*it->types() = newTypes;
103+
session.saveConfig(
104+
cli::ServiceType::AGENT, cli::ConfigActionLevel::HITLESS);
105+
return fmt::format(
106+
"Successfully updated traffic counter '{}' to type '{}'",
107+
name,
108+
typesToString(newTypes));
109+
}
110+
111+
cfg::TrafficCounter newCounter;
112+
newCounter.name() = name;
113+
newCounter.types() = newTypes;
114+
counters.push_back(std::move(newCounter));
115+
116+
session.saveConfig(cli::ServiceType::AGENT, cli::ConfigActionLevel::HITLESS);
117+
118+
return fmt::format(
119+
"Successfully created traffic counter '{}' with type '{}'",
120+
name,
121+
typesToString(newTypes));
122+
}
123+
124+
void CmdConfigTrafficCounter::printOutput(const RetType& output) {
125+
std::cout << output << std::endl;
126+
}
127+
128+
// Explicit template instantiation
129+
template void
130+
CmdHandler<CmdConfigTrafficCounter, CmdConfigTrafficCounterTraits>::run();
131+
132+
} // namespace facebook::fboss
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
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> [packets|bytes|both]
21+
class TrafficCounterArg : public utils::BaseObjectArgType<std::string> {
22+
public:
23+
/* implicit */ TrafficCounterArg( // NOLINT(google-explicit-constructor)
24+
std::vector<std::string> v);
25+
26+
const std::string& getName() const {
27+
return name_;
28+
}
29+
30+
const std::vector<cfg::CounterType>& getTypes() const {
31+
return types_;
32+
}
33+
34+
private:
35+
std::string name_;
36+
std::vector<cfg::CounterType> types_{cfg::CounterType::PACKETS};
37+
};
38+
39+
struct CmdConfigTrafficCounterTraits : public WriteCommandTraits {
40+
static void addCliArg(CLI::App& cmd, std::vector<std::string>& args) {
41+
cmd.add_option(
42+
"name_and_type",
43+
args,
44+
"Counter name followed by an optional type (packets|bytes|both); defaults to packets")
45+
->expected(1, 2);
46+
}
47+
using ObjectArgType = TrafficCounterArg;
48+
using RetType = std::string;
49+
};
50+
51+
class CmdConfigTrafficCounter : public CmdHandler<
52+
CmdConfigTrafficCounter,
53+
CmdConfigTrafficCounterTraits> {
54+
public:
55+
using ObjectArgType = CmdConfigTrafficCounterTraits::ObjectArgType;
56+
using RetType = CmdConfigTrafficCounterTraits::RetType;
57+
58+
RetType queryClient(
59+
const HostInfo& hostInfo,
60+
const ObjectArgType& trafficCounter);
61+
62+
void printOutput(const RetType& output);
63+
};
64+
65+
} // namespace facebook::fboss

0 commit comments

Comments
 (0)