Skip to content

Commit

Permalink
[vxlanorch]: p2mp tunnels not stored in VXLAN state table
Browse files Browse the repository at this point in the history
Whether a VXLAN tunnel is brought up as p2p or p2mp depends on the silicon
in use and isn't otherwise expressed to the end user.  The side effect of
this is `show vxlan remotevtep` shows no destinations when the silicon
only supports p2mp such as Mellanox Spectrum1.  This can cause a user to
believe the vxlan tunnel wasn't properly formed when it really was.

p2mp tunnels do not get a Port created, so do not rely on an operstatus
being set by portsorch to know if they are up or down.  This is presumably
why the tunnel entries were not recorded.  This introduces a new status
of 'p2mp' in this case to denote this difference.

Due to some past restructuring, calls into `updateRemoteEndPointIpRef()` were
omitted as p2mp tunnels are not calling `addTunnelUser()`.  This has been
resolved and this function now calls into `addRemoveStateTableEntry()` for
tracking of those remote vteps.

Result:
```
+------------+------------+-------------------+--------------+
| SIP        | DIP        | Creation Source   | OperStatus   |
+============+============+===================+==============+
| 172.16.0.3 | 172.16.0.1 | EVPN              | oper_p2mp    |
+------------+------------+-------------------+--------------+
| 172.16.0.3 | 172.16.0.2 | EVPN              | oper_p2mp    |
+------------+------------+-------------------+--------------+
Total count : 2
```

Signed-off-by: Brad House (@bradh352)
  • Loading branch information
bradh352 committed Jan 24, 2025
1 parent 4eb74f0 commit f390228
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 20 deletions.
39 changes: 32 additions & 7 deletions orchagent/vxlanorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -484,15 +484,15 @@ VxlanTunnel::VxlanTunnel(string name, IpAddress srcIp, IpAddress dstIp, tunnel_c
{
vtep_ptr = tunnel_orch->getVTEP(srcIp);
tunnel_orch->addRemoveStateTableEntry(name,srcIp, dstIp,
src, true);
src, "", true);
}
}

VxlanTunnel::~VxlanTunnel()
{
VxlanTunnelOrch* tunnel_orch = gDirectory.get<VxlanTunnelOrch*>();
tunnel_orch->addRemoveStateTableEntry(tunnel_name_,src_ip_, dst_ip_,
src_creation_, false);
src_creation_, "", false);
}

sai_object_id_t VxlanTunnel::addEncapMapperEntry(sai_object_id_t obj, uint32_t vni, tunnel_map_type_t type)
Expand Down Expand Up @@ -1034,8 +1034,12 @@ void VxlanTunnel::updateRemoteEndPointRefCnt(bool inc, tunnel_refcnt_t& tnl_refc

void VxlanTunnel::updateRemoteEndPointIpRef(const std::string remote_vtep, bool inc)
{
VxlanTunnelOrch* tunnel_orch = gDirectory.get<VxlanTunnelOrch*>();
string tunnel_name;
tunnel_refcnt_t tnl_refcnts;

tunnel_orch->getTunnelNameFromDIP(remote_vtep, tunnel_name);

auto it = tnl_users_.find(remote_vtep);
if (inc)
{
Expand All @@ -1044,6 +1048,8 @@ void VxlanTunnel::updateRemoteEndPointIpRef(const std::string remote_vtep, bool
memset(&tnl_refcnts, 0, sizeof(tunnel_refcnt_t));
tnl_refcnts.ip_refcnt++;
tnl_users_[remote_vtep] = tnl_refcnts;
auto dipaddr = IpAddress(remote_vtep);
tunnel_orch->addRemoveStateTableEntry(tunnel_name, src_ip_, dipaddr, TNL_CREATION_SRC_EVPN, "p2mp", true);
}
else
{
Expand All @@ -1066,6 +1072,8 @@ void VxlanTunnel::updateRemoteEndPointIpRef(const std::string remote_vtep, bool
if (it->second.ip_refcnt == 0)
{
tnl_users_.erase(remote_vtep);
auto dipaddr = IpAddress(remote_vtep);
tunnel_orch->addRemoveStateTableEntry(tunnel_name, src_ip_, dipaddr, TNL_CREATION_SRC_EVPN, "", false);
}
}
}
Expand Down Expand Up @@ -1802,9 +1810,11 @@ void VxlanTunnelOrch::updateDbTunnelOperStatus(string tunnel_portname,
m_stateVxlanTable.set(tunnel_name, fvVector);
}

void VxlanTunnelOrch::addRemoveStateTableEntry(string tunnel_name,
IpAddress& sip, IpAddress& dip,
tunnel_creation_src_t src, bool add)
void VxlanTunnelOrch::addRemoveStateTableEntry(const string tunnel_name,
IpAddress& sip, IpAddress& dip,
tunnel_creation_src_t src,
const string operstatus,
bool add)

{
std::vector<FieldValueTuple> fvVector, tmpFvVector;
Expand All @@ -1830,8 +1840,15 @@ void VxlanTunnelOrch::addRemoveStateTableEntry(string tunnel_name,
{
fvVector.emplace_back("tnl_src", "EVPN");
}

fvVector.emplace_back("operstatus", "down");

if (operstatus.empty())
{
fvVector.emplace_back("operstatus", "down");
}
else
{
fvVector.emplace_back("operstatus", operstatus);
}
m_stateVxlanTable.set(tunnel_name, fvVector);
SWSS_LOG_INFO("adding tunnel %s during warmboot", tunnel_name.c_str());
}
Expand Down Expand Up @@ -2549,6 +2566,8 @@ bool EvpnRemoteVnip2mpOrch::addOperation(const Request& request)
return false;
}

tunnel_orch->addTunnelUser(end_point_ip, vni_id, vlan_id, TUNNEL_USER_IMR);

auto src_vtep = vtep_ptr->getSrcIP().to_string();
if (tunnel_orch->getTunnelPort(src_vtep,tunnelPort, true))
{
Expand Down Expand Up @@ -2640,6 +2659,12 @@ bool EvpnRemoteVnip2mpOrch::delOperation(const Request& request)
return false;
}

if (!tunnel_orch->delTunnelUser(end_point_ip, vni_id, vlan_id, TUNNEL_USER_IMR))
{
SWSS_LOG_WARN("delTunnelUser remote failed: end_point_ip:%s vni:%d vlan:%d", end_point_ip.c_str(), vni_id, vlan_id);
return false;
}

if (vtep_ptr->del_tnl_hw_pending &&
!vtep_ptr->isTunnelReferenced())
{
Expand Down
2 changes: 1 addition & 1 deletion orchagent/vxlanorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ class VxlanTunnelOrch : public Orch2

void deleteTunnelPort(Port &tunnelPort);

void addRemoveStateTableEntry(const string, IpAddress&, IpAddress&, tunnel_creation_src_t, bool);
void addRemoveStateTableEntry(const string, IpAddress&, IpAddress&, tunnel_creation_src_t, const string, bool);

std::string getTunnelPortName(const std::string& vtep, bool local=false);
void getTunnelNameFromDIP(const string& dip, string& tunnel_name);
Expand Down
32 changes: 20 additions & 12 deletions tests/evpn_tunnel.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ def check_vxlan_tunnel_map_entry_removed(self, dvs, tunnel_name, vidlist, vnidli
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)
state_db = swsscommon.DBConnector(swsscommon.STATE_DB, dvs.redis_sock, 0)

tbl = swsscommon.Table(app_db, "VXLAN_TUNNEL_TABLE")
status, fvs = tbl.get(self.tunnel_appdb[tunnel_name])
Expand All @@ -527,11 +528,29 @@ def check_vxlan_sip_tunnel_delete(self, dvs, tunnel_name, sip, ignore_bp = True)
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(state_db, 'VXLAN_TUNNEL_TABLE')
status, fvs = tbl.get(tunnel_name)
assert status == False, "State Table 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_tunnel_state_table(self, dvs, tunnel_name, src_ip, dst_ip):
state_db = swsscommon.DBConnector(swsscommon.STATE_DB, dvs.redis_sock, 0)

expected_state_attributes = {
'src_ip': src_ip,
'dst_ip': dst_ip,
'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 not created"
assert len(ret) == 1, "More than 1 Tunn statetable entry created"
self.dip_tun_state_map[dip] = ret[0]

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,
tunnel_map_entry_count = 3):
Expand Down Expand Up @@ -646,19 +665,8 @@ def check_vxlan_dip_tunnel_delete(self, dvs, dip):

def check_vxlan_dip_tunnel(self, dvs, vtep_name, src_ip, dip):
asic_db = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
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 not created"
assert len(ret) == 1, "More than 1 Tunn statetable entry created"
self.dip_tun_state_map[dip] = ret[0]

self.check_vxlan_tunnel_state_table(dvs, tunnel_name, src_ip, dip)

tunnel_map_id = self.tunnel_map_map[vtep_name]

Expand Down
4 changes: 4 additions & 0 deletions tests/test_evpn_tunnel_p2mp.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def test_vlan_extension(self, dvs, testlog):

vxlan_obj.create_evpn_nvo(dvs, 'nvo1', tunnel_name)
vxlan_obj.create_evpn_remote_vni(dvs, 'Vlan100', '7.7.7.7', '1000')
vxlan_obj.check_vxlan_state_table(dvs, tunnel_name, '6.6.6.6', '7.7.7.7')

print("Testing VLAN 100 extension")
vxlan_obj.check_vlan_extension_p2mp(dvs, '100', '6.6.6.6', '7.7.7.7')
Expand All @@ -83,6 +84,9 @@ def test_vlan_extension(self, dvs, testlog):
print("Testing VLAN 102 extension")
vxlan_obj.check_vlan_extension_p2mp(dvs, '102', '6.6.6.6', '7.7.7.7')

# Make sure no new state entries were created
vxlan_obj.check_vxlan_state_table(dvs, tunnel_name, '6.6.6.6', '7.7.7.7')

print("Testing another remote endpoint to 8.8.8.8")
vxlan_obj.create_evpn_remote_vni(dvs, 'Vlan100', '8.8.8.8', '1000')
print("Testing remote endpoint creation to 8.8.8.8")
Expand Down

0 comments on commit f390228

Please sign in to comment.