diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 7ee60a9580..0294e9cfed 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -379,7 +379,7 @@ bool NeighOrch::addNextHop(NeighborContext& ctx) } assert(!hasNextHop(nexthop)); - sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(nh.alias); + sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(nexthop.alias); vector next_hop_attrs; @@ -457,7 +457,7 @@ bool NeighOrch::addNextHop(NeighborContext& ctx) next_hop_entry.nh_flags = 0; m_syncdNextHops[nexthop] = next_hop_entry; - m_intfsOrch->increaseRouterIntfsRefCount(nh.alias); + m_intfsOrch->increaseRouterIntfsRefCount(nexthop.alias); if (nexthop.isMplsNextHop()) { @@ -516,6 +516,15 @@ bool NeighOrch::processBulkAddNextHop(NeighborContext& ctx) } NextHopKey nexthop(nh); + if (m_intfsOrch->isRemoteSystemPortIntf(nh.alias)) + { + Port inbp; + gPortsOrch->getInbandPort(inbp); + assert(inbp.m_alias.length()); + + nexthop.alias = inbp.m_alias; + } + if (ctx.next_hop_id == SAI_NULL_OBJECT_ID) { sai_status_t bulker_status = gNextHopBulker.create_status(ctx.next_hop_id); @@ -549,7 +558,7 @@ bool NeighOrch::processBulkAddNextHop(NeighborContext& ctx) next_hop_entry.nh_flags = 0; m_syncdNextHops[nexthop] = next_hop_entry; - m_intfsOrch->increaseRouterIntfsRefCount(nh.alias); + m_intfsOrch->increaseRouterIntfsRefCount(nexthop.alias); if (nexthop.isMplsNextHop()) { @@ -778,13 +787,13 @@ bool NeighOrch::removeNextHop(const IpAddress &ipAddress, const string &alias) if (m_syncdNextHops[nexthop].ref_count > 0) { - SWSS_LOG_ERROR("Failed to remove still referenced next hop %s on %s", - ipAddress.to_string().c_str(), alias.c_str()); + SWSS_LOG_ERROR("Failed to remove still referenced next hop %s requested on %s, tracked on %s", + ipAddress.to_string().c_str(), alias.c_str(), nexthop.alias.c_str()); return false; } m_syncdNextHops.erase(nexthop); - m_intfsOrch->decreaseRouterIntfsRefCount(alias); + m_intfsOrch->decreaseRouterIntfsRefCount(nexthop.alias); return true; } diff --git a/tests/mock_tests/neighorch_ut.cpp b/tests/mock_tests/neighorch_ut.cpp index dc08749c37..a925b1d464 100644 --- a/tests/mock_tests/neighorch_ut.cpp +++ b/tests/mock_tests/neighorch_ut.cpp @@ -17,7 +17,9 @@ EXTERN_MOCK_FNS namespace neighorch_test { DEFINE_SAI_API_MOCK(neighbor); + DEFINE_SAI_GENERIC_API_MOCK(next_hop, next_hop); using namespace std; + using ::testing::Invoke; using namespace mock_orch_test; using ::testing::Return; using ::testing::Throw; @@ -49,6 +51,27 @@ namespace neighorch_test neigh_table.del(key); } + void ConfigureRemoteSystemPort(const string& remote_alias, const string& inband_alias) + { + Table intf_table = Table(m_app_db.get(), APP_INTF_TABLE_NAME); + intf_table.set(remote_alias, {{"NULL", "NULL"}}); + intf_table.set(inband_alias, {{"NULL", "NULL"}}); + gIntfsOrch->addExistingData(&intf_table); + static_cast(gIntfsOrch)->doTask(); + + Port remote_port; + ASSERT_TRUE(gPortsOrch->getPort(remote_alias, remote_port)); + remote_port.m_system_port_info.type = SAI_SYSTEM_PORT_TYPE_REMOTE; + remote_port.m_oper_status = SAI_PORT_OPER_STATUS_UP; + gPortsOrch->setPort(remote_alias, remote_port); + + Port inband_port; + ASSERT_TRUE(gPortsOrch->getPort(inband_alias, inband_port)); + inband_port.m_oper_status = SAI_PORT_OPER_STATUS_UP; + gPortsOrch->setPort(inband_alias, inband_port); + gPortsOrch->m_inbandPortName = inband_alias; + } + void ApplyInitialConfigs() { Table port_table = Table(m_app_db.get(), APP_PORT_TABLE_NAME); @@ -180,15 +203,76 @@ namespace neighorch_test void PostSetUp() override { INIT_SAI_API_MOCK(neighbor); + INIT_SAI_API_MOCK(next_hop); MockSaiApis(); } void PreTearDown() override { RestoreSaiApis(); + DEINIT_SAI_API_MOCK(next_hop); + DEINIT_SAI_API_MOCK(neighbor); } }; + TEST_F(NeighOrchTest, RemoteSystemPortNextHopUsesInbandRif) + { + ConfigureRemoteSystemPort(ETHERNET0, ETHERNET4); + + Port remote_port; + ASSERT_TRUE(gPortsOrch->getPort(ETHERNET0, remote_port)); + remote_port.m_oper_status = SAI_PORT_OPER_STATUS_DOWN; + gPortsOrch->setPort(ETHERNET0, remote_port); + + Port inband_port; + ASSERT_TRUE(gPortsOrch->getPort(ETHERNET4, inband_port)); + ASSERT_EQ(inband_port.m_oper_status, SAI_PORT_OPER_STATUS_UP); + + auto inband_rif = gIntfsOrch->getRouterIntfsId(ETHERNET4); + ASSERT_NE(inband_rif, SAI_NULL_OBJECT_ID); + auto remote_ref_count = gIntfsOrch->getSyncdIntfses().at(ETHERNET0).ref_count; + auto inband_ref_count = gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count; + NextHopKey inband_nexthop(TEST_IP, ETHERNET4); + + bool saw_inband_rif = false; + EXPECT_CALL(*mock_sai_next_hop_api, create_next_hop) + .WillOnce(Invoke([&](sai_object_id_t *next_hop_id, sai_object_id_t, uint32_t attr_count, + const sai_attribute_t *attr_list) { + *next_hop_id = 0x200000; + for (uint32_t i = 0; i < attr_count; ++i) + { + if (attr_list[i].id == SAI_NEXT_HOP_ATTR_ROUTER_INTERFACE_ID) + { + EXPECT_EQ(attr_list[i].value.oid, inband_rif); + saw_inband_rif = true; + } + } + return SAI_STATUS_SUCCESS; + })); + + NeighborContext ctx(NeighborEntry(TEST_IP, ETHERNET0)); + ASSERT_TRUE(gNeighOrch->addNextHop(ctx)); + ASSERT_TRUE(saw_inband_rif); + ASSERT_EQ(gNeighOrch->m_syncdNextHops.count(inband_nexthop), 1u); + ASSERT_TRUE(gNeighOrch->isNextHopFlagSet(inband_nexthop, NHFLAGS_IFDOWN)); + ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET0).ref_count, remote_ref_count); + ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count, inband_ref_count + 1); + + ASSERT_TRUE(gNeighOrch->removeNextHop(IpAddress(TEST_IP), ETHERNET0)); + ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count, inband_ref_count); + + NeighborContext bulk_ctx(NeighborEntry(TEST_IP, ETHERNET0), true); + bulk_ctx.next_hop_id = 0x200001; + ASSERT_TRUE(gNeighOrch->processBulkAddNextHop(bulk_ctx)); + ASSERT_EQ(gNeighOrch->m_syncdNextHops.count(inband_nexthop), 1u); + ASSERT_TRUE(gNeighOrch->isNextHopFlagSet(inband_nexthop, NHFLAGS_IFDOWN)); + ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET0).ref_count, remote_ref_count); + ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count, inband_ref_count + 1); + + ASSERT_TRUE(gNeighOrch->removeNextHop(IpAddress(TEST_IP), ETHERNET0)); + ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count, inband_ref_count); + } + TEST_F(NeighOrchTest, MultiVlanDuplicateNeighbor) { EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry); diff --git a/tests/test_virtual_chassis.py b/tests/test_virtual_chassis.py index 0a6f4bdbf8..bb4fba0e10 100644 --- a/tests/test_virtual_chassis.py +++ b/tests/test_virtual_chassis.py @@ -1370,7 +1370,7 @@ def test_remote_neighbor_add(self, vct): nexthop_entry = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP", nexthop_keys[0]) print("3:nexthop_entrty:",nexthop_entry) rif3 = nexthop_entry.get("SAI_NEXT_HOP_ATTR_ROUTER_INTERFACE_ID") - assert rif1 != rif3, "Neighbor is not replaced with new rif" + assert rif1 == rif3, "Remote neighbor next hop moved off the inband rif" #del the neighbor self.configure_neighbor(local_lc_dvs, "del", test_neigh_ip_1, test_neigh_mac_1, test_neigh_dev_2)