Skip to content

Commit

Permalink
[EVPN]Modified tunnel creation logic when creating tunnel in VRF-VNI …
Browse files Browse the repository at this point in the history 
…map creation flow (#2404)

Same as PR #2387

- What I did
To fix issue sonic-net/sonic-buildimage#11428
Modified the logic of tunnel map creation to create tunnel with tunnel map for vlan-vni map in addition to vrf-vni map when tunnel is created first time in the VRF-VNI map processing flow.
Modified the tunnel stats interval to 10 sec
Modified the logic to create bridge port for p2mp tunnel only when p2p tunnel is not supported

- Why I did it
During the configuration phase when VRF-VNI map arrives before VLAN-VNI map, the tunnel is created without a tunnel map for vlan-vni membership. This is problematic when VLAN to VNI map arrives later, tunnel map entry cannot be created since the tunnel map doesn't exist and its a create only attribute in SAI.

- How I verified it
Modified UT to add VRF-VNI map first and VLAN-VLAN map later
  • Loading branch information
@dgsudharsan
dgsudharsan authored Aug 8, 2022
1 parent 2489ad5 commit 168bd3b
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 148 deletions.
140 changes: 48 additions & 92 deletions orchagent/vxlanorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,67 +494,6 @@ VxlanTunnel::~VxlanTunnel()
src_creation_, false);
}

bool VxlanTunnel::createTunnel(MAP_T encap, MAP_T decap, uint8_t encap_ttl)
{
try
{
VxlanTunnelOrch* tunnel_orch = gDirectory.get<VxlanTunnelOrch*>();
sai_ip_address_t ips, ipd, *ip=nullptr;
uint8_t mapper_list = 0;
swss::copy(ips, src_ip_);

// Only a single mapper type is created

if (decap == MAP_T::VNI_TO_BRIDGE)
{
TUNNELMAP_SET_BRIDGE(mapper_list);
}
else if (decap == MAP_T::VNI_TO_VLAN_ID)
{
TUNNELMAP_SET_VLAN(mapper_list);
}
else
{
TUNNELMAP_SET_VRF(mapper_list);
}

createMapperHw(mapper_list, (encap == MAP_T::MAP_TO_INVALID) ?
TUNNEL_MAP_USE_DECAP_ONLY: TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP);

if (encap != MAP_T::MAP_TO_INVALID)
{
ip = &ips;
}

ids_.tunnel_id = create_tunnel(&ids_, ip, NULL, gUnderlayIfId, false, encap_ttl);

if (ids_.tunnel_id != SAI_NULL_OBJECT_ID)
{
tunnel_orch->addTunnelToFlexCounter(ids_.tunnel_id, tunnel_name_);
}

ip = nullptr;
if (!dst_ip_.isZero())
{
swss::copy(ipd, dst_ip_);
ip = &ipd;
}

ids_.tunnel_term_id = create_tunnel_termination(ids_.tunnel_id, ips, ip, gVirtualRouterId);
active_ = true;
tunnel_map_ = { encap, decap };
}
catch (const std::runtime_error& error)
{
SWSS_LOG_ERROR("Error creating tunnel %s: %s", tunnel_name_.c_str(), error.what());
// FIXME: add code to remove already created objects
return false;
}

SWSS_LOG_NOTICE("Vxlan tunnel '%s' was created", tunnel_name_.c_str());
return true;
}

sai_object_id_t VxlanTunnel::addEncapMapperEntry(sai_object_id_t obj, uint32_t vni, tunnel_map_type_t type)
{
const auto encap_id = getEncapMapId(type);
Expand Down Expand Up @@ -1924,20 +1863,18 @@ bool VxlanTunnel::isTunnelReferenced()
Port tunnelPort;
bool dip_tunnels_used = tunnel_orch->isDipTunnelsSupported();

ret = gPortsOrch->getPort(port_tunnel_name, tunnelPort);
if (!ret)
{
SWSS_LOG_ERROR("Get port failed for source vtep %s", port_tunnel_name.c_str());
return false;
}


if (dip_tunnels_used)
{
return (getDipTunnelCnt() != 0);
}
else
{
ret = gPortsOrch->getPort(port_tunnel_name, tunnelPort);
if (!ret)
{
SWSS_LOG_ERROR("Get port failed for source vtep %s", port_tunnel_name.c_str());
return false;
}
if (tunnelPort.m_fdb_count != 0)
{
return true;
Expand Down Expand Up @@ -2013,19 +1950,21 @@ bool VxlanTunnelMapOrch::addOperation(const Request& request)
if (!tunnel_obj->isActive())
{
//@Todo, currently only decap mapper is allowed
//tunnel_obj->createTunnel(MAP_T::MAP_TO_INVALID, MAP_T::VNI_TO_VLAN_ID);
uint8_t mapper_list = 0;
TUNNELMAP_SET_VLAN(mapper_list);
TUNNELMAP_SET_VRF(mapper_list);
tunnel_obj->createTunnelHw(mapper_list,TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP);
Port tunPort;
auto src_vtep = tunnel_obj->getSrcIP().to_string();
if (!tunnel_orch->getTunnelPort(src_vtep, tunPort, true))
if (!tunnel_orch->isDipTunnelsSupported())
{
auto port_tunnel_name = tunnel_orch->getTunnelPortName(src_vtep, true);
gPortsOrch->addTunnel(port_tunnel_name, tunnel_obj->getTunnelId(), false);
gPortsOrch->getPort(port_tunnel_name,tunPort);
gPortsOrch->addBridgePort(tunPort);
Port tunPort;
auto src_vtep = tunnel_obj->getSrcIP().to_string();
if (!tunnel_orch->getTunnelPort(src_vtep, tunPort, true))
{
auto port_tunnel_name = tunnel_orch->getTunnelPortName(src_vtep, true);
gPortsOrch->addTunnel(port_tunnel_name, tunnel_obj->getTunnelId(), false);
gPortsOrch->getPort(port_tunnel_name,tunPort);
gPortsOrch->addBridgePort(tunPort);
}
}
}

Expand Down Expand Up @@ -2117,26 +2056,27 @@ bool VxlanTunnelMapOrch::delOperation(const Request& request)
auto port_tunnel_name = tunnel_orch->getTunnelPortName(src_vtep, true);
bool ret;

ret = gPortsOrch->getPort(port_tunnel_name, tunnelPort);
// If there are Dynamic DIP Tunnels referring to this SIP Tunnel
// then mark it as pending for delete.
if (!tunnel_obj->isTunnelReferenced())
{
if (!ret)
if (!tunnel_orch->isDipTunnelsSupported())
{
SWSS_LOG_ERROR("Get port failed for source vtep %s", port_tunnel_name.c_str());
return true;
ret = gPortsOrch->getPort(port_tunnel_name, tunnelPort);
if (!ret)
{
SWSS_LOG_ERROR("Get port failed for source vtep %s", port_tunnel_name.c_str());
return true;
}
ret = gPortsOrch->removeBridgePort(tunnelPort);
if (!ret)
{
SWSS_LOG_ERROR("Remove Bridge port failed for source vtep = %s fdbcount = %d",
port_tunnel_name.c_str(), tunnelPort.m_fdb_count);
return true;
}
gPortsOrch->removeTunnel(tunnelPort);
}
ret = gPortsOrch->removeBridgePort(tunnelPort);
if (!ret)
{
SWSS_LOG_ERROR("Remove Bridge port failed for source vtep = %s fdbcount = %d",
port_tunnel_name.c_str(), tunnelPort.m_fdb_count);
return true;
}

gPortsOrch->removeTunnel(tunnelPort);

uint8_t mapper_list=0;
TUNNELMAP_SET_VLAN(mapper_list);
TUNNELMAP_SET_VRF(mapper_list);
Expand All @@ -2152,6 +2092,7 @@ bool VxlanTunnelMapOrch::delOperation(const Request& request)
}
else
{
gPortsOrch->getPort(port_tunnel_name, tunnelPort);
SWSS_LOG_WARN("Postponing the SIP Tunnel HW deletion Remote reference count = %d",
gPortsOrch->getBridgePortReferenceCount(tunnelPort));
}
Expand Down Expand Up @@ -2218,7 +2159,22 @@ bool VxlanVrfMapOrch::addOperation(const Request& request)
{
if (!tunnel_obj->isActive())
{
tunnel_obj->createTunnel(MAP_T::VRID_TO_VNI, MAP_T::VNI_TO_VRID);
uint8_t mapper_list = 0;
TUNNELMAP_SET_VLAN(mapper_list);
TUNNELMAP_SET_VRF(mapper_list);
tunnel_obj->createTunnelHw(mapper_list,TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP);
if (!tunnel_orch->isDipTunnelsSupported())
{
Port tunPort;
auto src_vtep = tunnel_obj->getSrcIP().to_string();
if (!tunnel_orch->getTunnelPort(src_vtep, tunPort, true))
{
auto port_tunnel_name = tunnel_orch->getTunnelPortName(src_vtep, true);
gPortsOrch->addTunnel(port_tunnel_name, tunnel_obj->getTunnelId(), false);
gPortsOrch->getPort(port_tunnel_name,tunPort);
gPortsOrch->addBridgePort(tunPort);
}
}
}
vrf_id = vrf_orch->getVRFid(vrf_name);
}
Expand Down
3 changes: 1 addition & 2 deletions orchagent/vxlanorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ typedef enum
#define IS_TUNNELMAP_SET_BRIDGE(x) ((x)& (1<<TUNNEL_MAP_T_BRIDGE))

#define TUNNEL_STAT_COUNTER_FLEX_COUNTER_GROUP "TUNNEL_STAT_COUNTER"
#define TUNNEL_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS 1000
#define TUNNEL_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS 10000
#define LOCAL_TUNNEL_PORT_PREFIX "Port_SRC_VTEP_"
#define EVPN_TUNNEL_PORT_PREFIX "Port_EVPN_"
#define EVPN_TUNNEL_NAME_PREFIX "EVPN_"
Expand Down Expand Up @@ -149,7 +149,6 @@ class VxlanTunnel
return active_;
}

bool createTunnel(MAP_T encap, MAP_T decap, uint8_t encap_ttl=0);
sai_object_id_t addEncapMapperEntry(sai_object_id_t obj, uint32_t vni,
tunnel_map_type_t type=TUNNEL_MAP_T_VIRTUAL_ROUTER);
sai_object_id_t addDecapMapperEntry(sai_object_id_t obj, uint32_t vni,
Expand Down
75 changes: 36 additions & 39 deletions tests/evpn_tunnel.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ def check_vxlan_tunnel_map_entry(self, dvs, tunnel_name, vidlist, vnidlist):
(exitcode, out) = dvs.runcmd(iplinkcmd)
assert exitcode == 0, "Kernel device not created"

def check_vxlan_sip_tunnel_delete(self, dvs, tunnel_name, sip):
def check_vxlan_sip_tunnel_delete(self, dvs, tunnel_name, sip, ignore_bp = True):
asic_db = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
app_db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)

Expand All @@ -511,36 +511,35 @@ def check_vxlan_sip_tunnel_delete(self, dvs, tunnel_name, sip):
status, fvs = tbl.get(self.tunnel_map_map[tunnel_name][3])
assert status == False, "SIP Tunnel mapper3 not deleted from ASIC_DB"

tbl = swsscommon.Table(asic_db, self.ASIC_BRIDGE_PORT)
status, fvs = tbl.get(self.bridgeport_map[sip])
assert status == False, "Tunnel bridgeport entry not deleted"
if not ignore_bp:
tbl = swsscommon.Table(asic_db, self.ASIC_BRIDGE_PORT)
status, fvs = tbl.get(self.bridgeport_map[sip])
assert status == False, "Tunnel bridgeport entry not deleted"

def check_vxlan_sip_tunnel(self, dvs, tunnel_name, src_ip, vidlist, vnidlist, dst_ip = '0.0.0.0', skip_dst_ip = 'True'):
def check_vxlan_sip_tunnel(self, dvs, tunnel_name, src_ip, vidlist, vnidlist,
dst_ip = '0.0.0.0', skip_dst_ip = 'True', ignore_bp = True):
asic_db = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
app_db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)

tunnel_map_id = self.helper.get_created_entries(asic_db, self.ASIC_TUNNEL_MAP, self.tunnel_map_ids, 4)
tunnel_id = self.helper.get_created_entry(asic_db, self.ASIC_TUNNEL_TABLE, self.tunnel_ids)
tunnel_term_id = self.helper.get_created_entry(asic_db, self.ASIC_TUNNEL_TERM_ENTRY, self.tunnel_term_ids)
tunnel_map_entry_id = self.helper.get_created_entries(asic_db, self.ASIC_TUNNEL_MAP_ENTRY, self.tunnel_map_entry_ids, 3)

# check that the vxlan tunnel termination are there
assert self.helper.how_many_entries_exist(asic_db, self.ASIC_TUNNEL_MAP) == (len(self.tunnel_map_ids) + 4), "The TUNNEL_MAP wasn't created"
assert self.helper.how_many_entries_exist(asic_db, self.ASIC_TUNNEL_MAP_ENTRY) == (len(self.tunnel_map_entry_ids) + 3), "The TUNNEL_MAP_ENTRY is created"
assert self.helper.how_many_entries_exist(asic_db, self.ASIC_TUNNEL_TABLE) == (len(self.tunnel_ids) + 1), "The TUNNEL wasn't created"
assert self.helper.how_many_entries_exist(asic_db, self.ASIC_TUNNEL_TERM_ENTRY) == (len(self.tunnel_term_ids) + 1), "The TUNNEL_TERM_TABLE_ENTRY wasm't created"

self.helper.check_object(asic_db, self.ASIC_TUNNEL_MAP, tunnel_map_id[2],
{
'SAI_TUNNEL_MAP_ATTR_TYPE': 'SAI_TUNNEL_MAP_TYPE_VNI_TO_VIRTUAL_ROUTER_ID',
}
)
expected_attributes_1 = {}
expected_attributes_1['SAI_TUNNEL_MAP_ATTR_TYPE'] = 'SAI_TUNNEL_MAP_TYPE_VNI_TO_VIRTUAL_ROUTER_ID'
ret = self.helper.get_key_with_attr(asic_db, self.ASIC_TUNNEL_MAP, expected_attributes_1)
assert len(ret) == 1, "Unexpected number of tunnel maps created for type SAI_TUNNEL_MAP_TYPE_VNI_TO_VIRTUAL_ROUTER_ID"

expected_attributes_1['SAI_TUNNEL_MAP_ATTR_TYPE'] = 'SAI_TUNNEL_MAP_TYPE_VIRTUAL_ROUTER_ID_TO_VNI'
ret = self.helper.get_key_with_attr(asic_db, self.ASIC_TUNNEL_MAP, expected_attributes_1)
assert len(ret) == 1, "Unexpected number of tunnel maps created for type SAI_TUNNEL_MAP_TYPE_VIRTUAL_ROUTER_ID_TO_VNI"

self.helper.check_object(asic_db, self.ASIC_TUNNEL_MAP, tunnel_map_id[3],
{
'SAI_TUNNEL_MAP_ATTR_TYPE': 'SAI_TUNNEL_MAP_TYPE_VIRTUAL_ROUTER_ID_TO_VNI',
}
)

decapstr = '2:' + tunnel_map_id[0] + ',' + tunnel_map_id[2]
encapstr = '2:' + tunnel_map_id[1] + ',' + tunnel_map_id[3]
Expand Down Expand Up @@ -571,15 +570,15 @@ def check_vxlan_sip_tunnel(self, dvs, tunnel_name, src_ip, vidlist, vnidlist, ds

expected_attributes_1 = {
'SAI_TUNNEL_MAP_ENTRY_ATTR_TUNNEL_MAP_TYPE': 'SAI_TUNNEL_MAP_TYPE_VNI_TO_VLAN_ID',
'SAI_TUNNEL_MAP_ENTRY_ATTR_TUNNEL_MAP': tunnel_map_id[0],
'SAI_TUNNEL_MAP_ENTRY_ATTR_VLAN_ID_VALUE': vidlist[0],
'SAI_TUNNEL_MAP_ENTRY_ATTR_VNI_ID_KEY': vnidlist[0],
}

for x in range(len(vidlist)):
expected_attributes_1['SAI_TUNNEL_MAP_ENTRY_ATTR_VLAN_ID_VALUE'] = vidlist[x]
expected_attributes_1['SAI_TUNNEL_MAP_ENTRY_ATTR_VNI_ID_KEY'] = vnidlist[x]
self.helper.check_object(asic_db, self.ASIC_TUNNEL_MAP_ENTRY, tunnel_map_entry_id[x], expected_attributes_1)
self.helper.get_key_with_attr(asic_db, self.ASIC_TUNNEL_MAP_ENTRY, expected_attributes_1)
assert len(ret) == 1, "Unexpected number of tunnel map entries created for VLAN to VNI mapping"

expected_siptnl_attributes = {
'src_ip': src_ip,
Expand All @@ -593,16 +592,18 @@ def check_vxlan_sip_tunnel(self, dvs, tunnel_name, src_ip, vidlist, vnidlist, ds
assert len(ret) == 1, "More than 1 Tunn statetable entry created"
self.tunnel_appdb[tunnel_name] = ret[0]

expected_bridgeport_attributes = {
'SAI_BRIDGE_PORT_ATTR_TYPE': 'SAI_BRIDGE_PORT_TYPE_TUNNEL',
'SAI_BRIDGE_PORT_ATTR_TUNNEL_ID': tunnel_id,
'SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE': 'SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE',
'SAI_BRIDGE_PORT_ATTR_ADMIN_STATE': 'true',
}
ret = self.helper.get_key_with_attr(asic_db, self.ASIC_BRIDGE_PORT, expected_bridgeport_attributes)
assert len(ret) > 0, "Bridgeport entry not created"
assert len(ret) == 1, "More than 1 bridgeport entry created"
self.bridgeport_map[src_ip] = ret[0]
if not ignore_bp:
expected_bridgeport_attributes = {
'SAI_BRIDGE_PORT_ATTR_TYPE': 'SAI_BRIDGE_PORT_TYPE_TUNNEL',
'SAI_BRIDGE_PORT_ATTR_TUNNEL_ID': tunnel_id,
'SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE': 'SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE',
'SAI_BRIDGE_PORT_ATTR_ADMIN_STATE': 'true',
}
ret = self.helper.get_key_with_attr(asic_db, self.ASIC_BRIDGE_PORT, expected_bridgeport_attributes)
assert len(ret) > 0, "Bridgeport entry not created"
assert len(ret) == 1, "More than 1 bridgeport entry created"
self.bridgeport_map[src_ip] = ret[0]

self.tunnel_map_ids.update(tunnel_map_id)
self.tunnel_ids.add(tunnel_id)
self.tunnel_term_ids.add(tunnel_term_id)
Expand Down Expand Up @@ -797,37 +798,33 @@ def check_vxlan_tunnel_entry(self, dvs, tunnel_name, vnet_name, vni_id):
def check_vxlan_tunnel_vrf_map_entry(self, dvs, tunnel_name, vrf_name, vni_id):
asic_db = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)

if (self.tunnel_map_map.get(tunnel_name) is None):
tunnel_map_id = self.helper.get_created_entries(asic_db, self.ASIC_TUNNEL_MAP, self.tunnel_map_ids, 4)
else:
tunnel_map_id = self.tunnel_map_map[tunnel_name]

tunnel_map_entry_id = self.helper.get_created_entries(asic_db, self.ASIC_TUNNEL_MAP_ENTRY, self.tunnel_map_entry_ids, 3)

# check that the vxlan tunnel termination are there
assert self.helper.how_many_entries_exist(asic_db, self.ASIC_TUNNEL_MAP_ENTRY) == (len(self.tunnel_map_entry_ids) + 3), "The TUNNEL_MAP_ENTRY is created too early"

self.helper.check_object(asic_db, self.ASIC_TUNNEL_MAP_ENTRY, tunnel_map_entry_id[1],
ret = self.helper.get_key_with_attr(asic_db, self.ASIC_TUNNEL_MAP_ENTRY,
{
'SAI_TUNNEL_MAP_ENTRY_ATTR_TUNNEL_MAP_TYPE': 'SAI_TUNNEL_MAP_TYPE_VIRTUAL_ROUTER_ID_TO_VNI',
'SAI_TUNNEL_MAP_ENTRY_ATTR_TUNNEL_MAP': tunnel_map_id[3],
'SAI_TUNNEL_MAP_ENTRY_ATTR_VIRTUAL_ROUTER_ID_KEY': self.vr_map[vrf_name].get('ing'),
'SAI_TUNNEL_MAP_ENTRY_ATTR_VNI_ID_VALUE': vni_id,
}
)

self.tunnel_map_vrf_entry_ids.update(tunnel_map_entry_id[1])
assert len(ret) == 1, "Invalid number of tunnel map entries for SAI_TUNNEL_MAP_TYPE_VIRTUAL_ROUTER_ID_TO_VNI"

self.tunnel_map_vrf_entry_ids.update(ret[0])

self.helper.check_object(asic_db, self.ASIC_TUNNEL_MAP_ENTRY, tunnel_map_entry_id[2],
ret = self.helper.get_key_with_attr(asic_db, self.ASIC_TUNNEL_MAP_ENTRY,
{
'SAI_TUNNEL_MAP_ENTRY_ATTR_TUNNEL_MAP_TYPE': 'SAI_TUNNEL_MAP_TYPE_VNI_TO_VIRTUAL_ROUTER_ID',
'SAI_TUNNEL_MAP_ENTRY_ATTR_TUNNEL_MAP': tunnel_map_id[2],
'SAI_TUNNEL_MAP_ENTRY_ATTR_VNI_ID_KEY': vni_id,
'SAI_TUNNEL_MAP_ENTRY_ATTR_VIRTUAL_ROUTER_ID_VALUE': self.vr_map[vrf_name].get('egr'),
}
)

self.tunnel_map_vrf_entry_ids.update(tunnel_map_entry_id[2])
assert len(ret) == 1, "Invalid number of tunnel map entries for SAI_TUNNEL_MAP_TYPE_VNI_TO_VIRTUAL_ROUTER_ID"
self.tunnel_map_vrf_entry_ids.update(ret[0])
self.tunnel_map_entry_ids.update(tunnel_map_entry_id)

def check_vxlan_tunnel_vrf_map_entry_remove(self, dvs, tunnel_name, vrf_name, vni_id):
Expand Down
Loading

0 comments on commit 168bd3b

Please sign in to comment.