[Nexthop][fboss2-dev] Add config and delete interface sflow sample-dest

cmake/CliFboss2.cmake

@@ -844,6 +844,8 @@ add_library(fboss2_config_lib
   fboss/cli/fboss2/commands/config/interface/ipv6/ndp/CmdConfigInterfaceIpv6Ndp.h
   fboss/cli/fboss2/commands/config/interface/pfc_config/CmdConfigInterfacePfcConfig.cpp
   fboss/cli/fboss2/commands/config/interface/pfc_config/CmdConfigInterfacePfcConfig.h
+  fboss/cli/fboss2/commands/config/interface/sflow/CmdConfigInterfaceSflow.cpp
+  fboss/cli/fboss2/commands/config/interface/sflow/CmdConfigInterfaceSflow.h
   fboss/cli/fboss2/commands/config/interface/pfc_config/PfcConfigUtils.h
   fboss/cli/fboss2/commands/config/interface/switchport/CmdConfigInterfaceSwitchport.cpp
   fboss/cli/fboss2/commands/config/interface/switchport/CmdConfigInterfaceSwitchport.h
@@ -1084,6 +1086,8 @@ add_library(fboss2_config_lib
   fboss/cli/fboss2/commands/delete/interface/ipv6/CmdDeleteInterfaceIpv6.h
   fboss/cli/fboss2/commands/delete/interface/ipv6/ndp/CmdDeleteInterfaceIpv6Ndp.cpp
   fboss/cli/fboss2/commands/delete/interface/ipv6/ndp/CmdDeleteInterfaceIpv6Ndp.h
+  fboss/cli/fboss2/commands/delete/interface/sflow/CmdDeleteInterfaceSflow.cpp
+  fboss/cli/fboss2/commands/delete/interface/sflow/CmdDeleteInterfaceSflow.h
   fboss/cli/fboss2/commands/delete/protocol/CmdDeleteProtocol.cpp
   fboss/cli/fboss2/commands/delete/protocol/CmdDeleteProtocol.h
   fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp

cmake/CliFboss2TestConfig.cmake

@@ -15,6 +15,7 @@ add_executable(fboss2_cmd_config_test
   fboss/cli/fboss2/test/config/CmdConfigHostnameTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigHistoryTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigInterfaceIpv6NdpTest.cpp
+  fboss/cli/fboss2/test/config/CmdConfigInterfaceSflowSampleDestTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigInterfaceSwitchportAccessVlanTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigInterfaceSwitchportTrunkAllowedVlanTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigInterfaceTest.cpp

cmake/CliFboss2TestIntegrationTest.cmake

@@ -11,6 +11,8 @@ add_executable(fboss2_integration_test
   fboss/cli/fboss2/test/integration_test/ConfigAdminDistanceTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigAclRuleTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigArpTest.cpp
+  fboss/cli/fboss2/test/integration_test/ConfigSflowRateTest.cpp
+  fboss/cli/fboss2/test/integration_test/ConfigSflowSampleDestTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigConcurrentSessionsTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigHostnameTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigIcmpV4UnavailableSrcAddrTest.cpp

fboss/cli/fboss2/BUCK

@@ -1091,6 +1091,7 @@ cpp_library(
         "commands/config/interface/ipv6/CmdConfigInterfaceIpv6.cpp",
         "commands/config/interface/ipv6/ndp/CmdConfigInterfaceIpv6Ndp.cpp",
         "commands/config/interface/pfc_config/CmdConfigInterfacePfcConfig.cpp",
+        "commands/config/interface/sflow/CmdConfigInterfaceSflow.cpp",
         "commands/config/interface/switchport/CmdConfigInterfaceSwitchport.cpp",
         "commands/config/interface/switchport/access/CmdConfigInterfaceSwitchportAccess.cpp",
         "commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.cpp",
@@ -1210,6 +1211,7 @@ cpp_library(
         "commands/delete/interface/CmdDeleteInterface.cpp",
         "commands/delete/interface/ipv6/CmdDeleteInterfaceIpv6.cpp",
         "commands/delete/interface/ipv6/ndp/CmdDeleteInterfaceIpv6Ndp.cpp",
+        "commands/delete/interface/sflow/CmdDeleteInterfaceSflow.cpp",
         "commands/delete/protocol/CmdDeleteProtocol.cpp",
         "commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp",
         "commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.cpp",
@@ -1248,6 +1250,7 @@ cpp_library(
         "commands/config/interface/ipv6/CmdConfigInterfaceIpv6.h",
         "commands/config/interface/ipv6/ndp/CmdConfigInterfaceIpv6Ndp.h",
         "commands/config/interface/pfc_config/CmdConfigInterfacePfcConfig.h",
+        "commands/config/interface/sflow/CmdConfigInterfaceSflow.h",
         "commands/config/interface/pfc_config/PfcConfigUtils.h",
         "commands/config/interface/switchport/CmdConfigInterfaceSwitchport.h",
         "commands/config/interface/switchport/access/CmdConfigInterfaceSwitchportAccess.h",
@@ -1352,6 +1355,7 @@ cpp_library(
         "commands/delete/interface/CmdDeleteInterface.h",
         "commands/delete/interface/ipv6/CmdDeleteInterfaceIpv6.h",
         "commands/delete/interface/ipv6/ndp/CmdDeleteInterfaceIpv6Ndp.h",
+        "commands/delete/interface/sflow/CmdDeleteInterfaceSflow.h",
         "commands/delete/protocol/CmdDeleteProtocol.h",
         "commands/delete/protocol/static/CmdDeleteProtocolStatic.h",
         "commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h",

fboss/cli/fboss2/CmdListConfig.cpp

@@ -24,6 +24,7 @@
 #include "fboss/cli/fboss2/commands/config/interface/ipv6/CmdConfigInterfaceIpv6.h"
 #include "fboss/cli/fboss2/commands/config/interface/ipv6/ndp/CmdConfigInterfaceIpv6Ndp.h"
 #include "fboss/cli/fboss2/commands/config/interface/pfc_config/CmdConfigInterfacePfcConfig.h"
+#include "fboss/cli/fboss2/commands/config/interface/sflow/CmdConfigInterfaceSflow.h"
 #include "fboss/cli/fboss2/commands/config/interface/switchport/CmdConfigInterfaceSwitchport.h"
 #include "fboss/cli/fboss2/commands/config/interface/switchport/access/CmdConfigInterfaceSwitchportAccess.h"
 #include "fboss/cli/fboss2/commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.h"
@@ -139,6 +140,7 @@
 #include "fboss/cli/fboss2/commands/delete/interface/CmdDeleteInterface.h"
 #include "fboss/cli/fboss2/commands/delete/interface/ipv6/CmdDeleteInterfaceIpv6.h"
 #include "fboss/cli/fboss2/commands/delete/interface/ipv6/ndp/CmdDeleteInterfaceIpv6Ndp.h"
+#include "fboss/cli/fboss2/commands/delete/interface/sflow/CmdDeleteInterfaceSflow.h"
 #include "fboss/cli/fboss2/commands/delete/protocol/CmdDeleteProtocol.h"
 #include "fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.h"
 #include "fboss/cli/fboss2/commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h"
@@ -278,6 +280,13 @@ const CommandTree& kConfigCommandTree() {
                    argRegistrar<CmdConfigInterfaceIpv6NdpTraits>,
                }},
            },
+           {
+               "sflow",
+               "Configure sFlow settings: sample-dest <cpu|mirror>, "
+               "ingress-rate <N>, egress-rate <N>",
+               commandHandler<CmdConfigInterfaceSflow>,
+               argRegistrar<CmdConfigInterfaceSflowTraits>,
+           },
            {
                "switchport",
                "Configure switchport settings",
@@ -1086,17 +1095,24 @@ const CommandTree& kConfigCommandTree() {
           commandHandler<CmdDeleteInterface>,
           argRegistrar<CmdDeleteInterfaceTraits>,
           {{
-              "ipv6",
-              "Delete (reset to default) IPv6 settings for interface",
-              commandHandler<CmdDeleteInterfaceIpv6>,
-              argTypeHandler<CmdDeleteInterfaceIpv6Traits>,
-              {{
-                  "ndp",
-                  "Reset IPv6 Neighbor Discovery (NDP/RA) settings to defaults",
-                  commandHandler<CmdDeleteInterfaceIpv6Ndp>,
-                  argRegistrar<CmdDeleteInterfaceIpv6NdpTraits>,
-              }},
-          }},
+               "ipv6",
+               "Delete (reset to default) IPv6 settings for interface",
+               commandHandler<CmdDeleteInterfaceIpv6>,
+               argTypeHandler<CmdDeleteInterfaceIpv6Traits>,
+               {{
+                   "ndp",
+                   "Reset IPv6 Neighbor Discovery (NDP/RA) settings to defaults",
+                   commandHandler<CmdDeleteInterfaceIpv6Ndp>,
+                   argRegistrar<CmdDeleteInterfaceIpv6NdpTraits>,
+               }},
+           },
+           {
+               "sflow",
+               "Delete (reset to default) sFlow settings for interface: "
+               "sample-dest, ingress-rate, egress-rate",
+               commandHandler<CmdDeleteInterfaceSflow>,
+               argRegistrar<CmdDeleteInterfaceSflowTraits>,
+           }},
       },
 
       {

fboss/cli/fboss2/commands/config/interface/sflow/CmdConfigInterfaceSflow.cpp

@@ -0,0 +1,198 @@
+/*
+ *  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/interface/sflow/CmdConfigInterfaceSflow.h"
+
+#include "fboss/cli/fboss2/CmdHandler.cpp"
+
+#include <fmt/format.h>
+#include <folly/Conv.h>
+#include <folly/String.h>
+#include <algorithm>
+#include <cctype>
+#include <iostream>
+#include "fboss/agent/gen-cpp2/switch_config_types.h"
+#include "fboss/cli/fboss2/session/ConfigSession.h"
+
+namespace facebook::fboss {
+
+namespace {
+constexpr std::string_view kAttrSampleDest = "sample-dest";
+constexpr std::string_view kAttrIngressRate = "ingress-rate";
+constexpr std::string_view kAttrEgressRate = "egress-rate";
+constexpr std::string_view kSampleDestCpu = "cpu";
+constexpr std::string_view kSampleDestMirror = "mirror";
+constexpr auto kValidSflowAttrs = "sample-dest, ingress-rate, egress-rate";
+
+std::string toLower(std::string s) {
+  std::transform(s.begin(), s.end(), s.begin(), [](unsigned char c) {
+    return std::tolower(c);
+  });
+  return s;
+}
+
+cfg::SampleDestination parseSampleDest(const std::string& token) {
+  if (token == kSampleDestCpu) {
+    return cfg::SampleDestination::CPU;
+  }
+  if (token == kSampleDestMirror) {
+    return cfg::SampleDestination::MIRROR;
+  }
+  throw std::invalid_argument(
+      fmt::format(
+          "Invalid sample destination '{}': must be {} or {}",
+          token,
+          kSampleDestCpu,
+          kSampleDestMirror));
+}
+
+// sFlowIngressRate/sFlowEgressRate are "every 1/rate packets sampled"; 0
+// disables sampling. Negative values are meaningless and would wrap when
+// handed to the SAI SDK's unsigned rate attribute.
+int64_t parseSampleRate(const std::string& attr, const std::string& value) {
+  int64_t rate = 0;
+  try {
+    rate = folly::to<int64_t>(value);
+  } catch (const std::exception&) {
+    throw std::invalid_argument(
+        fmt::format("Invalid {} value '{}': must be an integer", attr, value));
+  }
+  if (rate < 0) {
+    throw std::invalid_argument(
+        fmt::format(
+            "Invalid {} value '{}': must be a non-negative integer",
+            attr,
+            value));
+  }
+  return rate;
+}
+} // namespace
+
+SflowAttrArgs::SflowAttrArgs(std::vector<std::string> v) {
+  if (v.empty()) {
+    throw std::invalid_argument(
+        fmt::format(
+            "No sflow attribute provided. Valid attributes are: {}",
+            kValidSflowAttrs));
+  }
+  attr_ = toLower(v[0]);
+  if (v.size() != 2) {
+    throw std::invalid_argument(
+        fmt::format("Expected exactly one value for '{}'", attr_));
+  }
+  value_ = v[1];
+  data_ = std::move(v);
+}
+
+CmdConfigInterfaceSflowTraits::RetType CmdConfigInterfaceSflow::queryClient(
+    const HostInfo& /* hostInfo */,
+    const utils::InterfaceList& interfaces,
+    const ObjectArgType& sflowAttr) {
+  if (interfaces.empty()) {
+    throw std::invalid_argument("No interface name provided");
+  }
+
+  const std::string& attr = sflowAttr.attr();
+  if (attr != kAttrSampleDest && attr != kAttrIngressRate &&
+      attr != kAttrEgressRate) {
+    throw std::invalid_argument(
+        fmt::format(
+            "Unknown sflow attribute '{}'. Valid attributes are: {}",
+            attr,
+            kValidSflowAttrs));
+  }
+
+  cfg::SampleDestination dest = cfg::SampleDestination::CPU;
+  int64_t rate = 0;
+  std::string displayValue;
+  if (attr == kAttrSampleDest) {
+    displayValue = toLower(sflowAttr.value());
+    dest = parseSampleDest(displayValue);
+  } else {
+    rate = parseSampleRate(attr, sflowAttr.value());
+    displayValue = folly::to<std::string>(rate);
+  }
+
+  auto& session = ConfigSession::getInstance();
+
+  std::vector<std::string> updatedNames;
+  std::vector<std::string> skippedNames;
+  for (const utils::Intf& intf : interfaces) {
+    cfg::Port* port = intf.getPort();
+    if (!port) {
+      // Resolved as an L3 interface only (e.g. an SVI): these sflow
+      // attributes are all Port attributes, so there is nothing to set --
+      // report it rather than silently succeeding.
+      skippedNames.push_back(intf.name());
+      continue;
+    }
+    if (attr == kAttrSampleDest) {
+      // The agent rejects egress sampling to a mirror destination
+      // (ApplyThriftConfig throws for MIRROR + sFlowEgressRate > 0); fail
+      // here with a targeted message before touching the config.
+      if (dest == cfg::SampleDestination::MIRROR &&
+          *port->sFlowEgressRate() > 0) {
+        throw std::invalid_argument(
+            fmt::format(
+                "Port {}: sample-dest {} requires sFlowEgressRate 0 — egress "
+                "sampling to a mirror destination is unsupported",
+                *port->name(),
+                kSampleDestMirror));
+      }
+      port->sampleDest() = dest;
+    } else if (attr == kAttrIngressRate) {
+      port->sFlowIngressRate() = rate;
+    } else {
+      // Same MIRROR/egress-rate constraint as above, checked from the other
+      // side: refuse a non-zero egress-rate on a port whose sampleDest is
+      // already MIRROR.
+      if (rate > 0 && port->sampleDest().has_value() &&
+          *port->sampleDest() == cfg::SampleDestination::MIRROR) {
+        throw std::invalid_argument(
+            fmt::format(
+                "Port {}: egress-rate must be 0 while sample-dest is {} — "
+                "egress sampling to a mirror destination is unsupported",
+                *port->name(),
+                kSampleDestMirror));
+      }
+      port->sFlowEgressRate() = rate;
+    }
+    updatedNames.push_back(intf.name());
+  }
+  if (updatedNames.empty()) {
+    throw std::invalid_argument("No port found for the specified interface(s)");
+  }
+
+  session.saveConfig(cli::ServiceType::AGENT, cli::ConfigActionLevel::HITLESS);
+
+  std::string attrLabel = attr == kAttrSampleDest
+      ? "sample destination"
+      : (attr == kAttrIngressRate ? "ingress-rate" : "egress-rate");
+  std::string message = fmt::format(
+      "Successfully set sFlow {} for interface(s) {} to {}",
+      attrLabel,
+      folly::join(", ", updatedNames),
+      displayValue);
+  if (!skippedNames.empty()) {
+    message +=
+        fmt::format("; skipped (no port): {}", folly::join(", ", skippedNames));
+  }
+  return message;
+}
+
+void CmdConfigInterfaceSflow::printOutput(const RetType& logMsg) {
+  std::cout << logMsg << std::endl;
+}
+
+// Explicit template instantiation
+template void
+CmdHandler<CmdConfigInterfaceSflow, CmdConfigInterfaceSflowTraits>::run();
+
+} // namespace facebook::fboss