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

Handle learning duplicate IPs on different VRFs #3165

Merged
merged 10 commits into from
May 30, 2024
Merged

Handle learning duplicate IPs on different VRFs #3165

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0d44bee
Handle learning duplicate IPs on different VRFs
theasianpianist May 24, 2024
4541d7c
Merge branch 'master' into multi-vlan-vrf
theasianpianist May 24, 2024
c43d16e
Merge branch 'master' of github.com:sonic-net/sonic-swss into multi-v…
theasianpianist May 24, 2024
c3e0624
remove unused branch
theasianpianist May 24, 2024
7f75606
improve unit test coverage
theasianpianist May 24, 2024
d6e63a0
add missing else
theasianpianist May 28, 2024
56794c2
Merge branch 'master' of github.com:sonic-net/sonic-swss into multi-v…
theasianpianist May 28, 2024
3fe95c1
formatting
theasianpianist May 28, 2024
44a0ae6
Merge branch 'master' of github.com:sonic-net/sonic-swss into multi-v…
theasianpianist May 28, 2024
d83e5d8
Merge branch 'master' into multi-vlan-vrf
prsunny May 29, 2024
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
28 changes: 25 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,34 @@ 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());
}
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing 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
155 changes: 114 additions & 41 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 @@ -55,25 +57,43 @@ namespace neighorch_test
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 +104,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 +125,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 +189,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 +222,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