Skip to content

[neighorch]: Track remote next hops on inband RIF #4769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Xichen96 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[neighorch]: Track remote next hops on inband RIF #4769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
707997b
[neighorch]: Track remote next hops on inband RIF
Xichen96 Jul 20, 2026
4fa09fc
[neighorch]: Clarify remote next-hop port semantics
Xichen96 Jul 20, 2026
df387c7
[neighorch]: Update remote next-hop RIF expectation
Xichen96 Jul 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions orchagent/neighorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<sai_attribute_t> next_hop_attrs;

Expand Down Expand Up @@ -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())
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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())
{
Expand Down Expand Up @@ -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);
Comment thread
Xichen96 marked this conversation as resolved.
return true;
}

Expand Down
84 changes: 84 additions & 0 deletions tests/mock_tests/neighorch_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Orch *>(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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tests/test_virtual_chassis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading