[Nexthop][fboss2-dev] Add fboss2-dev config vlan default CLI command

cmake/CliFboss2.cmake

@@ -867,6 +867,8 @@ add_library(fboss2_config_lib
   fboss/cli/fboss2/commands/config/session/CmdConfigSessionRebase.cpp
   fboss/cli/fboss2/commands/config/vlan/CmdConfigVlan.cpp
   fboss/cli/fboss2/commands/config/vlan/CmdConfigVlan.h
+  fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanDefault.cpp
+  fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanDefault.h
   fboss/cli/fboss2/commands/config/vlan/port/CmdConfigVlanPort.cpp
   fboss/cli/fboss2/commands/config/vlan/port/CmdConfigVlanPort.h
   fboss/cli/fboss2/commands/config/vlan/port/tagging_mode/CmdConfigVlanPortTaggingMode.h

cmake/CliFboss2TestConfig.cmake

@@ -16,6 +16,7 @@ add_executable(fboss2_cmd_config_test
   fboss/cli/fboss2/test/config/CmdConfigSessionDiffTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigSessionTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigTestBase.cpp
+  fboss/cli/fboss2/test/config/CmdConfigVlanDefaultTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigVlanPortTaggingModeTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigVlanStaticMacTest.cpp
   fboss/cli/fboss2/test/config/ConfigSessionSystemdTest.cpp

cmake/CliFboss2TestIntegrationTest.cmake

@@ -18,6 +18,7 @@ add_executable(fboss2_integration_test
   fboss/cli/fboss2/test/integration_test/ConfigPortQueueConfigTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigQosPolicyMapTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigSessionClearTest.cpp
+  fboss/cli/fboss2/test/integration_test/ConfigVlanDefaultTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigVlanPortTaggingModeTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigVlanStaticMacTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigVlanSwitchportAccessTest.cpp

fboss/cli/fboss2/BUCK

@@ -1105,6 +1105,7 @@ cpp_library(
         "commands/config/session/CmdConfigSessionDiff.cpp",
         "commands/config/session/CmdConfigSessionRebase.cpp",
         "commands/config/vlan/CmdConfigVlan.cpp",
+        "commands/config/vlan/CmdConfigVlanDefault.cpp",
         "commands/config/vlan/port/CmdConfigVlanPort.cpp",
         "commands/config/vlan/port/tagging_mode/CmdConfigVlanPortTaggingMode.cpp",
         "commands/config/vlan/static_mac/CmdConfigVlanStaticMac.cpp",
@@ -1187,6 +1188,7 @@ cpp_library(
         "commands/config/session/CmdConfigSessionDiff.h",
         "commands/config/session/CmdConfigSessionRebase.h",
         "commands/config/vlan/CmdConfigVlan.h",
+        "commands/config/vlan/CmdConfigVlanDefault.h",
         "commands/config/vlan/port/CmdConfigVlanPort.h",
         "commands/config/vlan/port/tagging_mode/CmdConfigVlanPortTaggingMode.h",
         "commands/config/vlan/static_mac/CmdConfigVlanStaticMac.h",

fboss/cli/fboss2/CmdListConfig.cpp

@@ -95,6 +95,7 @@
 #include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h"
 #include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionRebase.h"
 #include "fboss/cli/fboss2/commands/config/vlan/CmdConfigVlan.h"
+#include "fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanDefault.h"
 #include "fboss/cli/fboss2/commands/config/vlan/port/CmdConfigVlanPort.h"
 #include "fboss/cli/fboss2/commands/config/vlan/port/tagging_mode/CmdConfigVlanPortTaggingMode.h"
 #include "fboss/cli/fboss2/commands/config/vlan/static_mac/CmdConfigVlanStaticMac.h"
@@ -774,6 +775,23 @@ const CommandTree& kConfigCommandTree() {
            }},
       },
 
+      // Registered separately (merged into the existing "config vlan" by
+      // CmdSubcommands::addCommandBranch) so "default" lands at depth 0 and
+      // owns data_[0] for its own VLAN ID arg, independent of the
+      // "config vlan <id>" context (whose arg also lives at data_[0] but is
+      // not populated when the "default" subcommand is matched).
+      {
+          "config",
+          "vlan",
+          "Configure VLAN settings",
+          {{
+              "default",
+              "Set global default VLAN ID for untagged traffic",
+              commandHandler<CmdConfigVlanDefault>,
+              argRegistrar<CmdConfigVlanDefaultTraits>,
+          }},
+      },
+
       {"delete",
        "config",
        "Delete config objects",

fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanDefault.cpp

@@ -0,0 +1,116 @@
+/*
+ *  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/vlan/CmdConfigVlanDefault.h"
+
+#include "fboss/cli/fboss2/CmdHandler.cpp"
+
+#include <fmt/format.h>
+#include <algorithm>
+#include <iostream>
+#include "fboss/agent/gen-cpp2/switch_config_types.h"
+#include "fboss/cli/fboss2/session/ConfigSession.h"
+
+namespace facebook::fboss {
+
+CmdConfigVlanDefaultTraits::RetType CmdConfigVlanDefault::queryClient(
+    const HostInfo& /* hostInfo */,
+    const ObjectArgType& vlanIdArg) {
+  auto& session = ConfigSession::getInstance();
+  auto& config = session.getAgentConfig();
+  auto& swConfig = *config.sw();
+  int32_t vlanId = vlanIdArg.getVlanId();
+  int32_t currentDefault = *swConfig.defaultVlan();
+
+  if (currentDefault == vlanId) {
+    return fmt::format("Default VLAN is already set to {}", vlanId);
+  }
+
+  // Small helper for ID-based membership checks on different config lists.
+  auto hasElemWithId = [](const auto& list, int id, auto accessor) {
+    return std::any_of(list.begin(), list.end(), [&](const auto& elem) {
+      return accessor(elem) == id;
+    });
+  };
+
+  // Validate the current default VLAN before changing it.
+  //
+  // 1) If the old default VLAN is used as ingressVlan for any port and also
+  //    has an Interface, we allow the change.
+  // 2) If the old default VLAN is used as ingressVlan for any port but has
+  //    no Interface, we refuse to proceed and ask the user to fix the config
+  //    first (this is the state that causes fboss_sw_agent to crash).
+  bool oldVlanHasInterface = hasElemWithId(
+      *swConfig.interfaces(), currentDefault, [](const auto& intf) {
+        return *intf.vlanID();
+      });
+
+  bool oldVlanUsedAsIngress =
+      hasElemWithId(*swConfig.ports(), currentDefault, [](const auto& port) {
+        return *port.ingressVlan();
+      });
+
+  if (oldVlanUsedAsIngress && !oldVlanHasInterface) {
+    return fmt::format(
+        "Refusing to change default VLAN from {} to {} because VLAN {} is "
+        "used as ingressVlan for one or more ports but has no interface. "
+        "Move those ports to a different VLAN or attach an interface before "
+        "changing the default.",
+        currentDefault,
+        vlanId,
+        currentDefault);
+  }
+
+  // New default VLAN handling:
+  // 3) If the new VLAN already exists in the VLAN table, do not touch it;
+  //    just update sw.defaultVlan.
+  // 4) If the new VLAN does not exist, create a non-routable placeholder
+  //    entry for it (no interface).
+  auto& vlans = *swConfig.vlans();
+  auto findById = [](auto& vlanList, int id) {
+    return std::find_if(
+        vlanList.begin(), vlanList.end(), [id](const auto& vlan) {
+          return *vlan.id() == id;
+        });
+  };
+
+  auto targetVlanIt = findById(vlans, vlanId);
+  if (targetVlanIt == vlans.end()) {
+    cfg::Vlan newVlan;
+    newVlan.id() = vlanId;
+    newVlan.name() = fmt::format("default_{}", vlanId);
+    newVlan.routable() = false;
+    vlans.push_back(newVlan);
+  }
+
+  // Remove the older VLAN id from the VLAN object, if it does not have any
+  // interface associated with it.
+  auto oldVlanIt = findById(vlans, currentDefault);
+  if (!oldVlanHasInterface && oldVlanIt != vlans.end()) {
+    vlans.erase(oldVlanIt);
+  }
+
+  swConfig.defaultVlan() = vlanId;
+
+  session.saveConfig();
+
+  return fmt::format(
+      "Successfully set default VLAN to {} (was {})", vlanId, currentDefault);
+}
+
+void CmdConfigVlanDefault::printOutput(const RetType& logMsg) {
+  std::cout << logMsg << std::endl;
+}
+
+// Explicit template instantiation
+template void
+CmdHandler<CmdConfigVlanDefault, CmdConfigVlanDefaultTraits>::run();
+
+} // namespace facebook::fboss

fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanDefault.h

@@ -0,0 +1,37 @@
+/*
+ *  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 "fboss/cli/fboss2/CmdHandler.h"
+#include "fboss/cli/fboss2/utils/CmdUtils.h"
+
+namespace facebook::fboss {
+
+struct CmdConfigVlanDefaultTraits : public WriteCommandTraits {
+  static void addCliArg(CLI::App& cmd, std::vector<std::string>& args) {
+    cmd.add_option("vlan_id", args, "VLAN ID (1-4094)");
+  }
+  using ObjectArgType = utils::VlanIdValue;
+  using RetType = std::string;
+};
+
+class CmdConfigVlanDefault
+    : public CmdHandler<CmdConfigVlanDefault, CmdConfigVlanDefaultTraits> {
+ public:
+  using ObjectArgType = CmdConfigVlanDefaultTraits::ObjectArgType;
+  using RetType = CmdConfigVlanDefaultTraits::RetType;
+
+  RetType queryClient(const HostInfo& hostInfo, const ObjectArgType& vlanId);
+
+  void printOutput(const RetType& logMsg);
+};
+
+} // namespace facebook::fboss

fboss/cli/fboss2/test/config/BUCK

@@ -18,6 +18,7 @@ cpp_unittest(
         "CmdConfigSessionDiffTest.cpp",
         "CmdConfigSessionTest.cpp",
         "CmdConfigTestBase.cpp",
+        "CmdConfigVlanDefaultTest.cpp",
         "CmdConfigVlanPortTaggingModeTest.cpp",
         "CmdConfigVlanStaticMacTest.cpp",
         "ConfigSessionSystemdTest.cpp",