Add fboss2-dev delete switch admin-distance and icmpv4-unavailable-src-addr

delete switch admin-distance <client-id> removes a routing client's
entry from sw.clientIdToAdminDistance. Once the entry is gone,
getAdminDistanceForClientId() finds no mapping and returns
AdminDistance::MAX_ADMIN_DISTANCE (255) for that client; the per-client
default in switch_config.thrift only applies when the whole map is
absent from the config. Saved at coldboot level because the map is
consulted only at route-program time and existing routes are not
re-stamped. Client-ids 1-4 (STATIC_ROUTE, INTERFACE_ROUTE,
LINKLOCAL_ROUTE, REMOTE_INTERFACE_ROUTE) are refused: their distances
are hardcoded in the agent, mirroring the refusal in config switch
admin-distance. The client-id parsing and forbidden-client check are
shared with the config command via parseAdminDistanceClientId().

delete switch icmpv4-unavailable-src-addr clears the optional
sw.icmpV4UnavailableSrcAddress field, returning it to its unset
default. Hitless.

Author: vybhav-nexthop

cmake/CliFboss2.cmake

@@ -1092,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/switch/CmdDeleteSwitch.cpp
+  fboss/cli/fboss2/commands/delete/switch/CmdDeleteSwitch.h
+  fboss/cli/fboss2/commands/delete/switch/admin_distance/CmdDeleteAdminDistance.cpp
+  fboss/cli/fboss2/commands/delete/switch/admin_distance/CmdDeleteAdminDistance.h
+  fboss/cli/fboss2/commands/delete/switch/icmpv4_unavailable_src_addr/CmdDeleteIcmpV4UnavailableSrcAddr.cpp
+  fboss/cli/fboss2/commands/delete/switch/icmpv4_unavailable_src_addr/CmdDeleteIcmpV4UnavailableSrcAddr.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

@@ -38,6 +38,8 @@ add_executable(fboss2_cmd_config_test
   fboss/cli/fboss2/test/config/CmdConfigVlanPortTaggingModeTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigVlanStaticMacTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteAclRuleTest.cpp
+  fboss/cli/fboss2/test/config/CmdDeleteAdminDistanceTest.cpp
+  fboss/cli/fboss2/test/config/CmdDeleteIcmpV4UnavailableSrcAddrTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteArpTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteConfigInterfaceTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteDhcpTest.cpp

fboss/cli/fboss2/BUCK

@@ -1217,6 +1217,9 @@ cpp_library(
         "commands/delete/qos/default_policy/CmdDeleteQosDefaultPolicy.cpp",
         "commands/delete/qos/queue_config/CmdDeleteQosQueueConfig.cpp",
         "commands/delete/qos/queue_config/CmdDeleteQosQueueConfigQueueId.cpp",
+        "commands/delete/switch/CmdDeleteSwitch.cpp",
+        "commands/delete/switch/admin_distance/CmdDeleteAdminDistance.cpp",
+        "commands/delete/switch/icmpv4_unavailable_src_addr/CmdDeleteIcmpV4UnavailableSrcAddr.cpp",
         "commands/delete/tunnel/CmdDeleteTunnel.cpp",
         "commands/delete/tunnel/ip_in_ip/CmdDeleteTunnelIpInIp.cpp",
         "commands/delete/tunnel/ip_in_ip/TunnelIpInIpDeleteUtils.cpp",
@@ -1359,6 +1362,9 @@ cpp_library(
         "commands/delete/qos/default_policy/CmdDeleteQosDefaultPolicy.h",
         "commands/delete/qos/queue_config/CmdDeleteQosQueueConfig.h",
         "commands/delete/qos/queue_config/CmdDeleteQosQueueConfigQueueId.h",
+        "commands/delete/switch/CmdDeleteSwitch.h",
+        "commands/delete/switch/admin_distance/CmdDeleteAdminDistance.h",
+        "commands/delete/switch/icmpv4_unavailable_src_addr/CmdDeleteIcmpV4UnavailableSrcAddr.h",
         "commands/delete/tunnel/CmdDeleteTunnel.h",
         "commands/delete/tunnel/ip_in_ip/CmdDeleteTunnelIpInIp.h",
         "commands/delete/tunnel/ip_in_ip/TunnelIpInIpDeleteUtils.h",

fboss/cli/fboss2/CmdListConfig.cpp

@@ -146,6 +146,9 @@
 #include "fboss/cli/fboss2/commands/delete/qos/default_policy/CmdDeleteQosDefaultPolicy.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/switch/CmdDeleteSwitch.h"
+#include "fboss/cli/fboss2/commands/delete/switch/admin_distance/CmdDeleteAdminDistance.h"
+#include "fboss/cli/fboss2/commands/delete/switch/icmpv4_unavailable_src_addr/CmdDeleteIcmpV4UnavailableSrcAddr.h"
 #include "fboss/cli/fboss2/commands/delete/tunnel/CmdDeleteTunnel.h"
 #include "fboss/cli/fboss2/commands/delete/tunnel/ip_in_ip/CmdDeleteTunnelIpInIp.h"
 #include "fboss/cli/fboss2/commands/delete/tunnel/ip_in_ip/decap/CmdDeleteTunnelIpInIpDecap.h"
@@ -1230,6 +1233,19 @@ const CommandTree& kConfigCommandTree() {
        commandHandler<CmdDeleteVlan>,
        argRegistrar<CmdDeleteVlanTraits>},
 
+      {"delete",
+       "switch",
+       "Delete switch-level settings",
+       commandHandler<CmdDeleteSwitch>,
+       argTypeHandler<CmdDeleteSwitchTraits>,
+       {{"admin-distance",
+         "Remove the admin distance entry for a routing client: <client-id>",
+         commandHandler<CmdDeleteAdminDistance>,
+         argRegistrar<CmdDeleteAdminDistanceTraits>},
+        {"icmpv4-unavailable-src-addr",
+         "Remove the ICMPv4 unavailable source address (agent falls back to the RFC 7600 default)",
+         commandHandler<CmdDeleteIcmpV4UnavailableSrcAddr>,
+         argRegistrar<CmdDeleteIcmpV4UnavailableSrcAddrTraits>}}},
   };
   stable_sort(root.begin(), root.end());
   return root;

fboss/cli/fboss2/commands/config/switch/admin_distance/CmdConfigAdminDistance.cpp

@@ -21,14 +21,10 @@ namespace facebook::fboss {
 
 namespace {
 constexpr int32_t kMaxAdminDistance = 255;
+} // namespace
 
-// ClientIDs whose admin distance is hardcoded in the agent and cannot be
-// overridden via clientIdToAdminDistance config:
-//   STATIC_ROUTE (1)         -> AdminDistance::STATIC_ROUTE
-//   INTERFACE_ROUTE (2)      -> AdminDistance::DIRECTLY_CONNECTED
-//   LINKLOCAL_ROUTE (3)      -> AdminDistance::DIRECTLY_CONNECTED
-//   REMOTE_INTERFACE_ROUTE (4) -> AdminDistance::DIRECTLY_CONNECTED
-const std::unordered_map<int32_t, std::string>& forbiddenClients() {
+const std::unordered_map<int32_t, std::string>&
+forbiddenAdminDistanceClients() {
   static const std::unordered_map<int32_t, std::string> kForbidden = {
       {1,
        "STATIC_ROUTE is hardcoded to AdminDistance::STATIC_ROUTE in the agent"},
@@ -41,36 +37,43 @@ const std::unordered_map<int32_t, std::string>& forbiddenClients() {
   };
   return kForbidden;
 }
-} // namespace
-
-AdminDistanceArg::AdminDistanceArg(std::vector<std::string> v) {
-  if (v.size() != 2) {
-    throw std::invalid_argument(
-        fmt::format(
-            "Expected exactly two arguments: <client-id> <distance>, got {}",
-            v.size()));
-  }
 
+int32_t parseAdminDistanceClientId(
+    const std::string& token,
+    std::string_view action) {
+  int32_t clientId = 0;
   try {
-    clientId_ = folly::to<int32_t>(v[0]);
+    clientId = folly::to<int32_t>(token);
   } catch (const folly::ConversionError&) {
     throw std::invalid_argument(
-        fmt::format("Invalid client-id '{}': must be an integer", v[0]));
+        fmt::format("Invalid client-id '{}': must be an integer", token));
   }
-  if (clientId_ < 0) {
+  if (clientId < 0) {
     throw std::invalid_argument(
-        fmt::format("client-id must be a non-negative integer, got {}", v[0]));
+        fmt::format("client-id must be a non-negative integer, got {}", token));
   }
-
-  const auto& forbidden = forbiddenClients();
-  auto it = forbidden.find(clientId_);
+  const auto& forbidden = forbiddenAdminDistanceClients();
+  auto it = forbidden.find(clientId);
   if (it != forbidden.end()) {
     throw std::invalid_argument(
         fmt::format(
-            "FBOSS does not allow changing admin distance for client-id {}: {}",
-            clientId_,
+            "FBOSS does not allow {} for client-id {}: {}",
+            action,
+            clientId,
             it->second));
   }
+  return clientId;
+}
+
+AdminDistanceArg::AdminDistanceArg(std::vector<std::string> v) {
+  if (v.size() != 2) {
+    throw std::invalid_argument(
+        fmt::format(
+            "Expected exactly two arguments: <client-id> <distance>, got {}",
+            v.size()));
+  }
+
+  clientId_ = parseAdminDistanceClientId(v[0], "changing admin distance");
 
   try {
     distance_ = folly::to<int32_t>(v[1]);

fboss/cli/fboss2/commands/config/switch/admin_distance/CmdConfigAdminDistance.h

@@ -10,11 +10,33 @@
 
 #pragma once
 
+#include <cstdint>
+#include <string>
+#include <string_view>
+#include <unordered_map>
 #include "fboss/cli/fboss2/CmdHandler.h"
 #include "fboss/cli/fboss2/commands/config/switch/CmdConfigSwitch.h"
 
 namespace facebook::fboss {
 
+// ClientIDs whose admin distance is hardcoded in the agent, so a
+// clientIdToAdminDistance entry for them is never consulted:
+//   STATIC_ROUTE (1)           -> AdminDistance::STATIC_ROUTE
+//   INTERFACE_ROUTE (2)        -> AdminDistance::DIRECTLY_CONNECTED
+//   LINKLOCAL_ROUTE (3)        -> AdminDistance::DIRECTLY_CONNECTED
+//   REMOTE_INTERFACE_ROUTE (4) -> AdminDistance::DIRECTLY_CONNECTED
+// Maps client-id to the reason it is rejected. Both `config switch
+// admin-distance` and `delete switch admin-distance` refuse these ids.
+const std::unordered_map<int32_t, std::string>& forbiddenAdminDistanceClients();
+
+// Parses and validates the <client-id> token shared by `config switch
+// admin-distance` and `delete switch admin-distance`: must be a non-negative
+// integer and not a forbidden client. `action` names the operation for the
+// refusal message (e.g. "changing admin distance").
+int32_t parseAdminDistanceClientId(
+    const std::string& token,
+    std::string_view action);
+
 // Parses the two positional arguments of
 //   config switch admin-distance <client-id> <distance>
 // where <client-id> is a routing ClientID (e.g. 0=BGP, 786=OpenR)

fboss/cli/fboss2/commands/delete/switch/CmdDeleteSwitch.cpp

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

fboss/cli/fboss2/commands/delete/switch/CmdDeleteSwitch.h

@@ -0,0 +1,36 @@
+/*
+ *  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 <string>
+#include "fboss/cli/fboss2/CmdHandler.h"
+
+namespace facebook::fboss {
+
+struct CmdDeleteSwitchTraits : public WriteCommandTraits {
+  static constexpr utils::ObjectArgTypeId ObjectArgTypeId =
+      utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_NONE;
+  using ObjectArgType = std::monostate;
+  using RetType = std::string;
+};
+
+class CmdDeleteSwitch
+    : public CmdHandler<CmdDeleteSwitch, CmdDeleteSwitchTraits> {
+ public:
+  RetType queryClient(const HostInfo& /* hostInfo */) {
+    throw std::runtime_error(
+        "Incomplete command, please use one of the subcommands (e.g. admin-distance)");
+  }
+
+  void printOutput(const RetType& /* model */) {}
+};
+
+} // namespace facebook::fboss

fboss/cli/fboss2/commands/delete/switch/admin_distance/CmdDeleteAdminDistance.cpp

@@ -0,0 +1,81 @@
+/*
+ *  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/switch/admin_distance/CmdDeleteAdminDistance.h"
+
+#include "fboss/cli/fboss2/CmdHandler.cpp"
+
+#include <fmt/format.h>
+#include <iostream>
+#include "fboss/agent/FbossError.h"
+#include "fboss/cli/fboss2/commands/config/switch/admin_distance/CmdConfigAdminDistance.h"
+#include "fboss/cli/fboss2/session/ConfigSession.h"
+
+namespace facebook::fboss {
+
+AdminDistanceDeleteArg::AdminDistanceDeleteArg(std::vector<std::string> v) {
+  if (v.size() != 1) {
+    throw std::invalid_argument(
+        fmt::format(
+            "Expected exactly one argument: <client-id>, got {}", v.size()));
+  }
+
+  // Shared with `config switch admin-distance`: an entry for the forbidden
+  // clients is never consulted by the agent, so neither setting nor removing
+  // one is meaningful.
+  clientId_ =
+      parseAdminDistanceClientId(v[0], "removing the admin distance entry");
+
+  data_ = std::move(v);
+}
+
+CmdDeleteAdminDistanceTraits::RetType CmdDeleteAdminDistance::queryClient(
+    const HostInfo& /* hostInfo */,
+    const ObjectArgType& arg) {
+  auto& session = ConfigSession::getInstance();
+  auto& swConfig = *session.getAgentConfig().sw();
+
+  int32_t clientId = arg.getClientId();
+
+  // Once the entry is gone, getAdminDistanceForClientId() (Utils.cpp, reached
+  // via SwSwitch::clientIdToAdminDistance) finds no mapping and returns
+  // AdminDistance::MAX_ADMIN_DISTANCE (255) for this client -- it does not fall
+  // back to the per-client default in switch_config.thrift, since that default
+  // only applies when the whole clientIdToAdminDistance field is absent from
+  // the config.
+  auto& adminDistanceMap = *swConfig.clientIdToAdminDistance();
+  auto it = adminDistanceMap.find(clientId);
+  if (it == adminDistanceMap.end()) {
+    throw FbossError("No admin distance configured for client-id ", clientId);
+  }
+  adminDistanceMap.erase(it);
+
+  // clientIdToAdminDistance is only consulted at route-program time; existing
+  // routes are not re-stamped when the map changes. A coldboot is required to
+  // flush and re-program all routes with the restored default distance.
+  session.saveConfig(
+      cli::ServiceType::AGENT, cli::ConfigActionLevel::AGENT_COLDBOOT);
+
+  return fmt::format(
+      "Successfully removed admin distance entry for client-id {}. Routes from "
+      "this client will use MAX_ADMIN_DISTANCE (255). A coldboot is required to "
+      "apply the change to existing routes.",
+      clientId);
+}
+
+void CmdDeleteAdminDistance::printOutput(const RetType& output) {
+  std::cout << output << std::endl;
+}
+
+// Explicit template instantiation
+template void
+CmdHandler<CmdDeleteAdminDistance, CmdDeleteAdminDistanceTraits>::run();
+
+} // namespace facebook::fboss

fboss/cli/fboss2/commands/delete/switch/admin_distance/CmdDeleteAdminDistance.h

@@ -0,0 +1,67 @@
+/*
+ *  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 <string>
+#include <vector>
+#include "fboss/cli/fboss2/CmdHandler.h"
+#include "fboss/cli/fboss2/commands/delete/switch/CmdDeleteSwitch.h"
+
+namespace facebook::fboss {
+
+// Parses the single positional argument of
+//   delete admin-distance <client-id>
+// where <client-id> is a routing ClientID (e.g. 0=BGPD, 700=STATIC_INTERNAL,
+// 786=OPENR) whose admin-distance override should be removed from the config.
+// ClientIDs 1 (STATIC_ROUTE), 2 (INTERFACE_ROUTE), 3 (LINKLOCAL_ROUTE), and
+// 4 (REMOTE_INTERFACE_ROUTE) are forbidden here for the same reason they are
+// forbidden in `config switch admin-distance`: the agent hardcodes their
+// distances, so their map entries are inert. See
+// forbiddenAdminDistanceClients() in CmdConfigAdminDistance.h.
+class AdminDistanceDeleteArg : public utils::BaseObjectArgType<std::string> {
+ public:
+  /* implicit */ AdminDistanceDeleteArg( // NOLINT(google-explicit-constructor)
+      std::vector<std::string> v);
+
+  int32_t getClientId() const {
+    return clientId_;
+  }
+
+ private:
+  int32_t clientId_{0};
+};
+
+struct CmdDeleteAdminDistanceTraits : public WriteCommandTraits {
+  using ParentCmd = CmdDeleteSwitch;
+  static void addCliArg(CLI::App& cmd, std::vector<std::string>& args) {
+    cmd.add_option(
+        "client_id",
+        args,
+        "<client-id> - routing client whose admin distance entry to remove "
+        "(e.g. 0=BGP, 786=OpenR). "
+        "Client-ids 1/2/3/4 are hardcoded and cannot be removed.");
+  }
+  using ObjectArgType = AdminDistanceDeleteArg;
+  using RetType = std::string;
+};
+
+class CmdDeleteAdminDistance
+    : public CmdHandler<CmdDeleteAdminDistance, CmdDeleteAdminDistanceTraits> {
+ public:
+  using ObjectArgType = CmdDeleteAdminDistanceTraits::ObjectArgType;
+  using RetType = CmdDeleteAdminDistanceTraits::RetType;
+
+  RetType queryClient(const HostInfo& hostInfo, const ObjectArgType& arg);
+
+  void printOutput(const RetType& output);
+};
+
+} // namespace facebook::fboss