Skip to content

Commit 97c7f3e

Browse files
theasianpianisttheasianpianist
authored
Fix multi VLAN neighbor learning (#3049)
What I did When adding a new neighbor, check if the neighbor IP has already been learned on a different VLAN. If it has, remove the old neighbor entry before adding the new one. Why I did it On Gemini devices, if a neighbor IP moves from an active port in one VLAN to a second VLAN, then back to the first VLAN (with 3 different MAC addresses), orchagent will crash. Even though the MAC address of the last move is different from the first MAC address, orchagent believes the last MAC address to already be programmed in the hardware and tries to set an attribute of the entry which doesn't exist.
1 parent 2617d4c commit 97c7f3e

File tree

11 files changed

+643
-334
lines changed

11 files changed

+643
-334
lines changed

‎orchagent/neighorch.cpp‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,27 @@ bool NeighOrch::addNeighbor(const NeighborEntry &neighborEntry, const MacAddress
933933
}
934934
}
935935

936+
PortsOrch* ports_orch = gDirectory.get<PortsOrch*>();
937+
auto vlan_ports = ports_orch->getAllVlans();
938+
939+
for (auto vlan_port: vlan_ports)
940+
{
941+
if (vlan_port == alias)
942+
{
943+
continue;
944+
}
945+
NeighborEntry temp_entry = { ip_address, vlan_port };
946+
if (m_syncdNeighbors.find(temp_entry) != m_syncdNeighbors.end())
947+
{
948+
SWSS_LOG_NOTICE("Neighbor %s on %s already exists, removing before adding new neighbor", ip_address.to_string().c_str(), vlan_port.c_str());
949+
if (!removeNeighbor(temp_entry))
950+
{
951+
SWSS_LOG_ERROR("Failed to remove neighbor %s on %s", ip_address.to_string().c_str(), vlan_port.c_str());
952+
return false;
953+
}
954+
}
955+
}
956+
936957
MuxOrch* mux_orch = gDirectory.get<MuxOrch*>();
937958
bool hw_config = isHwConfigured(neighborEntry);
938959

‎orchagent/portsorch.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,11 @@ map<string, Port>& PortsOrch::getAllPorts()
11561156
return m_portList;
11571157
}
11581158

1159+
unordered_set<string>& PortsOrch::getAllVlans()
1160+
{
1161+
return m_vlanPorts;
1162+
}
1163+
11591164
bool PortsOrch::getPort(string alias, Port &p)
11601165
{
11611166
SWSS_LOG_ENTER();
@@ -5743,6 +5748,7 @@ bool PortsOrch::addVlan(string vlan_alias)
57435748
m_portList[vlan_alias] = vlan;
57445749
m_port_ref_count[vlan_alias] = 0;
57455750
saiOidToAlias[vlan_oid] = vlan_alias;
5751+
m_vlanPorts.emplace(vlan_alias);
57465752

57475753
return true;
57485754
}
@@ -5809,6 +5815,7 @@ bool PortsOrch::removeVlan(Port vlan)
58095815
saiOidToAlias.erase(vlan.m_vlan_info.vlan_oid);
58105816
m_portList.erase(vlan.m_alias);
58115817
m_port_ref_count.erase(vlan.m_alias);
5818+
m_vlanPorts.erase(vlan.m_alias);
58125819

58135820
return true;
58145821
}

‎orchagent/portsorch.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#define SWSS_PORTSORCH_H
33

44
#include <map>
5+
#include <unordered_set>
56

67
#include "acltable.h"
78
#include "orch.h"
@@ -150,6 +151,8 @@ class PortsOrch : public Orch, public Subject
150151
bool createVlanHostIntf(Port& vl, string hostif_name);
151152
bool removeVlanHostIntf(Port vl);
152153

154+
unordered_set<string>& getAllVlans();
155+
153156
bool createBindAclTableGroup(sai_object_id_t port_oid,
154157
sai_object_id_t acl_table_oid,
155158
sai_object_id_t &group_oid,
@@ -306,6 +309,7 @@ class PortsOrch : public Orch, public Subject
306309
map<int, gearbox_port_t> m_gearboxPortMap;
307310
map<sai_object_id_t, tuple<sai_object_id_t, sai_object_id_t>> m_gearboxPortListLaneMap;
308311

312+
unordered_set<string> m_vlanPorts;
309313
port_config_state_t m_portConfigState = PORT_CONFIG_MISSING;
310314
sai_uint32_t m_portCount;
311315
map<set<uint32_t>, sai_object_id_t> m_portListLaneMap;

‎tests/mock_tests/Makefile.am‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ tests_SOURCES = aclorch_ut.cpp \
4444
mock_table.cpp \
4545
mock_hiredis.cpp \
4646
mock_redisreply.cpp \
47+
mock_sai_api.cpp \
4748
bulker_ut.cpp \
4849
portmgr_ut.cpp \
4950
sflowmgrd_ut.cpp \
@@ -56,6 +57,7 @@ tests_SOURCES = aclorch_ut.cpp \
5657
warmrestartassist_ut.cpp \
5758
test_failure_handling.cpp \
5859
warmrestarthelper_ut.cpp \
60+
neighorch_ut.cpp \
5961
$(top_srcdir)/warmrestart/warmRestartHelper.cpp \
6062
$(top_srcdir)/lib/gearboxutils.cpp \
6163
$(top_srcdir)/lib/subintf.cpp \

‎tests/mock_tests/flowcounterrouteorch_ut.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ namespace flowcounterrouteorch_test
135135

136136
ASSERT_EQ(gPortsOrch, nullptr);
137137
gPortsOrch = new PortsOrch(m_app_db.get(), m_state_db.get(), ports_tables, m_chassis_app_db.get());
138+
gDirectory.set(gPortsOrch);
138139

139140
vector<string> vnet_tables = {
140141
APP_VNET_RT_TABLE_NAME,

0 commit comments

Comments
 (0)