Skip to content
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

Jump to bottom

[202311] Handle learning duplicate IPs on different VRFs (#3165) #3179

Merged
merged 1 commit into from
Jun 3, 2024
Merged

[202311] Handle learning duplicate IPs on different VRFs (#3165) #3179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 27 additions & 3 deletions orchagent/neighorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -945,12 +945,36 @@ bool NeighOrch::addNeighbor(const NeighborEntry &neighborEntry, const MacAddress
NeighborEntry temp_entry = { ip_address, vlan_port };
if (m_syncdNeighbors.find(temp_entry) != m_syncdNeighbors.end())
{
SWSS_LOG_NOTICE("Neighbor %s on %s already exists, removing before adding new neighbor", ip_address.to_string().c_str(), vlan_port.c_str());
if (!removeNeighbor(temp_entry))
// Neighbor already exists on another VLAN. If they belong to the same VRF, delete the old neighbor
Port existing_vlan, new_vlan;
if (!gPortsOrch->getPort(vlan_port, new_vlan))
{
SWSS_LOG_ERROR("Failed to remove neighbor %s on %s", ip_address.to_string().c_str(), vlan_port.c_str());
SWSS_LOG_ERROR("Failed to get port for %s", vlan_port.c_str());
return false;
}
if (!gPortsOrch->getPort(alias, existing_vlan))
{
SWSS_LOG_ERROR("Failed to get port for %s", alias.c_str());
return false;
}
if (existing_vlan.m_vr_id == new_vlan.m_vr_id)
{
std::string vrf_name = gDirectory.get<VRFOrch*>()->getVRFname(existing_vlan.m_vr_id);
if (vrf_name.empty())
{
SWSS_LOG_NOTICE("Neighbor %s already learned on %s, removing before adding new neighbor", ip_address.to_string().c_str(), vlan_port.c_str());
}
else
{
SWSS_LOG_NOTICE("Neighbor %s already learned on %s in VRF %s, removing before adding new neighbor", ip_address.to_string().c_str(), vlan_port.c_str(), vrf_name.c_str());
}

if (!removeNeighbor(temp_entry))
{
SWSS_LOG_ERROR("Failed to remove neighbor %s on %s", ip_address.to_string().c_str(), vlan_port.c_str());
return false;
}
}
}
}

Expand Down
8 changes: 8 additions & 0 deletions tests/mock_tests/mock_orch_test.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,24 @@ namespace mock_orch_test
static const string PEER_IPV4_ADDRESS = "1.1.1.1";
static const string ACTIVE_INTERFACE = "Ethernet4";
static const string STANDBY_INTERFACE = "Ethernet8";
static const string ETHERNET0 = "Ethernet0";
static const string ETHERNET4 = "Ethernet4";
static const string ETHERNET8 = "Ethernet8";
static const string ETHERNET12 = "Ethernet12";
static const string ACTIVE_STATE = "active";
static const string STANDBY_STATE = "standby";
static const string STATE = "state";
static const string VLAN_1000 = "Vlan1000";
static const string VLAN_2000 = "Vlan2000";
static const string VLAN_3000 = "Vlan3000";
static const string VLAN_4000 = "Vlan4000";
static const string SERVER_IP1 = "192.168.0.2";
static const string SERVER_IP2 = "192.168.0.3";
static const string MAC1 = "62:f9:65:10:2f:01";
static const string MAC2 = "62:f9:65:10:2f:02";
static const string MAC3 = "62:f9:65:10:2f:03";
static const string MAC4 = "62:f9:65:10:2f:04";
static const string MAC5 = "62:f9:65:10:2f:05";

class MockOrchTest: public ::testing::Test
{
Expand Down
158 changes: 114 additions & 44 deletions tests/mock_tests/neighorch_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "mock_sai_api.h"
#include "mock_orch_test.h"


EXTERN_MOCK_FNS

namespace neighorch_test
Expand All @@ -21,15 +20,18 @@ namespace neighorch_test
using ::testing::Throw;

static const string TEST_IP = "10.10.10.10";
static const NeighborEntry VLAN1000_NEIGH = NeighborEntry(TEST_IP, VLAN_1000);
static const string VRF_3000 = "Vrf3000";
static const NeighborEntry VLAN1000_NEIGH = NeighborEntry(TEST_IP, VLAN_1000);
static const NeighborEntry VLAN2000_NEIGH = NeighborEntry(TEST_IP, VLAN_2000);
static const NeighborEntry VLAN3000_NEIGH = NeighborEntry(TEST_IP, VLAN_3000);
static const NeighborEntry VLAN4000_NEIGH = NeighborEntry(TEST_IP, VLAN_4000);

class NeighOrchTest: public MockOrchTest
class NeighOrchTest : public MockOrchTest
{
protected:
void SetAndAssertMuxState(std::string interface, std::string state)
{
MuxCable* muxCable = m_MuxOrch->getMuxCable(interface);
MuxCable *muxCable = m_MuxOrch->getMuxCable(interface);
muxCable->setState(state);
EXPECT_EQ(state, muxCable->getState());
}
Expand All @@ -46,34 +48,49 @@ namespace neighorch_test

void ApplyInitialConfigs()
{
Table peer_switch_table = Table(m_config_db.get(), CFG_PEER_SWITCH_TABLE_NAME);
Table tunnel_table = Table(m_app_db.get(), APP_TUNNEL_DECAP_TABLE_NAME);
Table mux_cable_table = Table(m_config_db.get(), CFG_MUX_CABLE_TABLE_NAME);
Table port_table = Table(m_app_db.get(), APP_PORT_TABLE_NAME);
Table vlan_table = Table(m_app_db.get(), APP_VLAN_TABLE_NAME);
Table vlan_member_table = Table(m_app_db.get(), APP_VLAN_MEMBER_TABLE_NAME);
Table neigh_table = Table(m_app_db.get(), APP_NEIGH_TABLE_NAME);
Table intf_table = Table(m_app_db.get(), APP_INTF_TABLE_NAME);
Table fdb_table = Table(m_app_db.get(), APP_FDB_TABLE_NAME);
Table vrf_table = Table(m_app_db.get(), APP_VRF_TABLE_NAME);

auto ports = ut_helper::getInitialSaiPorts();
port_table.set(ACTIVE_INTERFACE, ports[ACTIVE_INTERFACE]);
port_table.set(STANDBY_INTERFACE, ports[STANDBY_INTERFACE]);
port_table.set(ETHERNET0, ports[ETHERNET0]);
port_table.set(ETHERNET4, ports[ETHERNET4]);
port_table.set(ETHERNET8, ports[ETHERNET8]);
port_table.set("PortConfigDone", { { "count", to_string(1) } });
port_table.set("PortInitDone", { {} });

vrf_table.set(VRF_3000, { {"NULL", "NULL"} });

vlan_table.set(VLAN_1000, { { "admin_status", "up" },
{ "mtu", "9100" },
{ "mac", "00:aa:bb:cc:dd:ee" } });
vlan_table.set(VLAN_2000, { { "admin_status", "up"},
vlan_table.set(VLAN_2000, { { "admin_status", "up" },
{ "mtu", "9100" },
{ "mac", "aa:11:bb:22:cc:33" } });
vlan_table.set(VLAN_3000, { { "admin_status", "up" },
{ "mtu", "9100" },
{ "mac", "99:ff:88:ee:77:dd" } });
vlan_table.set(VLAN_4000, { { "admin_status", "up" },
{ "mtu", "9100" },
{ "mac", "99:ff:88:ee:77:dd" } });
vlan_member_table.set(
VLAN_1000 + vlan_member_table.getTableNameSeparator() + ETHERNET0,
{ { "tagging_mode", "untagged" } });

vlan_member_table.set(
VLAN_2000 + vlan_member_table.getTableNameSeparator() + ETHERNET4,
{ { "tagging_mode", "untagged" } });

vlan_member_table.set(
VLAN_1000 + vlan_member_table.getTableNameSeparator() + ACTIVE_INTERFACE,
VLAN_3000 + vlan_member_table.getTableNameSeparator() + ETHERNET8,
{ { "tagging_mode", "untagged" } });

vlan_member_table.set(
VLAN_2000 + vlan_member_table.getTableNameSeparator() + STANDBY_INTERFACE,
VLAN_4000 + vlan_member_table.getTableNameSeparator() + ETHERNET12,
{ { "tagging_mode", "untagged" } });

intf_table.set(VLAN_1000, { { "grat_arp", "enabled" },
Expand All @@ -84,6 +101,16 @@ namespace neighorch_test
{ "proxy_arp", "enabled" },
{ "mac_addr", "00:00:00:00:00:00" } });

intf_table.set(VLAN_3000, { { "grat_arp", "enabled" },
{ "proxy_arp", "enabled" },
{ "vrf_name", VRF_3000 },
{ "mac_addr", "00:00:00:00:00:00" } });

intf_table.set(VLAN_4000, { { "grat_arp", "enabled" },
{ "proxy_arp", "enabled" },
{ "vrf_name", VRF_3000 },
{ "mac_addr", "00:00:00:00:00:00" } });

intf_table.set(
VLAN_1000 + neigh_table.getTableNameSeparator() + "192.168.0.1/24", {
{ "scope", "global" },
Expand All @@ -95,60 +122,56 @@ namespace neighorch_test
{ "scope", "global" },
{ "family", "IPv4" },
});
tunnel_table.set(MUX_TUNNEL, { { "dscp_mode", "uniform" },
{ "dst_ip", "2.2.2.2" },
{ "ecn_mode", "copy_from_outer" },
{ "encap_ecn_mode", "standard" },
{ "ttl_mode", "pipe" },
{ "tunnel_type", "IPINIP" } });

peer_switch_table.set(PEER_SWITCH_HOSTNAME, { { "address_ipv4", PEER_IPV4_ADDRESS } });

mux_cable_table.set(ACTIVE_INTERFACE, { { "server_ipv4", SERVER_IP1 + "/32" },
{ "server_ipv6", "a::a/128" },
{ "state", "auto" } });
intf_table.set(
VLAN_3000 + neigh_table.getTableNameSeparator() + "192.168.3.1/24", {
{ "scope", "global" },
{ "family", "IPv4" },
});

mux_cable_table.set(STANDBY_INTERFACE, { { "server_ipv4", SERVER_IP2+ "/32" },
{ "server_ipv6", "a::b/128" },
{ "state", "auto" } });
intf_table.set(
VLAN_4000 + neigh_table.getTableNameSeparator() + "192.168.3.1/24", {
{ "scope", "global" },
{ "family", "IPv4" },
});

gPortsOrch->addExistingData(&port_table);
gPortsOrch->addExistingData(&vlan_table);
gPortsOrch->addExistingData(&vlan_member_table);
static_cast<Orch *>(gPortsOrch)->doTask();

gVrfOrch->addExistingData(&vrf_table);
static_cast<Orch *>(gVrfOrch)->doTask();

gIntfsOrch->addExistingData(&intf_table);
static_cast<Orch *>(gIntfsOrch)->doTask();

m_TunnelDecapOrch->addExistingData(&tunnel_table);
static_cast<Orch *>(m_TunnelDecapOrch)->doTask();

m_MuxOrch->addExistingData(&peer_switch_table);
static_cast<Orch *>(m_MuxOrch)->doTask();

m_MuxOrch->addExistingData(&mux_cable_table);
static_cast<Orch *>(m_MuxOrch)->doTask();

fdb_table.set(
VLAN_1000 + fdb_table.getTableNameSeparator() + MAC1,
{ { "type", "dynamic" },
{ "port", ACTIVE_INTERFACE } });
{ "port", ETHERNET0 } });

fdb_table.set(
VLAN_2000 + fdb_table.getTableNameSeparator() + MAC2,
{ { "type", "dynamic" },
{ "port", STANDBY_INTERFACE} });
{ "port", ETHERNET4 } });

fdb_table.set(
VLAN_1000 + fdb_table.getTableNameSeparator() + MAC3,
{ { "type", "dynamic" },
{ "port", ACTIVE_INTERFACE} });
{ "port", ETHERNET0 } });

fdb_table.set(
VLAN_3000 + fdb_table.getTableNameSeparator() + MAC4,
{ { "type", "dynamic" },
{ "port", ETHERNET8 } });

fdb_table.set(
VLAN_4000 + fdb_table.getTableNameSeparator() + MAC5,
{ { "type", "dynamic" },
{ "port", ETHERNET12 } });

gFdbOrch->addExistingData(&fdb_table);
static_cast<Orch *>(gFdbOrch)->doTask();

SetAndAssertMuxState(ACTIVE_INTERFACE, ACTIVE_STATE);
SetAndAssertMuxState(STANDBY_INTERFACE, STANDBY_STATE);
}

void PostSetUp() override
Expand All @@ -163,18 +186,19 @@ namespace neighorch_test
}
};

TEST_F(NeighOrchTest, MultiVlanIpLearning)
TEST_F(NeighOrchTest, MultiVlanDuplicateNeighbor)
{

EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry);
LearnNeighbor(VLAN_1000, TEST_IP, MAC1);
ASSERT_EQ(gNeighOrch->m_syncdNeighbors.count(VLAN1000_NEIGH), 1);

EXPECT_CALL(*mock_sai_neighbor_api, remove_neighbor_entry);
EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry);
LearnNeighbor(VLAN_2000, TEST_IP, MAC2);
ASSERT_EQ(gNeighOrch->m_syncdNeighbors.count(VLAN1000_NEIGH), 0);
ASSERT_EQ(gNeighOrch->m_syncdNeighbors.count(VLAN2000_NEIGH), 1);

EXPECT_CALL(*mock_sai_neighbor_api, remove_neighbor_entry);
EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry);
LearnNeighbor(VLAN_1000, TEST_IP, MAC3);
ASSERT_EQ(gNeighOrch->m_syncdNeighbors.count(VLAN1000_NEIGH), 1);
Expand All @@ -195,4 +219,50 @@ namespace neighorch_test
ASSERT_EQ(gNeighOrch->m_syncdNeighbors.count(VLAN1000_NEIGH), 1);
ASSERT_EQ(gNeighOrch->m_syncdNeighbors.count(VLAN2000_NEIGH), 0);
}

TEST_F(NeighOrchTest, MultiVlanDifferentVrfDuplicateNeighbor)
{
EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry);
LearnNeighbor(VLAN_1000, TEST_IP, MAC1);
ASSERT_EQ(gNeighOrch->m_syncdNeighbors.count(VLAN1000_NEIGH), 1);

EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry);
EXPECT_CALL(*mock_sai_neighbor_api, remove_neighbor_entry).Times(0);
LearnNeighbor(VLAN_3000, TEST_IP, MAC4);
ASSERT_EQ(gNeighOrch->m_syncdNeighbors.count(VLAN1000_NEIGH), 1);
ASSERT_EQ(gNeighOrch->m_syncdNeighbors.count(VLAN3000_NEIGH), 1);
}

TEST_F(NeighOrchTest, MultiVlanSameVrfDuplicateNeighbor)
{
EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry);
LearnNeighbor(VLAN_3000, TEST_IP, MAC4);
ASSERT_EQ(gNeighOrch->m_syncdNeighbors.count(VLAN3000_NEIGH), 1);

EXPECT_CALL(*mock_sai_neighbor_api, remove_neighbor_entry);
EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry);
LearnNeighbor(VLAN_4000, TEST_IP, MAC5);
ASSERT_EQ(gNeighOrch->m_syncdNeighbors.count(VLAN3000_NEIGH), 0);
ASSERT_EQ(gNeighOrch->m_syncdNeighbors.count(VLAN4000_NEIGH), 1);
}

TEST_F(NeighOrchTest, MultiVlanDuplicateNeighborMissingExistingVlanPort)
{
LearnNeighbor(VLAN_1000, TEST_IP, MAC1);

EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry).Times(0);
EXPECT_CALL(*mock_sai_neighbor_api, remove_neighbor_entry).Times(0);
gPortsOrch->m_portList.erase(VLAN_1000);
LearnNeighbor(VLAN_2000, TEST_IP, MAC2);
}

TEST_F(NeighOrchTest, MultiVlanDuplicateNeighborMissingNewVlanPort)
{
LearnNeighbor(VLAN_1000, TEST_IP, MAC1);

EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry).Times(0);
EXPECT_CALL(*mock_sai_neighbor_api, remove_neighbor_entry).Times(0);
gPortsOrch->m_portList.erase(VLAN_2000);
LearnNeighbor(VLAN_2000, TEST_IP, MAC2);
}
}
Loading