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

[EVPN]Handling race condition when remote VNI arrives before tunnel map entry #2642

Merged
merged 1 commit into from
Feb 2, 2023
Merged

[EVPN]Handling race condition when remote VNI arrives before tunnel map entry #2642

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
23 changes: 23 additions & 0 deletions orchagent/vxlanorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2349,13 +2349,24 @@ bool EvpnRemoteVnip2pOrch::addOperation(const Request& request)

VxlanTunnelOrch* tunnel_orch = gDirectory.get<VxlanTunnelOrch*>();
Port tunnelPort, vlanPort;
VxlanTunnelMapOrch* vxlan_tun_map_orch = gDirectory.get<VxlanTunnelMapOrch*>();
std::string vniVlanMapName;
uint32_t tmp_vlan_id = 0;
sai_object_id_t tnl_map_entry_id = SAI_NULL_OBJECT_ID;

if (!gPortsOrch->getVlanByVlanId(vlan_id, vlanPort))
{
SWSS_LOG_WARN("Vxlan tunnel map vlan id doesn't exist: %d", vlan_id);
return false;
}

/* Remote end point can be added only after local VLAN to VNI map gets created */
Copy link
Contributor

@srj102 srj102 Feb 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don't see why we need this change.
Is there any constraint imposed by the SAI API for this ?
What happens without this change ?

The errors listed seem specific to the implementation. I think this should be handled within the SAI driver implementation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think the ordering is correct logically. You cannot add the remote end point without the respective maps being created. We following the same approach as linux and expect the same ordering. In linux we first add the vlan to vni map and them advertise it after which we receive the remote end point which we program. The same should be the behavior in orchagent. I don't think there is anything wrong in adding this check.

if (!vxlan_tun_map_orch->isVniVlanMapExists(vni_id, vniVlanMapName, &tnl_map_entry_id, &tmp_vlan_id))
{
SWSS_LOG_WARN("Vxlan tunnel map is not created for vni:%d", vni_id);
return false;
}

if (tunnel_orch->getTunnelPort(remote_vtep,tunnelPort))
{
SWSS_LOG_INFO("Vxlan tunnelPort exists: %s", remote_vtep.c_str());
Expand Down Expand Up @@ -2492,6 +2503,11 @@ bool EvpnRemoteVnip2mpOrch::addOperation(const Request& request)
}

VxlanTunnelOrch* tunnel_orch = gDirectory.get<VxlanTunnelOrch*>();
VxlanTunnelMapOrch* vxlan_tun_map_orch = gDirectory.get<VxlanTunnelMapOrch*>();
std::string vniVlanMapName;
uint32_t tmp_vlan_id = 0;
sai_object_id_t tnl_map_entry_id = SAI_NULL_OBJECT_ID;

Port tunnelPort, vlanPort;
auto vtep_ptr = evpn_orch->getEVPNVtep();
if (!vtep_ptr)
Expand All @@ -2507,6 +2523,13 @@ bool EvpnRemoteVnip2mpOrch::addOperation(const Request& request)
return false;
}

/* Remote end point can be added only after local VLAN to VNI map gets created */
if (!vxlan_tun_map_orch->isVniVlanMapExists(vni_id, vniVlanMapName, &tnl_map_entry_id, &tmp_vlan_id))
{
SWSS_LOG_WARN("Vxlan tunnel map is not created for vni: %d", vni_id);
return false;
}

auto src_vtep = vtep_ptr->getSrcIP().to_string();
if (tunnel_orch->getTunnelPort(src_vtep,tunnelPort, true))
{
Expand Down
38 changes: 38 additions & 0 deletions tests/evpn_tunnel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,18 @@ def check_vxlan_dip_tunnel(self, dvs, vtep_name, src_ip, dip):

self.bridgeport_map[dip] = ret[0]

def check_vxlan_dip_tunnel_not_created(self, dvs, vtep_name, src_ip, dip):
state_db = swsscommon.DBConnector(swsscommon.STATE_DB, dvs.redis_sock, 0)

expected_state_attributes = {
'src_ip': src_ip,
'dst_ip': dip,
'tnl_src': 'EVPN',
}

ret = self.helper.get_key_with_attr(state_db, 'VXLAN_TUNNEL_TABLE', expected_state_attributes)
assert len(ret) == 0, "Tunnel Statetable entry created"

def check_vlan_extension_delete(self, dvs, vlan_name, dip):
asic_db = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)

Expand Down Expand Up @@ -777,6 +789,32 @@ def check_vlan_extension_p2mp(self, dvs, vlan_name, sip, dip):
assert len(ret) == 1, "More than 1 L2MC group member created"
self.l2mcgroup_member_map[dip+vlan_name] = ret[0]

def check_vlan_extension_not_created_p2mp(self, dvs, vlan_name, sip, dip):
asic_db = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
tbl = swsscommon.Table(asic_db, 'ASIC_STATE:SAI_OBJECT_TYPE_VLAN')
expected_vlan_attributes = {
'SAI_VLAN_ATTR_VLAN_ID': vlan_name,
}
ret = self.helper.get_key_with_attr(asic_db, 'ASIC_STATE:SAI_OBJECT_TYPE_VLAN', expected_vlan_attributes)
assert len(ret) > 0, "VLAN entry not created"
assert len(ret) == 1, "More than 1 VLAN entry created"

self.vlan_id_map[vlan_name] = ret[0]
status, fvs = tbl.get(self.vlan_id_map[vlan_name])

print(fvs)

uuc_flood_type = None
bc_flood_type = None
uuc_flood_group = None
bc_flood_group = None

for attr,value in fvs:
assert attr != 'SAI_VLAN_ATTR_UNKNOWN_UNICAST_FLOOD_CONTROL_TYPE', "Unknown unicast flood control type is set"
assert attr != 'SAI_VLAN_ATTR_BROADCAST_FLOOD_CONTROL_TYPE', "Broadcast flood control type is set"
assert attr != 'SAI_VLAN_ATTR_UNKNOWN_UNICAST_FLOOD_GROUP', "Unknown unicast flood group is set"
assert attr != 'SAI_VLAN_ATTR_UNKNOWN_UNICAST_FLOOD_CONTROL_TYPE', "Broadcast flood group is set"

def check_vxlan_tunnel_entry(self, dvs, tunnel_name, vnet_name, vni_id):
asic_db = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)

Expand Down
46 changes: 46 additions & 0 deletions tests/test_evpn_tunnel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ def test_p2p_tunnel(self, dvs, testlog):
vnilist = ['1000', '1001', '1002']

vxlan_obj.fetch_exist_entries(dvs)
vxlan_obj.create_vlan1(dvs,"Vlan100")
vxlan_obj.create_vlan1(dvs,"Vlan101")
vxlan_obj.create_vlan1(dvs,"Vlan102")
vxlan_obj.create_vxlan_tunnel(dvs, tunnel_name, '6.6.6.6')
vxlan_obj.create_vxlan_tunnel_map(dvs, tunnel_name, map_name, '1000', 'Vlan100')
vxlan_obj.create_vxlan_tunnel_map(dvs, tunnel_name, map_name_1, '1001', 'Vlan101')
Expand Down Expand Up @@ -161,3 +164,46 @@ def test_p2mp_tunnel_with_dip(self, dvs, testlog):
print("Testing SIP Tunnel Deletion")
vxlan_obj.remove_vxlan_tunnel(dvs, tunnel_name)
vxlan_obj.check_vxlan_sip_tunnel_delete(dvs, tunnel_name, '6.6.6.6')

def test_delayed_vlan_vni_map(self, dvs, testlog):
vxlan_obj = self.get_vxlan_obj()

tunnel_name = 'tunnel_2'
map_name = 'map_1000_100'
map_name_1 = 'map_1001_101'
vlanlist = ['100']
vnilist = ['1000']

vxlan_obj.fetch_exist_entries(dvs)
vxlan_obj.create_vlan1(dvs,"Vlan100")
vxlan_obj.create_vlan1(dvs,"Vlan101")

vxlan_obj.create_vxlan_tunnel(dvs, tunnel_name, '6.6.6.6')
vxlan_obj.create_vxlan_tunnel_map(dvs, tunnel_name, map_name, '1000', 'Vlan100')

vxlan_obj.check_vxlan_sip_tunnel(dvs, tunnel_name, '6.6.6.6', vlanlist, vnilist, tunnel_map_entry_count = 1)
vxlan_obj.check_vxlan_tunnel_map_entry(dvs, tunnel_name, vlanlist, vnilist)

vxlan_obj.create_evpn_nvo(dvs, 'nvo1', tunnel_name)

vxlan_obj.create_evpn_remote_vni(dvs, 'Vlan101', '7.7.7.7', '1001')
vxlan_obj.check_vxlan_dip_tunnel_not_created(dvs, tunnel_name, '6.6.6.6', '7.7.7.7')
vxlan_obj.create_vxlan_tunnel_map(dvs, tunnel_name, map_name_1, '1001', 'Vlan101')

print("Testing VLAN 101 extension")
vxlan_obj.check_vxlan_dip_tunnel(dvs, tunnel_name, '6.6.6.6', '7.7.7.7')
vxlan_obj.check_vlan_extension(dvs, '101', '7.7.7.7')

print("Testing Vlan Extension removal")
vxlan_obj.remove_evpn_remote_vni(dvs, 'Vlan101', '7.7.7.7')
vxlan_obj.check_vlan_extension_delete(dvs, '101', '7.7.7.7')
vxlan_obj.check_vxlan_dip_tunnel_delete(dvs, '7.7.7.7')

vxlan_obj.remove_vxlan_tunnel_map(dvs, tunnel_name, map_name, '1000', 'Vlan100')
vxlan_obj.remove_vxlan_tunnel_map(dvs, tunnel_name, map_name_1, '1001', 'Vlan101')
vxlan_obj.check_vxlan_tunnel_map_entry_delete(dvs, tunnel_name, vlanlist, vnilist)

print("Testing SIP Tunnel Deletion")
vxlan_obj.remove_evpn_nvo(dvs, 'nvo1')
vxlan_obj.remove_vxlan_tunnel(dvs, tunnel_name)
vxlan_obj.check_vxlan_sip_tunnel_delete(dvs, tunnel_name, '6.6.6.6')
44 changes: 44 additions & 0 deletions tests/test_evpn_tunnel_p2mp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ def test_vlan_extension(self, dvs, testlog):
vnilist = ['1000', '1001', '1002']

vxlan_obj.fetch_exist_entries(dvs)
vxlan_obj.create_vlan1(dvs,"Vlan100")
vxlan_obj.create_vlan1(dvs,"Vlan101")
vxlan_obj.create_vlan1(dvs,"Vlan102")
vxlan_obj.create_vxlan_tunnel(dvs, tunnel_name, '6.6.6.6')
vxlan_obj.create_vxlan_tunnel_map(dvs, tunnel_name, map_name, '1000', 'Vlan100')
vxlan_obj.create_vxlan_tunnel_map(dvs, tunnel_name, map_name_1, '1001', 'Vlan101')
Expand Down Expand Up @@ -122,3 +125,44 @@ def test_vlan_extension(self, dvs, testlog):
vxlan_obj.remove_evpn_nvo(dvs, 'nvo1')
vxlan_obj.remove_vxlan_tunnel(dvs, tunnel_name)
vxlan_obj.check_vxlan_sip_tunnel_delete(dvs, tunnel_name, '6.6.6.6', ignore_bp=False)

def test_delayed_vlan_vni_map(self, dvs, testlog):
vxlan_obj = self.get_vxlan_obj()

tunnel_name = 'tunnel_2'
map_name = 'map_1000_100'
map_name_1 = 'map_1001_101'
vlanlist = ['100']
vnilist = ['1000']

vxlan_obj.fetch_exist_entries(dvs)
vxlan_obj.create_vlan1(dvs,"Vlan100")
vxlan_obj.create_vlan1(dvs,"Vlan101")

vxlan_obj.create_vxlan_tunnel(dvs, tunnel_name, '6.6.6.6')
vxlan_obj.create_vxlan_tunnel_map(dvs, tunnel_name, map_name, '1000', 'Vlan100')

vxlan_obj.check_vxlan_sip_tunnel(dvs, tunnel_name, '6.6.6.6', vlanlist, vnilist, ignore_bp=False, tunnel_map_entry_count = 1)
vxlan_obj.check_vxlan_tunnel_map_entry(dvs, tunnel_name, vlanlist, vnilist)

vxlan_obj.create_evpn_nvo(dvs, 'nvo1', tunnel_name)

vxlan_obj.create_evpn_remote_vni(dvs, 'Vlan101', '7.7.7.7', '1001')
vxlan_obj.check_vlan_extension_not_created_p2mp(dvs, '101', '6.6.6.6', '7.7.7.7')
vxlan_obj.create_vxlan_tunnel_map(dvs, tunnel_name, map_name_1, '1001', 'Vlan101')

print("Testing VLAN 101 extension")
vxlan_obj.check_vlan_extension_p2mp(dvs, '101', '6.6.6.6', '7.7.7.7')

print("Testing Vlan Extension removal")
vxlan_obj.remove_evpn_remote_vni(dvs, 'Vlan101', '7.7.7.7')
vxlan_obj.check_vlan_extension_delete_p2mp(dvs, '101', '6.6.6.6', '7.7.7.7')

vxlan_obj.remove_vxlan_tunnel_map(dvs, tunnel_name, map_name, '1000', 'Vlan100')
vxlan_obj.remove_vxlan_tunnel_map(dvs, tunnel_name, map_name_1, '1001', 'Vlan101')
vxlan_obj.check_vxlan_tunnel_map_entry_delete(dvs, tunnel_name, vlanlist, vnilist)

print("Testing SIP Tunnel Deletion")
vxlan_obj.remove_evpn_nvo(dvs, 'nvo1')
vxlan_obj.remove_vxlan_tunnel(dvs, tunnel_name)
vxlan_obj.check_vxlan_sip_tunnel_delete(dvs, tunnel_name, '6.6.6.6', ignore_bp=False)