Add fboss2-dev delete copp subcommands (cpu-queue, reason)

Delete-side counterparts for the copp config commands:

  delete copp cpu-queue <id>       - remove the whole sw.cpuQueues[]
      entry; refused while rxReason mappings (the ordered list or the
      deprecated rxReasonToCPUQueue map, which the agent still reads as
      a fallback) or matchToAction actions (send-to-queue,
      user-defined-trap) still reference the queue id, naming each
      referencing entry
  delete copp reason <reason-name> - remove the
      rxReasonToQueueOrderedList entry

Per-attribute deletes (cpu-queue <id> name, cpu-queue <id> rate-limit)
are left for a follow-up.

Queue-id parsing/lookup and reason-name parsing move out of
CmdConfigCopp.cpp into the copp_cpu_queue and copp_reason namespaces in
a new CoppUtils.{h,cpp} so the config and delete trees share one
implementation. Both handlers save with ConfigActionLevel::HITLESS,
matching the config-side handlers.

This also introduces the `delete copp` parent command node, which does
not exist upstream yet (#1379 adds it too, for the cpu-traffic-policy
delete; whichever lands second is a trivial merge).

Author: vybhav-nexthop

cmake/CliFboss2.cmake

@@ -824,6 +824,8 @@ add_library(fboss2_config_lib
   fboss/cli/fboss2/commands/config/arp/CmdConfigArp.h
   fboss/cli/fboss2/commands/config/copp/CmdConfigCopp.cpp
   fboss/cli/fboss2/commands/config/copp/CmdConfigCopp.h
+  fboss/cli/fboss2/commands/config/copp/CoppUtils.cpp
+  fboss/cli/fboss2/commands/config/copp/CoppUtils.h
   fboss/cli/fboss2/commands/config/dhcp/CmdConfigDhcp.cpp
   fboss/cli/fboss2/commands/config/dhcp/CmdConfigDhcp.h
   fboss/cli/fboss2/commands/config/dhcp/relay_source_override/CmdConfigDhcpRelaySourceOverride.cpp
@@ -1084,6 +1086,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/copp/CmdDeleteCopp.cpp
+  fboss/cli/fboss2/commands/delete/copp/CmdDeleteCopp.h
+  fboss/cli/fboss2/commands/delete/copp/cpu_queue/CmdDeleteCoppCpuQueue.cpp
+  fboss/cli/fboss2/commands/delete/copp/cpu_queue/CmdDeleteCoppCpuQueue.h
+  fboss/cli/fboss2/commands/delete/copp/reason/CmdDeleteCoppReason.cpp
+  fboss/cli/fboss2/commands/delete/copp/reason/CmdDeleteCoppReason.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

cmake/CliFboss2TestConfig.cmake

@@ -39,6 +39,8 @@ add_executable(fboss2_cmd_config_test
   fboss/cli/fboss2/test/config/CmdDeleteAclRuleTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteArpTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteConfigInterfaceTest.cpp
+  fboss/cli/fboss2/test/config/CmdDeleteCoppCpuQueueTest.cpp
+  fboss/cli/fboss2/test/config/CmdDeleteCoppReasonTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteDhcpTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteInterfaceIpv6NdpTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteInterfaceTest.cpp

cmake/CliFboss2TestIntegrationTest.cmake

@@ -18,6 +18,7 @@ add_executable(fboss2_integration_test
   fboss/cli/fboss2/test/integration_test/ConfigDhcpSourceOverrideTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigInterfaceDescriptionTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigInterfaceIpv6NdpTest.cpp
+  fboss/cli/fboss2/test/integration_test/DeleteCoppTest.cpp
   fboss/cli/fboss2/test/integration_test/DeleteInterfaceTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigInterfaceMtuTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigInterfaceProfileTest.cpp

fboss/cli/fboss2/BUCK

@@ -1074,6 +1074,7 @@ cpp_library(
         "commands/config/acl/rule/CmdConfigAclRule.cpp",
         "commands/config/arp/CmdConfigArp.cpp",
         "commands/config/copp/CmdConfigCopp.cpp",
+        "commands/config/copp/CoppUtils.cpp",
         "commands/config/dhcp/CmdConfigDhcp.cpp",
         "commands/config/dhcp/relay_source_override/CmdConfigDhcpRelaySourceOverride.cpp",
         "commands/config/dhcp/reply_source_override/CmdConfigDhcpReplySourceOverride.cpp",
@@ -1194,6 +1195,9 @@ cpp_library(
         "commands/delete/acl/CmdDeleteAcl.cpp",
         "commands/delete/acl/rule/CmdDeleteAclRule.cpp",
         "commands/delete/arp/CmdDeleteArp.cpp",
+        "commands/delete/copp/CmdDeleteCopp.cpp",
+        "commands/delete/copp/cpu_queue/CmdDeleteCoppCpuQueue.cpp",
+        "commands/delete/copp/reason/CmdDeleteCoppReason.cpp",
         "commands/delete/dhcp/CmdDeleteDhcp.cpp",
         "commands/delete/dhcp/relay_source_override/CmdDeleteDhcpRelaySourceOverride.cpp",
         "commands/delete/dhcp/reply_source_override/CmdDeleteDhcpReplySourceOverride.cpp",
@@ -1226,6 +1230,7 @@ cpp_library(
         "commands/config/acl/rule/CmdConfigAclRule.h",
         "commands/config/arp/CmdConfigArp.h",
         "commands/config/copp/CmdConfigCopp.h",
+        "commands/config/copp/CoppUtils.h",
         "commands/config/dhcp/CmdConfigDhcp.h",
         "commands/config/dhcp/relay_source_override/CmdConfigDhcpRelaySourceOverride.h",
         "commands/config/dhcp/reply_source_override/CmdConfigDhcpReplySourceOverride.h",
@@ -1333,6 +1338,9 @@ cpp_library(
         "commands/delete/acl/CmdDeleteAcl.h",
         "commands/delete/acl/rule/CmdDeleteAclRule.h",
         "commands/delete/arp/CmdDeleteArp.h",
+        "commands/delete/copp/CmdDeleteCopp.h",
+        "commands/delete/copp/cpu_queue/CmdDeleteCoppCpuQueue.h",
+        "commands/delete/copp/reason/CmdDeleteCoppReason.h",
         "commands/delete/dhcp/CmdDeleteDhcp.h",
         "commands/delete/dhcp/relay_source_override/CmdDeleteDhcpRelaySourceOverride.h",
         "commands/delete/dhcp/reply_source_override/CmdDeleteDhcpReplySourceOverride.h",

fboss/cli/fboss2/CmdListConfig.cpp

@@ -132,6 +132,9 @@
 #include "fboss/cli/fboss2/commands/delete/acl/rule/CmdDeleteAclRule.h"
 #include "fboss/cli/fboss2/commands/delete/arp/CmdDeleteArp.h"
 #include "fboss/cli/fboss2/commands/delete/config/CmdDeleteConfig.h"
+#include "fboss/cli/fboss2/commands/delete/copp/CmdDeleteCopp.h"
+#include "fboss/cli/fboss2/commands/delete/copp/cpu_queue/CmdDeleteCoppCpuQueue.h"
+#include "fboss/cli/fboss2/commands/delete/copp/reason/CmdDeleteCoppReason.h"
 #include "fboss/cli/fboss2/commands/delete/dhcp/CmdDeleteDhcp.h"
 #include "fboss/cli/fboss2/commands/delete/dhcp/relay_source_override/CmdDeleteDhcpRelaySourceOverride.h"
 #include "fboss/cli/fboss2/commands/delete/dhcp/reply_source_override/CmdDeleteDhcpReplySourceOverride.h"
@@ -1163,6 +1166,26 @@ const CommandTree& kConfigCommandTree() {
        commandHandler<CmdDeleteConfig>,
        argRegistrar<CmdDeleteConfigTraits>},
 
+      {
+          "delete",
+          "copp",
+          "Delete COPP (Control Plane Policing) configuration",
+          commandHandler<CmdDeleteCopp>,
+          argRegistrar<CmdDeleteCoppTraits>,
+          {{
+               "cpu-queue",
+               "Delete a CPU queue entry",
+               commandHandler<CmdDeleteCoppCpuQueue>,
+               argRegistrar<CmdDeleteCoppCpuQueueTraits>,
+           },
+           {
+               "reason",
+               "Delete a packet-rx reason to CPU queue mapping",
+               commandHandler<CmdDeleteCoppReason>,
+               argRegistrar<CmdDeleteCoppReasonTraits>,
+           }},
+      },
+
       {
           "delete",
           "dhcp",

fboss/cli/fboss2/commands/config/copp/CmdConfigCopp.cpp

@@ -15,16 +15,15 @@
 
 #include <fmt/format.h>
 #include <folly/Conv.h>
-#include <folly/String.h>
 #include <thrift/lib/cpp/util/EnumUtils.h>
-#include <cctype>
 #include <cstdint>
 #include <iostream>
 #include <stdexcept>
 #include <string>
 #include <string_view>
 #include <utility>
 #include <vector>
+#include "fboss/cli/fboss2/commands/config/copp/CoppUtils.h"
 #include "fboss/cli/fboss2/gen-cpp2/cli_metadata_types.h"
 #include "fboss/cli/fboss2/session/ConfigSession.h"
 #include "fboss/cli/fboss2/utils/HostInfo.h"
@@ -39,55 +38,12 @@ constexpr std::string_view kSubCmdRateLimit = "rate-limit";
 constexpr std::string_view kRateUnitKbps = "kbps";
 constexpr std::string_view kRateUnitPps = "pps";
 
+using copp_cpu_queue::parseQueueId;
+
 // Literal tokens accepted between the reason name and queue id in
 // `config copp reason <reason-name> queue <id>`.
 constexpr std::string_view kSubCmdQueue = "queue";
 
-// CPU queue IDs are a small platform-bounded set; reject anything that is
-// clearly out of range before we construct a PortQueue. The actual per-ASIC
-// cap is enforced by the agent (SaiHostifManager::getMaxCpuQueues) at apply
-// time.
-constexpr int16_t kMaxCpuQueueId = 255;
-
-// Normalize a user-typed reason name: uppercase + dashes->underscores, so
-// that "arp", "ARP", "bgp-v6", "bgpv6" all match the cfg::PacketRxReason
-// enum names ("ARP", "BGPV6", ...).
-std::string normalizeReason(const std::string& v) {
-  std::string out;
-  out.reserve(v.size());
-  for (unsigned char c : v) {
-    out.push_back(c == '-' ? '_' : std::toupper(c));
-  }
-  return out;
-}
-
-std::string validReasonNames() {
-  std::vector<std::string> names;
-  for (auto value : apache::thrift::TEnumTraits<cfg::PacketRxReason>::values) {
-    names.push_back(apache::thrift::util::enumNameSafe(value));
-  }
-  return folly::join(", ", names);
-}
-
-int16_t parseQueueId(const std::string& s, std::string_view context) {
-  int16_t parsed = 0;
-  try {
-    parsed = folly::to<int16_t>(s);
-  } catch (const folly::ConversionError&) {
-    throw std::invalid_argument(
-        fmt::format("Queue ID ({}) must be an integer, got '{}'", context, s));
-  }
-  if (parsed < 0 || parsed > kMaxCpuQueueId) {
-    throw std::invalid_argument(
-        fmt::format(
-            "Queue ID ({}) must be in [0, {}], got {}",
-            context,
-            kMaxCpuQueueId,
-            parsed));
-  }
-  return parsed;
-}
-
 int32_t parseRateMax(const std::string& s, std::string_view unit) {
   int32_t parsed = 0;
   try {
@@ -112,10 +68,9 @@ int32_t parseRateMax(const std::string& s, std::string_view unit) {
 // populated cpuQueues list.
 cfg::PortQueue& findOrCreateCpuQueue(cfg::SwitchConfig& swConfig, int16_t id) {
   auto& queues = *swConfig.cpuQueues();
-  for (auto& q : queues) {
-    if (*q.id() == id) {
-      return q;
-    }
+  auto it = copp_cpu_queue::findCpuQueue(queues, id);
+  if (it != queues.end()) {
+    return *it;
   }
   cfg::PortQueue q;
   q.id() = id;
@@ -202,13 +157,7 @@ CoppReasonArgs::CoppReasonArgs(std::vector<std::string> v) {
             kSubCmdQueue,
             v.size()));
   }
-  cfg::PacketRxReason reason{};
-  if (!apache::thrift::TEnumTraits<cfg::PacketRxReason>::findValue(
-          normalizeReason(v[0]), &reason)) {
-    throw std::invalid_argument(
-        fmt::format(
-            "Unknown reason name '{}'. Valid: {}", v[0], validReasonNames()));
-  }
+  cfg::PacketRxReason reason = copp_reason::parseReason(v[0]);
   if (v[1] != kSubCmdQueue) {
     throw std::invalid_argument(
         fmt::format(

fboss/cli/fboss2/commands/config/copp/CoppUtils.cpp

@@ -0,0 +1,87 @@
+/*
+ *  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/copp/CoppUtils.h"
+
+#include <fmt/format.h>
+#include <folly/Conv.h>
+#include <folly/String.h>
+#include <thrift/lib/cpp/util/EnumUtils.h>
+#include <algorithm>
+#include <cctype>
+#include <stdexcept>
+
+namespace facebook::fboss {
+
+namespace copp_cpu_queue {
+
+int16_t parseQueueId(const std::string& s, std::string_view context) {
+  int16_t parsed = 0;
+  try {
+    parsed = folly::to<int16_t>(s);
+  } catch (const folly::ConversionError&) {
+    throw std::invalid_argument(
+        fmt::format("Queue ID ({}) must be an integer, got '{}'", context, s));
+  }
+  if (parsed < 0 || parsed > kMaxCpuQueueId) {
+    throw std::invalid_argument(
+        fmt::format(
+            "Queue ID ({}) must be in [0, {}], got {}",
+            context,
+            kMaxCpuQueueId,
+            parsed));
+  }
+  return parsed;
+}
+
+std::vector<cfg::PortQueue>::iterator findCpuQueue(
+    std::vector<cfg::PortQueue>& queues,
+    int16_t id) {
+  return std::find_if(
+      queues.begin(), queues.end(), [id](const cfg::PortQueue& q) {
+        return *q.id() == id;
+      });
+}
+
+} // namespace copp_cpu_queue
+
+namespace copp_reason {
+
+std::string normalizeReason(const std::string& v) {
+  std::string out;
+  out.reserve(v.size());
+  for (unsigned char c : v) {
+    out.push_back(c == '-' ? '_' : std::toupper(c));
+  }
+  return out;
+}
+
+std::string validReasonNames() {
+  std::vector<std::string> names;
+  for (auto value : apache::thrift::TEnumTraits<cfg::PacketRxReason>::values) {
+    names.push_back(apache::thrift::util::enumNameSafe(value));
+  }
+  return folly::join(", ", names);
+}
+
+cfg::PacketRxReason parseReason(const std::string& s) {
+  cfg::PacketRxReason reason{};
+  if (!apache::thrift::TEnumTraits<cfg::PacketRxReason>::findValue(
+          normalizeReason(s), &reason)) {
+    throw std::invalid_argument(
+        fmt::format(
+            "Unknown reason name '{}'. Valid: {}", s, validReasonNames()));
+  }
+  return reason;
+}
+
+} // namespace copp_reason
+
+} // namespace facebook::fboss

fboss/cli/fboss2/commands/config/copp/CoppUtils.h

@@ -0,0 +1,56 @@
+/*
+ *  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 <cstdint>
+#include <string>
+#include <string_view>
+#include <vector>
+#include "fboss/agent/gen-cpp2/switch_config_types.h"
+
+namespace facebook::fboss {
+
+// Shared vocabulary for the copp cpu-queue commands, used by both the
+// `config` and `delete` command trees so queue-id parsing and lookup live
+// in one place.
+namespace copp_cpu_queue {
+
+// CPU queue IDs are a small platform-bounded set; reject anything that is
+// clearly out of range before touching the config. The actual per-ASIC cap
+// is enforced by the agent (SaiHostifManager::getMaxCpuQueues) at apply
+// time.
+constexpr int16_t kMaxCpuQueueId = 255;
+
+int16_t parseQueueId(const std::string& s, std::string_view context);
+
+// Return an iterator to the cpuQueues entry with `id`, or end().
+std::vector<cfg::PortQueue>::iterator findCpuQueue(
+    std::vector<cfg::PortQueue>& queues,
+    int16_t id);
+
+} // namespace copp_cpu_queue
+
+// Shared vocabulary for the copp reason commands, used by both the `config`
+// and `delete` command trees so reason-name parsing stays consistent.
+namespace copp_reason {
+
+// Normalize a user-typed reason name: uppercase + dashes->underscores, so
+// that "arp", "ARP", "ttl-1", "ttl_1" all match the cfg::PacketRxReason
+// enum names ("ARP", "TTL_1", ...).
+std::string normalizeReason(const std::string& v);
+
+std::string validReasonNames();
+
+cfg::PacketRxReason parseReason(const std::string& s);
+
+} // namespace copp_reason
+
+} // namespace facebook::fboss

fboss/cli/fboss2/commands/delete/copp/CmdDeleteCopp.cpp

@@ -0,0 +1,20 @@
+/*
+ *  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/copp/CmdDeleteCopp.h"
+
+#include "fboss/cli/fboss2/CmdHandler.cpp"
+
+namespace facebook::fboss {
+
+// Explicit template instantiation
+template void CmdHandler<CmdDeleteCopp, CmdDeleteCoppTraits>::run();
+
+} // namespace facebook::fboss