Skip to content

Commit 0bfa7f9

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 0d22049 commit 0bfa7f9

14 files changed

Lines changed: 642 additions & 6 deletions

‎cmake/CliFboss2.cmake‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,8 @@ add_library(fboss2_config_lib
977977
fboss/cli/fboss2/commands/config/session/CmdConfigSessionRebase.cpp
978978
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlan.cpp
979979
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlan.h
980+
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanConfig.cpp
981+
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanConfig.h
980982
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanDefault.cpp
981983
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanDefault.h
982984
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
@@ -21,6 +21,7 @@ add_executable(fboss2_cmd_config_test
2121
fboss/cli/fboss2/test/config/CmdConfigSessionDiffTest.cpp
2222
fboss/cli/fboss2/test/config/CmdConfigSessionTest.cpp
2323
fboss/cli/fboss2/test/config/CmdConfigTestBase.cpp
24+
fboss/cli/fboss2/test/config/CmdConfigVlanConfigTest.cpp
2425
fboss/cli/fboss2/test/config/CmdConfigVlanDefaultTest.cpp
2526
fboss/cli/fboss2/test/config/CmdConfigVlanManagerTest.cpp
2627
fboss/cli/fboss2/test/config/CmdConfigVlanPortTaggingModeTest.cpp

‎cmake/CliFboss2TestIntegrationTest.cmake‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ add_executable(fboss2_integration_test
3030
fboss/cli/fboss2/test/integration_test/ConfigSessionClearTest.cpp
3131
fboss/cli/fboss2/test/integration_test/ConfigVlanCreateTest.cpp
3232
fboss/cli/fboss2/test/integration_test/ConfigVlanDefaultTest.cpp
33+
fboss/cli/fboss2/test/integration_test/ConfigVlanNameTest.cpp
3334
fboss/cli/fboss2/test/integration_test/ConfigVlanPortTaggingModeTest.cpp
3435
fboss/cli/fboss2/test/integration_test/ConfigVlanStaticMacTest.cpp
3536
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
@@ -1127,6 +1127,7 @@ cpp_library(
11271127
"commands/config/session/CmdConfigSessionDiff.cpp",
11281128
"commands/config/session/CmdConfigSessionRebase.cpp",
11291129
"commands/config/vlan/CmdConfigVlan.cpp",
1130+
"commands/config/vlan/CmdConfigVlanConfig.cpp",
11301131
"commands/config/vlan/CmdConfigVlanDefault.cpp",
11311132
"commands/config/vlan/VlanManager.cpp",
11321133
"commands/config/vlan/port/CmdConfigVlanPort.cpp",
@@ -1223,6 +1224,7 @@ cpp_library(
12231224
"commands/config/session/CmdConfigSessionDiff.h",
12241225
"commands/config/session/CmdConfigSessionRebase.h",
12251226
"commands/config/vlan/CmdConfigVlan.h",
1227+
"commands/config/vlan/CmdConfigVlanConfig.h",
12261228
"commands/config/vlan/CmdConfigVlanDefault.h",
12271229
"commands/config/vlan/VlanManager.h",
12281230
"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
@@ -103,6 +103,7 @@
103103
#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h"
104104
#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionRebase.h"
105105
#include "fboss/cli/fboss2/commands/config/vlan/CmdConfigVlan.h"
106+
#include "fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanConfig.h"
106107
#include "fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanDefault.h"
107108
#include "fboss/cli/fboss2/commands/config/vlan/port/CmdConfigVlanPort.h"
108109
#include "fboss/cli/fboss2/commands/config/vlan/port/tagging_mode/CmdConfigVlanPortTaggingMode.h"
@@ -833,6 +834,12 @@ const CommandTree& kConfigCommandTree() {
833834
commandHandler<CmdConfigVlan>,
834835
argRegistrar<CmdConfigVlanTraits>,
835836
{{
837+
"name",
838+
"Configure the name of a VLAN",
839+
commandHandler<CmdConfigVlanConfig>,
840+
argRegistrar<CmdConfigVlanConfigTraits>,
841+
},
842+
{
836843
"port",
837844
"Configure VLAN port settings",
838845
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
@@ -24,6 +24,7 @@ cpp_unittest(
2424
"CmdConfigSessionDiffTest.cpp",
2525
"CmdConfigSessionTest.cpp",
2626
"CmdConfigTestBase.cpp",
27+
"CmdConfigVlanConfigTest.cpp",
2728
"CmdConfigVlanDefaultTest.cpp",
2829
"CmdConfigVlanManagerTest.cpp",
2930
"CmdConfigVlanPortTaggingModeTest.cpp",

0 commit comments

Comments
 (0)