[Celestica] Ladakh800bcls: Agent Test: A VLAN left with no ports must be dropped; otherwise it diverges between cold boot and warmboot. - #1520
Conversation
cd3b824 to
ccc06bb
Compare
|
two optimizations.1: If the vlan is the default vlan, it will not be deleted. 2: If it is a Cisco chip, the deletion operation will be skipped.(We don't have Cisco fboss equipment, so we couldn't verify this logic; we simply added the code, please test. and when I do fboss/skills/fboss-code-standards/SKILL.md check This modification violates this rule.Agent No hardcoded ASIC types Use feature/property lookups, not ASIC name checks， since it's asic diff, I ignore it) |
|
@mikechoifb has imported this pull request. If you are a Meta employee, you can view this in D116804477. |
| auto asicType = *switchInfo.asicType(); | ||
| if (asicType == cfg::AsicType::ASIC_TYPE_EBRO || | ||
| asicType == cfg::AsicType::ASIC_TYPE_YUBA || | ||
| asicType == cfg::AsicType::ASIC_TYPE_G202X) { |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
ok, I will add ASIC_TYPE_P200 and ASIC_TYPE_GARONNE
|
@cel-gl there is no UT coverage for these changes. Is it possible to add some tests? |
AgentTrunkLoadBalancerTests tests can cover "A VLAN left with no ports must be dropped; otherwise it diverges between cold boot and warmboot." before change it test failed, after changed it can pass, or are there any other modifications or tests that require attention? |
ccc06bb to
8b8ee74
Compare
|
@cel-gl has updated the pull request. You must reimport the pull request before landing. |
Looks good @cel-gl . |
|
LGTM |
| cfg::SwitchConfig* config, | ||
| const std::set<int32_t>& memberOldVlans, | ||
| int32_t aggVlan) { | ||
| // Check ASIC type. Do not perform cleanup for Cisco chips. |
There was a problem hiding this comment.
add a FIXME or TODO to remove this once cisco fixes the LAG vlan removal issue
…se it diverges between cold boot and warmboot
8b8ee74 to
f7fd7bd
Compare
|
@cel-gl has updated the pull request. You must reimport the pull request before landing. |
4 participants
Pre-submission checklist
pip install -r requirements-dev.txt && pre-commit installpre-commit runclang-format.............................................................Passed
shellcheck...........................................(no files to check)Skipped
shfmt................................................(no files to check)Skipped
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check json...........................................(no files to check)Skipped
check for merge conflicts................................................Passed
ruff check...........................................(no files to check)Skipped
ruff format..........................................(no files to check)Skipped
Prevent sai_impl in fboss manifest.......................................Passed
Summary
Issue Summary
On multi-NPU hardware platforms, the AgentTrunkLoadBalancerTest suite was experiencing a fatal warmboot failure, The failure manifested as a no sai vlan for VlanID 2001 error
Root Cause Analysis
after debugging ,
1: I found in AgentTrunkLoadBalancerTests config call onePortPerInterfaceConfig
2: then addAggPort 3 ports, before that logical port 1(vlan2000) 2(2001) 3(2002), then changed to
port 1(vlan2000) 2(2000) 3(2000), vlan/vlaninterface 2001 2002 empty member ports
3: then new config goto ApplyThriftConfig.cpp new_->resetVlans(
toMultiSwitchMap(newVlans, scopeResolver_));
const HwSwitchMatcher SwitchIdScopeResolver::scope(
const std::shared_ptr& vlan)
if (vlan->getPortsInfo().empty())
// Return the first switchId.
// TODO: Remove this after scope resolution is updated to return single
// switchId based on virtual interface and switchId configuration.
return HwSwitchMatcher(
std::unordered_set(
{*allSwitchMatcher().switchIds().begin()}));
debug log : SwitchIdScopeResolver.cpp:253] willtest Vlan 2001 has no ports, scoping to first switch ID: 1
4: since vlan 2001 scope to switch id 1, when thrift config send to agent , delta will remove vlan 2001,
5: after cold boot test, then enter warmboot restoring the environment , in high level config there are vlan 2001, but in agent sai , there is no vlan sai 2001 handler, so this case fail on npu0 warmboot,
why this case issue doesn't happen on other cases which call addAggPort , Because several conditions need to be met. (onePortPerInterfaceConfig ,addAggPort more then one ports) trunk/LAG/SRv6/MPLS/MAC/copp tests they do not.
Solution Implemented
In void addAggPort(
When adding a port to agg, check which vlan it is previously part of another vlan and if that vlan does not have any remaining ports, then delete the vlan from the config if vlan==defaultvlan or asic type cisco skip delete process
Test Plan
cold_boot.AgentMacLearningAndNeighborResolutionTest/2.learnMacLinkDownNeighborResolve,PASSED
warm_boot.AgentMacLearningAndNeighborResolutionTest/2.learnMacLinkDownNeighborResolve,PASSED
cold_boot.AgentMacLearningAndNeighborResolutionTest/3.learnMacAndProgramNeighbors,PASSED
warm_boot.AgentMacLearningAndNeighborResolutionTest/3.learnMacAndProgramNeighbors,PASSED
cold_boot.AgentMacLearningAndNeighborResolutionTest/3.learnMacProgramNeighborsAndAgeMac,PASSED
warm_boot.AgentMacLearningAndNeighborResolutionTest/3.learnMacProgramNeighborsAndAgeMac,PASSED
cold_boot.AgentMacLearningAndNeighborResolutionTest/3.learnMacProgramNeighborsAndUpdateMac,PASSED
warm_boot.AgentMacLearningAndNeighborResolutionTest/3.learnMacProgramNeighborsAndUpdateMac,PASSED
cold_boot.AgentMacLearningAndNeighborResolutionTest/3.flapMacAndNeighbors,PASSED
warm_boot.AgentMacLearningAndNeighborResolutionTest/3.flapMacAndNeighbors,PASSED
cold_boot.AgentMacLearningAndNeighborResolutionTest/3.learnMacProgramNeighborsAndMove,PASSED
warm_boot.AgentMacLearningAndNeighborResolutionTest/3.learnMacProgramNeighborsAndMove,PASSED
cold_boot.AgentMacLearningAndNeighborResolutionTest/3.learnMacLinkDownNeighborResolve,PASSED
warm_boot.AgentMacLearningAndNeighborResolutionTest/3.learnMacLinkDownNeighborResolve,PASSED
cold_boot.AgentTrunkLoadBalancerTest.ECMPFullTrunkHalfHash4X3WideTrunksCpuTraffic,PASSED
warm_boot.AgentTrunkLoadBalancerTest.ECMPFullTrunkHalfHash4X3WideTrunksCpuTraffic,PASSED
cold_boot.AgentTrunkLoadBalancerTest.ECMPFullTrunkHalfHash4X2WideTrunksCpuTraffic,PASSED
warm_boot.AgentTrunkLoadBalancerTest.ECMPFullTrunkHalfHash4X2WideTrunksCpuTraffic,PASSED
cold_boot.AgentTrunkLoadBalancerTest.ECMPFullTrunkHalf4X3WideTrunksFrontPanelTraffic,PASSED
warm_boot.AgentTrunkLoadBalancerTest.ECMPFullTrunkHalf4X3WideTrunksFrontPanelTraffic,PASSED
cold_boot.AgentTrunkLoadBalancerTest.ECMPFullTrunkHalf4X2WideTrunksFrontPanelTraffic,PASSED
warm_boot.AgentTrunkLoadBalancerTest.ECMPFullTrunkHalf4X2WideTrunksFrontPanelTraffic,PASSED
cold_boot.AgentTrunkLoadBalancerTest.ECMPHalfTrunkFullHash4X3WideTrunksCpuTraffic,PASSED
warm_boot.AgentTrunkLoadBalancerTest.ECMPHalfTrunkFullHash4X3WideTrunksCpuTraffic,PASSED
cold_boot.AgentTrunkLoadBalancerTest.ECMPHalfTrunkFullHash4X2WideTrunksCpuTraffic,PASSED
warm_boot.AgentTrunkLoadBalancerTest.ECMPHalfTrunkFullHash4X2WideTrunksCpuTraffic,PASSED
cold_boot.AgentTrunkLoadBalancerTest.ECMPHalfTrunkFull4X3WideTrunksFrontPanelTraffic,PASSED
warm_boot.AgentTrunkLoadBalancerTest.ECMPHalfTrunkFull4X3WideTrunksFrontPanelTraffic,PASSED
cold_boot.AgentTrunkLoadBalancerTest.ECMPHalfTrunkFull4X2WideTrunksFrontPanelTraffic,PASSED
warm_boot.AgentTrunkLoadBalancerTest.ECMPHalfTrunkFull4X2WideTrunksFrontPanelTraffic,PASSED
cold_boot.AgentEcmpSpilloverTest.VerifyEcmpSpillover,TIMEOUT
cold_boot.AgentEcmpSpilloverTest.VerifyEcmpDecompress,PASSED
warm_boot.AgentEcmpSpilloverTest.VerifyEcmpDecompress,PASSED
cold_boot.AgentEcmpSpilloverTest.VerifyEcmpSpilloverForcedRollback,PASSED
warm_boot.AgentEcmpSpilloverTest.VerifyEcmpSpilloverForcedRollback,PASSED
cold_boot.AgentEcmpSpilloverTest.VerifyEcmpSpilloverForcedRollbackPartial,PASSED
warm_boot.AgentEcmpSpilloverTest.VerifyEcmpSpilloverForcedRollbackPartial,PASSED