[Nexthop][fboss2-dev] Add fboss2-dev config vlan default CLI command#1153
[Nexthop][fboss2-dev] Add fboss2-dev config vlan default CLI command#1153vybhav-nexthop wants to merge 1 commit into
Conversation
|
Hi @vybhav-nexthop! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
dbb9af6 to
127ef90
Compare
0a716ce to
2997f97
Compare
2997f97 to
3ee02a1
Compare
Pre-submission checklist
pip install -r requirements-dev.txt && pre-commit installpre-commit runBased on upstream PR -> #1091
Why I did it
sw.defaultVlan(thrift:SwitchConfig.defaultVlan) is alwaysinitialized to 4094 in the base agent config and was not previously
configurable via CLI. This command exposes it so operators can redirect
untagged traffic to a different VLAN at bring-up without hand-editing
the JSON config.
How I did it
New command:
fboss2-dev config vlan default <vlan-id>, registeredin
CmdListConfig.cppas a sibling of theconfig vlan <id>subtree(separate node so the integer arg resolves at the right depth).
Implementation (
CmdConfigVlanDefault.cpp): The command implementsthe following logic:
sw.defaultVlanalready equals the target,return immediately with a clear message.
ingressVlanfor any port but has no matchingcfg::Interfaceentry,refuse with a descriptive error (this is the config state that causes
fboss_sw_agentto crash on startup).sw.vlansby ID. If the target VLANalready exists, reuse it as-is. If not, auto-create a non-routable
placeholder:
cfg::Vlan{id=vlanId, name="default_<id>", routable=false}.port still uses it as
ingressVlan, remove it from the VLAN table.sw.defaultVlan\to the new ID and callsession.saveConfig().New files:
fboss/cli/fboss2/commands/config/vlan/CmdConfigVlanDefault.h/.cpp— command implementation
fboss/cli/fboss2/test/config/CmdConfigVlanDefaultTest.cpp— unittests
fboss/cli/fboss2/test/integration_test/ConfigVlanDefaultTest.cpp—integration tests
cmake/CliFboss2.cmake,cmake/CliFboss2TestConfig.cmake,cmake/CliFboss2TestIntegrationTest.cmake,fboss/cli/fboss2/BUCK,fboss/cli/fboss2/test/config/BUCK,fboss/cli/fboss2/test/integration_test/BUCKTest Plan
Unit tests (
CmdConfigVlanDefaultTest) cover the core logic paths:setDefaultVlanAlreadySet— no-op when target equals current defaultsetDefaultVlanSuccess— happy path: creates new target VLAN, updatesdefaultVlansetDefaultVlanUpdatesOnlyOneEntry— no duplicate VLAN entries createdchangeDefaultVlanMultipleTimes— sequential changes accumulatecorrectly
createsNewVlanWhenNeitherExists— auto-creates target when VLAN tablelacks it
reusesExistingTargetVlan— no duplication when target VLAN alreadypresent
succeedsWithNoDefaultVlanField— handles Thrift zero-default whendefaultVlanabsent from configIntegration tests (
ConfigVlanDefaultTest) run on fboss-sim:SetDefaultVlanTo300— moves eth ports off old default, sets newdefault, commits, verifies
sw.defaultVlanviagetRunningConfig()thrift, restores original state
NoOpWhenDefaultVlanUnchanged— idempotency: re-setting currentdefault returns a clear message with no config change
RefuseWhenPortOnDefaultVlanWithNoInterface— validation guard:refuses change when old default VLAN is used as
ingressVlanbut hasno interface (skipped if precondition not met in running config)
ChangeDefaultVlanWithPortInNonDefaultVlan— regression: command doesnot crash when ports have already been moved off the current default
VLAN