From 707997b91287c012f809c8659b49a7bff78aad7f Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 20 Jul 2026 16:29:26 +0000 Subject: [PATCH 1/3] [neighorch]: Track remote next hops on inband RIF Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079 Signed-off-by: Xichen96 --- orchagent/neighorch.cpp | 17 +++++-- tests/mock_tests/neighorch_ut.cpp | 73 +++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 7ee60a95803..35335c6ca11 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()) { @@ -784,7 +793,7 @@ bool NeighOrch::removeNextHop(const IpAddress &ipAddress, const string &alias) } 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 dc08749c370..fceb38d8229 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,65 @@ 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); + + 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_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_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); From 4fa09fc8310d0fc043a63f889f6d947a31774c07 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 20 Jul 2026 17:07:15 +0000 Subject: [PATCH 2/3] [neighorch]: Clarify remote next-hop port semantics Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079 Signed-off-by: Xichen96 --- orchagent/neighorch.cpp | 4 ++-- tests/mock_tests/neighorch_ut.cpp | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 35335c6ca11..0294e9cfeda 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -787,8 +787,8 @@ 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; } diff --git a/tests/mock_tests/neighorch_ut.cpp b/tests/mock_tests/neighorch_ut.cpp index fceb38d8229..a925b1d4640 100644 --- a/tests/mock_tests/neighorch_ut.cpp +++ b/tests/mock_tests/neighorch_ut.cpp @@ -219,6 +219,15 @@ namespace neighorch_test { 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; @@ -245,6 +254,7 @@ namespace neighorch_test 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); @@ -255,6 +265,7 @@ namespace neighorch_test 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); From df387c75cabd5ec325682a1304d1b6ac74882a1a Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Wed, 22 Jul 2026 03:19:33 +1000 Subject: [PATCH 3/3] [neighorch]: Update remote next-hop RIF expectation Remote system-port next hops remain on the global inband RIF when their neighbor moves between remote ports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079 Signed-off-by: Xichen96 --- tests/test_virtual_chassis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_virtual_chassis.py b/tests/test_virtual_chassis.py index 0a6f4bdbf8b..bb4fba0e108 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)