diff --git a/fboss/agent/ApplyThriftConfig.cpp b/fboss/agent/ApplyThriftConfig.cpp index 0799ab4d94420..8e8569b79972a 100644 --- a/fboss/agent/ApplyThriftConfig.cpp +++ b/fboss/agent/ApplyThriftConfig.cpp @@ -1271,6 +1271,12 @@ void ThriftConfigApplier::processReachabilityGroup( auto newPortMap = new_->getPorts()->modify(&new_); for (const auto& fabricSwitchId : localFabricSwitchIds) { std::unordered_map destinationId2ReachabilityGroup; + + // For reachability groups with parallel links, use index 1-128. + // Else use index for single link groups in range 129-256 + int nextParallelLinkGroup = 1; + int nextSingleLinkGroup = 129; + for (const auto& portCfg : *cfg_->ports()) { if (scopeResolver_.scope(portCfg).has(SwitchID(fabricSwitchId)) && portCfg.expectedNeighborReachability()->size() > 0) { @@ -1280,26 +1286,24 @@ void ThriftConfigApplier::processReachabilityGroup( new_->getDsfNodes()->getNode(neighborRemoteSwitchId); int destinationId; - // For parallel VOQ links, assign links reaching the same VOQ - // switch. - if (parallelVoqLinks && - neighborDsfNode->getType() == cfg::DsfNodeType::INTERFACE_NODE) { + if (neighborDsfNode->getType() == cfg::DsfNodeType::INTERFACE_NODE) { destinationId = neighborRemoteSwitchId; - } else if (neighborDsfNode->getClusterId() == std::nullopt) { - // Assign links based on cluster ID. Note that in dual stage, - // FE2 has no cluster ID: use -1 for grouping purpose. - CHECK_EQ( - static_cast(cfg::DsfNodeType::FABRIC_NODE), - static_cast(neighborDsfNode->getType())); - destinationId = -1; } else { - destinationId = neighborDsfNode->getClusterId().value(); + // Fabric node links: assign links based on cluster ID. Note that in + // dual stage, FE2 has no cluster ID: use -1 for grouping purpose. + destinationId = neighborDsfNode->getClusterId().value_or(-1); } + bool singleLinkGroup = + neighborDsfNode->getType() == cfg::DsfNodeType::INTERFACE_NODE && + !parallelVoqLinks; + int& nextGroupId = + singleLinkGroup ? nextSingleLinkGroup : nextParallelLinkGroup; auto [it, inserted] = destinationId2ReachabilityGroup.insert( - {destinationId, destinationId2ReachabilityGroup.size() + 1}); + {destinationId, nextGroupId}); auto reachabilityGroupId = it->second; if (inserted) { + nextGroupId++; XLOG(DBG2) << "Create new reachability group " << reachabilityGroupId << " towards node " << neighborDsfNode->getName() << " with switchId " diff --git a/fboss/agent/test/ReachabilityGroupTests.cpp b/fboss/agent/test/ReachabilityGroupTests.cpp index 14813a551a960..81008600a1f82 100644 --- a/fboss/agent/test/ReachabilityGroupTests.cpp +++ b/fboss/agent/test/ReachabilityGroupTests.cpp @@ -161,8 +161,11 @@ TEST_F(ReachabilityGroupSingleStageFdswNoParallelIntfLinkTest, test) { TEST_F(ReachabilityGroupDualStageFdswNoParallelIntfLinkTest, test) { // Dual stage FDSW with no parallel links to interface node. // One group for uplinks to SDSW (1) and one group for downlink to RDSW (2). - verifyReachabilityGroupListSize(2); + // However, SDK requires one group per RDSW downlink, hence we have 1 group + // for uplinks to SDSW and 18 groups to RDSW. + verifyReachabilityGroupListSize(19); + int expectedInterfaceNodeReachabilityGroup = 129; for (const auto& [_, portMap] : std::as_const(*sw_->getState()->getPorts())) { for (const auto& [_, port] : std::as_const(*portMap)) { auto neighborDsfNodeType = @@ -171,7 +174,9 @@ TEST_F(ReachabilityGroupDualStageFdswNoParallelIntfLinkTest, test) { EXPECT_TRUE(port->getReachabilityGroupId().has_value()); EXPECT_EQ( port->getReachabilityGroupId().value(), - neighborDsfNodeType == cfg::DsfNodeType::FABRIC_NODE ? 1 : 2); + neighborDsfNodeType == cfg::DsfNodeType::FABRIC_NODE + ? 1 + : expectedInterfaceNodeReachabilityGroup++); } } } @@ -229,7 +234,9 @@ TEST_F( configUpdateToParallelIntfLinks) { // Dual stage FDSW with no parallel links to interface node. // One group for uplinks to SDSW (1) and one group for downlink to RDSW (2). - verifyReachabilityGroupListSize(2); + // However, SDK requires one group per RDSW downlink, hence we have 1 group + // for uplinks to SDSW and 18 groups to RDSW. + verifyReachabilityGroupListSize(19); const auto newConfig = testConfigFabricSwitch( true /* dualStage*/,