Skip to content

Commit ceec311

Browse files
Add 'config vlan <id> name <value>' CLI command
Adds a new `config vlan <id> name <value>` command to configure the name attribute of a VLAN. The implementation follows the `CmdConfigInterface` pattern: a single handler (`CmdConfigVlanConfig`) dispatches on attribute name, so adding future attributes (e.g. `routable`) requires only a new subcommand entry in `CmdListConfig.cpp` and a new `else if` branch in `queryClient` — no new handler class. If the specified VLAN does not already exist, the command auto-creates it (via `VlanManager::getOrCreateVlan`) and prints "Created VLAN <id>" before configuring the attribute — consistent with all other `config vlan` subcommands. The auto-create test uses VLAN 3100 (not 3998/3999) because `getTableIdForNpu` maps IDs in [3000, 3152] to Linux routing table IDs in [101, 253]. Values above 3152 overflow the 253-table limit and crash `TunManager` with `Check failed: tableId <= 253`. ``` Note: Google Test filter = *VlanConfig* [==========] Running 9 tests from 1 test suite. [----------] 9 tests from CmdConfigVlanConfigTestFixture [ RUN ] CmdConfigVlanConfigTestFixture.setNameOnExistingVlan [ OK ] CmdConfigVlanConfigTestFixture.setNameOnExistingVlan (171 ms) [ RUN ] CmdConfigVlanConfigTestFixture.setNameUpdatesSwConfig [ OK ] CmdConfigVlanConfigTestFixture.setNameUpdatesSwConfig (308 ms) [ RUN ] CmdConfigVlanConfigTestFixture.setNameOnSecondVlan [ OK ] CmdConfigVlanConfigTestFixture.setNameOnSecondVlan (195 ms) [ RUN ] CmdConfigVlanConfigTestFixture.setNameOnNonExistentVlanAutoCreates [ OK ] CmdConfigVlanConfigTestFixture.setNameOnNonExistentVlanAutoCreates (105 ms) [ RUN ] CmdConfigVlanConfigTestFixture.setNameDoesNotAffectOtherVlans [ OK ] CmdConfigVlanConfigTestFixture.setNameDoesNotAffectOtherVlans (99 ms) [ RUN ] CmdConfigVlanConfigTestFixture.vlanNameArgGetAttrName [ OK ] CmdConfigVlanConfigTestFixture.vlanNameArgGetAttrName (29 ms) [ RUN ] CmdConfigVlanConfigTestFixture.vlanNameArgGetValue [ OK ] CmdConfigVlanConfigTestFixture.vlanNameArgGetValue (39 ms) [ RUN ] CmdConfigVlanConfigTestFixture.vlanNameArgRejectsMissingValue [ OK ] CmdConfigVlanConfigTestFixture.vlanNameArgRejectsMissingValue (33 ms) [ RUN ] CmdConfigVlanConfigTestFixture.vlanNameArgRejectsTooManyArgs [ OK ] CmdConfigVlanConfigTestFixture.vlanNameArgRejectsTooManyArgs (36 ms) [----------] 9 tests from CmdConfigVlanConfigTestFixture (1020 ms total) [==========] 9 tests from 1 test suite ran. (1020 ms total) [ PASSED ] 9 tests. ``` ``` Note: Google Test filter = ConfigVlanName* [==========] Running 2 tests from 1 test suite. [----------] 2 tests from ConfigVlanNameTest [ RUN ] ConfigVlanNameTest.SetNameOnExistingVlan I0325 14:00:25 [Step 1] Finding an eth interface... I0325 14:00:25 Using interface eth1/1/1 (VLAN 2122) I0325 14:00:25 [Step 2] Reading current VLAN 2122 name... I0325 14:00:25 Current name: vlan2122 I0325 14:00:25 [Step 3] Setting VLAN 2122 name to "TestVlanName"... I0325 14:00:25 Output: {"localhost":"Successfully configured VLAN 2122: name=\"TestVlanName\""} I0325 14:00:25 [Step 4] Committing... I0325 14:00:25 Committed. I0325 14:00:25 [Step 5] Restoring original name "vlan2122"... I0325 14:00:25 Restored. TEST PASSED [ OK ] ConfigVlanNameTest.SetNameOnExistingVlan (375 ms) [ RUN ] ConfigVlanNameTest.AutoCreateVlanAndSetName I0325 14:00:25 [Step 0] Ensuring VLAN 3100 is absent from running config... I0325 14:00:25 [Cleanup] VLAN 3100 not present — no cleanup needed. I0325 14:00:25 [Step 1] Setting name on non-existent VLAN 3100... I0325 14:00:25 Output: Created VLAN 3100 I0325 14:00:25 {"localhost":"Successfully configured VLAN 3100: name=\"TestAutoCreateVlan\""} I0325 14:00:25 [Step 2] Committing session... I0325 14:00:25 Config committed. I0325 14:00:25 [Step 3] Cleaning up VLAN 3100... I0325 14:00:25 Cleanup complete. TEST PASSED [ OK ] ConfigVlanNameTest.AutoCreateVlanAndSetName (262 ms) [----------] 2 tests from ConfigVlanNameTest (637 ms total) [==========] 2 tests from 1 test suite ran. (638 ms total) [ PASSED ] 2 tests. ``` ``` fboss2-dev config vlan 2122 name myvlan fboss2-dev config session commit fboss2-dev config vlan 3100 name newvlan fboss2-dev config session commit ```
1 parent 0817373 commit ceec311

14 files changed

Lines changed: 633 additions & 6 deletions

‎cmake/CliFboss2.cmake‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -993,6 +993,8 @@ add_library(fboss2_config_lib
993993
fboss/cli/fboss2/commands/config/tunnel/ip_in_ip/encap/CmdConfigTunnelIpInIpEncap.h
994994
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlan.cpp
995995
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlan.h
996+
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanConfig.cpp
997+
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanConfig.h
996998
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanDefault.cpp
997999
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanDefault.h
9981000
fboss/cli/fboss2/commands/config/vlan/VlanManager.cpp

‎cmake/CliFboss2TestConfig.cmake‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ add_executable(fboss2_cmd_config_test
2323
fboss/cli/fboss2/test/config/CmdConfigSessionDiffTest.cpp
2424
fboss/cli/fboss2/test/config/CmdConfigSessionTest.cpp
2525
fboss/cli/fboss2/test/config/CmdConfigTestBase.cpp
26+
fboss/cli/fboss2/test/config/CmdConfigVlanConfigTest.cpp
2627
fboss/cli/fboss2/test/config/CmdConfigVlanDefaultTest.cpp
2728
fboss/cli/fboss2/test/config/CmdConfigTunnelIpInIpTest.cpp
2829
fboss/cli/fboss2/test/config/CmdConfigVlanManagerTest.cpp

‎cmake/CliFboss2TestIntegrationTest.cmake‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ add_executable(fboss2_integration_test
3131
fboss/cli/fboss2/test/integration_test/ConfigVlanCreateTest.cpp
3232
fboss/cli/fboss2/test/integration_test/ConfigInterfaceSwitchportTrunkAllowedVlanTest.cpp
3333
fboss/cli/fboss2/test/integration_test/ConfigVlanDefaultTest.cpp
34+
fboss/cli/fboss2/test/integration_test/ConfigVlanNameTest.cpp
3435
fboss/cli/fboss2/test/integration_test/ConfigVlanPortTaggingModeTest.cpp
3536
fboss/cli/fboss2/test/integration_test/ConfigVlanStaticMacTest.cpp
3637
fboss/cli/fboss2/test/integration_test/ConfigVlanSwitchportAccessTest.cpp

‎fboss/cli/fboss2/BUCK‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,6 +1137,7 @@ cpp_library(
11371137
"commands/config/tunnel/ip_in_ip/decap/CmdConfigTunnelIpInIpDecap.cpp",
11381138
"commands/config/tunnel/ip_in_ip/encap/CmdConfigTunnelIpInIpEncap.cpp",
11391139
"commands/config/vlan/CmdConfigVlan.cpp",
1140+
"commands/config/vlan/CmdConfigVlanConfig.cpp",
11401141
"commands/config/vlan/CmdConfigVlanDefault.cpp",
11411142
"commands/config/vlan/VlanManager.cpp",
11421143
"commands/config/vlan/port/CmdConfigVlanPort.cpp",
@@ -1253,6 +1254,7 @@ cpp_library(
12531254
"commands/config/tunnel/ip_in_ip/decap/CmdConfigTunnelIpInIpDecap.h",
12541255
"commands/config/tunnel/ip_in_ip/encap/CmdConfigTunnelIpInIpEncap.h",
12551256
"commands/config/vlan/CmdConfigVlan.h",
1257+
"commands/config/vlan/CmdConfigVlanConfig.h",
12561258
"commands/config/vlan/CmdConfigVlanDefault.h",
12571259
"commands/config/vlan/VlanManager.h",
12581260
"commands/config/vlan/port/CmdConfigVlanPort.h",

‎fboss/cli/fboss2/CmdListConfig.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@
112112
#include "fboss/cli/fboss2/commands/config/tunnel/ip_in_ip/decap/CmdConfigTunnelIpInIpDecap.h"
113113
#include "fboss/cli/fboss2/commands/config/tunnel/ip_in_ip/encap/CmdConfigTunnelIpInIpEncap.h"
114114
#include "fboss/cli/fboss2/commands/config/vlan/CmdConfigVlan.h"
115+
#include "fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanConfig.h"
115116
#include "fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanDefault.h"
116117
#include "fboss/cli/fboss2/commands/config/vlan/port/CmdConfigVlanPort.h"
117118
#include "fboss/cli/fboss2/commands/config/vlan/port/tagging_mode/CmdConfigVlanPortTaggingMode.h"
@@ -923,6 +924,12 @@ const CommandTree& kConfigCommandTree() {
923924
commandHandler<CmdConfigVlan>,
924925
argRegistrar<CmdConfigVlanTraits>,
925926
{{
927+
"name",
928+
"Configure the name of a VLAN",
929+
commandHandler<CmdConfigVlanConfig>,
930+
argRegistrar<CmdConfigVlanConfigTraits>,
931+
},
932+
{
926933
"port",
927934
"Configure VLAN port settings",
928935
commandHandler<CmdConfigVlanPort>,

‎fboss/cli/fboss2/CmdSubcommands.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ CLI::App* CmdSubcommands::addCommand(
261261
case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_NONE:
262262
case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_REVISION_LIST:
263263
case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_VLAN_ID:
264+
default:
264265
break;
265266
}
266267
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
* Copyright (c) 2004-present, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
*/
10+
11+
#include "fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanConfig.h"
12+
13+
#include <fmt/format.h>
14+
#include <cstdint>
15+
#include <iostream>
16+
#include <ostream>
17+
#include <stdexcept>
18+
#include <string>
19+
#include "fboss/agent/gen-cpp2/switch_config_types.h"
20+
#include "fboss/agent/types.h"
21+
#include "fboss/cli/fboss2/CmdHandler.cpp"
22+
#include "fboss/cli/fboss2/commands/config/vlan/CmdConfigVlan.h"
23+
#include "fboss/cli/fboss2/commands/config/vlan/VlanManager.h"
24+
#include "fboss/cli/fboss2/session/ConfigSession.h"
25+
#include "fboss/cli/fboss2/utils/HostInfo.h"
26+
27+
namespace facebook::fboss {
28+
29+
CmdConfigVlanConfigTraits::RetType CmdConfigVlanConfig::queryClient(
30+
const HostInfo& /* hostInfo */,
31+
const VlanId& vlanIdArg,
32+
const ObjectArgType& arg) {
33+
auto& swConfig = *ConfigSession::getInstance().getAgentConfig().sw();
34+
VlanID vlanId(vlanIdArg.getVlanId());
35+
36+
// Ensure VLAN exists, auto-creating it if needed
37+
auto [created, vlan] = VlanManager::createVlan(swConfig, vlanId);
38+
if (created) {
39+
std::cout << fmt::format("Created VLAN {}", static_cast<uint16_t>(vlanId))
40+
<< std::endl;
41+
}
42+
43+
const std::string& attr = arg.getAttrName();
44+
const std::string& value = arg.getValue();
45+
46+
if (attr == "name") {
47+
vlan->name() = value;
48+
} else {
49+
throw std::invalid_argument(
50+
fmt::format("Unknown VLAN attribute '{}'. Supported: name", attr));
51+
}
52+
53+
ConfigSession::getInstance().saveConfig();
54+
55+
return fmt::format(
56+
"Successfully configured VLAN {}: {}=\"{}\"",
57+
static_cast<uint16_t>(vlanId),
58+
attr,
59+
value);
60+
}
61+
62+
void CmdConfigVlanConfig::printOutput(const RetType& logMsg) {
63+
std::cout << logMsg << std::endl;
64+
}
65+
66+
// Explicit template instantiation
67+
template void CmdHandler<CmdConfigVlanConfig, CmdConfigVlanConfigTraits>::run();
68+
69+
} // namespace facebook::fboss
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* Copyright (c) 2004-present, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
*/
10+
11+
#pragma once
12+
13+
#include <stdexcept>
14+
#include <string>
15+
#include <utility>
16+
#include <vector>
17+
#include "fboss/cli/fboss2/CmdHandler.h"
18+
#include "fboss/cli/fboss2/commands/config/vlan/CmdConfigVlan.h"
19+
#include "fboss/cli/fboss2/utils/CmdUtilsCommon.h"
20+
#include "fboss/cli/fboss2/utils/HostInfo.h"
21+
22+
namespace facebook::fboss {
23+
24+
/**
25+
* Argument type for VLAN attribute configuration.
26+
*
27+
* Each concrete subclass encodes one attribute name (e.g. "name") so the
28+
* handler can dispatch on it. Adding a new attribute requires only:
29+
* 1. A new subclass below with the appropriate getAttrName() value.
30+
* 2. A new else-if branch in CmdConfigVlanConfig::queryClient().
31+
* 3. A new subcommand entry in CmdListConfig.cpp pointing to the same
32+
* handler with the new arg class as ObjectArgType.
33+
*/
34+
class VlanConfigArg : public utils::BaseObjectArgType<std::string> {
35+
public:
36+
/* implicit */ VlanConfigArg( // NOLINT(google-explicit-constructor)
37+
std::vector<std::string> v) {
38+
if (v.size() != 1) {
39+
throw std::invalid_argument(
40+
"Expected <value>, got " + std::to_string(v.size()) + " argument(s)");
41+
}
42+
value_ = std::move(v[0]);
43+
}
44+
45+
virtual std::string getAttrName() const = 0;
46+
47+
const std::string& getValue() const {
48+
return value_;
49+
}
50+
51+
private:
52+
std::string value_;
53+
};
54+
55+
/** Argument type for 'config vlan <id> name <value>'. */
56+
class VlanNameArg : public VlanConfigArg {
57+
public:
58+
/* implicit */ VlanNameArg( // NOLINT(google-explicit-constructor)
59+
std::vector<std::string> v)
60+
: VlanConfigArg(std::move(v)) {}
61+
62+
std::string getAttrName() const override {
63+
return "name";
64+
}
65+
};
66+
67+
struct CmdConfigVlanConfigTraits : public WriteCommandTraits {
68+
using ParentCmd = CmdConfigVlan;
69+
static void addCliArg(CLI::App& cmd, std::vector<std::string>& args) {
70+
cmd.add_option(
71+
"vlan_config_value", args, "New value for the VLAN attribute");
72+
}
73+
using ObjectArgType = VlanNameArg;
74+
using RetType = std::string;
75+
};
76+
77+
class CmdConfigVlanConfig
78+
: public CmdHandler<CmdConfigVlanConfig, CmdConfigVlanConfigTraits> {
79+
public:
80+
using ObjectArgType = CmdConfigVlanConfigTraits::ObjectArgType;
81+
using RetType = CmdConfigVlanConfigTraits::RetType;
82+
83+
RetType queryClient(
84+
const HostInfo& hostInfo,
85+
const VlanId& vlanId,
86+
const ObjectArgType& arg);
87+
88+
void printOutput(const RetType& logMsg);
89+
};
90+
91+
} // namespace facebook::fboss

‎fboss/cli/fboss2/test/config/BUCK‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ cpp_unittest(
2727
"CmdConfigSessionTest.cpp",
2828
"CmdConfigTestBase.cpp",
2929
"CmdConfigTunnelIpInIpTest.cpp",
30+
"CmdConfigVlanConfigTest.cpp",
3031
"CmdConfigVlanDefaultTest.cpp",
3132
"CmdConfigVlanManagerTest.cpp",
3233
"CmdConfigVlanPortTaggingModeTest.cpp",

0 commit comments

Comments
 (0)