Skip to content

[Celestica] Ladakh800bcls: Agent Test: A VLAN left with no ports must be dropped; otherwise it diverges between cold boot and warmboot. #1520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cel-gl wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Celestica] Ladakh800bcls: Agent Test: A VLAN left with no ports must be dropped; otherwise it diverges between cold boot and warmboot. #1520

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 66 additions & 1 deletion fboss/agent/test/TrunkUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,64 @@ cfg::AggregatePortMember makePortMember(int32_t port, cfg::LacpPortRate rate) {
return aggMember;
}

namespace {
// A VLAN left with no ports must be dropped; otherwise it diverges between cold
// boot and warmboot.
void removeUnreferencedVlans(
cfg::SwitchConfig* config,
const std::set<int32_t>& memberOldVlans,
int32_t aggVlan) {
// TODO:remove this once cisco fixes the LAG vlan removal issue
bool isCiscoAsic = false;
for (const auto& [_, switchInfo] :
*config->switchSettings()->switchIdToSwitchInfo()) {
auto asicType = *switchInfo.asicType();
if (asicType == cfg::AsicType::ASIC_TYPE_EBRO ||
asicType == cfg::AsicType::ASIC_TYPE_YUBA ||
asicType == cfg::AsicType::ASIC_TYPE_P200 ||
asicType == cfg::AsicType::ASIC_TYPE_GARONNE ||
asicType == cfg::AsicType::ASIC_TYPE_G202X) {

@jchallag jchallag Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isCiscoAsic check only matches ASIC_TYPE_EBRO, ASIC_TYPE_YUBA, and ASIC_TYPE_G202X, but per the comment the intent is to skip cleanup for all Cisco/Tajo chips. ASIC_TYPE_P200 (which inherits from TajoAsic) and ASIC_TYPE_GARONNE are also Cisco chips and are grouped together with the ones listed here in similar codebase checks (e.g., fboss/agent/mnpu/MultiSwitchHwSwitchHandler.cpp groups EBRO+P200+YUBA+G202X).

@cel-gl cel-gl Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I will add ASIC_TYPE_P200 and ASIC_TYPE_GARONNE

@cel-gl cel-gl Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

isCiscoAsic = true;
break;
}
}
if (isCiscoAsic) {
return;
}

std::set<int32_t> stillReferenced;
for (const auto& vlanPort : *config->vlanPorts()) {
stillReferenced.insert(vlanPort.vlanID().value());
}

for (auto oldVlan : memberOldVlans) {
if (oldVlan == aggVlan || stillReferenced.contains(oldVlan) ||
(oldVlan == *config->defaultVlan())) {
continue;
}

auto& vlans = *config->vlans();
vlans.erase(
std::remove_if(
vlans.begin(),
vlans.end(),
[&](const auto& v) { return v.id().value() == oldVlan; }),
vlans.end());

auto& intfs = *config->interfaces();
intfs.erase(
std::remove_if(
intfs.begin(),
intfs.end(),
[&](const auto& i) {
return i.type().value() == cfg::InterfaceType::VLAN &&
i.vlanID().value() == oldVlan;
}),
intfs.end());
}
}
} // namespace

void addAggPort(
int key,
const std::vector<int32_t>& ports,
Expand Down Expand Up @@ -54,15 +112,22 @@ void addAggPort(
// Set VLAN for all members to be the same
std::set<uint32_t> memberPorts(ports.begin(), ports.end());
std::optional<int32_t> aggVlan;
std::set<int32_t> memberOldVlans;
for (auto& vlanPort : *config->vlanPorts()) {
if (memberPorts.contains(vlanPort.logicalPort().value())) {
int32_t oldVlan = vlanPort.vlanID().value();
memberOldVlans.insert(oldVlan);
if (!aggVlan) {
aggVlan = vlanPort.vlanID().value();
aggVlan = oldVlan;
}
vlanPort.vlanID() = *aggVlan;
}
}

if (aggVlan.has_value()) {
removeUnreferencedVlans(config, memberOldVlans, *aggVlan);
}

if (aggregatePortType == cfg::AggregatePortType::LAG_PORT) {
// Set ingress VLAN for all members to be the same
for (auto& port : *config->ports()) {
Expand Down
Loading