From f7fd7bd54b407fbde0ae6eaceab92a3af5e97e28 Mon Sep 17 00:00:00 2001 From: Heshi Wu Date: Mon, 17 Aug 2026 02:02:28 +0000 Subject: [PATCH] Fix TrunkUtils.cpp A VLAN left with no ports must be dropped; otherwise it diverges between cold boot and warmboot --- fboss/agent/test/TrunkUtils.cpp | 67 ++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/fboss/agent/test/TrunkUtils.cpp b/fboss/agent/test/TrunkUtils.cpp index 0d2524d6a0811..e71edc78b1a87 100644 --- a/fboss/agent/test/TrunkUtils.cpp +++ b/fboss/agent/test/TrunkUtils.cpp @@ -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& 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) { + isCiscoAsic = true; + break; + } + } + if (isCiscoAsic) { + return; + } + + std::set 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& ports, @@ -54,15 +112,22 @@ void addAggPort( // Set VLAN for all members to be the same std::set memberPorts(ports.begin(), ports.end()); std::optional aggVlan; + std::set 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()) {