diff --git a/cmake/CliFboss2.cmake b/cmake/CliFboss2.cmake index 75a97f40d27b9..945ea0da93197 100644 --- a/cmake/CliFboss2.cmake +++ b/cmake/CliFboss2.cmake @@ -995,6 +995,8 @@ add_library(fboss2_config_lib fboss/cli/fboss2/commands/config/qos/PortQueueConfigUtils.h fboss/cli/fboss2/commands/config/qos/default_policy/CmdConfigQosDefaultPolicy.cpp fboss/cli/fboss2/commands/config/qos/default_policy/CmdConfigQosDefaultPolicy.h + fboss/cli/fboss2/commands/config/qos/QosPolicyUtils.cpp + fboss/cli/fboss2/commands/config/qos/QosPolicyUtils.h fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.cpp fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.h fboss/cli/fboss2/commands/delete/qos/default_policy/CmdDeleteQosDefaultPolicy.cpp @@ -1090,6 +1092,12 @@ add_library(fboss2_config_lib fboss/cli/fboss2/commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h fboss/cli/fboss2/commands/delete/arp/CmdDeleteArp.cpp fboss/cli/fboss2/commands/delete/arp/CmdDeleteArp.h + fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.cpp + fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.h + fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.cpp + fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.h + fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.cpp + fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.h fboss/cli/fboss2/commands/delete/tunnel/CmdDeleteTunnel.cpp fboss/cli/fboss2/commands/delete/tunnel/CmdDeleteTunnel.h fboss/cli/fboss2/commands/delete/tunnel/ip_in_ip/CmdDeleteTunnelIpInIp.cpp diff --git a/cmake/CliFboss2TestConfig.cmake b/cmake/CliFboss2TestConfig.cmake index d0e15d2497718..382041e2f646e 100644 --- a/cmake/CliFboss2TestConfig.cmake +++ b/cmake/CliFboss2TestConfig.cmake @@ -43,6 +43,7 @@ add_executable(fboss2_cmd_config_test fboss/cli/fboss2/test/config/CmdDeleteDhcpTest.cpp fboss/cli/fboss2/test/config/CmdDeleteInterfaceIpv6NdpTest.cpp fboss/cli/fboss2/test/config/CmdDeleteInterfaceTest.cpp + fboss/cli/fboss2/test/config/CmdDeleteQosPolicyTest.cpp fboss/cli/fboss2/test/config/CmdDeleteQosQueueConfigTest.cpp fboss/cli/fboss2/test/config/CmdDeleteTunnelIpInIpTest.cpp fboss/cli/fboss2/test/config/CmdDeleteVlanTest.cpp diff --git a/cmake/CliFboss2TestIntegrationTest.cmake b/cmake/CliFboss2TestIntegrationTest.cmake index 1873293ac8710..6cb4f26541d12 100644 --- a/cmake/CliFboss2TestIntegrationTest.cmake +++ b/cmake/CliFboss2TestIntegrationTest.cmake @@ -45,6 +45,7 @@ add_executable(fboss2_integration_test fboss/cli/fboss2/test/integration_test/DeleteArpTest.cpp fboss/cli/fboss2/test/integration_test/DeleteDhcpSourceOverrideTest.cpp fboss/cli/fboss2/test/integration_test/DeleteInterfaceIpv6NdpTest.cpp + fboss/cli/fboss2/test/integration_test/DeleteQosPolicyTest.cpp fboss/cli/fboss2/test/integration_test/DeleteVlanTest.cpp fboss/cli/fboss2/test/integration_test/TunnelIpInIpTest.cpp fboss/cli/fboss2/utils/CmdInitUtils.cpp diff --git a/fboss/cli/fboss2/BUCK b/fboss/cli/fboss2/BUCK index 9e37cb04308c1..fe92d87f28a17 100644 --- a/fboss/cli/fboss2/BUCK +++ b/fboss/cli/fboss2/BUCK @@ -1161,6 +1161,7 @@ cpp_library( "commands/config/qos/PortQueueConfigUtils.cpp", "commands/config/qos/buffer_pool/CmdConfigQosBufferPool.cpp", "commands/config/qos/default_policy/CmdConfigQosDefaultPolicy.cpp", + "commands/config/qos/QosPolicyUtils.cpp", "commands/config/qos/policy/CmdConfigQosPolicy.cpp", "commands/config/qos/policy/CmdConfigQosPolicyMap.cpp", "commands/config/qos/priority_group_policy/CmdConfigQosPriorityGroupPolicy.cpp", @@ -1207,6 +1208,8 @@ cpp_library( "commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.cpp", "commands/delete/qos/CmdDeleteQos.cpp", "commands/delete/qos/default_policy/CmdDeleteQosDefaultPolicy.cpp", + "commands/delete/qos/policy/CmdDeleteQosPolicy.cpp", + "commands/delete/qos/policy/CmdDeleteQosPolicyMap.cpp", "commands/delete/qos/queue_config/CmdDeleteQosQueueConfig.cpp", "commands/delete/qos/queue_config/CmdDeleteQosQueueConfigQueueId.cpp", "commands/delete/tunnel/CmdDeleteTunnel.cpp", @@ -1303,6 +1306,7 @@ cpp_library( "commands/config/qos/PortQueueConfigUtils.h", "commands/config/qos/buffer_pool/CmdConfigQosBufferPool.h", "commands/config/qos/default_policy/CmdConfigQosDefaultPolicy.h", + "commands/config/qos/QosPolicyUtils.h", "commands/config/qos/policy/CmdConfigQosPolicy.h", "commands/config/qos/policy/CmdConfigQosPolicyMap.h", "commands/config/qos/priority_group_policy/CmdConfigQosPriorityGroupPolicy.h", @@ -1349,6 +1353,8 @@ cpp_library( "commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h", "commands/delete/qos/CmdDeleteQos.h", "commands/delete/qos/default_policy/CmdDeleteQosDefaultPolicy.h", + "commands/delete/qos/policy/CmdDeleteQosPolicy.h", + "commands/delete/qos/policy/CmdDeleteQosPolicyMap.h", "commands/delete/qos/queue_config/CmdDeleteQosQueueConfig.h", "commands/delete/qos/queue_config/CmdDeleteQosQueueConfigQueueId.h", "commands/delete/tunnel/CmdDeleteTunnel.h", diff --git a/fboss/cli/fboss2/CmdListConfig.cpp b/fboss/cli/fboss2/CmdListConfig.cpp index 164a2fdca30c9..af42b2a4e065e 100644 --- a/fboss/cli/fboss2/CmdListConfig.cpp +++ b/fboss/cli/fboss2/CmdListConfig.cpp @@ -144,6 +144,8 @@ #include "fboss/cli/fboss2/commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h" #include "fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.h" #include "fboss/cli/fboss2/commands/delete/qos/default_policy/CmdDeleteQosDefaultPolicy.h" +#include "fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.h" +#include "fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.h" #include "fboss/cli/fboss2/commands/delete/qos/queue_config/CmdDeleteQosQueueConfig.h" #include "fboss/cli/fboss2/commands/delete/qos/queue_config/CmdDeleteQosQueueConfigQueueId.h" #include "fboss/cli/fboss2/commands/delete/tunnel/CmdDeleteTunnel.h" @@ -1122,6 +1124,18 @@ const CommandTree& kConfigCommandTree() { commandHandler, argRegistrar, }}, + }, + { + "policy", + "Delete a QoS policy or one of its map entries", + commandHandler, + argRegistrar, + {{ + "map", + "Remove a QoS map entry (dscp, tc-to-queue)", + commandHandler, + argRegistrar, + }}, }}, }, diff --git a/fboss/cli/fboss2/commands/config/qos/QosPolicyUtils.cpp b/fboss/cli/fboss2/commands/config/qos/QosPolicyUtils.cpp new file mode 100644 index 0000000000000..5a5e3b9925ba3 --- /dev/null +++ b/fboss/cli/fboss2/commands/config/qos/QosPolicyUtils.cpp @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2004-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + */ + +#include "fboss/cli/fboss2/commands/config/qos/QosPolicyUtils.h" + +#include +#include +#include + +namespace facebook::fboss::utils { + +std::vector::iterator findQosPolicy( + std::vector& qosPolicies, + const std::string& name) { + return std::find_if( + qosPolicies.begin(), qosPolicies.end(), [&name](const auto& policy) { + return *policy.name() == name; + }); +} + +std::vector::iterator findQosPolicyOrThrow( + std::vector& qosPolicies, + const std::string& name) { + auto it = findQosPolicy(qosPolicies, name); + if (it == qosPolicies.end()) { + throw std::runtime_error( + fmt::format("No QoS policy named '{}' exists", name)); + } + return it; +} + +} // namespace facebook::fboss::utils diff --git a/fboss/cli/fboss2/commands/config/qos/QosPolicyUtils.h b/fboss/cli/fboss2/commands/config/qos/QosPolicyUtils.h new file mode 100644 index 0000000000000..d503549df180f --- /dev/null +++ b/fboss/cli/fboss2/commands/config/qos/QosPolicyUtils.h @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2004-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + */ + +#pragma once + +#include +#include + +#include "fboss/agent/gen-cpp2/switch_config_types.h" + +namespace facebook::fboss::utils { + +// DSCP is a 6-bit field (RFC 2474): valid codepoints are 0..63. Shared so the +// config and delete qos-policy-map commands validate against one definition +// rather than each carrying their own copy. +constexpr int16_t kMinDscp = 0; +constexpr int16_t kMaxDscp = 63; + +// Locates the QosPolicy named `name` in `qosPolicies`, or end() when absent. +// sw.qosPolicies is a list keyed only by the name field, so every command that +// touches a policy has to do this scan; sharing it keeps `config qos policy` +// and `delete qos policy` agreeing on what "the policy called X" means. +// +// Returns an iterator rather than a pointer so callers can erase. +std::vector::iterator findQosPolicy( + std::vector& qosPolicies, + const std::string& name); + +// Like findQosPolicy, but throws std::runtime_error with a uniform +// "No QoS policy named '' exists" message when absent, so every delete +// command reports the same error instead of repeating the find-then-throw. +std::vector::iterator findQosPolicyOrThrow( + std::vector& qosPolicies, + const std::string& name); + +} // namespace facebook::fboss::utils diff --git a/fboss/cli/fboss2/commands/config/qos/policy/CmdConfigQosPolicyMap.cpp b/fboss/cli/fboss2/commands/config/qos/policy/CmdConfigQosPolicyMap.cpp index e34e87b4ff256..e1efa8221d6fc 100644 --- a/fboss/cli/fboss2/commands/config/qos/policy/CmdConfigQosPolicyMap.cpp +++ b/fboss/cli/fboss2/commands/config/qos/policy/CmdConfigQosPolicyMap.cpp @@ -25,6 +25,7 @@ #include #include "fboss/agent/gen-cpp2/switch_config_types.h" +#include "fboss/cli/fboss2/commands/config/qos/QosPolicyUtils.h" #include "fboss/cli/fboss2/commands/config/qos/policy/CmdConfigQosPolicy.h" #include "fboss/cli/fboss2/session/ConfigSession.h" #include "fboss/cli/fboss2/utils/HostInfo.h" @@ -35,9 +36,8 @@ namespace { constexpr int16_t kMinTCValue = 0; constexpr int16_t kMaxTCValue = 7; -// DSCP: 6-bit field (RFC 2474) -constexpr int8_t kMinDscpValue = 0; -constexpr int8_t kMaxDscpValue = 63; +// DSCP bounds (0..63) are shared with the delete command via utils::kMinDscp / +// utils::kMaxDscp in QosPolicyUtils.h. // MPLS EXP/TC: 3-bit field (RFC 3032, RFC 5462) constexpr int8_t kMinExpValue = 0; constexpr int8_t kMaxExpValue = 7; @@ -74,12 +74,12 @@ std::string getMapTypeString(QosMapType mapType, QosMapDirection direction) { // Validates value range based on type token and returns the map type. QosMapType validateAndGetMapType(const std::string& typeToken, int16_t value) { if (typeToken == "dscp") { - if (value < kMinDscpValue || value > kMaxDscpValue) { + if (value < utils::kMinDscp || value > utils::kMaxDscp) { throw std::invalid_argument( fmt::format( "DSCP value must be between {} and {}, got: {}", - kMinDscpValue, - kMaxDscpValue, + utils::kMinDscp, + utils::kMaxDscp, value)); } return QosMapType::DSCP; @@ -317,11 +317,9 @@ CmdConfigQosPolicyMapTraits::RetType CmdConfigQosPolicyMap::queryClient( // Find or create the QosPolicy with the given name cfg::QosPolicy* targetPolicy = nullptr; - for (auto& policy : qosPolicies) { - if (*policy.name() == name) { - targetPolicy = &policy; - break; - } + if (auto it = utils::findQosPolicy(qosPolicies, name); + it != qosPolicies.end()) { + targetPolicy = &*it; } if (targetPolicy == nullptr) { diff --git a/fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.cpp b/fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.cpp new file mode 100644 index 0000000000000..ef24c14a6b59c --- /dev/null +++ b/fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.cpp @@ -0,0 +1,119 @@ +/* + * Copyright (c) 2004-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + */ + +#include "fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.h" + +#include "fboss/cli/fboss2/CmdHandler.cpp" + +#include +#include +#include +#include +#include +#include + +#include "fboss/agent/gen-cpp2/switch_config_types.h" +#include "fboss/cli/fboss2/commands/config/qos/QosPolicyUtils.h" +#include "fboss/cli/fboss2/session/ConfigSession.h" +#include "fboss/cli/fboss2/utils/HostInfo.h" + +namespace facebook::fboss { + +namespace { + +// Collect every place a TrafficPolicyConfig can name a QoS policy, so the +// delete refuses instead of leaving a config that points at a policy which no +// longer exists. +void collectReferences( + const cfg::TrafficPolicyConfig& policyConfig, + const std::string& name, + const std::string& location, + std::vector& refs) { + if (policyConfig.defaultQosPolicy().has_value() && + *policyConfig.defaultQosPolicy() == name) { + refs.push_back(fmt::format("{}.defaultQosPolicy", location)); + } + if (policyConfig.portIdToQosPolicy().has_value()) { + for (const auto& [portId, policyName] : *policyConfig.portIdToQosPolicy()) { + if (policyName == name) { + refs.push_back( + fmt::format("{}.portIdToQosPolicy[{}]", location, portId)); + } + } + } +} + +std::vector findReferences( + const cfg::SwitchConfig& switchConfig, + const std::string& name) { + std::vector refs; + if (switchConfig.dataPlaneTrafficPolicy().has_value()) { + collectReferences( + *switchConfig.dataPlaneTrafficPolicy(), + name, + "dataPlaneTrafficPolicy", + refs); + } + if (switchConfig.cpuTrafficPolicy().has_value() && + switchConfig.cpuTrafficPolicy()->trafficPolicy().has_value()) { + collectReferences( + *switchConfig.cpuTrafficPolicy()->trafficPolicy(), + name, + "cpuTrafficPolicy.trafficPolicy", + refs); + } + return refs; +} + +} // namespace + +CmdDeleteQosPolicyTraits::RetType CmdDeleteQosPolicy::queryClient( + const HostInfo& /* hostInfo */, + const ObjectArgType& policyName) { + auto& session = ConfigSession::getInstance(); + auto& agentConfig = session.getAgentConfig(); + auto& switchConfig = *agentConfig.sw(); + + const std::string name = policyName.getName(); + if (name.empty()) { + throw std::runtime_error("qos policy name is required"); + } + auto& qosPolicies = *switchConfig.qosPolicies(); + + auto it = utils::findQosPolicyOrThrow(qosPolicies, name); + + // Refuse rather than cascade: clearing the referring field would silently + // change forwarding behaviour on ports the user did not name. Point at the + // exact fields so the operator knows what to unset first — there is no CLI + // today for defaultQosPolicy / portIdToQosPolicy (unlike + // `delete interface … queuing-policy`). + auto refs = findReferences(switchConfig, name); + if (!refs.empty()) { + throw std::runtime_error( + fmt::format( + "Cannot delete QoS policy '{}': still referenced by {}. " + "Unset those fields first, then retry the delete.", + name, + folly::join(", ", refs))); + } + + qosPolicies.erase(it); + session.saveConfig(); + + return fmt::format("Successfully deleted QoS policy '{}'", name); +} + +void CmdDeleteQosPolicy::printOutput(const RetType& logMsg) { + std::cout << logMsg << std::endl; +} + +template void CmdHandler::run(); + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.h b/fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.h new file mode 100644 index 0000000000000..f7a6ea4813b5b --- /dev/null +++ b/fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.h @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2004-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + */ + +#pragma once + +#include +#include +#include "fboss/cli/fboss2/CmdHandler.h" +#include "fboss/cli/fboss2/commands/config/qos/policy/CmdConfigQosPolicy.h" +#include "fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.h" +#include "fboss/cli/fboss2/utils/HostInfo.h" + +namespace facebook::fboss { + +struct CmdDeleteQosPolicyTraits : public WriteCommandTraits { + using ParentCmd = CmdDeleteQos; + static void addCliArg(CLI::App& cmd, std::vector& args) { + // required() + expected(1) keeps CLI11 from reclassifying the policy name + // as a subcommand when it happens to match one elsewhere in the tree. + cmd.add_option( + "qos_policy_name", + args, + "Name of the QoS policy to remove from qosPolicies") + ->required() + ->expected(1); + } + // Shared with `config qos policy` so the accepted name syntax cannot drift + // between the two commands. + using ObjectArgType = QosPolicyName; + using RetType = std::string; +}; + +class CmdDeleteQosPolicy + : public CmdHandler { + public: + using ObjectArgType = CmdDeleteQosPolicyTraits::ObjectArgType; + using RetType = CmdDeleteQosPolicyTraits::RetType; + + RetType queryClient( + const HostInfo& hostInfo, + const ObjectArgType& policyName); + + void printOutput(const RetType& logMsg); +}; + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.cpp b/fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.cpp new file mode 100644 index 0000000000000..1e0985ec664ea --- /dev/null +++ b/fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.cpp @@ -0,0 +1,320 @@ +/* + * Copyright (c) 2004-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + */ + +#include "fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.h" + +#include "fboss/cli/fboss2/CmdHandler.cpp" + +#include +#include +#include +#include +#include +#include +#include + +#include "fboss/agent/gen-cpp2/switch_config_types.h" +#include "fboss/cli/fboss2/commands/config/qos/QosPolicyUtils.h" +#include "fboss/cli/fboss2/session/ConfigSession.h" +#include "fboss/cli/fboss2/utils/HostInfo.h" + +namespace facebook::fboss { + +namespace { +constexpr auto kSupportedMapTypes = + "dscp, mpls-exp, dot1p, tc-to-queue, pfc-pri-to-queue, tc-to-pg, " + "pfc-pri-to-pg"; +// Traffic classes, PFC priorities, EXP and PCP codepoints are all 3-bit. +constexpr int16_t kMinMapKey = 0; +constexpr int16_t kMaxMapKey = 7; +} // namespace + +DeleteQosMapEntry::DeleteQosMapEntry(std::vector v) { + if (v.size() != 2) { + throw std::invalid_argument( + fmt::format( + "Expected ' ', got {} argument(s). " + "Valid map types: {}", + v.size(), + kSupportedMapTypes)); + } + + const std::string& mapType = v[0]; + if (mapType == "dscp") { + mapType_ = DeleteQosMapType::DSCP; + } else if (mapType == "mpls-exp") { + mapType_ = DeleteQosMapType::MPLS_EXP; + } else if (mapType == "dot1p") { + mapType_ = DeleteQosMapType::DOT1P; + } else if (mapType == "tc-to-queue") { + mapType_ = DeleteQosMapType::TC_TO_QUEUE; + } else if (mapType == "pfc-pri-to-queue") { + mapType_ = DeleteQosMapType::PFC_PRI_TO_QUEUE; + } else if (mapType == "tc-to-pg") { + mapType_ = DeleteQosMapType::TC_TO_PG; + } else if (mapType == "pfc-pri-to-pg") { + mapType_ = DeleteQosMapType::PFC_PRI_TO_PG; + } else { + throw std::invalid_argument( + fmt::format( + "Unknown map type '{}'. Valid map types: {}", + mapType, + kSupportedMapTypes)); + } + + auto parsed = folly::tryTo(v[1]); + if (!parsed.hasValue()) { + throw std::invalid_argument( + fmt::format("Invalid value '{}'. Must be an integer", v[1])); + } + key_ = parsed.value(); + + if (mapType_ == DeleteQosMapType::DSCP) { + if (key_ < utils::kMinDscp || key_ > utils::kMaxDscp) { + throw std::invalid_argument( + fmt::format( + "dscp value must be between {} and {}, got: {}", + utils::kMinDscp, + utils::kMaxDscp, + v[1])); + } + } else if (key_ < kMinMapKey || key_ > kMaxMapKey) { + throw std::invalid_argument( + fmt::format( + "{} value must be between {} and {}, got: {}", + mapType, + kMinMapKey, + kMaxMapKey, + v[1])); + } + + data_ = std::move(v); +} + +namespace { + +// Removes `codepoint` from whichever map entry's ingress list carries it. An +// entry that ends up with no ingress codepoints and no egress rewrite carries +// no information, so it is dropped rather than left behind as an empty shell. +// Shared by the three structurally identical list maps (DscpQosMap, ExpQosMap, +// PcpQosMap); the projections select the ingress list and the egress rewrite. +template +bool eraseCodepoint( + std::vector& maps, + int16_t codepoint, + GetIngressList getIngressList, + GetEgressValue getEgressValue) { + auto byteVal = static_cast(codepoint); + bool erased = false; + + for (auto& entry : maps) { + auto& fromList = getIngressList(entry); + auto it = std::find(fromList.begin(), fromList.end(), byteVal); + if (it != fromList.end()) { + fromList.erase(it); + erased = true; + } + } + + if (erased) { + maps.erase( + std::remove_if( + maps.begin(), + maps.end(), + [&](const MapEntry& entry) { + return getIngressList(const_cast(entry)).empty() && + !getEgressValue(entry).has_value(); + }), + maps.end()); + } + + return erased; +} + +bool eraseDscp(std::vector& dscpMaps, int16_t dscp) { + return eraseCodepoint( + dscpMaps, + dscp, + [](cfg::DscpQosMap& e) -> auto& { return *e.fromDscpToTrafficClass(); }, + [](const cfg::DscpQosMap& e) { return e.fromTrafficClassToDscp(); }); +} + +bool eraseExp(std::vector& expMaps, int16_t exp) { + return eraseCodepoint( + expMaps, + exp, + [](cfg::ExpQosMap& e) -> auto& { return *e.fromExpToTrafficClass(); }, + [](const cfg::ExpQosMap& e) { return e.fromTrafficClassToExp(); }); +} + +bool erasePcp(std::vector& pcpMaps, int16_t pcp) { + return eraseCodepoint( + pcpMaps, + pcp, + [](cfg::PcpQosMap& e) -> auto& { return *e.fromPcpToTrafficClass(); }, + [](const cfg::PcpQosMap& e) { return e.fromTrafficClassToPcp(); }); +} + +// Erases `key` from an optional map field. Returns false when the +// field is unset or the key is absent. A map left empty by the erase is +// reset, so the field returns to its unset default. +template +bool eraseOptionalMapKey(OptionalMapRef field, int16_t key) { + if (!field.has_value()) { + return false; + } + if (field->erase(key) == 0) { + return false; + } + if (field->empty()) { + field.reset(); + } + return true; +} + +} // namespace + +CmdDeleteQosPolicyMapTraits::RetType CmdDeleteQosPolicyMap::queryClient( + const HostInfo& /* hostInfo */, + const QosPolicyName& policyName, + const ObjectArgType& entry) { + auto& session = ConfigSession::getInstance(); + auto& agentConfig = session.getAgentConfig(); + auto& switchConfig = *agentConfig.sw(); + + const std::string name = policyName.getName(); + auto& qosPolicies = *switchConfig.qosPolicies(); + + auto policyIt = utils::findQosPolicyOrThrow(qosPolicies, name); + + if (!policyIt->qosMap().has_value()) { + throw std::runtime_error( + fmt::format("QoS policy '{}' has no qosMap configured", name)); + } + auto& qosMap = *policyIt->qosMap(); + + switch (entry.getMapType()) { + case DeleteQosMapType::DSCP: { + if (!eraseDscp(*qosMap.dscpMaps(), entry.getKey())) { + throw std::runtime_error( + fmt::format( + "QoS policy '{}' has no dscp mapping for value {}", + name, + entry.getKey())); + } + session.saveConfig(); + return fmt::format( + "Successfully deleted QoS policy '{}' dscp mapping for value {}", + name, + entry.getKey()); + } + case DeleteQosMapType::MPLS_EXP: { + if (!eraseExp(*qosMap.expMaps(), entry.getKey())) { + throw std::runtime_error( + fmt::format( + "QoS policy '{}' has no mpls-exp mapping for value {}", + name, + entry.getKey())); + } + session.saveConfig(); + return fmt::format( + "Successfully deleted QoS policy '{}' mpls-exp mapping for value {}", + name, + entry.getKey()); + } + case DeleteQosMapType::DOT1P: { + bool erased = qosMap.pcpMaps().has_value() && + erasePcp(*qosMap.pcpMaps(), entry.getKey()); + if (!erased) { + throw std::runtime_error( + fmt::format( + "QoS policy '{}' has no dot1p mapping for value {}", + name, + entry.getKey())); + } + if (qosMap.pcpMaps()->empty()) { + qosMap.pcpMaps().reset(); + } + session.saveConfig(); + return fmt::format( + "Successfully deleted QoS policy '{}' dot1p mapping for value {}", + name, + entry.getKey()); + } + case DeleteQosMapType::TC_TO_QUEUE: { + auto& tcToQueue = *qosMap.trafficClassToQueueId(); + if (tcToQueue.erase(entry.getKey()) == 0) { + throw std::runtime_error( + fmt::format( + "QoS policy '{}' has no tc-to-queue mapping for traffic class {}", + name, + entry.getKey())); + } + session.saveConfig(); + return fmt::format( + "Successfully deleted QoS policy '{}' tc-to-queue mapping for traffic class {}", + name, + entry.getKey()); + } + case DeleteQosMapType::PFC_PRI_TO_QUEUE: { + if (!eraseOptionalMapKey(qosMap.pfcPriorityToQueueId(), entry.getKey())) { + throw std::runtime_error( + fmt::format( + "QoS policy '{}' has no pfc-pri-to-queue mapping for priority {}", + name, + entry.getKey())); + } + session.saveConfig(); + return fmt::format( + "Successfully deleted QoS policy '{}' pfc-pri-to-queue mapping for priority {}", + name, + entry.getKey()); + } + case DeleteQosMapType::TC_TO_PG: { + if (!eraseOptionalMapKey(qosMap.trafficClassToPgId(), entry.getKey())) { + throw std::runtime_error( + fmt::format( + "QoS policy '{}' has no tc-to-pg mapping for traffic class {}", + name, + entry.getKey())); + } + session.saveConfig(); + return fmt::format( + "Successfully deleted QoS policy '{}' tc-to-pg mapping for traffic class {}", + name, + entry.getKey()); + } + case DeleteQosMapType::PFC_PRI_TO_PG: { + if (!eraseOptionalMapKey(qosMap.pfcPriorityToPgId(), entry.getKey())) { + throw std::runtime_error( + fmt::format( + "QoS policy '{}' has no pfc-pri-to-pg mapping for priority {}", + name, + entry.getKey())); + } + session.saveConfig(); + return fmt::format( + "Successfully deleted QoS policy '{}' pfc-pri-to-pg mapping for priority {}", + name, + entry.getKey()); + } + } + + throw std::runtime_error("Unhandled map type"); +} + +void CmdDeleteQosPolicyMap::printOutput(const RetType& logMsg) { + std::cout << logMsg << std::endl; +} + +template void +CmdHandler::run(); + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.h b/fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.h new file mode 100644 index 0000000000000..7e9285d137740 --- /dev/null +++ b/fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.h @@ -0,0 +1,104 @@ +/* + * Copyright (c) 2004-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + */ + +#pragma once + +#include +#include +#include +#include "fboss/cli/fboss2/CmdHandler.h" +#include "fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.h" +#include "fboss/cli/fboss2/utils/CmdUtilsCommon.h" +#include "fboss/cli/fboss2/utils/HostInfo.h" + +namespace facebook::fboss { + +/** + * Which QosMap a delete targets — every map type CmdConfigQosPolicyMap can + * write is removable, so config and delete cannot drift apart. + */ +enum class DeleteQosMapType { + DSCP, // QosMap.dscpMaps + MPLS_EXP, // QosMap.expMaps + DOT1P, // QosMap.pcpMaps + TC_TO_QUEUE, // QosMap.trafficClassToQueueId + PFC_PRI_TO_QUEUE, // QosMap.pfcPriorityToQueueId + TC_TO_PG, // QosMap.trafficClassToPgId + PFC_PRI_TO_PG, // QosMap.pfcPriorityToPgId +}; + +/** + * Parses the entry to remove: + * + * dscp - drop a DSCP codepoint from the policy's dscpMaps + * mpls-exp - drop an EXP codepoint from expMaps + * dot1p - drop a PCP codepoint from pcpMaps + * tc-to-queue - drop a traffic class from trafficClassToQueueId + * pfc-pri-to-queue - drop a PFC priority from pfcPriorityToQueueId + * tc-to-pg - drop a traffic class from trafficClassToPgId + * pfc-pri-to-pg - drop a PFC priority from pfcPriorityToPgId + * + * The value alone identifies the entry: a codepoint appears in at most one + * ingress map entry, and the rest are map keys. + */ +class DeleteQosMapEntry : public utils::BaseObjectArgType { + public: + // NOLINTNEXTLINE(google-explicit-constructor) + /* implicit */ DeleteQosMapEntry(std::vector v); + + DeleteQosMapType getMapType() const { + return mapType_; + } + + int16_t getKey() const { + return key_; + } + + private: + DeleteQosMapType mapType_{DeleteQosMapType::TC_TO_QUEUE}; + int16_t key_{0}; +}; + +struct CmdDeleteQosPolicyMapTraits : public WriteCommandTraits { + using ParentCmd = CmdDeleteQosPolicy; + static void addCliArg(CLI::App& cmd, std::vector& args) { + cmd.add_option( + "map_entry", + args, + " where map-type is one of:\n" + " dscp remove a DSCP to traffic-class mapping\n" + " mpls-exp remove an EXP to traffic-class mapping\n" + " dot1p remove a PCP to traffic-class mapping\n" + " tc-to-queue remove a traffic-class to queue mapping\n" + " pfc-pri-to-queue remove a PFC priority to queue mapping\n" + " tc-to-pg remove a traffic-class to PG mapping\n" + " pfc-pri-to-pg remove a PFC priority to PG mapping") + ->required() + ->expected(2); + } + using ObjectArgType = DeleteQosMapEntry; + using RetType = std::string; +}; + +class CmdDeleteQosPolicyMap + : public CmdHandler { + public: + using ObjectArgType = CmdDeleteQosPolicyMapTraits::ObjectArgType; + using RetType = CmdDeleteQosPolicyMapTraits::RetType; + + RetType queryClient( + const HostInfo& hostInfo, + const QosPolicyName& policyName, + const ObjectArgType& entry); + + void printOutput(const RetType& logMsg); +}; + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/test/config/BUCK b/fboss/cli/fboss2/test/config/BUCK index ed105d2d8998b..e7e972266cfeb 100644 --- a/fboss/cli/fboss2/test/config/BUCK +++ b/fboss/cli/fboss2/test/config/BUCK @@ -50,6 +50,7 @@ cpp_unittest( "CmdDeleteTunnelIpInIpTest.cpp", "CmdDeleteVlanTest.cpp", "ConfigSessionSystemdTest.cpp", + "CmdDeleteQosPolicyTest.cpp", "ProfileValidationTest.cpp", ], headers = [ diff --git a/fboss/cli/fboss2/test/config/CmdDeleteQosPolicyTest.cpp b/fboss/cli/fboss2/test/config/CmdDeleteQosPolicyTest.cpp new file mode 100644 index 0000000000000..ef3c357a8f568 --- /dev/null +++ b/fboss/cli/fboss2/test/config/CmdDeleteQosPolicyTest.cpp @@ -0,0 +1,478 @@ +// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary. + +#include "fboss/agent/gen-cpp2/switch_config_types.h" +#include "fboss/cli/fboss2/test/config/CmdConfigTestBase.h" + +#include +#include +#include +#include + +#include "fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.h" +#include "fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.h" +#include "fboss/cli/fboss2/session/ConfigSession.h" + +namespace facebook::fboss { + +// Two policies: "unreferenced" is free to delete, "in-use" is named by +// dataPlaneTrafficPolicy.defaultQosPolicy so the delete must refuse it. +// dscpMaps groups codepoints under an internalTrafficClass, matching the +// shape CmdConfigQosPolicyMap writes. +static const std::string kSeedConfig = R"({ + "sw": { + "qosPolicies": [ + { + "name": "unreferenced", + "rules": [], + "qosMap": { + "dscpMaps": [ + {"internalTrafficClass": 0, "fromDscpToTrafficClass": [0, 1, 2]}, + {"internalTrafficClass": 1, "fromDscpToTrafficClass": [8], "fromTrafficClassToDscp": 8} + ], + "expMaps": [ + {"internalTrafficClass": 2, "fromExpToTrafficClass": [5]} + ], + "pcpMaps": [ + {"internalTrafficClass": 3, "fromPcpToTrafficClass": [6]} + ], + "trafficClassToQueueId": {"0": 0, "1": 1, "2": 2}, + "pfcPriorityToQueueId": {"3": 3}, + "trafficClassToPgId": {"4": 4, "5": 5}, + "pfcPriorityToPgId": {"6": 6} + } + }, + { + "name": "in-use", + "rules": [], + "qosMap": { + "dscpMaps": [], + "expMaps": [], + "trafficClassToQueueId": {"0": 0} + } + }, + {"name": "port-ref", "rules": []}, + {"name": "cpu-ref", "rules": []}, + {"name": "no-map", "rules": []} + ], + "dataPlaneTrafficPolicy": { + "defaultQosPolicy": "in-use", + "portIdToQosPolicy": {"1": "port-ref"} + }, + "cpuTrafficPolicy": { + "trafficPolicy": { + "defaultQosPolicy": "cpu-ref" + } + } + } +})"; + +class CmdDeleteQosPolicyTestFixture : public CmdConfigTestBase { + public: + CmdDeleteQosPolicyTestFixture() + : CmdConfigTestBase( + "fboss_del_qos_policy_test_%%%%-%%%%-%%%%-%%%%", + kSeedConfig) {} + + protected: + static const cfg::QosPolicy* findPolicy(const std::string& name) { + const auto& policies = + *ConfigSession::getInstance().getAgentConfig().sw()->qosPolicies(); + for (const auto& policy : policies) { + if (*policy.name() == name) { + return &policy; + } + } + return nullptr; + } + + static const cfg::DscpQosMap* findDscpEntry( + const std::string& policyName, + int16_t trafficClass) { + const auto* policy = findPolicy(policyName); + if (policy == nullptr || !policy->qosMap().has_value()) { + return nullptr; + } + for (const auto& entry : *policy->qosMap()->dscpMaps()) { + if (*entry.internalTrafficClass() == trafficClass) { + return &entry; + } + } + return nullptr; + } +}; + +// ---------------------------------------------------------------- arg parsing + +// The name type is shared with `config qos policy`; arity is enforced by +// CLI11 (required/expected(1)), so only the empty-name guard is ours. +TEST_F(CmdDeleteQosPolicyTestFixture, policyNameArgValidation) { + EXPECT_EQ(QosPolicyName({"p1"}).getName(), "p1"); + EXPECT_EQ(QosPolicyName({}).getName(), ""); + + setupTestableConfigSession("delete qos policy", ""); + auto cmd = CmdDeleteQosPolicy(); + EXPECT_THROW( + cmd.queryClient(localhost(), QosPolicyName({})), std::runtime_error); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, mapEntryArgValidation) { + EXPECT_EQ(DeleteQosMapEntry({"dscp", "40"}).getKey(), 40); + EXPECT_EQ( + DeleteQosMapEntry({"dscp", "40"}).getMapType(), DeleteQosMapType::DSCP); + EXPECT_EQ( + DeleteQosMapEntry({"tc-to-queue", "3"}).getMapType(), + DeleteQosMapType::TC_TO_QUEUE); + + // wrong arity + EXPECT_THROW(DeleteQosMapEntry({"dscp"}), std::invalid_argument); + EXPECT_THROW(DeleteQosMapEntry({"dscp", "1", "2"}), std::invalid_argument); + // unknown map type + EXPECT_THROW(DeleteQosMapEntry({"no-such-map", "1"}), std::invalid_argument); + // non-integer value + EXPECT_THROW(DeleteQosMapEntry({"dscp", "abc"}), std::invalid_argument); + // dscp out of range + EXPECT_THROW(DeleteQosMapEntry({"dscp", "64"}), std::invalid_argument); + EXPECT_THROW(DeleteQosMapEntry({"dscp", "-1"}), std::invalid_argument); + // negative traffic class + EXPECT_THROW(DeleteQosMapEntry({"tc-to-queue", "-1"}), std::invalid_argument); + // boundary values accepted + EXPECT_NO_THROW(DeleteQosMapEntry({"dscp", "0"})); + EXPECT_NO_THROW(DeleteQosMapEntry({"dscp", "63"})); +} + +// -------------------------------------------------------- delete whole policy + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteUnreferencedPolicy) { + setupTestableConfigSession("delete qos policy", "unreferenced"); + + auto cmd = CmdDeleteQosPolicy(); + auto result = cmd.queryClient(localhost(), QosPolicyName({"unreferenced"})); + + EXPECT_THAT(result, ::testing::HasSubstr("Successfully deleted")); + EXPECT_EQ(findPolicy("unreferenced"), nullptr); + // the other policy survives + EXPECT_NE(findPolicy("in-use"), nullptr); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteReferencedPolicyRefused) { + setupTestableConfigSession("delete qos policy", "in-use"); + + auto cmd = CmdDeleteQosPolicy(); + try { + cmd.queryClient(localhost(), QosPolicyName({"in-use"})); + FAIL() << "expected delete of referenced policy to throw"; + } catch (const std::runtime_error& e) { + EXPECT_THAT( + e.what(), + ::testing::AllOf( + ::testing::HasSubstr("dataPlaneTrafficPolicy.defaultQosPolicy"), + ::testing::HasSubstr("Unset those fields first"))); + } + + // refusal leaves the policy in place + EXPECT_NE(findPolicy("in-use"), nullptr); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteMissingPolicyFails) { + setupTestableConfigSession("delete qos policy", "nope"); + + auto cmd = CmdDeleteQosPolicy(); + EXPECT_THROW( + cmd.queryClient(localhost(), QosPolicyName({"nope"})), + std::runtime_error); +} + +// ------------------------------------------------------------ delete map dscp + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteDscpFromSharedEntry) { + setupTestableConfigSession("delete qos policy map", "dscp 1"); + + auto cmd = CmdDeleteQosPolicyMap(); + auto result = cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"dscp", "1"})); + + EXPECT_THAT(result, ::testing::HasSubstr("Successfully deleted")); + + // tc 0 keeps its other codepoints; the entry itself survives + const auto* entry = findDscpEntry("unreferenced", 0); + ASSERT_NE(entry, nullptr); + EXPECT_THAT(*entry->fromDscpToTrafficClass(), ::testing::ElementsAre(0, 2)); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteLastDscpDropsEntry) { + setupTestableConfigSession("delete qos policy map", "dscp 0"); + + auto cmd = CmdDeleteQosPolicyMap(); + // remove every codepoint mapped to tc 0 + for (const auto& dscp : {"0", "1", "2"}) { + cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"dscp", dscp})); + } + + // entry carried no egress rewrite, so it is dropped rather than left empty + EXPECT_EQ(findDscpEntry("unreferenced", 0), nullptr); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteDscpKeepsEntryWithEgressRewrite) { + setupTestableConfigSession("delete qos policy map", "dscp 8"); + + auto cmd = CmdDeleteQosPolicyMap(); + cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"dscp", "8"})); + + // fromTrafficClassToDscp is still set, so the entry must stay + const auto* entry = findDscpEntry("unreferenced", 1); + ASSERT_NE(entry, nullptr); + EXPECT_TRUE(entry->fromDscpToTrafficClass()->empty()); + EXPECT_TRUE(entry->fromTrafficClassToDscp().has_value()); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteMissingDscpFails) { + setupTestableConfigSession("delete qos policy map", "dscp 63"); + + auto cmd = CmdDeleteQosPolicyMap(); + EXPECT_THROW( + cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"dscp", "63"})), + std::runtime_error); +} + +// ----------------------------------------------------- delete map tc-to-queue + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteTcToQueue) { + setupTestableConfigSession("delete qos policy map", "tc-to-queue 1"); + + auto cmd = CmdDeleteQosPolicyMap(); + auto result = cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"tc-to-queue", "1"})); + + EXPECT_THAT(result, ::testing::HasSubstr("Successfully deleted")); + + const auto* policy = findPolicy("unreferenced"); + ASSERT_NE(policy, nullptr); + const auto& tcToQueue = *policy->qosMap()->trafficClassToQueueId(); + EXPECT_EQ(tcToQueue.count(1), 0); + // neighbouring keys untouched + EXPECT_EQ(tcToQueue.at(0), 0); + EXPECT_EQ(tcToQueue.at(2), 2); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteMissingTcToQueueFails) { + setupTestableConfigSession("delete qos policy map", "tc-to-queue 7"); + + auto cmd = CmdDeleteQosPolicyMap(); + EXPECT_THROW( + cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"tc-to-queue", "7"})), + std::runtime_error); +} + +// ------------------------------------------------- delete map remaining kinds + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteMplsExpDropsEntry) { + setupTestableConfigSession("delete qos policy map", "mpls-exp 5"); + + auto cmd = CmdDeleteQosPolicyMap(); + auto result = cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"mpls-exp", "5"})); + + EXPECT_THAT(result, ::testing::HasSubstr("Successfully deleted")); + // 5 was the entry's only codepoint and it has no egress rewrite, so the + // whole ExpQosMap entry is dropped. + const auto* policy = findPolicy("unreferenced"); + ASSERT_NE(policy, nullptr); + EXPECT_TRUE(policy->qosMap()->expMaps()->empty()); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteMissingMplsExpFails) { + setupTestableConfigSession("delete qos policy map", "mpls-exp 0"); + + auto cmd = CmdDeleteQosPolicyMap(); + EXPECT_THROW( + cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"mpls-exp", "0"})), + std::runtime_error); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteLastDot1pResetsPcpMaps) { + setupTestableConfigSession("delete qos policy map", "dot1p 6"); + + auto cmd = CmdDeleteQosPolicyMap(); + auto result = cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"dot1p", "6"})); + + EXPECT_THAT(result, ::testing::HasSubstr("Successfully deleted")); + // 6 was the only PCP codepoint; the emptied optional pcpMaps is reset. + const auto* policy = findPolicy("unreferenced"); + ASSERT_NE(policy, nullptr); + EXPECT_FALSE(policy->qosMap()->pcpMaps().has_value()); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteMissingDot1pFails) { + setupTestableConfigSession("delete qos policy map", "dot1p 0"); + + auto cmd = CmdDeleteQosPolicyMap(); + EXPECT_THROW( + cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"dot1p", "0"})), + std::runtime_error); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteLastPfcPriToQueueResetsMap) { + setupTestableConfigSession("delete qos policy map", "pfc-pri-to-queue 3"); + + auto cmd = CmdDeleteQosPolicyMap(); + auto result = cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"pfc-pri-to-queue", "3"})); + + EXPECT_THAT(result, ::testing::HasSubstr("Successfully deleted")); + // 3 was the only key; the emptied optional map is reset to unset. + const auto* policy = findPolicy("unreferenced"); + ASSERT_NE(policy, nullptr); + EXPECT_FALSE(policy->qosMap()->pfcPriorityToQueueId().has_value()); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deleteTcToPgKeepsRemainingKeys) { + setupTestableConfigSession("delete qos policy map", "tc-to-pg 4"); + + auto cmd = CmdDeleteQosPolicyMap(); + auto result = cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"tc-to-pg", "4"})); + + EXPECT_THAT(result, ::testing::HasSubstr("Successfully deleted")); + const auto* policy = findPolicy("unreferenced"); + ASSERT_NE(policy, nullptr); + const auto& tcToPg = policy->qosMap()->trafficClassToPgId(); + ASSERT_TRUE(tcToPg.has_value()); + EXPECT_EQ(tcToPg->count(4), 0); + EXPECT_EQ(tcToPg->at(5), 5); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deletePfcPriToPgOnUnsetKeyFails) { + setupTestableConfigSession("delete qos policy map", "pfc-pri-to-pg 0"); + + auto cmd = CmdDeleteQosPolicyMap(); + EXPECT_THROW( + cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"pfc-pri-to-pg", "0"})), + std::runtime_error); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, deletePfcPriToPg) { + setupTestableConfigSession("delete qos policy map", "pfc-pri-to-pg 6"); + + auto cmd = CmdDeleteQosPolicyMap(); + auto result = cmd.queryClient( + localhost(), + QosPolicyName({"unreferenced"}), + DeleteQosMapEntry({"pfc-pri-to-pg", "6"})); + + EXPECT_THAT(result, ::testing::HasSubstr("Successfully deleted")); + const auto* policy = findPolicy("unreferenced"); + ASSERT_NE(policy, nullptr); + EXPECT_FALSE(policy->qosMap()->pfcPriorityToPgId().has_value()); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, mapEntryValueRangeValidation) { + // Non-dscp kinds are 3-bit: 8 is out of range for all of them. + EXPECT_THROW(DeleteQosMapEntry({"dot1p", "8"}), std::invalid_argument); + EXPECT_THROW( + DeleteQosMapEntry({"pfc-pri-to-queue", "8"}), std::invalid_argument); + EXPECT_THROW(DeleteQosMapEntry({"tc-to-pg", "-1"}), std::invalid_argument); + EXPECT_THROW(DeleteQosMapEntry({"mpls-exp", "8"}), std::invalid_argument); +} + +TEST_F(CmdDeleteQosPolicyTestFixture, mapDeleteOnMissingPolicyFails) { + setupTestableConfigSession("delete qos policy map", "tc-to-queue 0"); + + auto cmd = CmdDeleteQosPolicyMap(); + EXPECT_THROW( + cmd.queryClient( + localhost(), + QosPolicyName({"nope"}), + DeleteQosMapEntry({"tc-to-queue", "0"})), + std::runtime_error); +} + +// A policy named only through portIdToQosPolicy (not defaultQosPolicy) is still +// refused, and the error points at the per-port field so the operator knows +// which port binding to unset. +TEST_F(CmdDeleteQosPolicyTestFixture, deletePortReferencedPolicyRefused) { + setupTestableConfigSession("delete qos policy", "port-ref"); + + auto cmd = CmdDeleteQosPolicy(); + try { + cmd.queryClient(localhost(), QosPolicyName({"port-ref"})); + FAIL() << "expected delete of port-referenced policy to throw"; + } catch (const std::runtime_error& e) { + EXPECT_THAT( + std::string(e.what()), + ::testing::HasSubstr("dataPlaneTrafficPolicy.portIdToQosPolicy")); + } + + EXPECT_NE(findPolicy("port-ref"), nullptr); +} + +// A policy named by cpuTrafficPolicy.trafficPolicy is refused, exercising the +// second TrafficPolicyConfig findReferences scans. +TEST_F(CmdDeleteQosPolicyTestFixture, deleteCpuReferencedPolicyRefused) { + setupTestableConfigSession("delete qos policy", "cpu-ref"); + + auto cmd = CmdDeleteQosPolicy(); + try { + cmd.queryClient(localhost(), QosPolicyName({"cpu-ref"})); + FAIL() << "expected delete of cpu-referenced policy to throw"; + } catch (const std::runtime_error& e) { + EXPECT_THAT( + std::string(e.what()), + ::testing::HasSubstr("cpuTrafficPolicy.trafficPolicy")); + } + + EXPECT_NE(findPolicy("cpu-ref"), nullptr); +} + +// Deleting a map entry on a policy that has no qosMap reports the missing map +// rather than dereferencing an unset optional. +TEST_F(CmdDeleteQosPolicyTestFixture, mapDeleteOnPolicyWithNoQosMapFails) { + setupTestableConfigSession("delete qos policy map", "dscp 1"); + + auto cmd = CmdDeleteQosPolicyMap(); + try { + cmd.queryClient( + localhost(), + QosPolicyName({"no-map"}), + DeleteQosMapEntry({"dscp", "1"})); + FAIL() << "expected map delete on policy without qosMap to throw"; + } catch (const std::runtime_error& e) { + EXPECT_THAT( + std::string(e.what()), ::testing::HasSubstr("no qosMap configured")); + } +} + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/test/integration_test/BUCK b/fboss/cli/fboss2/test/integration_test/BUCK index b6ec5b952c37b..5ec6f4fa10eb8 100644 --- a/fboss/cli/fboss2/test/integration_test/BUCK +++ b/fboss/cli/fboss2/test/integration_test/BUCK @@ -53,6 +53,7 @@ cpp_binary( "DeleteArpTest.cpp", "DeleteDhcpSourceOverrideTest.cpp", "DeleteInterfaceIpv6NdpTest.cpp", + "DeleteQosPolicyTest.cpp", "DeleteInterfaceTest.cpp", "DeleteVlanTest.cpp", "Fboss2IntegrationTest.cpp", diff --git a/fboss/cli/fboss2/test/integration_test/DeleteQosPolicyTest.cpp b/fboss/cli/fboss2/test/integration_test/DeleteQosPolicyTest.cpp new file mode 100644 index 0000000000000..2a50cae45fe2f --- /dev/null +++ b/fboss/cli/fboss2/test/integration_test/DeleteQosPolicyTest.cpp @@ -0,0 +1,178 @@ +// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary. + +/** + * End-to-end test for `delete qos policy`: + * + * - delete qos policy map dscp — drops a codepoint from + * sw.qosPolicies[*].qosMap.dscpMaps + * - delete qos policy map tc-to-queue — drops a key from + * sw.qosPolicies[*].qosMap.trafficClassToQueueId + * - delete qos policy — removes the policy + * + * The three run in one test because each is the inverse of a step that set the + * policy up: the scratch policy is created here, dismantled entry by entry, + * then removed, so nothing is left behind and the test does not depend on DUT + * state. A stock switch has no qosPolicies at all. + * + * The refusal path (deleting a policy still named by dataPlaneTrafficPolicy or + * cpuTrafficPolicy.trafficPolicy) is not covered here: no CLI command writes + * defaultQosPolicy or portIdToQosPolicy, so a reference cannot be created from + * the command line. That path is covered by CmdDeleteQosPolicyTest, which + * seeds the reference directly into the config. + * + * QoS map edits commit at HITLESS, so the agents do not restart; post-commit + * state is still read through waitForRunningConfig() to avoid racing the + * config reload. + */ + +#include +#include +#include +#include + +#include "fboss/cli/fboss2/test/integration_test/Fboss2IntegrationTest.h" + +using namespace facebook::fboss; + +class DeleteQosPolicyTest : public Fboss2IntegrationTest { + protected: + // Name used for the scratch policy this test creates and removes. + static constexpr auto kPolicy = "fboss2-it-scratch-qos"; + static constexpr auto kDscp = "40"; + static constexpr auto kTrafficClass = "5"; + static constexpr auto kQueueId = "0"; + + static const folly::dynamic* findPolicy( + const folly::dynamic& config, + const std::string& name) { + if (!config.count("sw") || !config["sw"].count("qosPolicies")) { + return nullptr; + } + for (const auto& policy : config["sw"]["qosPolicies"]) { + if (policy.count("name") && policy["name"].asString() == name) { + return &policy; + } + } + return nullptr; + } + + static bool hasPolicy(const folly::dynamic& config, const std::string& name) { + return findPolicy(config, name) != nullptr; + } + + // True when the policy maps `dscp` to some traffic class. + static bool + hasDscp(const folly::dynamic& config, const std::string& name, int dscp) { + const auto* policy = findPolicy(config, name); + if (policy == nullptr || !policy->count("qosMap") || + !(*policy)["qosMap"].count("dscpMaps")) { + return false; + } + for (const auto& entry : (*policy)["qosMap"]["dscpMaps"]) { + if (!entry.count("fromDscpToTrafficClass")) { + continue; + } + for (const auto& value : entry["fromDscpToTrafficClass"]) { + if (value.asInt() == dscp) { + return true; + } + } + } + return false; + } + + // True when the policy maps traffic class `tc` to a queue. + static bool hasTcToQueue( + const folly::dynamic& config, + const std::string& name, + const std::string& tc) { + const auto* policy = findPolicy(config, name); + if (policy == nullptr || !policy->count("qosMap") || + !(*policy)["qosMap"].count("trafficClassToQueueId")) { + return false; + } + return (*policy)["qosMap"]["trafficClassToQueueId"].count(tc) > 0; + } +}; + +TEST_F(DeleteQosPolicyTest, DeleteMapEntriesThenPolicy) { + // 1. Create a scratch policy carrying one dscp mapping and one tc-to-queue + // mapping. `config qos policy map ...` creates the policy when it + // does not already exist. + XLOG(INFO) << "Creating qos policy " << kPolicy; + auto result = runCli( + {"config", + "qos", + "policy", + kPolicy, + "map", + "dscp", + kDscp, + "traffic-class", + kTrafficClass}); + ASSERT_EQ(result.exitCode, 0) + << "stdout=" << result.stdout << " stderr=" << result.stderr; + + result = runCli( + {"config", + "qos", + "policy", + kPolicy, + "map", + "tc-to-queue", + kTrafficClass, + kQueueId}); + ASSERT_EQ(result.exitCode, 0) + << "stdout=" << result.stdout << " stderr=" << result.stderr; + commitConfig(); + + auto config = waitForRunningConfig([&](const folly::dynamic& cfg) { + return hasPolicy(cfg, kPolicy) && hasDscp(cfg, kPolicy, std::stoi(kDscp)) && + hasTcToQueue(cfg, kPolicy, kTrafficClass); + }); + ASSERT_TRUE(hasPolicy(config, kPolicy)) + << "qos policy " << kPolicy << " missing after commit"; + ASSERT_TRUE(hasDscp(config, kPolicy, std::stoi(kDscp))); + ASSERT_TRUE(hasTcToQueue(config, kPolicy, kTrafficClass)); + + // 2. Delete the dscp mapping. The tc-to-queue mapping must survive. + XLOG(INFO) << "Deleting dscp " << kDscp << " from " << kPolicy; + result = runCli({"delete", "qos", "policy", kPolicy, "map", "dscp", kDscp}); + ASSERT_EQ(result.exitCode, 0) + << "stdout=" << result.stdout << " stderr=" << result.stderr; + commitConfig(); + config = waitForRunningConfig([&](const folly::dynamic& cfg) { + return !hasDscp(cfg, kPolicy, std::stoi(kDscp)); + }); + EXPECT_FALSE(hasDscp(config, kPolicy, std::stoi(kDscp))); + EXPECT_TRUE(hasTcToQueue(config, kPolicy, kTrafficClass)) + << "deleting the dscp mapping must not touch trafficClassToQueueId"; + + // 3. Delete the tc-to-queue mapping. + XLOG(INFO) << "Deleting tc-to-queue " << kTrafficClass << " from " << kPolicy; + result = runCli( + {"delete", + "qos", + "policy", + kPolicy, + "map", + "tc-to-queue", + kTrafficClass}); + ASSERT_EQ(result.exitCode, 0) + << "stdout=" << result.stdout << " stderr=" << result.stderr; + commitConfig(); + config = waitForRunningConfig([&](const folly::dynamic& cfg) { + return !hasTcToQueue(cfg, kPolicy, kTrafficClass); + }); + EXPECT_FALSE(hasTcToQueue(config, kPolicy, kTrafficClass)); + + // 4. Delete the policy itself. Nothing references it, so this is allowed. + XLOG(INFO) << "Deleting qos policy " << kPolicy; + result = runCli({"delete", "qos", "policy", kPolicy}); + ASSERT_EQ(result.exitCode, 0) + << "stdout=" << result.stdout << " stderr=" << result.stderr; + commitConfig(); + config = waitForRunningConfig( + [&](const folly::dynamic& cfg) { return !hasPolicy(cfg, kPolicy); }); + EXPECT_FALSE(hasPolicy(config, kPolicy)); +}