From 16c7f0a351961338e231553520b1a7e7e639c48a Mon Sep 17 00:00:00 2001 From: Tapash Das Date: Tue, 21 Apr 2020 04:40:59 -0700 Subject: [PATCH 1/7] Added support for EVPN L3 VXLAN as described in the PR Azure/SONiC#437 --- cfgmgr/vrfmgr.cpp | 186 +++++++++++++++++++++++++--- cfgmgr/vrfmgr.h | 20 ++- cfgmgr/vrfmgrd.cpp | 1 + orchagent/neighorch.cpp | 93 +++++++++++++- orchagent/neighorch.h | 4 + orchagent/nexthopgroupkey.h | 26 +++- orchagent/nexthopkey.h | 37 +++++- orchagent/port.h | 1 + orchagent/portsorch.cpp | 41 ++++++ orchagent/portsorch.h | 1 + orchagent/routeorch.cpp | 240 +++++++++++++++++++++++++++++++++--- orchagent/routeorch.h | 5 + orchagent/vnetorch.cpp | 5 + orchagent/vrforch.cpp | 115 +++++++++++++++++ orchagent/vrforch.h | 41 ++++++ 15 files changed, 777 insertions(+), 39 deletions(-) diff --git a/cfgmgr/vrfmgr.cpp b/cfgmgr/vrfmgr.cpp index 25ca1253e3..caa5df11cd 100644 --- a/cfgmgr/vrfmgr.cpp +++ b/cfgmgr/vrfmgr.cpp @@ -19,6 +19,7 @@ VrfMgr::VrfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, con Orch(cfgDb, tableNames), m_appVrfTableProducer(appDb, APP_VRF_TABLE_NAME), m_appVnetTableProducer(appDb, APP_VNET_TABLE_NAME), + m_appVxlanVrfTableProducer(appDb, APP_VXLAN_VRF_TABLE_NAME), m_stateVrfTable(stateDb, STATE_VRF_TABLE_NAME), m_stateVrfObjectTable(stateDb, STATE_VRF_OBJECT_TABLE_NAME) { @@ -203,23 +204,32 @@ void VrfMgr::doTask(Consumer &consumer) string op = kfvOp(t); if (op == SET_COMMAND) { - if (!setLink(vrfName)) + if (consumer.getTableName() == CFG_VXLAN_EVPN_NVO_TABLE_NAME) { - SWSS_LOG_ERROR("Failed to create vrf netdev %s", vrfName.c_str()); - } - - vector fvVector; - fvVector.emplace_back("state", "ok"); - m_stateVrfTable.set(vrfName, fvVector); - - SWSS_LOG_NOTICE("Created vrf netdev %s", vrfName.c_str()); - if (consumer.getTableName() == CFG_VRF_TABLE_NAME) - { - m_appVrfTableProducer.set(vrfName, kfvFieldsValues(t)); + doVrfEvpnNvoAddTask(t); } else { - m_appVnetTableProducer.set(vrfName, kfvFieldsValues(t)); + if (!setLink(vrfName)) + { + SWSS_LOG_ERROR("Failed to create vrf netdev %s", vrfName.c_str()); + } + + vector fvVector; + fvVector.emplace_back("state", "ok"); + m_stateVrfTable.set(vrfName, fvVector); + + SWSS_LOG_NOTICE("Created vrf netdev %s", vrfName.c_str()); + if (consumer.getTableName() == CFG_VRF_TABLE_NAME) + { + m_appVrfTableProducer.set(vrfName, kfvFieldsValues(t)); + + doVrfVxlanTableCreateTask (t); + } + else + { + m_appVnetTableProducer.set(vrfName, kfvFieldsValues(t)); + } } } else if (op == DEL_COMMAND) @@ -229,7 +239,11 @@ void VrfMgr::doTask(Consumer &consumer) * Now state VRF_TABLE|Vrf represent vrf exist in appDB, if it exist vrf device is always effective. * VRFOrch add/del state VRF_OBJECT_TABLE|Vrf to represent object existence. VNETOrch is not do so now. */ - if (consumer.getTableName() == CFG_VRF_TABLE_NAME) + if (consumer.getTableName() == CFG_VXLAN_EVPN_NVO_TABLE_NAME) + { + doVrfEvpnNvoDelTask (t); + } + else if (consumer.getTableName() == CFG_VRF_TABLE_NAME) { vector temp; @@ -242,6 +256,7 @@ void VrfMgr::doTask(Consumer &consumer) continue; } + doVrfVxlanTableRemoveTask (t); m_appVrfTableProducer.del(vrfName); m_stateVrfTable.del(vrfName); } @@ -273,3 +288,146 @@ void VrfMgr::doTask(Consumer &consumer) it = consumer.m_toSync.erase(it); } } + +bool VrfMgr::doVrfEvpnNvoAddTask(const KeyOpFieldsValuesTuple & t) +{ + SWSS_LOG_ENTER(); + string tunnel_name = ""; + const vector& data = kfvFieldsValues(t); + for (auto idx : data) + { + const auto &field = fvField(idx); + const auto &value = fvValue(idx); + + if (field == "source_vtep") { + tunnel_name = value; + } + } + + if(m_evpnVxlanTunnel.empty()) { + m_evpnVxlanTunnel = tunnel_name; + VrfVxlanTableSync(true); + } + + SWSS_LOG_NOTICE("Added evpn nvo tunnel %s", m_evpnVxlanTunnel.c_str()); + return true; +} + +bool VrfMgr::doVrfEvpnNvoDelTask(const KeyOpFieldsValuesTuple & t) +{ + SWSS_LOG_ENTER(); + + if(!m_evpnVxlanTunnel.empty()) { + VrfVxlanTableSync(false); + m_evpnVxlanTunnel = ""; + } + + SWSS_LOG_NOTICE("Removed evpn nvo tunnel %s", m_evpnVxlanTunnel.c_str()); + return true; +} + +bool VrfMgr::doVrfVxlanTableCreateTask(const KeyOpFieldsValuesTuple & t) +{ + SWSS_LOG_ENTER(); + + auto vrf_name = kfvKey(t); + uint32_t vni = 0, old_vni = 0; + string s_vni = ""; + bool add = true; + + const vector& data = kfvFieldsValues(t); + for (auto idx : data) + { + const auto &field = fvField(idx); + const auto &value = fvValue(idx); + + if (field == "vni") { + s_vni = value; + vni = static_cast(stoul(value)); + } + } + + old_vni = getVRFmappedVNI(vrf_name); + SWSS_LOG_NOTICE("VRF VNI map update vrf %s, vni %d, old_vni %d", vrf_name.c_str(), vni, old_vni); + if (vni != old_vni) + { + if (vni == 0) { + vrf_vni_map_table_.erase(vrf_name); + s_vni = to_string(old_vni); + add = false; + } else { + vrf_vni_map_table_[vrf_name] = vni; + } + + } + + if ((vni == 0) && add) + { + return true; + } + + SWSS_LOG_NOTICE("VRF VNI map update vrf %s, s_vni %s, add %d", vrf_name.c_str(), s_vni.c_str(), add); + doVrfVxlanTableUpdate(vrf_name, s_vni, add); + return true; +} + +bool VrfMgr::doVrfVxlanTableRemoveTask(const KeyOpFieldsValuesTuple & t) +{ + SWSS_LOG_ENTER(); + + auto vrf_name = kfvKey(t); + uint32_t vni = 0; + string s_vni = ""; + + vni = getVRFmappedVNI(vrf_name); + SWSS_LOG_NOTICE("VRF VNI map remove vrf %s, vni %d", vrf_name.c_str(), vni); + if (vni != 0) { + s_vni = to_string(vni); + doVrfVxlanTableUpdate(vrf_name, s_vni, false); + vrf_vni_map_table_.erase(vrf_name); + } + + return true; +} + +bool VrfMgr::doVrfVxlanTableUpdate(const string& vrf_name, const string& vni, bool add) +{ + SWSS_LOG_ENTER(); + string key; + + if(m_evpnVxlanTunnel.empty()) { + SWSS_LOG_NOTICE("NVO Tunnel not present. vrf %s, vni %s, add %d", vrf_name.c_str(), vni.c_str(), add); + return false; + } + + key = m_evpnVxlanTunnel + ":" + "evpn_map_" + vni + "_" + vrf_name; + + std::vector fvVector; + FieldValueTuple v1("vni", vni); + FieldValueTuple v2("vrf", vrf_name); + fvVector.push_back(v1); + fvVector.push_back(v2); + + SWSS_LOG_NOTICE("VRF VNI map table update vrf %s, vni %s, add %d", vrf_name.c_str(), vni.c_str(), add); + if (add) { + m_appVxlanVrfTableProducer.set(key, fvVector); + } else { + m_appVxlanVrfTableProducer.del(key); + } + + return true; +} + +void VrfMgr::VrfVxlanTableSync(bool add) +{ + SWSS_LOG_ENTER(); + string s_vni = ""; + + for (auto itr : vrf_vni_map_table_) + { + s_vni = to_string(itr.second); + SWSS_LOG_NOTICE("vrf %s, vni %s, add %d", (itr.first).c_str(), s_vni.c_str(), add); + doVrfVxlanTableUpdate(itr.first, s_vni, add); + } +} + diff --git a/cfgmgr/vrfmgr.h b/cfgmgr/vrfmgr.h index f8da87d318..5973469960 100644 --- a/cfgmgr/vrfmgr.h +++ b/cfgmgr/vrfmgr.h @@ -12,11 +12,22 @@ using namespace std; namespace swss { +typedef std::unordered_map VRFNameVNIMapTable; + class VrfMgr : public Orch { public: VrfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const std::vector &tableNames); using Orch::doTask; + std::string m_evpnVxlanTunnel; + + uint32_t getVRFmappedVNI(const std::string& vrf_name) const + { + if (vrf_vni_map_table_.find(vrf_name) != std::end(vrf_vni_map_table_)) + return vrf_vni_map_table_.at(vrf_name); + else + return 0; + } private: bool delLink(const std::string& vrfName); @@ -25,13 +36,20 @@ class VrfMgr : public Orch void recycleTable(uint32_t table); uint32_t getFreeTable(void); void handleVnetConfigSet(KeyOpFieldsValuesTuple &t); + bool doVrfEvpnNvoAddTask(const KeyOpFieldsValuesTuple & t); + bool doVrfEvpnNvoDelTask(const KeyOpFieldsValuesTuple & t); + bool doVrfVxlanTableCreateTask(const KeyOpFieldsValuesTuple & t); + bool doVrfVxlanTableRemoveTask(const KeyOpFieldsValuesTuple & t); + bool doVrfVxlanTableUpdate(const string& vrf_name, const string& vni, bool add); + void VrfVxlanTableSync(bool add); void doTask(Consumer &consumer); std::map m_vrfTableMap; std::set m_freeTables; + VRFNameVNIMapTable vrf_vni_map_table_; Table m_stateVrfTable, m_stateVrfObjectTable; - ProducerStateTable m_appVrfTableProducer, m_appVnetTableProducer; + ProducerStateTable m_appVrfTableProducer, m_appVnetTableProducer, m_appVxlanVrfTableProducer; }; } diff --git a/cfgmgr/vrfmgrd.cpp b/cfgmgr/vrfmgrd.cpp index c81f7bdadd..2ecdf7968f 100644 --- a/cfgmgr/vrfmgrd.cpp +++ b/cfgmgr/vrfmgrd.cpp @@ -44,6 +44,7 @@ int main(int argc, char **argv) vector cfg_vrf_tables = { CFG_VRF_TABLE_NAME, CFG_VNET_TABLE_NAME, + CFG_VXLAN_EVPN_NVO_TABLE_NAME, }; DBConnector cfgDb("CONFIG_DB", 0); diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index ae0db0751c..67b6a08b5f 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -4,6 +4,7 @@ #include "swssnet.h" #include "crmorch.h" #include "routeorch.h" +#include "directory.h" extern sai_neighbor_api_t* sai_neighbor_api; extern sai_next_hop_api_t* sai_next_hop_api; @@ -12,6 +13,7 @@ extern PortsOrch *gPortsOrch; extern sai_object_id_t gSwitchId; extern CrmOrch *gCrmOrch; extern RouteOrch *gRouteOrch; +extern Directory gDirectory; const int neighorch_pri = 30; @@ -228,6 +230,23 @@ bool NeighOrch::removeNextHop(const IpAddress &ipAddress, const string &alias) return true; } +bool NeighOrch::removeOverlayNextHop(const NextHopKey &nexthop) +{ + SWSS_LOG_ENTER(); + + assert(hasNextHop(nexthop)); + + if (m_syncdNextHops[nexthop].ref_count > 0) + { + SWSS_LOG_ERROR("Failed to remove still referenced next hop %s on %s", + nexthop.ip_address.to_string().c_str(), nexthop.alias.c_str()); + return false; + } + + m_syncdNextHops.erase(nexthop); + return true; +} + sai_object_id_t NeighOrch::getNextHopId(const NextHopKey &nexthop) { assert(hasNextHop(nexthop)); @@ -425,8 +444,8 @@ bool NeighOrch::addNeighbor(const NeighborEntry &neighborEntry, const MacAddress return false; } } - - SWSS_LOG_NOTICE("Created neighbor %s on %s", macAddress.to_string().c_str(), alias.c_str()); + SWSS_LOG_NOTICE("Created neighbor ip %s, %s on %s", ip_address.to_string().c_str(), + macAddress.to_string().c_str(), alias.c_str()); m_intfsOrch->increaseRouterIntfsRefCount(alias); if (neighbor_entry.ip_address.addr_family == SAI_IP_ADDR_FAMILY_IPV4) @@ -581,3 +600,73 @@ bool NeighOrch::removeNeighbor(const NeighborEntry &neighborEntry) return true; } + +sai_object_id_t NeighOrch::addTunnelNextHop(const NextHopKey& nh) +{ + SWSS_LOG_ENTER(); + sai_object_id_t nh_id = SAI_NULL_OBJECT_ID; + + EvpnNvoOrch* evpn_orch = gDirectory.get(); + auto vtep_ptr = evpn_orch->getEVPNVtep(); + + if(!vtep_ptr) + { + SWSS_LOG_ERROR("Add Tunnel next hop unable to find EVPN VTEP"); + return nh_id; + } + + auto tun_name = vtep_ptr->getTunnelName(); + + VxlanTunnelOrch* vxlan_orch = gDirectory.get(); + IpAddress tnl_dip = nh.ip_address; + nh_id = vxlan_orch->createNextHopTunnel(tun_name, tnl_dip, nh.mac_address, nh.vni); + + if (nh_id == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_ERROR("Failed to create Tunnel next hop %s, %s|%d|%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), + nh.vni, nh.mac_address.to_string().c_str()); + throw std::runtime_error("NH Tunnel create failed for " + tun_name + " ip " + nh.ip_address.to_string()); + } + + SWSS_LOG_NOTICE("Created Tunnel next hop %s, %s|%d|%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), + nh.vni, nh.mac_address.to_string().c_str()); + + NextHopEntry next_hop_entry; + next_hop_entry.next_hop_id = nh_id; + next_hop_entry.ref_count = 0; + next_hop_entry.nh_flags = 0; + m_syncdNextHops[nh] = next_hop_entry; + + return nh_id; +} + +bool NeighOrch::removeTunnelNextHop(const NextHopKey& nh) +{ + SWSS_LOG_ENTER(); + + EvpnNvoOrch* evpn_orch = gDirectory.get(); + auto vtep_ptr = evpn_orch->getEVPNVtep(); + + if(!vtep_ptr) + { + SWSS_LOG_ERROR("Remove Tunnel next hop unable to find EVPN VTEP"); + return false; + } + + auto tun_name = vtep_ptr->getTunnelName(); + + VxlanTunnelOrch* vxlan_orch = gDirectory.get(); + + IpAddress tnl_dip = nh.ip_address; + if (!vxlan_orch->removeNextHopTunnel(tun_name, tnl_dip, nh.mac_address, nh.vni)) + { + SWSS_LOG_ERROR("Failed to remove Tunnel next hop %s, %s|%d|%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), + nh.vni, nh.mac_address.to_string().c_str()); + return false; + } + + SWSS_LOG_NOTICE("Removed Tunnel next hop %s, %s|%d|%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), + nh.vni, nh.mac_address.to_string().c_str()); + return true; +} + diff --git a/orchagent/neighorch.h b/orchagent/neighorch.h index 60c40f9053..909a002c24 100644 --- a/orchagent/neighorch.h +++ b/orchagent/neighorch.h @@ -48,9 +48,13 @@ class NeighOrch : public Orch, public Subject bool getNeighborEntry(const NextHopKey&, NeighborEntry&, MacAddress&); bool getNeighborEntry(const IpAddress&, NeighborEntry&, MacAddress&); + sai_object_id_t addTunnelNextHop(const NextHopKey&); + bool removeTunnelNextHop(const NextHopKey&); + bool ifChangeInformNextHop(const string &, bool); bool isNextHopFlagSet(const NextHopKey &, const uint32_t); + bool removeOverlayNextHop(const NextHopKey &); private: IntfsOrch *m_intfsOrch; diff --git a/orchagent/nexthopgroupkey.h b/orchagent/nexthopgroupkey.h index d60aee8a32..48029c344a 100644 --- a/orchagent/nexthopgroupkey.h +++ b/orchagent/nexthopgroupkey.h @@ -11,6 +11,7 @@ class NextHopGroupKey /* ip_string@if_alias separated by ',' */ NextHopGroupKey(const std::string &nexthops) { + m_overlay_nexthops = false; auto nhv = tokenize(nexthops, NHG_DELIMITER); for (const auto &nh : nhv) { @@ -18,6 +19,18 @@ class NextHopGroupKey } } + /* ip_string|if_alias|vni|router_mac separated by ',' */ + NextHopGroupKey(const std::string &nexthops, bool overlay_nh) + { + m_overlay_nexthops = true; + auto nhv = tokenize(nexthops, NHG_DELIMITER); + for (const auto &nh_str : nhv) + { + auto nh = NextHopKey(nh_str, overlay_nh); + m_nexthops.insert(nh); + } + } + inline const std::set &getNextHops() const { return m_nexthops; @@ -124,15 +137,24 @@ class NextHopGroupKey { nhs_str += NHG_DELIMITER; } - - nhs_str += it->to_string(); + if (m_overlay_nexthops) { + nhs_str += it->to_string(m_overlay_nexthops); + } else { + nhs_str += it->to_string(); + } } return nhs_str; } + inline bool is_overlay_nexthop() const + { + return m_overlay_nexthops; + } + private: std::set m_nexthops; + bool m_overlay_nexthops; }; #endif /* SWSS_NEXTHOPGROUPKEY_H */ diff --git a/orchagent/nexthopkey.h b/orchagent/nexthopkey.h index 757436226b..1454d22618 100644 --- a/orchagent/nexthopkey.h +++ b/orchagent/nexthopkey.h @@ -13,10 +13,12 @@ struct NextHopKey { IpAddress ip_address; // neighbor IP address string alias; // incoming interface alias + uint32_t vni; // Encap VNI overlay nexthop + MacAddress mac_address; // Overlay Nexthop MAC. NextHopKey() = default; - NextHopKey(const std::string &ipstr, const std::string &alias) : ip_address(ipstr), alias(alias) {} - NextHopKey(const IpAddress &ip, const std::string &alias) : ip_address(ip), alias(alias) {} + NextHopKey(const std::string &ipstr, const std::string &alias) : ip_address(ipstr), alias(alias), vni(0), mac_address() {} + NextHopKey(const IpAddress &ip, const std::string &alias) : ip_address(ip), alias(alias), vni(0), mac_address() {} NextHopKey(const std::string &str) { if (str.find(NHG_DELIMITER) != string::npos) @@ -25,6 +27,8 @@ struct NextHopKey throw std::invalid_argument(err); } auto keys = tokenize(str, NH_DELIMITER); + vni = 0; + mac_address = MacAddress(); if (keys.size() == 1) { ip_address = keys[0]; @@ -45,19 +49,44 @@ struct NextHopKey throw std::invalid_argument(err); } } + NextHopKey(const std::string &str, bool overlay_nh) + { + if (str.find(NHG_DELIMITER) != string::npos) + { + std::string err = "Error converting " + str + " to NextHop"; + throw std::invalid_argument(err); + } + auto keys = tokenize(str, NH_DELIMITER); + if (keys.size() != 4) + { + std::string err = "Error converting " + str + " to NextHop"; + throw std::invalid_argument(err); + } + ip_address = keys[0]; + alias = keys[1]; + vni = static_cast(std::stoul(keys[2])); + mac_address = keys[3]; + } + const std::string to_string() const { return ip_address.to_string() + NH_DELIMITER + alias; } + const std::string to_string(bool overlay_nh) const + { + std::string s_vni = std::to_string(vni); + return ip_address.to_string() + NH_DELIMITER + alias + NH_DELIMITER + s_vni + NH_DELIMITER + mac_address.to_string(); + } + bool operator<(const NextHopKey &o) const { - return tie(ip_address, alias) < tie(o.ip_address, o.alias); + return tie(ip_address, alias, vni, mac_address) < tie(o.ip_address, o.alias, o.vni, o.mac_address); } bool operator==(const NextHopKey &o) const { - return (ip_address == o.ip_address) && (alias == o.alias); + return (ip_address == o.ip_address) && (alias == o.alias) && (vni == o.vni) && (mac_address == o.mac_address); } bool operator!=(const NextHopKey &o) const diff --git a/orchagent/port.h b/orchagent/port.h index 74f805ddec..504f143285 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -78,6 +78,7 @@ class Port bool m_autoneg = false; bool m_admin_state_up = false; bool m_init = false; + bool m_l3_vni = false; sai_object_id_t m_port_id = 0; sai_port_fec_mode_t m_fec_mode = SAI_PORT_FEC_MODE_NONE; VlanInfo m_vlan_info; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 4596336347..6929003762 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -3983,3 +3983,44 @@ void PortsOrch::getPortSerdesVal(const std::string& val_str, lane_values.push_back(lane_val); } } + +bool PortsOrch::updateL3VniStatus(uint16_t vlan_id, bool isUp) +{ + Port vlan; + string vlan_alias; + + vlan_alias = VLAN_PREFIX + to_string(vlan_id); + SWSS_LOG_NOTICE("update L3Vni Status for Vlan %d with isUp %d vlan %s", + vlan_id, isUp, vlan_alias.c_str()); + + if (!getPort(vlan_alias, vlan)) + { + SWSS_LOG_NOTICE("Failed to locate VLAN %d", vlan_id); + return false; + } + + SWSS_LOG_NOTICE("member count %d, l3vni %d", vlan.m_up_member_count, vlan.m_l3_vni); + if (isUp) { + auto old_count = vlan.m_up_member_count; + vlan.m_up_member_count++; + if (old_count == 0) + { + updateVlanOperStatus(vlan, true); + } + vlan.m_l3_vni = true; + } else { + vlan.m_up_member_count--; + if (vlan.m_up_member_count == 0) + { + updateVlanOperStatus(vlan, false); + } + vlan.m_l3_vni = false; + } + + m_portList[vlan_alias] = vlan; + + SWSS_LOG_NOTICE("Updated L3Vni status of VLAN %d member count %d", vlan_id, vlan.m_up_member_count); + + return true; +} + diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 34d7dfd8bf..042259a177 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -106,6 +106,7 @@ class PortsOrch : public Orch, public Subject bool addSubPort(Port &port, const string &alias, const bool &adminUp = true, const uint32_t &mtu = 0); bool removeSubPort(const string &alias); + bool updateL3VniStatus(uint16_t vlan_id, bool status); private: unique_ptr m_counterTable; unique_ptr
m_counterLagTable; diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index eb28ff824b..ed9aca26f7 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -4,6 +4,7 @@ #include "logger.h" #include "swssnet.h" #include "crmorch.h" +#include "directory.h" extern sai_object_id_t gVirtualRouterId; extern sai_object_id_t gSwitchId; @@ -14,6 +15,7 @@ extern sai_switch_api_t* sai_switch_api; extern PortsOrch *gPortsOrch; extern CrmOrch *gCrmOrch; +extern Directory gDirectory; /* Default maximum number of next hop groups */ #define DEFAULT_NUMBER_OF_ECMP_GROUPS 128 @@ -453,7 +455,10 @@ void RouteOrch::doTask(Consumer& consumer) { string ips; string aliases; + string vni_labels; + string remote_macs; bool excp_intfs_flag = false; + bool overlay_nh = false; for (auto i : kfvFieldsValues(t)) { @@ -462,9 +467,20 @@ void RouteOrch::doTask(Consumer& consumer) if (fvField(i) == "ifname") aliases = fvValue(i); + + if (fvField(i) == "vni_label") { + vni_labels = fvValue(i); + overlay_nh = true; + } + + if (fvField(i) == "router_mac") + remote_macs = fvValue(i); } + vector ipv = tokenize(ips, ','); vector alsv = tokenize(aliases, ','); + vector vni_labelv = tokenize(vni_labels, ','); + vector rmacv = tokenize(remote_macs, ','); for (auto alias : alsv) { @@ -487,13 +503,28 @@ void RouteOrch::doTask(Consumer& consumer) continue; } - string nhg_str = ipv[0] + NH_DELIMITER + alsv[0]; - for (uint32_t i = 1; i < ipv.size(); i++) - { - nhg_str += NHG_DELIMITER + ipv[i] + NH_DELIMITER + alsv[i]; - } + string nhg_str = ""; + NextHopGroupKey nhg; + + if (overlay_nh == false) { + nhg_str = ipv[0] + NH_DELIMITER + alsv[0]; + + for (uint32_t i = 1; i < ipv.size(); i++) + { + nhg_str += NHG_DELIMITER + ipv[i] + NH_DELIMITER + alsv[i]; + } + + nhg = NextHopGroupKey(nhg_str); - NextHopGroupKey nhg(nhg_str); + } else { + nhg_str = ipv[0] + NH_DELIMITER + "vni" + alsv[0] + NH_DELIMITER + vni_labelv[0] + NH_DELIMITER + rmacv[0]; + for (uint32_t i = 1; i < ipv.size(); i++) + { + nhg_str += NHG_DELIMITER + ipv[i] + NH_DELIMITER + "vni" + alsv[i] + NH_DELIMITER + vni_labelv[i] + NH_DELIMITER + rmacv[i]; + } + + nhg = NextHopGroupKey(nhg_str, overlay_nh); + } if (ipv.size() == 1 && IpAddress(ipv[0]).isZero()) { @@ -648,7 +679,14 @@ void RouteOrch::increaseNextHopRefCount(const NextHopGroupKey &nexthops) } else if (nexthops.getSize() == 1) { - NextHopKey nexthop(nexthops.to_string()); + NextHopKey nexthop; + bool overlay_nh = nexthops.is_overlay_nexthop(); + if (overlay_nh) { + nexthop = NextHopKey (nexthops.to_string(), overlay_nh); + } else { + nexthop = NextHopKey (nexthops.to_string()); + } + if (nexthop.ip_address.isZero()) m_intfsOrch->increaseRouterIntfsRefCount(nexthop.alias); else @@ -659,6 +697,7 @@ void RouteOrch::increaseNextHopRefCount(const NextHopGroupKey &nexthops) m_syncdNextHopGroups[nexthops].ref_count ++; } } + void RouteOrch::decreaseNextHopRefCount(const NextHopGroupKey &nexthops) { /* Return when there is no next hop (dropped) */ @@ -668,7 +707,14 @@ void RouteOrch::decreaseNextHopRefCount(const NextHopGroupKey &nexthops) } else if (nexthops.getSize() == 1) { - NextHopKey nexthop(nexthops.to_string()); + NextHopKey nexthop; + bool overlay_nh = nexthops.is_overlay_nexthop(); + if (overlay_nh) { + nexthop = NextHopKey (nexthops.to_string(), overlay_nh); + } else { + nexthop = NextHopKey (nexthops.to_string()); + } + if (nexthop.ip_address.isZero()) m_intfsOrch->decreaseRouterIntfsRefCount(nexthop.alias); else @@ -904,7 +950,9 @@ bool RouteOrch::addRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix, const SWSS_LOG_ENTER(); /* next_hop_id indicates the next hop id or next hop group id of this route */ - sai_object_id_t next_hop_id; + sai_object_id_t next_hop_id = SAI_NULL_OBJECT_ID; + bool overlay_nh = false; + bool status = false; if (m_syncdRoutes.find(vrf_id) == m_syncdRoutes.end()) { @@ -912,12 +960,23 @@ bool RouteOrch::addRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix, const m_vrfOrch->increaseVrfRefCount(vrf_id); } + if(nextHops.is_overlay_nexthop()) + { + overlay_nh = true; + } + auto it_route = m_syncdRoutes.at(vrf_id).find(ipPrefix); /* The route is pointing to a next hop */ if (nextHops.getSize() == 1) { - NextHopKey nexthop(nextHops.to_string()); + NextHopKey nexthop; + if (overlay_nh) { + nexthop = NextHopKey(nextHops.to_string(), overlay_nh); + } else { + nexthop = NextHopKey(nextHops.to_string()); + } + if (nexthop.ip_address.isZero()) { next_hop_id = m_intfsOrch->getRouterIntfsId(nexthop.alias); @@ -937,9 +996,23 @@ bool RouteOrch::addRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix, const } else { - SWSS_LOG_INFO("Failed to get next hop %s for %s", - nextHops.to_string().c_str(), ipPrefix.to_string().c_str()); - return false; + if(overlay_nh) + { + SWSS_LOG_NOTICE("create remote vtep %s", nexthop.to_string(overlay_nh).c_str()); + status = createRemoteVtep(vrf_id, nexthop); + if (status == false) + { + SWSS_LOG_NOTICE("Failed to create remote vtep %s", nexthop.to_string(overlay_nh).c_str()); + return false; + } + next_hop_id = m_neighOrch->addTunnelNextHop(nexthop); + } + else + { + SWSS_LOG_INFO("Failed to get next hop %s for %s", + nextHops.to_string().c_str(), ipPrefix.to_string().c_str()); + return false; + } } } } @@ -952,13 +1025,47 @@ bool RouteOrch::addRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix, const /* Try to create a new next hop group */ if (!addNextHopGroup(nextHops)) { + /* NextHopGroup is in "Ip1|alias1,Ip2|alias2,..." format*/ + std::vector nhops = tokenize(nextHops.to_string(), ','); + for(auto it = nhops.begin(); it != nhops.end(); ++it) + { + NextHopKey nextHop; + if (overlay_nh) { + nextHop = NextHopKey(*it, overlay_nh); + } else { + nextHop = NextHopKey(*it); + } + + if(!m_neighOrch->hasNextHop(nextHop)) + { + if(overlay_nh) + { + SWSS_LOG_NOTICE("create remote vtep %s ecmp", nextHop.to_string(overlay_nh).c_str()); + status = createRemoteVtep(vrf_id, nextHop); + if (status == false) + { + SWSS_LOG_NOTICE("Failed to create remote vtep %s ecmp", nextHop.to_string(overlay_nh).c_str()); + return false; + } + next_hop_id = m_neighOrch->addTunnelNextHop(nextHop); + } + } + } /* Failed to create the next hop group and check if a temporary route is needed */ /* If the current next hop is part of the next hop group to sync, * then return false and no need to add another temporary route. */ if (it_route != m_syncdRoutes.at(vrf_id).end() && it_route->second.getSize() == 1) { - NextHopKey nexthop(it_route->second.to_string()); + NextHopKey nexthop; + auto old_nextHops = it_route->second; + + if (old_nextHops.is_overlay_nexthop()) { + nexthop = NextHopKey(it_route->second.to_string(), true); + } else { + nexthop = NextHopKey(it_route->second.to_string()); + } + if (nextHops.contains(nexthop)) { return false; @@ -1021,7 +1128,7 @@ bool RouteOrch::addRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix, const /* Increase the ref_count for the next hop (group) entry */ increaseNextHopRefCount(nextHops); - SWSS_LOG_INFO("Create route %s with next hop(s) %s", + SWSS_LOG_NOTICE("Create route %s with next hop(s) %s", ipPrefix.to_string().c_str(), nextHops.to_string().c_str()); } else @@ -1059,12 +1166,37 @@ bool RouteOrch::addRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix, const increaseNextHopRefCount(nextHops); decreaseNextHopRefCount(it_route->second); + auto ol_nextHops = it_route->second; if (it_route->second.getSize() > 1 && m_syncdNextHopGroups[it_route->second].ref_count == 0) { removeNextHopGroup(it_route->second); + } else if (ol_nextHops.is_overlay_nexthop() && ( m_syncdNextHopGroups[it_route->second].ref_count == 0)){ + + SWSS_LOG_NOTICE("Update overlay Nexthop %s", nextHops.to_string().c_str()); + for (auto &tunnel_nh : ol_nextHops.getNextHops()) + { + if (!m_neighOrch->getNextHopRefCount(tunnel_nh)) + { + if(!m_neighOrch->removeTunnelNextHop(tunnel_nh)) + { + SWSS_LOG_ERROR("Tunnel Nexthop %s delete failed", it_route->second.to_string().c_str()); + } + else + { + m_neighOrch->removeOverlayNextHop(tunnel_nh); + SWSS_LOG_NOTICE("Tunnel Nexthop %s delete success", it_route->second.to_string().c_str()); + SWSS_LOG_NOTICE("delete remote vtep %s", tunnel_nh.to_string(overlay_nh).c_str()); + status = deleteRemoteVtep(vrf_id, tunnel_nh); + if (status == false) + { + SWSS_LOG_NOTICE("Failed to delete remote vtep %s ecmp", tunnel_nh.to_string(overlay_nh).c_str()); + } + } + } + } } - SWSS_LOG_INFO("Set route %s with next hop(s) %s", + SWSS_LOG_NOTICE("Set route %s with next hop(s) %s", ipPrefix.to_string().c_str(), nextHops.to_string().c_str()); } @@ -1077,6 +1209,8 @@ bool RouteOrch::addRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix, const bool RouteOrch::removeRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix) { SWSS_LOG_ENTER(); + bool status = false; + bool overlay_nh = false; auto it_route_table = m_syncdRoutes.find(vrf_id); if (it_route_table == m_syncdRoutes.end()) @@ -1155,6 +1289,34 @@ bool RouteOrch::removeRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix) * to remove the next hop group. */ decreaseNextHopRefCount(it_route->second); + + auto nextHops = it_route->second; + if(nextHops.is_overlay_nexthop()) + { + overlay_nh = true; + for (auto &tunnel_nh : nextHops.getNextHops()) + { + if (!m_neighOrch->getNextHopRefCount(tunnel_nh)) + { + if(!m_neighOrch->removeTunnelNextHop(tunnel_nh)) + { + SWSS_LOG_ERROR("Tunnel Nexthop %s delete failed", it_route->second.to_string().c_str()); + } + else + { + m_neighOrch->removeOverlayNextHop(tunnel_nh); + SWSS_LOG_NOTICE("Tunnel Nexthop %s delete success", it_route->second.to_string().c_str()); + SWSS_LOG_NOTICE("delete remote vtep %s", tunnel_nh.to_string(overlay_nh).c_str()); + status = deleteRemoteVtep(vrf_id, tunnel_nh); + if (status == false) + { + SWSS_LOG_NOTICE("Failed to delete remote vtep %s ecmp", tunnel_nh.to_string(overlay_nh).c_str()); + } + } + } + } + } + if (it_route->second.getSize() > 1 && m_syncdNextHopGroups[it_route->second].ref_count == 0) { @@ -1187,3 +1349,49 @@ bool RouteOrch::removeRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix) return true; } + +bool RouteOrch::createRemoteVtep(sai_object_id_t vrf_id, const NextHopKey &nextHop) +{ + SWSS_LOG_ENTER(); + EvpnNvoOrch* evpn_orch = gDirectory.get(); + VxlanTunnelOrch* tunnel_orch = gDirectory.get(); + bool status = false; + int ip_refcnt = -2; + + SWSS_LOG_NOTICE("Routeorch Add Remote VTEP %s, VNI %d, VR_ID %lx", + nextHop.ip_address.to_string().c_str(), nextHop.vni, vrf_id); + status = tunnel_orch->addTunnelUser(nextHop.ip_address.to_string(), nextHop.vni, 0, TNL_SRC_IP, vrf_id); + + auto vtep_ptr = evpn_orch->getEVPNVtep(); + if (vtep_ptr) + { + ip_refcnt = vtep_ptr->getDipTunnelIPRefCnt(nextHop.ip_address.to_string()); + } + SWSS_LOG_NOTICE("Routeorch Add Remote VTEP %s, VNI %d, VR_ID %lx, IP ref_cnt %d", + nextHop.ip_address.to_string().c_str(), nextHop.vni, vrf_id, ip_refcnt); + return status; +} + +bool RouteOrch::deleteRemoteVtep(sai_object_id_t vrf_id, const NextHopKey &nextHop) +{ + SWSS_LOG_ENTER(); + EvpnNvoOrch* evpn_orch = gDirectory.get(); + VxlanTunnelOrch* tunnel_orch = gDirectory.get(); + bool status = false; + int ip_refcnt = -2; + + SWSS_LOG_NOTICE("Routeorch Del Remote VTEP %s, VNI %d, VR_ID %lx", + nextHop.ip_address.to_string().c_str(), nextHop.vni, vrf_id); + status = tunnel_orch->delTunnelUser(nextHop.ip_address.to_string(), nextHop.vni, 0, TNL_SRC_IP, vrf_id); + + auto vtep_ptr = evpn_orch->getEVPNVtep(); + if (vtep_ptr) + { + ip_refcnt = vtep_ptr->getDipTunnelIPRefCnt(nextHop.ip_address.to_string()); + } + + SWSS_LOG_NOTICE("Routeorch Del Remote VTEP %s, VNI %d, VR_ID %lx, IP ref_cnt %d", + nextHop.ip_address.to_string().c_str(), nextHop.vni, vrf_id, ip_refcnt); + return status; +} + diff --git a/orchagent/routeorch.h b/orchagent/routeorch.h index 84550d7589..43b3dbccbc 100644 --- a/orchagent/routeorch.h +++ b/orchagent/routeorch.h @@ -5,6 +5,8 @@ #include "observer.h" #include "intfsorch.h" #include "neighorch.h" +#include "vnetorch.h" +#include "vxlanorch.h" #include "ipaddress.h" #include "ipaddresses.h" @@ -75,6 +77,9 @@ class RouteOrch : public Orch, public Subject bool validnexthopinNextHopGroup(const NextHopKey&); bool invalidnexthopinNextHopGroup(const NextHopKey&); + bool createRemoteVtep(sai_object_id_t, const NextHopKey&); + bool deleteRemoteVtep(sai_object_id_t, const NextHopKey&); + void notifyNextHopChangeObservers(sai_object_id_t, const IpPrefix&, const NextHopGroupKey&, bool); private: NeighOrch *m_neighOrch; diff --git a/orchagent/vnetorch.cpp b/orchagent/vnetorch.cpp index 9155d31693..cec538caf1 100644 --- a/orchagent/vnetorch.cpp +++ b/orchagent/vnetorch.cpp @@ -1653,6 +1653,11 @@ bool VNetRouteOrch::doRouteTask(const string& vnet, IpPrefix& ipP bool is_subnet = (!nh.ips.getSize() || nh.ips.contains("0.0.0.0")) ? true : false; + /*With VRF changes nexthop IP is 0.0.0.0*/ + if ((nh.ips.getSize() == 1) && (IpAddress(nh.ips.to_string()).isZero())) { + is_subnet = true; + } + Port port; if (is_subnet && (!gPortsOrch->getPort(nh.ifname, port) || (port.m_rif_id == SAI_NULL_OBJECT_ID))) { diff --git a/orchagent/vrforch.cpp b/orchagent/vrforch.cpp index 62b56e1e0e..6f04e4905e 100644 --- a/orchagent/vrforch.cpp +++ b/orchagent/vrforch.cpp @@ -10,16 +10,24 @@ #include "orch.h" #include "request_parser.h" #include "vrforch.h" +#include "vxlanorch.h" +#include "routeorch.h" +#include "directory.h" using namespace std; using namespace swss; extern sai_virtual_router_api_t* sai_virtual_router_api; extern sai_object_id_t gSwitchId; +extern Directory gDirectory; +extern PortsOrch* gPortsOrch; +extern RouteOrch* gRouteOrch; bool VRFOrch::addOperation(const Request& request) { SWSS_LOG_ENTER(); + uint32_t vni = 0; + bool error = true; sai_attribute_t attr; vector attrs; @@ -57,6 +65,11 @@ bool VRFOrch::addOperation(const Request& request) attr.id = SAI_VIRTUAL_ROUTER_ATTR_UNKNOWN_L3_MULTICAST_PACKET_ACTION; attr.value.s32 = request.getAttrPacketAction("l3_mc_action"); } + else if (name == "vni") + { + vni = static_cast(request.getAttrUint(name)); + continue; + } else { SWSS_LOG_ERROR("Logic error: Unknown attribute: %s", name.c_str()); @@ -84,6 +97,12 @@ bool VRFOrch::addOperation(const Request& request) vrf_table_[vrf_name].vrf_id = router_id; vrf_table_[vrf_name].ref_count = 0; vrf_id_table_[router_id] = vrf_name; + if (vni != 0) { + SWSS_LOG_NOTICE("VRF '%s' vni %d add", vrf_name.c_str(), vni); + error = updateVrfVNIMap(vrf_name, vni); + if (error == false) + return false; + } m_stateVrfObjectTable.hset(vrf_name, "state", "ok"); SWSS_LOG_NOTICE("VRF '%s' was added", vrf_name.c_str()); } @@ -103,6 +122,11 @@ bool VRFOrch::addOperation(const Request& request) } } + SWSS_LOG_NOTICE("VRF '%s' vni %d modify", vrf_name.c_str(), vni); + error = updateVrfVNIMap(vrf_name, vni); + if (error == false) + return false; + SWSS_LOG_NOTICE("VRF '%s' was updated", vrf_name.c_str()); } @@ -112,6 +136,7 @@ bool VRFOrch::addOperation(const Request& request) bool VRFOrch::delOperation(const Request& request) { SWSS_LOG_ENTER(); + bool error = true; const std::string& vrf_name = request.getKeyString(0); if (vrf_table_.find(vrf_name) == std::end(vrf_table_)) @@ -133,9 +158,99 @@ bool VRFOrch::delOperation(const Request& request) vrf_table_.erase(vrf_name); vrf_id_table_.erase(router_id); + error = delVrfVNIMap(vrf_name, 0); + if (error == false) + return false; m_stateVrfObjectTable.del(vrf_name); SWSS_LOG_NOTICE("VRF '%s' was removed", vrf_name.c_str()); return true; } + +bool VRFOrch::updateVrfVNIMap(const std::string& vrf_name, uint32_t vni) +{ + SWSS_LOG_ENTER(); + uint32_t old_vni = 0; + uint16_t vlan_id = 0; + EvpnNvoOrch* evpn_orch = gDirectory.get(); + VxlanTunnelOrch* tunnel_orch = gDirectory.get(); + bool error = true; + + old_vni = getVRFmappedVNI(vrf_name); + SWSS_LOG_NOTICE("VRF '%s' vni %d old_vni %d", vrf_name.c_str(), vni, old_vni); + + if (old_vni != vni) { + if (vni == 0) { + error = delVrfVNIMap(vrf_name, old_vni); + if (error == false) + return false; + } else { + //update l3vni table, if vlan/vni is received later will be able to update L3VniStatus. + l3vni_table_[vni].vlan_id = 0; + l3vni_table_[vni].l3_vni = true; + vrf_vni_map_table_[vrf_name] = vni; + auto evpn_vtep_ptr = evpn_orch->getEVPNVtep(); + if(!evpn_vtep_ptr) + { + SWSS_LOG_NOTICE("updateVrfVNIMap unable to find EVPN VTEP"); + return false; + } + + vlan_id = tunnel_orch->getVlanMappedToVni(vni); + l3vni_table_[vni].vlan_id = vlan_id; + SWSS_LOG_NOTICE("addL3VniStatus vni %d vlan %d", vni, vlan_id); + if (vlan_id != 0) + { + /*call VE UP*/ + error = gPortsOrch->updateL3VniStatus(vlan_id, true); + SWSS_LOG_NOTICE("addL3VniStatus vni %d vlan %d, status %d", vni, vlan_id, error); + } + } + SWSS_LOG_NOTICE("VRF '%s' vni %d map update", vrf_name.c_str(), vni); + } + + return true; +} + +bool VRFOrch::delVrfVNIMap(const std::string& vrf_name, uint32_t vni) +{ + SWSS_LOG_ENTER(); + bool status = true; + uint16_t vlan_id = 0; + + SWSS_LOG_NOTICE("VRF '%s' VNI %d map", vrf_name.c_str(), vni); + if (vni == 0) { + vni = getVRFmappedVNI(vrf_name); + } + + if (vni != 0) + { + vlan_id = l3vni_table_[vni].vlan_id; + SWSS_LOG_NOTICE("delL3VniStatus vni %d vlan %d", vni, vlan_id); + if (vlan_id != 0) + { + /*call VE Down*/ + status = gPortsOrch->updateL3VniStatus(vlan_id, false); + SWSS_LOG_NOTICE("delL3VniStatus vni %d vlan %d, status %d", vni, vlan_id, status); + } + l3vni_table_.erase(vni); + vrf_vni_map_table_.erase(vrf_name); + } + + SWSS_LOG_NOTICE("VRF '%s' VNI %d map removed", vrf_name.c_str(), vni); + return true; +} + +int VRFOrch::updateL3VniVlan(uint32_t vni, uint16_t vlan_id) +{ + bool status = true; + l3vni_table_[vni].vlan_id = vlan_id; + + SWSS_LOG_NOTICE("updateL3VniStatus vni %d vlan %d", vni, vlan_id); + /*call VE UP*/ + status = gPortsOrch->updateL3VniStatus(vlan_id, true); + SWSS_LOG_NOTICE("updateL3VniStatus vni %d vlan %d, status %d", vni, vlan_id, status); + + return 0; +} diff --git a/orchagent/vrforch.h b/orchagent/vrforch.h index 41ad55a003..f4007629df 100644 --- a/orchagent/vrforch.h +++ b/orchagent/vrforch.h @@ -11,8 +11,16 @@ struct VrfEntry int ref_count; }; +struct L3VNIEntry +{ + uint16_t vlan_id; + bool l3_vni; +}; + typedef std::unordered_map VRFTable; typedef std::unordered_map VRFIdNameTable; +typedef std::unordered_map VRFNameVNIMapTable; +typedef std::unordered_map L3VNITable; const request_description_t request_description = { { REQ_T_STRING }, @@ -24,6 +32,7 @@ const request_description_t request_description = { { "ip_opt_action", REQ_T_PACKET_ACTION }, { "l3_mc_action", REQ_T_PACKET_ACTION }, { "fallback", REQ_T_BOOL }, + { "vni", REQ_T_UINT } }, { } // no mandatory attributes }; @@ -109,14 +118,46 @@ class VRFOrch : public Orch2 } } + int getVrfRefCount(const std::string& name) + { + if (vrf_table_.find(name) != std::end(vrf_table_)) + return vrf_table_.at(name).ref_count; + else + return -1; + } + + uint32_t getVRFmappedVNI(const std::string& vrf_name) const + { + if (vrf_vni_map_table_.find(vrf_name) != std::end(vrf_vni_map_table_)) + { + return vrf_vni_map_table_.at(vrf_name); + } + else + { + return 0; + } + } + + int getL3VniVlan(const uint32_t vni) const + { + if (l3vni_table_.find(vni) != std::end(l3vni_table_)) + return l3vni_table_.at(vni).vlan_id; + else + return (-1); + } + int updateL3VniVlan(uint32_t vni, uint16_t vlan_id); private: virtual bool addOperation(const Request& request); virtual bool delOperation(const Request& request); + bool updateVrfVNIMap(const std::string& vrf_name, uint32_t vni); + bool delVrfVNIMap(const std::string& vrf_name, uint32_t vni); VRFTable vrf_table_; VRFIdNameTable vrf_id_table_; VRFRequest request_; + VRFNameVNIMapTable vrf_vni_map_table_; swss::Table m_stateVrfObjectTable; + L3VNITable l3vni_table_; }; #endif // __VRFORCH_H From 11cf1786d4109e80601e840b4e4e3d2f56301b69 Mon Sep 17 00:00:00 2001 From: Tapash Das Date: Wed, 24 Jun 2020 00:36:02 -0700 Subject: [PATCH 2/7] Added Unit Test Fixes and code to handle overlay ECMP group --- cfgmgr/vrfmgr.cpp | 7 ++- orchagent/neighorch.cpp | 8 +-- orchagent/routeorch.cpp | 109 +++++++++++++++++++++------------------- orchagent/routeorch.h | 1 + orchagent/vxlanorch.cpp | 60 +++++++++++++++++++++- 5 files changed, 126 insertions(+), 59 deletions(-) diff --git a/cfgmgr/vrfmgr.cpp b/cfgmgr/vrfmgr.cpp index caa5df11cd..8ce7db4025 100644 --- a/cfgmgr/vrfmgr.cpp +++ b/cfgmgr/vrfmgr.cpp @@ -273,9 +273,12 @@ void VrfMgr::doTask(Consumer &consumer) m_stateVrfTable.del(vrfName); } - if (!delLink(vrfName)) + if (consumer.getTableName() != CFG_VXLAN_EVPN_NVO_TABLE_NAME) { - SWSS_LOG_ERROR("Failed to remove vrf netdev %s", vrfName.c_str()); + if (!delLink(vrfName)) + { + SWSS_LOG_ERROR("Failed to remove vrf netdev %s", vrfName.c_str()); + } } SWSS_LOG_NOTICE("Removed vrf netdev %s", vrfName.c_str()); diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 67b6a08b5f..32b6375bf1 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -623,12 +623,12 @@ sai_object_id_t NeighOrch::addTunnelNextHop(const NextHopKey& nh) if (nh_id == SAI_NULL_OBJECT_ID) { - SWSS_LOG_ERROR("Failed to create Tunnel next hop %s, %s|%d|%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), + SWSS_LOG_ERROR("Failed to create Tunnel next hop %s, %s@%d@%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), nh.vni, nh.mac_address.to_string().c_str()); throw std::runtime_error("NH Tunnel create failed for " + tun_name + " ip " + nh.ip_address.to_string()); } - SWSS_LOG_NOTICE("Created Tunnel next hop %s, %s|%d|%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), + SWSS_LOG_NOTICE("Created Tunnel next hop %s, %s@%d@%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), nh.vni, nh.mac_address.to_string().c_str()); NextHopEntry next_hop_entry; @@ -660,12 +660,12 @@ bool NeighOrch::removeTunnelNextHop(const NextHopKey& nh) IpAddress tnl_dip = nh.ip_address; if (!vxlan_orch->removeNextHopTunnel(tun_name, tnl_dip, nh.mac_address, nh.vni)) { - SWSS_LOG_ERROR("Failed to remove Tunnel next hop %s, %s|%d|%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), + SWSS_LOG_ERROR("Failed to remove Tunnel next hop %s, %s@%d@%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), nh.vni, nh.mac_address.to_string().c_str()); return false; } - SWSS_LOG_NOTICE("Removed Tunnel next hop %s, %s|%d|%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), + SWSS_LOG_NOTICE("Removed Tunnel next hop %s, %s@%d@%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), nh.vni, nh.mac_address.to_string().c_str()); return true; } diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index ed9aca26f7..165d9ebebc 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -859,6 +859,7 @@ bool RouteOrch::removeNextHopGroup(const NextHopGroupKey &nexthops) sai_object_id_t next_hop_group_id; auto next_hop_group_entry = m_syncdNextHopGroups.find(nexthops); sai_status_t status; + bool overlay_nh = nexthops.is_overlay_nexthop(); assert(next_hop_group_entry != m_syncdNextHopGroups.end()); @@ -907,6 +908,24 @@ bool RouteOrch::removeNextHopGroup(const NextHopGroupKey &nexthops) for (auto it : next_hop_set) { m_neighOrch->decreaseNextHopRefCount(it); + if (overlay_nh && !m_neighOrch->getNextHopRefCount(it)) + { + if(!m_neighOrch->removeTunnelNextHop(it)) + { + SWSS_LOG_ERROR("Tunnel Nexthop %s delete failed", nexthops.to_string().c_str()); + } + else + { + m_neighOrch->removeOverlayNextHop(it); + SWSS_LOG_NOTICE("Tunnel Nexthop %s delete success", nexthops.to_string().c_str()); + SWSS_LOG_NOTICE("delete remote vtep %s", it.to_string(true).c_str()); + status = deleteRemoteVtep(SAI_NULL_OBJECT_ID, it); + if (status == false) + { + SWSS_LOG_NOTICE("Failed to delete remote vtep %s ecmp", it.to_string(true).c_str()); + } + } + } } m_syncdNextHopGroups.erase(nexthops); @@ -1171,30 +1190,10 @@ bool RouteOrch::addRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix, const && m_syncdNextHopGroups[it_route->second].ref_count == 0) { removeNextHopGroup(it_route->second); - } else if (ol_nextHops.is_overlay_nexthop() && ( m_syncdNextHopGroups[it_route->second].ref_count == 0)){ + } else if (ol_nextHops.is_overlay_nexthop()){ - SWSS_LOG_NOTICE("Update overlay Nexthop %s", nextHops.to_string().c_str()); - for (auto &tunnel_nh : ol_nextHops.getNextHops()) - { - if (!m_neighOrch->getNextHopRefCount(tunnel_nh)) - { - if(!m_neighOrch->removeTunnelNextHop(tunnel_nh)) - { - SWSS_LOG_ERROR("Tunnel Nexthop %s delete failed", it_route->second.to_string().c_str()); - } - else - { - m_neighOrch->removeOverlayNextHop(tunnel_nh); - SWSS_LOG_NOTICE("Tunnel Nexthop %s delete success", it_route->second.to_string().c_str()); - SWSS_LOG_NOTICE("delete remote vtep %s", tunnel_nh.to_string(overlay_nh).c_str()); - status = deleteRemoteVtep(vrf_id, tunnel_nh); - if (status == false) - { - SWSS_LOG_NOTICE("Failed to delete remote vtep %s ecmp", tunnel_nh.to_string(overlay_nh).c_str()); - } - } - } - } + SWSS_LOG_NOTICE("Update overlay Nexthop %s", ol_nextHops.to_string().c_str()); + removeOverlayNextHops(vrf_id, ol_nextHops); } SWSS_LOG_NOTICE("Set route %s with next hop(s) %s", ipPrefix.to_string().c_str(), nextHops.to_string().c_str()); @@ -1209,8 +1208,6 @@ bool RouteOrch::addRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix, const bool RouteOrch::removeRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix) { SWSS_LOG_ENTER(); - bool status = false; - bool overlay_nh = false; auto it_route_table = m_syncdRoutes.find(vrf_id); if (it_route_table == m_syncdRoutes.end()) @@ -1290,37 +1287,16 @@ bool RouteOrch::removeRoute(sai_object_id_t vrf_id, const IpPrefix &ipPrefix) */ decreaseNextHopRefCount(it_route->second); - auto nextHops = it_route->second; - if(nextHops.is_overlay_nexthop()) - { - overlay_nh = true; - for (auto &tunnel_nh : nextHops.getNextHops()) - { - if (!m_neighOrch->getNextHopRefCount(tunnel_nh)) - { - if(!m_neighOrch->removeTunnelNextHop(tunnel_nh)) - { - SWSS_LOG_ERROR("Tunnel Nexthop %s delete failed", it_route->second.to_string().c_str()); - } - else - { - m_neighOrch->removeOverlayNextHop(tunnel_nh); - SWSS_LOG_NOTICE("Tunnel Nexthop %s delete success", it_route->second.to_string().c_str()); - SWSS_LOG_NOTICE("delete remote vtep %s", tunnel_nh.to_string(overlay_nh).c_str()); - status = deleteRemoteVtep(vrf_id, tunnel_nh); - if (status == false) - { - SWSS_LOG_NOTICE("Failed to delete remote vtep %s ecmp", tunnel_nh.to_string(overlay_nh).c_str()); - } - } - } - } - } + auto ol_nextHops = it_route->second; if (it_route->second.getSize() > 1 && m_syncdNextHopGroups[it_route->second].ref_count == 0) { removeNextHopGroup(it_route->second); + } else if (ol_nextHops.is_overlay_nexthop()){ + + SWSS_LOG_NOTICE("Remove overlay Nexthop %s", ol_nextHops.to_string().c_str()); + removeOverlayNextHops(vrf_id, ol_nextHops); } SWSS_LOG_INFO("Remove route %s with next hop(s) %s", @@ -1395,3 +1371,34 @@ bool RouteOrch::deleteRemoteVtep(sai_object_id_t vrf_id, const NextHopKey &nextH return status; } +bool RouteOrch::removeOverlayNextHops(sai_object_id_t vrf_id, const NextHopGroupKey &ol_nextHops) +{ + SWSS_LOG_ENTER(); + bool status = false; + + SWSS_LOG_NOTICE("Remove overlay Nexthop %s", ol_nextHops.to_string().c_str()); + for (auto &tunnel_nh : ol_nextHops.getNextHops()) + { + if (!m_neighOrch->getNextHopRefCount(tunnel_nh)) + { + if(!m_neighOrch->removeTunnelNextHop(tunnel_nh)) + { + SWSS_LOG_ERROR("Tunnel Nexthop %s delete failed", ol_nextHops.to_string().c_str()); + } + else + { + m_neighOrch->removeOverlayNextHop(tunnel_nh); + SWSS_LOG_NOTICE("Tunnel Nexthop %s delete success", ol_nextHops.to_string().c_str()); + SWSS_LOG_NOTICE("delete remote vtep %s", tunnel_nh.to_string(true).c_str()); + status = deleteRemoteVtep(vrf_id, tunnel_nh); + if (status == false) + { + SWSS_LOG_NOTICE("Failed to delete remote vtep %s ecmp", tunnel_nh.to_string(true).c_str()); + } + } + } + } + + return true; +} + diff --git a/orchagent/routeorch.h b/orchagent/routeorch.h index 43b3dbccbc..43515f6d06 100644 --- a/orchagent/routeorch.h +++ b/orchagent/routeorch.h @@ -79,6 +79,7 @@ class RouteOrch : public Orch, public Subject bool createRemoteVtep(sai_object_id_t, const NextHopKey&); bool deleteRemoteVtep(sai_object_id_t, const NextHopKey&); + bool removeOverlayNextHops(sai_object_id_t, const NextHopGroupKey&); void notifyNextHopChangeObservers(sai_object_id_t, const IpPrefix&, const NextHopGroupKey&, bool); private: diff --git a/orchagent/vxlanorch.cpp b/orchagent/vxlanorch.cpp index e385c64c20..4bd32f9585 100644 --- a/orchagent/vxlanorch.cpp +++ b/orchagent/vxlanorch.cpp @@ -857,6 +857,8 @@ bool VxlanTunnelMapOrch::delOperation(const Request& request) return true; } +//------------------- VXLAN_VRF_MAP Table --------------------------// + bool VxlanVrfMapOrch::addOperation(const Request& request) { SWSS_LOG_ENTER(); @@ -889,9 +891,13 @@ bool VxlanVrfMapOrch::addOperation(const Request& request) string vrf_name = request.getAttrString("vrf"); VRFOrch* vrf_orch = gDirectory.get(); + SWSS_LOG_NOTICE("VRF VNI mapping '%s' update vrf %s, vni %d", + full_map_entry_name.c_str(), vrf_name.c_str(), vni_id); if (vrf_orch->isVRFexists(vrf_name)) { - tunnel_obj->createTunnel(MAP_T::VRID_TO_VNI, MAP_T::VNI_TO_VRID); + if (!tunnel_obj->isActive()) { + tunnel_obj->createTunnel(MAP_T::VRID_TO_VNI, MAP_T::VNI_TO_VRID); + } vrf_id = vrf_orch->getVRFid(vrf_name); } else @@ -908,7 +914,9 @@ bool VxlanVrfMapOrch::addOperation(const Request& request) * Create encap and decap mapper */ entry.encap_id = tunnel_obj->addEncapMapperEntry(vrf_id, vni_id); + vrf_orch->increaseVrfRefCount(vrf_name); entry.decap_id = tunnel_obj->addDecapMapperEntry(vrf_id, vni_id); + vrf_orch->increaseVrfRefCount(vrf_name); SWSS_LOG_DEBUG("Vxlan tunnel encap entry '%" PRIx64 "' decap entry '0x%" PRIx64 "'", entry.encap_id, entry.decap_id); @@ -932,7 +940,55 @@ bool VxlanVrfMapOrch::delOperation(const Request& request) { SWSS_LOG_ENTER(); - SWSS_LOG_ERROR("DEL operation is not implemented"); + VRFOrch* vrf_orch = gDirectory.get(); + const auto full_map_entry_name = request.getFullKey(); + + if (!isVrfMapExists(full_map_entry_name)) + { + SWSS_LOG_ERROR("VxlanVrfMapOrch Vxlan map '%s' do not exist", full_map_entry_name.c_str()); + return false; + } + size_t pos = full_map_entry_name.find("Vrf"); + if (pos == string::npos) { + SWSS_LOG_ERROR("VxlanVrfMapOrch no VRF in Vxlan map '%s'", full_map_entry_name.c_str()); + return false; + } + string vrf_name = full_map_entry_name.substr(pos); + + if (!vrf_orch->isVRFexists(vrf_name)) + { + SWSS_LOG_ERROR("VxlanVrfMapOrch VRF '%s' not present", vrf_name.c_str()); + return false; + } + SWSS_LOG_NOTICE("VxlanVrfMapOrch VRF VNI mapping '%s' remove vrf %s", full_map_entry_name.c_str(), vrf_name.c_str()); + vrf_map_entry_t entry; + try + { + /* + * Remove encap and decap mapper + */ + entry = vxlan_vrf_table_[full_map_entry_name]; + + SWSS_LOG_NOTICE("VxlanVrfMapOrch Vxlan tunnel VRF encap entry '%lx' decap entry '0x%lx'", + entry.encap_id, entry.decap_id); + + remove_tunnel_map_entry(entry.encap_id); + vrf_orch->decreaseVrfRefCount(vrf_name); + remove_tunnel_map_entry(entry.decap_id); + vrf_orch->decreaseVrfRefCount(vrf_name); + vxlan_vrf_table_.erase(full_map_entry_name); + vxlan_vrf_tunnel_.erase(vrf_name); + } + catch(const std::runtime_error& error) + { + SWSS_LOG_ERROR("VxlanVrfMapOrch Error removing tunnel map entry. Entry: %s. Error: %s", + full_map_entry_name.c_str(), error.what()); + return false; + } + + SWSS_LOG_NOTICE("VxlanVrfMapOrch Vxlan vrf map entry '%s' is removed. VRF Refcnt %d", full_map_entry_name.c_str(), + vrf_orch->getVrfRefCount(vrf_name)); return true; } + From b0c93421555a07d1b4d1990993590d96c7a24fca Mon Sep 17 00:00:00 2001 From: Tapash Das Date: Sat, 10 Oct 2020 00:31:11 -0700 Subject: [PATCH 3/7] Updated Review Comments. And fixed master branch merge issues. --- cfgmgr/vrfmgr.cpp | 52 ++++++++++++++++++++++++++++++----------- cfgmgr/vrfmgr.h | 10 ++------ orchagent/neighorch.cpp | 2 +- orchagent/portsorch.cpp | 28 ++++++++++++++++++---- orchagent/routeorch.cpp | 32 +++++++++++++------------ orchagent/routeorch.h | 1 - orchagent/vnetorch.cpp | 5 ---- orchagent/vrforch.cpp | 24 +++++++++---------- orchagent/vrforch.h | 12 ++++++++-- 9 files changed, 103 insertions(+), 63 deletions(-) diff --git a/cfgmgr/vrfmgr.cpp b/cfgmgr/vrfmgr.cpp index 8ce7db4025..a273a8f726 100644 --- a/cfgmgr/vrfmgr.cpp +++ b/cfgmgr/vrfmgr.cpp @@ -302,12 +302,14 @@ bool VrfMgr::doVrfEvpnNvoAddTask(const KeyOpFieldsValuesTuple & t) const auto &field = fvField(idx); const auto &value = fvValue(idx); - if (field == "source_vtep") { + if (field == "source_vtep") + { tunnel_name = value; } } - if(m_evpnVxlanTunnel.empty()) { + if (m_evpnVxlanTunnel.empty()) + { m_evpnVxlanTunnel = tunnel_name; VrfVxlanTableSync(true); } @@ -320,7 +322,8 @@ bool VrfMgr::doVrfEvpnNvoDelTask(const KeyOpFieldsValuesTuple & t) { SWSS_LOG_ENTER(); - if(!m_evpnVxlanTunnel.empty()) { + if (!m_evpnVxlanTunnel.empty()) + { VrfVxlanTableSync(false); m_evpnVxlanTunnel = ""; } @@ -344,7 +347,8 @@ bool VrfMgr::doVrfVxlanTableCreateTask(const KeyOpFieldsValuesTuple & t) const auto &field = fvField(idx); const auto &value = fvValue(idx); - if (field == "vni") { + if (field == "vni") + { s_vni = value; vni = static_cast(stoul(value)); } @@ -354,12 +358,15 @@ bool VrfMgr::doVrfVxlanTableCreateTask(const KeyOpFieldsValuesTuple & t) SWSS_LOG_NOTICE("VRF VNI map update vrf %s, vni %d, old_vni %d", vrf_name.c_str(), vni, old_vni); if (vni != old_vni) { - if (vni == 0) { - vrf_vni_map_table_.erase(vrf_name); + if (vni == 0) + { + m_vrfVniMapTable.erase(vrf_name); s_vni = to_string(old_vni); add = false; - } else { - vrf_vni_map_table_[vrf_name] = vni; + } + else + { + m_vrfVniMapTable[vrf_name] = vni; } } @@ -384,10 +391,11 @@ bool VrfMgr::doVrfVxlanTableRemoveTask(const KeyOpFieldsValuesTuple & t) vni = getVRFmappedVNI(vrf_name); SWSS_LOG_NOTICE("VRF VNI map remove vrf %s, vni %d", vrf_name.c_str(), vni); - if (vni != 0) { + if (vni != 0) + { s_vni = to_string(vni); doVrfVxlanTableUpdate(vrf_name, s_vni, false); - vrf_vni_map_table_.erase(vrf_name); + m_vrfVniMapTable.erase(vrf_name); } return true; @@ -398,7 +406,8 @@ bool VrfMgr::doVrfVxlanTableUpdate(const string& vrf_name, const string& vni, bo SWSS_LOG_ENTER(); string key; - if(m_evpnVxlanTunnel.empty()) { + if (m_evpnVxlanTunnel.empty()) + { SWSS_LOG_NOTICE("NVO Tunnel not present. vrf %s, vni %s, add %d", vrf_name.c_str(), vni.c_str(), add); return false; } @@ -412,9 +421,12 @@ bool VrfMgr::doVrfVxlanTableUpdate(const string& vrf_name, const string& vni, bo fvVector.push_back(v2); SWSS_LOG_NOTICE("VRF VNI map table update vrf %s, vni %s, add %d", vrf_name.c_str(), vni.c_str(), add); - if (add) { + if (add) + { m_appVxlanVrfTableProducer.set(key, fvVector); - } else { + } + else + { m_appVxlanVrfTableProducer.del(key); } @@ -426,7 +438,7 @@ void VrfMgr::VrfVxlanTableSync(bool add) SWSS_LOG_ENTER(); string s_vni = ""; - for (auto itr : vrf_vni_map_table_) + for (auto itr : m_vrfVniMapTable) { s_vni = to_string(itr.second); SWSS_LOG_NOTICE("vrf %s, vni %s, add %d", (itr.first).c_str(), s_vni.c_str(), add); @@ -434,3 +446,15 @@ void VrfMgr::VrfVxlanTableSync(bool add) } } +uint32_t VrfMgr::getVRFmappedVNI(const std::string& vrf_name) +{ + if (m_vrfVniMapTable.find(vrf_name) != std::end(m_vrfVniMapTable)) + { + return m_vrfVniMapTable.at(vrf_name); + } + else + { + return 0; + } +} + diff --git a/cfgmgr/vrfmgr.h b/cfgmgr/vrfmgr.h index 5973469960..5e656813bb 100644 --- a/cfgmgr/vrfmgr.h +++ b/cfgmgr/vrfmgr.h @@ -21,13 +21,7 @@ class VrfMgr : public Orch using Orch::doTask; std::string m_evpnVxlanTunnel; - uint32_t getVRFmappedVNI(const std::string& vrf_name) const - { - if (vrf_vni_map_table_.find(vrf_name) != std::end(vrf_vni_map_table_)) - return vrf_vni_map_table_.at(vrf_name); - else - return 0; - } + uint32_t getVRFmappedVNI(const std::string& vrf_name); private: bool delLink(const std::string& vrfName); @@ -46,7 +40,7 @@ class VrfMgr : public Orch std::map m_vrfTableMap; std::set m_freeTables; - VRFNameVNIMapTable vrf_vni_map_table_; + VRFNameVNIMapTable m_vrfVniMapTable; Table m_stateVrfTable, m_stateVrfObjectTable; ProducerStateTable m_appVrfTableProducer, m_appVnetTableProducer, m_appVxlanVrfTableProducer; diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 374665a554..d363ab15eb 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -630,7 +630,7 @@ sai_object_id_t NeighOrch::addTunnelNextHop(const NextHopKey& nh) { SWSS_LOG_ERROR("Failed to create Tunnel next hop %s, %s@%d@%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), nh.vni, nh.mac_address.to_string().c_str()); - throw std::runtime_error("NH Tunnel create failed for " + tun_name + " ip " + nh.ip_address.to_string()); + return nh_id; } SWSS_LOG_NOTICE("Created Tunnel next hop %s, %s@%d@%s", tun_name.c_str(), nh.ip_address.to_string().c_str(), diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index ce803d069a..fea880dc80 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -4274,28 +4274,47 @@ void PortsOrch::getPortSerdesVal(const std::string& val_str, } } +void PortsOrch::updateVlanOperStatus(const Port &vlan, bool isUp) +{ + struct PortOperStateUpdate update; + update.port = vlan; + update.up = isUp; + notify(SUBJECT_TYPE_PORT_OPER_STATE_CHANGE, &update); + + if (isUp) + { + updateDbVlanOperStatus(vlan, "up"); + } + else + { + updateDbVlanOperStatus(vlan, "down"); + } +} + +/* Bring up/down Vlan interface associated with L3 VNI*/ bool PortsOrch::updateL3VniStatus(uint16_t vlan_id, bool isUp) { Port vlan; string vlan_alias; vlan_alias = VLAN_PREFIX + to_string(vlan_id); - SWSS_LOG_NOTICE("update L3Vni Status for Vlan %d with isUp %d vlan %s", + SWSS_LOG_INFO("update L3Vni Status for Vlan %d with isUp %d vlan %s", vlan_id, isUp, vlan_alias.c_str()); if (!getPort(vlan_alias, vlan)) { - SWSS_LOG_NOTICE("Failed to locate VLAN %d", vlan_id); + SWSS_LOG_INFO("Failed to locate VLAN %d", vlan_id); return false; } - SWSS_LOG_NOTICE("member count %d, l3vni %d", vlan.m_up_member_count, vlan.m_l3_vni); + SWSS_LOG_INFO("member count %d, l3vni %d", vlan.m_up_member_count, vlan.m_l3_vni); if (isUp) { auto old_count = vlan.m_up_member_count; vlan.m_up_member_count++; if (old_count == 0) { updateVlanOperStatus(vlan, true); + vlan.m_oper_status = SAI_PORT_OPER_STATUS_UP; } vlan.m_l3_vni = true; } else { @@ -4303,13 +4322,14 @@ bool PortsOrch::updateL3VniStatus(uint16_t vlan_id, bool isUp) if (vlan.m_up_member_count == 0) { updateVlanOperStatus(vlan, false); + vlan.m_oper_status = SAI_PORT_OPER_STATUS_DOWN; } vlan.m_l3_vni = false; } m_portList[vlan_alias] = vlan; - SWSS_LOG_NOTICE("Updated L3Vni status of VLAN %d member count %d", vlan_id, vlan.m_up_member_count); + SWSS_LOG_INFO("Updated L3Vni status of VLAN %d member count %d", vlan_id, vlan.m_up_member_count); return true; } diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index 991ad9639e..dbd2957611 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -473,7 +473,7 @@ void RouteOrch::doTask(Consumer& consumer) sai_object_id_t& vrf_id = ctx.vrf_id; IpPrefix& ip_prefix = ctx.ip_prefix; - + if (!key.compare(0, strlen(VRF_PREFIX), VRF_PREFIX)) { size_t found = key.find(':'); @@ -492,20 +492,20 @@ void RouteOrch::doTask(Consumer& consumer) vrf_id = gVirtualRouterId; ip_prefix = IpPrefix(key); } - + if (op == SET_COMMAND) { string ips; string aliases; string vni_labels; string remote_macs; - bool excp_intfs_flag = false; + bool& excp_intfs_flag = ctx.excp_intfs_flag; bool overlay_nh = false; for (auto i : kfvFieldsValues(t)) { if (fvField(i) == "nexthop") - ips = fvValue(i); + ips = fvValue(i); if (fvField(i) == "ifname") aliases = fvValue(i); @@ -521,7 +521,7 @@ void RouteOrch::doTask(Consumer& consumer) vector& ipv = ctx.ipv; ipv = tokenize(ips, ','); - vector alsv = tokenize(aliases, ','); + vector alsv = tokenize(aliases, ','); vector vni_labelv = tokenize(vni_labels, ','); vector rmacv = tokenize(remote_macs, ','); @@ -578,7 +578,7 @@ void RouteOrch::doTask(Consumer& consumer) string nhg_str = ""; NextHopGroupKey& nhg = ctx.nhg; - + if (overlay_nh == false) { nhg_str = ipv[0] + NH_DELIMITER + alsv[0]; @@ -597,8 +597,8 @@ void RouteOrch::doTask(Consumer& consumer) } nhg = NextHopGroupKey(nhg_str, overlay_nh); - } - + } + if (ipv.size() == 1 && IpAddress(ipv[0]).isZero()) { /* blackhole to be done */ @@ -1425,7 +1425,13 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey /* The route is pointing to a next hop */ if (nextHops.getSize() == 1) { - NextHopKey nexthop(nextHops.to_string()); + NextHopKey nexthop; + if(nextHops.is_overlay_nexthop()) { + nexthop = NextHopKey(nextHops.to_string(), true); + } else { + nexthop = NextHopKey(nextHops.to_string()); + } + if (nexthop.ip_address.isZero()) { next_hop_id = m_intfsOrch->getRouterIntfsId(nexthop.alias); @@ -1716,10 +1722,8 @@ bool RouteOrch::createRemoteVtep(sai_object_id_t vrf_id, const NextHopKey &nextH EvpnNvoOrch* evpn_orch = gDirectory.get(); VxlanTunnelOrch* tunnel_orch = gDirectory.get(); bool status = false; - int ip_refcnt = -2; + int ip_refcnt = 0; - SWSS_LOG_NOTICE("Routeorch Add Remote VTEP %s, VNI %d, VR_ID %lx", - nextHop.ip_address.to_string().c_str(), nextHop.vni, vrf_id); status = tunnel_orch->addTunnelUser(nextHop.ip_address.to_string(), nextHop.vni, 0, TNL_SRC_IP, vrf_id); auto vtep_ptr = evpn_orch->getEVPNVtep(); @@ -1738,10 +1742,8 @@ bool RouteOrch::deleteRemoteVtep(sai_object_id_t vrf_id, const NextHopKey &nextH EvpnNvoOrch* evpn_orch = gDirectory.get(); VxlanTunnelOrch* tunnel_orch = gDirectory.get(); bool status = false; - int ip_refcnt = -2; + int ip_refcnt = 0; - SWSS_LOG_NOTICE("Routeorch Del Remote VTEP %s, VNI %d, VR_ID %lx", - nextHop.ip_address.to_string().c_str(), nextHop.vni, vrf_id); status = tunnel_orch->delTunnelUser(nextHop.ip_address.to_string(), nextHop.vni, 0, TNL_SRC_IP, vrf_id); auto vtep_ptr = evpn_orch->getEVPNVtep(); diff --git a/orchagent/routeorch.h b/orchagent/routeorch.h index d7f1a4d562..d9b4804790 100644 --- a/orchagent/routeorch.h +++ b/orchagent/routeorch.h @@ -6,7 +6,6 @@ #include "switchorch.h" #include "intfsorch.h" #include "neighorch.h" -#include "vnetorch.h" #include "vxlanorch.h" #include "ipaddress.h" diff --git a/orchagent/vnetorch.cpp b/orchagent/vnetorch.cpp index bd4b20094e..5795b0aae2 100644 --- a/orchagent/vnetorch.cpp +++ b/orchagent/vnetorch.cpp @@ -1730,11 +1730,6 @@ bool VNetRouteOrch::doRouteTask(const string& vnet, IpPrefix& ipP bool is_subnet = (!nh.ips.getSize() || nh.ips.contains("0.0.0.0")) ? true : false; - /*With VRF changes nexthop IP is 0.0.0.0*/ - if ((nh.ips.getSize() == 1) && (IpAddress(nh.ips.to_string()).isZero())) { - is_subnet = true; - } - Port port; if (is_subnet && (!gPortsOrch->getPort(nh.ifname, port) || (port.m_rif_id == SAI_NULL_OBJECT_ID))) { diff --git a/orchagent/vrforch.cpp b/orchagent/vrforch.cpp index 6f04e4905e..035399420a 100644 --- a/orchagent/vrforch.cpp +++ b/orchagent/vrforch.cpp @@ -11,7 +11,6 @@ #include "request_parser.h" #include "vrforch.h" #include "vxlanorch.h" -#include "routeorch.h" #include "directory.h" using namespace std; @@ -21,7 +20,6 @@ extern sai_virtual_router_api_t* sai_virtual_router_api; extern sai_object_id_t gSwitchId; extern Directory gDirectory; extern PortsOrch* gPortsOrch; -extern RouteOrch* gRouteOrch; bool VRFOrch::addOperation(const Request& request) { @@ -178,7 +176,7 @@ bool VRFOrch::updateVrfVNIMap(const std::string& vrf_name, uint32_t vni) bool error = true; old_vni = getVRFmappedVNI(vrf_name); - SWSS_LOG_NOTICE("VRF '%s' vni %d old_vni %d", vrf_name.c_str(), vni, old_vni); + SWSS_LOG_INFO("VRF '%s' vni %d old_vni %d", vrf_name.c_str(), vni, old_vni); if (old_vni != vni) { if (vni == 0) { @@ -189,7 +187,6 @@ bool VRFOrch::updateVrfVNIMap(const std::string& vrf_name, uint32_t vni) //update l3vni table, if vlan/vni is received later will be able to update L3VniStatus. l3vni_table_[vni].vlan_id = 0; l3vni_table_[vni].l3_vni = true; - vrf_vni_map_table_[vrf_name] = vni; auto evpn_vtep_ptr = evpn_orch->getEVPNVtep(); if(!evpn_vtep_ptr) { @@ -197,17 +194,18 @@ bool VRFOrch::updateVrfVNIMap(const std::string& vrf_name, uint32_t vni) return false; } + vrf_vni_map_table_[vrf_name] = vni; vlan_id = tunnel_orch->getVlanMappedToVni(vni); l3vni_table_[vni].vlan_id = vlan_id; - SWSS_LOG_NOTICE("addL3VniStatus vni %d vlan %d", vni, vlan_id); + SWSS_LOG_INFO("addL3VniStatus vni %d vlan %d", vni, vlan_id); if (vlan_id != 0) { /*call VE UP*/ error = gPortsOrch->updateL3VniStatus(vlan_id, true); - SWSS_LOG_NOTICE("addL3VniStatus vni %d vlan %d, status %d", vni, vlan_id, error); + SWSS_LOG_INFO("addL3VniStatus vni %d vlan %d, status %d", vni, vlan_id, error); } } - SWSS_LOG_NOTICE("VRF '%s' vni %d map update", vrf_name.c_str(), vni); + SWSS_LOG_INFO("VRF '%s' vni %d map update", vrf_name.c_str(), vni); } return true; @@ -219,7 +217,7 @@ bool VRFOrch::delVrfVNIMap(const std::string& vrf_name, uint32_t vni) bool status = true; uint16_t vlan_id = 0; - SWSS_LOG_NOTICE("VRF '%s' VNI %d map", vrf_name.c_str(), vni); + SWSS_LOG_INFO("VRF '%s' VNI %d map", vrf_name.c_str(), vni); if (vni == 0) { vni = getVRFmappedVNI(vrf_name); } @@ -227,18 +225,18 @@ bool VRFOrch::delVrfVNIMap(const std::string& vrf_name, uint32_t vni) if (vni != 0) { vlan_id = l3vni_table_[vni].vlan_id; - SWSS_LOG_NOTICE("delL3VniStatus vni %d vlan %d", vni, vlan_id); + SWSS_LOG_INFO("delL3VniStatus vni %d vlan %d", vni, vlan_id); if (vlan_id != 0) { /*call VE Down*/ status = gPortsOrch->updateL3VniStatus(vlan_id, false); - SWSS_LOG_NOTICE("delL3VniStatus vni %d vlan %d, status %d", vni, vlan_id, status); + SWSS_LOG_INFO("delL3VniStatus vni %d vlan %d, status %d", vni, vlan_id, status); } l3vni_table_.erase(vni); vrf_vni_map_table_.erase(vrf_name); } - SWSS_LOG_NOTICE("VRF '%s' VNI %d map removed", vrf_name.c_str(), vni); + SWSS_LOG_INFO("VRF '%s' VNI %d map removed", vrf_name.c_str(), vni); return true; } @@ -247,10 +245,10 @@ int VRFOrch::updateL3VniVlan(uint32_t vni, uint16_t vlan_id) bool status = true; l3vni_table_[vni].vlan_id = vlan_id; - SWSS_LOG_NOTICE("updateL3VniStatus vni %d vlan %d", vni, vlan_id); + SWSS_LOG_INFO("updateL3VniStatus vni %d vlan %d", vni, vlan_id); /*call VE UP*/ status = gPortsOrch->updateL3VniStatus(vlan_id, true); - SWSS_LOG_NOTICE("updateL3VniStatus vni %d vlan %d, status %d", vni, vlan_id, status); + SWSS_LOG_INFO("updateL3VniStatus vni %d vlan %d, status %d", vni, vlan_id, status); return 0; } diff --git a/orchagent/vrforch.h b/orchagent/vrforch.h index f4007629df..68a871b809 100644 --- a/orchagent/vrforch.h +++ b/orchagent/vrforch.h @@ -11,7 +11,7 @@ struct VrfEntry int ref_count; }; -struct L3VNIEntry +struct VNIEntry { uint16_t vlan_id; bool l3_vni; @@ -20,7 +20,7 @@ struct L3VNIEntry typedef std::unordered_map VRFTable; typedef std::unordered_map VRFIdNameTable; typedef std::unordered_map VRFNameVNIMapTable; -typedef std::unordered_map L3VNITable; +typedef std::unordered_map L3VNITable; const request_description_t request_description = { { REQ_T_STRING }, @@ -121,9 +121,13 @@ class VRFOrch : public Orch2 int getVrfRefCount(const std::string& name) { if (vrf_table_.find(name) != std::end(vrf_table_)) + { return vrf_table_.at(name).ref_count; + } else + { return -1; + } } uint32_t getVRFmappedVNI(const std::string& vrf_name) const @@ -141,9 +145,13 @@ class VRFOrch : public Orch2 int getL3VniVlan(const uint32_t vni) const { if (l3vni_table_.find(vni) != std::end(l3vni_table_)) + { return l3vni_table_.at(vni).vlan_id; + } else + { return (-1); + } } int updateL3VniVlan(uint32_t vni, uint16_t vlan_id); private: From 8419ee26663dfdc388be7054f650602a13697721 Mon Sep 17 00:00:00 2001 From: Tapash Das Date: Wed, 2 Dec 2020 06:35:50 -0800 Subject: [PATCH 4/7] Resolved Code merge issues after PR1264 merge. --- orchagent/port.h | 1 + orchagent/portsorch.cpp | 2 ++ orchagent/portsorch.h | 1 + orchagent/routeorch.cpp | 4 ++-- 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/orchagent/port.h b/orchagent/port.h index 66331e86b7..8462aca120 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -109,6 +109,7 @@ class Port uint32_t m_nat_zone_id = 0; uint32_t m_vnid = VNID_NONE; uint32_t m_fdb_count = 0; + uint32_t m_up_member_count = 0; /* * Following two bit vectors are used to lock diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index f057fa7c4b..32ccb4afd5 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -4387,6 +4387,7 @@ void PortsOrch::getPortSerdesVal(const std::string& val_str, void PortsOrch::updateVlanOperStatus(const Port &vlan, bool isUp) { +#if 0 /* Dependent on PR 1275 */ struct PortOperStateUpdate update; update.port = vlan; update.up = isUp; @@ -4400,6 +4401,7 @@ void PortsOrch::updateVlanOperStatus(const Port &vlan, bool isUp) { updateDbVlanOperStatus(vlan, "down"); } +#endif } /* Bring up/down Vlan interface associated with L3 VNI*/ diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 42cb7278b8..1e8ce2d219 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -283,6 +283,7 @@ class PortsOrch : public Orch, public Subject sai_acl_bind_point_type_t &sai_acl_bind_type); void initGearbox(); bool initGearboxPort(Port &port); + void updateVlanOperStatus(const Port &vlan, bool isUp); }; #endif /* SWSS_PORTSORCH_H */ diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index 22b0aab6ad..a4ca2f186a 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -1730,7 +1730,7 @@ bool RouteOrch::createRemoteVtep(sai_object_id_t vrf_id, const NextHopKey &nextH bool status = false; int ip_refcnt = 0; - status = tunnel_orch->addTunnelUser(nextHop.ip_address.to_string(), nextHop.vni, 0, TNL_SRC_IP, vrf_id); + status = tunnel_orch->addTunnelUser(nextHop.ip_address.to_string(), nextHop.vni, 0, TUNNEL_USER_IP, vrf_id); auto vtep_ptr = evpn_orch->getEVPNVtep(); if (vtep_ptr) @@ -1750,7 +1750,7 @@ bool RouteOrch::deleteRemoteVtep(sai_object_id_t vrf_id, const NextHopKey &nextH bool status = false; int ip_refcnt = 0; - status = tunnel_orch->delTunnelUser(nextHop.ip_address.to_string(), nextHop.vni, 0, TNL_SRC_IP, vrf_id); + status = tunnel_orch->delTunnelUser(nextHop.ip_address.to_string(), nextHop.vni, 0, TUNNEL_USER_IP, vrf_id); auto vtep_ptr = evpn_orch->getEVPNVtep(); if (vtep_ptr) From cd8c5df31ec70690bff086b999993342f3c6d005 Mon Sep 17 00:00:00 2001 From: Tapash Das Date: Sun, 6 Dec 2020 01:05:17 -0800 Subject: [PATCH 5/7] Fix for mirror VS test failures. --- orchagent/mirrororch.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/orchagent/mirrororch.cpp b/orchagent/mirrororch.cpp index 1abb522e21..b9656ba216 100644 --- a/orchagent/mirrororch.cpp +++ b/orchagent/mirrororch.cpp @@ -64,8 +64,9 @@ MirrorEntry::MirrorEntry(const string& platform) : greType = 0x88be; } + string alias = ""; nexthopInfo.prefix = IpPrefix("0.0.0.0/0"); - nexthopInfo.nexthop = NextHopKey("0.0.0.0", ""); + nexthopInfo.nexthop = NextHopKey("0.0.0.0", alias); } MirrorOrch::MirrorOrch(TableConnector stateDbConnector, TableConnector confDbConnector, @@ -1203,7 +1204,8 @@ void MirrorOrch::updateNextHop(const NextHopUpdate& update) } else { - session.nexthopInfo.nexthop = NextHopKey("0.0.0.0", ""); + string alias = ""; + session.nexthopInfo.nexthop = NextHopKey("0.0.0.0", alias); } // Update State DB Nexthop From f542d8f3ded676d5ddc08cf2f38fdb3a5f3f2315 Mon Sep 17 00:00:00 2001 From: Tapash Das Date: Mon, 14 Dec 2020 21:41:13 -0800 Subject: [PATCH 6/7] Updated Review Comments. --- cfgmgr/vrfmgr.cpp | 43 ++++++++++++++++++++++++++++++++--------- orchagent/portsorch.cpp | 23 ++-------------------- orchagent/portsorch.h | 1 - orchagent/routeorch.cpp | 35 +++++++++++++++++++++------------ orchagent/vrforch.cpp | 4 ++-- 5 files changed, 61 insertions(+), 45 deletions(-) diff --git a/cfgmgr/vrfmgr.cpp b/cfgmgr/vrfmgr.cpp index a273a8f726..d9164f47c5 100644 --- a/cfgmgr/vrfmgr.cpp +++ b/cfgmgr/vrfmgr.cpp @@ -215,6 +215,7 @@ void VrfMgr::doTask(Consumer &consumer) SWSS_LOG_ERROR("Failed to create vrf netdev %s", vrfName.c_str()); } + bool status = true; vector fvVector; fvVector.emplace_back("state", "ok"); m_stateVrfTable.set(vrfName, fvVector); @@ -222,9 +223,16 @@ void VrfMgr::doTask(Consumer &consumer) SWSS_LOG_NOTICE("Created vrf netdev %s", vrfName.c_str()); if (consumer.getTableName() == CFG_VRF_TABLE_NAME) { + status = doVrfVxlanTableCreateTask (t); + if (status == false) + { + SWSS_LOG_ERROR("VRF VNI Map Config Failed"); + it = consumer.m_toSync.erase(it); + continue; + } + m_appVrfTableProducer.set(vrfName, kfvFieldsValues(t)); - doVrfVxlanTableCreateTask (t); } else { @@ -314,7 +322,7 @@ bool VrfMgr::doVrfEvpnNvoAddTask(const KeyOpFieldsValuesTuple & t) VrfVxlanTableSync(true); } - SWSS_LOG_NOTICE("Added evpn nvo tunnel %s", m_evpnVxlanTunnel.c_str()); + SWSS_LOG_INFO("Added evpn nvo tunnel %s", m_evpnVxlanTunnel.c_str()); return true; } @@ -328,7 +336,7 @@ bool VrfMgr::doVrfEvpnNvoDelTask(const KeyOpFieldsValuesTuple & t) m_evpnVxlanTunnel = ""; } - SWSS_LOG_NOTICE("Removed evpn nvo tunnel %s", m_evpnVxlanTunnel.c_str()); + SWSS_LOG_INFO("Removed evpn nvo tunnel %s", m_evpnVxlanTunnel.c_str()); return true; } @@ -354,8 +362,20 @@ bool VrfMgr::doVrfVxlanTableCreateTask(const KeyOpFieldsValuesTuple & t) } } + if (vni != 0) + { + for (auto itr : m_vrfVniMapTable) + { + if (vni == itr.second) + { + SWSS_LOG_ERROR(" vni %d is already mapped to vrf %s", vni, itr.first.c_str()); + return false; + } + } + } + old_vni = getVRFmappedVNI(vrf_name); - SWSS_LOG_NOTICE("VRF VNI map update vrf %s, vni %d, old_vni %d", vrf_name.c_str(), vni, old_vni); + SWSS_LOG_INFO("VRF VNI map update vrf %s, vni %d, old_vni %d", vrf_name.c_str(), vni, old_vni); if (vni != old_vni) { if (vni == 0) @@ -366,6 +386,11 @@ bool VrfMgr::doVrfVxlanTableCreateTask(const KeyOpFieldsValuesTuple & t) } else { + if (old_vni != 0) + { + SWSS_LOG_ERROR(" vrf %s is already mapped to vni %d", vrf_name.c_str(), old_vni); + return false; + } m_vrfVniMapTable[vrf_name] = vni; } @@ -376,7 +401,7 @@ bool VrfMgr::doVrfVxlanTableCreateTask(const KeyOpFieldsValuesTuple & t) return true; } - SWSS_LOG_NOTICE("VRF VNI map update vrf %s, s_vni %s, add %d", vrf_name.c_str(), s_vni.c_str(), add); + SWSS_LOG_INFO("VRF VNI map update vrf %s, s_vni %s, add %d", vrf_name.c_str(), s_vni.c_str(), add); doVrfVxlanTableUpdate(vrf_name, s_vni, add); return true; } @@ -390,7 +415,7 @@ bool VrfMgr::doVrfVxlanTableRemoveTask(const KeyOpFieldsValuesTuple & t) string s_vni = ""; vni = getVRFmappedVNI(vrf_name); - SWSS_LOG_NOTICE("VRF VNI map remove vrf %s, vni %d", vrf_name.c_str(), vni); + SWSS_LOG_INFO("VRF VNI map remove vrf %s, vni %d", vrf_name.c_str(), vni); if (vni != 0) { s_vni = to_string(vni); @@ -408,7 +433,7 @@ bool VrfMgr::doVrfVxlanTableUpdate(const string& vrf_name, const string& vni, bo if (m_evpnVxlanTunnel.empty()) { - SWSS_LOG_NOTICE("NVO Tunnel not present. vrf %s, vni %s, add %d", vrf_name.c_str(), vni.c_str(), add); + SWSS_LOG_INFO("NVO Tunnel not present. vrf %s, vni %s, add %d", vrf_name.c_str(), vni.c_str(), add); return false; } @@ -420,7 +445,7 @@ bool VrfMgr::doVrfVxlanTableUpdate(const string& vrf_name, const string& vni, bo fvVector.push_back(v1); fvVector.push_back(v2); - SWSS_LOG_NOTICE("VRF VNI map table update vrf %s, vni %s, add %d", vrf_name.c_str(), vni.c_str(), add); + SWSS_LOG_INFO("VRF VNI map table update vrf %s, vni %s, add %d", vrf_name.c_str(), vni.c_str(), add); if (add) { m_appVxlanVrfTableProducer.set(key, fvVector); @@ -441,7 +466,7 @@ void VrfMgr::VrfVxlanTableSync(bool add) for (auto itr : m_vrfVniMapTable) { s_vni = to_string(itr.second); - SWSS_LOG_NOTICE("vrf %s, vni %s, add %d", (itr.first).c_str(), s_vni.c_str(), add); + SWSS_LOG_INFO("vrf %s, vni %s, add %d", (itr.first).c_str(), s_vni.c_str(), add); doVrfVxlanTableUpdate(itr.first, s_vni, add); } } diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 32ccb4afd5..3afa68e24d 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -4385,25 +4385,6 @@ void PortsOrch::getPortSerdesVal(const std::string& val_str, } } -void PortsOrch::updateVlanOperStatus(const Port &vlan, bool isUp) -{ -#if 0 /* Dependent on PR 1275 */ - struct PortOperStateUpdate update; - update.port = vlan; - update.up = isUp; - notify(SUBJECT_TYPE_PORT_OPER_STATE_CHANGE, &update); - - if (isUp) - { - updateDbVlanOperStatus(vlan, "up"); - } - else - { - updateDbVlanOperStatus(vlan, "down"); - } -#endif -} - /* Bring up/down Vlan interface associated with L3 VNI*/ bool PortsOrch::updateL3VniStatus(uint16_t vlan_id, bool isUp) { @@ -4426,7 +4407,7 @@ bool PortsOrch::updateL3VniStatus(uint16_t vlan_id, bool isUp) vlan.m_up_member_count++; if (old_count == 0) { - updateVlanOperStatus(vlan, true); + /* updateVlanOperStatus(vlan, true); */ /* TBD */ vlan.m_oper_status = SAI_PORT_OPER_STATUS_UP; } vlan.m_l3_vni = true; @@ -4434,7 +4415,7 @@ bool PortsOrch::updateL3VniStatus(uint16_t vlan_id, bool isUp) vlan.m_up_member_count--; if (vlan.m_up_member_count == 0) { - updateVlanOperStatus(vlan, false); + /* updateVlanOperStatus(vlan, false); */ /* TBD */ vlan.m_oper_status = SAI_PORT_OPER_STATUS_DOWN; } vlan.m_l3_vni = false; diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 1e8ce2d219..42cb7278b8 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -283,7 +283,6 @@ class PortsOrch : public Orch, public Subject sai_acl_bind_point_type_t &sai_acl_bind_type); void initGearbox(); bool initGearboxPort(Port &port); - void updateVlanOperStatus(const Port &vlan, bool isUp); }; #endif /* SWSS_PORTSORCH_H */ diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index a4ca2f186a..1222dbb0af 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -1160,12 +1160,12 @@ bool RouteOrch::removeNextHopGroup(const NextHopGroupKey &nexthops) else { m_neighOrch->removeOverlayNextHop(it); - SWSS_LOG_NOTICE("Tunnel Nexthop %s delete success", nexthops.to_string().c_str()); - SWSS_LOG_NOTICE("delete remote vtep %s", it.to_string(true).c_str()); + SWSS_LOG_INFO("Tunnel Nexthop %s delete success", nexthops.to_string().c_str()); + SWSS_LOG_INFO("delete remote vtep %s", it.to_string(true).c_str()); status = deleteRemoteVtep(SAI_NULL_OBJECT_ID, it); if (status == false) { - SWSS_LOG_NOTICE("Failed to delete remote vtep %s ecmp", it.to_string(true).c_str()); + SWSS_LOG_ERROR("Failed to delete remote vtep %s ecmp", it.to_string(true).c_str()); } } } @@ -1276,14 +1276,19 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) { if(overlay_nh) { - SWSS_LOG_NOTICE("create remote vtep %s", nexthop.to_string(overlay_nh).c_str()); + SWSS_LOG_INFO("create remote vtep %s", nexthop.to_string(overlay_nh).c_str()); status = createRemoteVtep(vrf_id, nexthop); if (status == false) { - SWSS_LOG_NOTICE("Failed to create remote vtep %s", nexthop.to_string(overlay_nh).c_str()); + SWSS_LOG_ERROR("Failed to create remote vtep %s", nexthop.to_string(overlay_nh).c_str()); return false; } next_hop_id = m_neighOrch->addTunnelNextHop(nexthop); + if (next_hop_id == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_ERROR("Failed to create Tunnel Nexthop %s", nexthop.to_string(overlay_nh).c_str()); + return false; + } } else { @@ -1318,14 +1323,19 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) { if(overlay_nh) { - SWSS_LOG_NOTICE("create remote vtep %s ecmp", nextHop.to_string(overlay_nh).c_str()); + SWSS_LOG_INFO("create remote vtep %s ecmp", nextHop.to_string(overlay_nh).c_str()); status = createRemoteVtep(vrf_id, nextHop); if (status == false) { - SWSS_LOG_NOTICE("Failed to create remote vtep %s ecmp", nextHop.to_string(overlay_nh).c_str()); + SWSS_LOG_ERROR("Failed to create remote vtep %s ecmp", nextHop.to_string(overlay_nh).c_str()); return false; } next_hop_id = m_neighOrch->addTunnelNextHop(nextHop); + if (next_hop_id == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_ERROR("Failed to create Tunnel Nexthop %s", nextHop.to_string(overlay_nh).c_str()); + return false; + } } } } @@ -1737,7 +1747,7 @@ bool RouteOrch::createRemoteVtep(sai_object_id_t vrf_id, const NextHopKey &nextH { ip_refcnt = vtep_ptr->getDipTunnelIPRefCnt(nextHop.ip_address.to_string()); } - SWSS_LOG_NOTICE("Routeorch Add Remote VTEP %s, VNI %d, VR_ID %lx, IP ref_cnt %d", + SWSS_LOG_INFO("Routeorch Add Remote VTEP %s, VNI %d, VR_ID %lx, IP ref_cnt %d", nextHop.ip_address.to_string().c_str(), nextHop.vni, vrf_id, ip_refcnt); return status; } @@ -1758,7 +1768,7 @@ bool RouteOrch::deleteRemoteVtep(sai_object_id_t vrf_id, const NextHopKey &nextH ip_refcnt = vtep_ptr->getDipTunnelIPRefCnt(nextHop.ip_address.to_string()); } - SWSS_LOG_NOTICE("Routeorch Del Remote VTEP %s, VNI %d, VR_ID %lx, IP ref_cnt %d", + SWSS_LOG_INFO("Routeorch Del Remote VTEP %s, VNI %d, VR_ID %lx, IP ref_cnt %d", nextHop.ip_address.to_string().c_str(), nextHop.vni, vrf_id, ip_refcnt); return status; } @@ -1780,12 +1790,13 @@ bool RouteOrch::removeOverlayNextHops(sai_object_id_t vrf_id, const NextHopGroup else { m_neighOrch->removeOverlayNextHop(tunnel_nh); - SWSS_LOG_NOTICE("Tunnel Nexthop %s delete success", ol_nextHops.to_string().c_str()); - SWSS_LOG_NOTICE("delete remote vtep %s", tunnel_nh.to_string(true).c_str()); + SWSS_LOG_INFO("Tunnel Nexthop %s delete success", ol_nextHops.to_string().c_str()); + SWSS_LOG_INFO("delete remote vtep %s", tunnel_nh.to_string(true).c_str()); status = deleteRemoteVtep(vrf_id, tunnel_nh); if (status == false) { - SWSS_LOG_NOTICE("Failed to delete remote vtep %s ecmp", tunnel_nh.to_string(true).c_str()); + SWSS_LOG_ERROR("Failed to delete remote vtep %s ecmp", tunnel_nh.to_string(true).c_str()); + return false; } } } diff --git a/orchagent/vrforch.cpp b/orchagent/vrforch.cpp index 035399420a..a87a822ddc 100644 --- a/orchagent/vrforch.cpp +++ b/orchagent/vrforch.cpp @@ -96,7 +96,7 @@ bool VRFOrch::addOperation(const Request& request) vrf_table_[vrf_name].ref_count = 0; vrf_id_table_[router_id] = vrf_name; if (vni != 0) { - SWSS_LOG_NOTICE("VRF '%s' vni %d add", vrf_name.c_str(), vni); + SWSS_LOG_INFO("VRF '%s' vni %d add", vrf_name.c_str(), vni); error = updateVrfVNIMap(vrf_name, vni); if (error == false) return false; @@ -120,7 +120,7 @@ bool VRFOrch::addOperation(const Request& request) } } - SWSS_LOG_NOTICE("VRF '%s' vni %d modify", vrf_name.c_str(), vni); + SWSS_LOG_INFO("VRF '%s' vni %d modify", vrf_name.c_str(), vni); error = updateVrfVNIMap(vrf_name, vni); if (error == false) return false; From c9ccfe41f83093ed8d42c5f4fadd666c5d91def8 Mon Sep 17 00:00:00 2001 From: Tapash Das Date: Thu, 17 Dec 2020 22:46:04 -0800 Subject: [PATCH 7/7] Updated review comments. --- orchagent/nexthopkey.h | 4 ++-- orchagent/routeorch.cpp | 37 ++++++++++++++++++++++++++----------- orchagent/vrforch.cpp | 17 ++++++++++++++--- orchagent/vxlanorch.cpp | 2 +- 4 files changed, 43 insertions(+), 17 deletions(-) diff --git a/orchagent/nexthopkey.h b/orchagent/nexthopkey.h index 1454d22618..69a94505ae 100644 --- a/orchagent/nexthopkey.h +++ b/orchagent/nexthopkey.h @@ -50,7 +50,7 @@ struct NextHopKey } } NextHopKey(const std::string &str, bool overlay_nh) - { + { if (str.find(NHG_DELIMITER) != string::npos) { std::string err = "Error converting " + str + " to NextHop"; @@ -74,7 +74,7 @@ struct NextHopKey } const std::string to_string(bool overlay_nh) const - { + { std::string s_vni = std::to_string(vni); return ip_address.to_string() + NH_DELIMITER + alias + NH_DELIMITER + s_vni + NH_DELIMITER + mac_address.to_string(); } diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index 1222dbb0af..03efd31543 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -579,7 +579,8 @@ void RouteOrch::doTask(Consumer& consumer) string nhg_str = ""; NextHopGroupKey& nhg = ctx.nhg; - if (overlay_nh == false) { + if (overlay_nh == false) + { nhg_str = ipv[0] + NH_DELIMITER + alsv[0]; for (uint32_t i = 1; i < ipv.size(); i++) @@ -589,7 +590,9 @@ void RouteOrch::doTask(Consumer& consumer) nhg = NextHopGroupKey(nhg_str); - } else { + } + else + { nhg_str = ipv[0] + NH_DELIMITER + "vni" + alsv[0] + NH_DELIMITER + vni_labelv[0] + NH_DELIMITER + rmacv[0]; for (uint32_t i = 1; i < ipv.size(); i++) { @@ -843,9 +846,12 @@ void RouteOrch::increaseNextHopRefCount(const NextHopGroupKey &nexthops) { NextHopKey nexthop; bool overlay_nh = nexthops.is_overlay_nexthop(); - if (overlay_nh) { + if (overlay_nh) + { nexthop = NextHopKey (nexthops.to_string(), overlay_nh); - } else { + } + else + { nexthop = NextHopKey (nexthops.to_string()); } @@ -871,9 +877,12 @@ void RouteOrch::decreaseNextHopRefCount(const NextHopGroupKey &nexthops) { NextHopKey nexthop; bool overlay_nh = nexthops.is_overlay_nexthop(); - if (overlay_nh) { + if (overlay_nh) + { nexthop = NextHopKey (nexthops.to_string(), overlay_nh); - } else { + } + else + { nexthop = NextHopKey (nexthops.to_string()); } @@ -1238,7 +1247,7 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) m_vrfOrch->increaseVrfRefCount(vrf_id); } - if(nextHops.is_overlay_nexthop()) + if (nextHops.is_overlay_nexthop()) { overlay_nh = true; } @@ -1249,9 +1258,12 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) if (nextHops.getSize() == 1) { NextHopKey nexthop; - if (overlay_nh) { + if (overlay_nh) + { nexthop = NextHopKey(nextHops.to_string(), overlay_nh); - } else { + } + else + { nexthop = NextHopKey(nextHops.to_string()); } @@ -1313,9 +1325,12 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) for(auto it = nhops.begin(); it != nhops.end(); ++it) { NextHopKey nextHop; - if (overlay_nh) { + if (overlay_nh) + { nextHop = NextHopKey(*it, overlay_nh); - } else { + } + else + { nextHop = NextHopKey(*it); } diff --git a/orchagent/vrforch.cpp b/orchagent/vrforch.cpp index a87a822ddc..ee0d9d4ac2 100644 --- a/orchagent/vrforch.cpp +++ b/orchagent/vrforch.cpp @@ -95,11 +95,14 @@ bool VRFOrch::addOperation(const Request& request) vrf_table_[vrf_name].vrf_id = router_id; vrf_table_[vrf_name].ref_count = 0; vrf_id_table_[router_id] = vrf_name; - if (vni != 0) { + if (vni != 0) + { SWSS_LOG_INFO("VRF '%s' vni %d add", vrf_name.c_str(), vni); error = updateVrfVNIMap(vrf_name, vni); if (error == false) + { return false; + } } m_stateVrfObjectTable.hset(vrf_name, "state", "ok"); SWSS_LOG_NOTICE("VRF '%s' was added", vrf_name.c_str()); @@ -123,7 +126,9 @@ bool VRFOrch::addOperation(const Request& request) SWSS_LOG_INFO("VRF '%s' vni %d modify", vrf_name.c_str(), vni); error = updateVrfVNIMap(vrf_name, vni); if (error == false) + { return false; + } SWSS_LOG_NOTICE("VRF '%s' was updated", vrf_name.c_str()); } @@ -158,7 +163,9 @@ bool VRFOrch::delOperation(const Request& request) vrf_id_table_.erase(router_id); error = delVrfVNIMap(vrf_name, 0); if (error == false) + { return false; + } m_stateVrfObjectTable.del(vrf_name); SWSS_LOG_NOTICE("VRF '%s' was removed", vrf_name.c_str()); @@ -178,11 +185,15 @@ bool VRFOrch::updateVrfVNIMap(const std::string& vrf_name, uint32_t vni) old_vni = getVRFmappedVNI(vrf_name); SWSS_LOG_INFO("VRF '%s' vni %d old_vni %d", vrf_name.c_str(), vni, old_vni); - if (old_vni != vni) { - if (vni == 0) { + if (old_vni != vni) + { + if (vni == 0) + { error = delVrfVNIMap(vrf_name, old_vni); if (error == false) + { return false; + } } else { //update l3vni table, if vlan/vni is received later will be able to update L3VniStatus. l3vni_table_[vni].vlan_id = 0; diff --git a/orchagent/vxlanorch.cpp b/orchagent/vxlanorch.cpp index aff3bbf454..4f20d7d60b 100644 --- a/orchagent/vxlanorch.cpp +++ b/orchagent/vxlanorch.cpp @@ -2134,7 +2134,7 @@ bool EvpnNvoOrch::addOperation(const Request& request) source_vtep_ptr = tunnel_orch->getVxlanTunnel(vtep_name); SWSS_LOG_INFO("evpnnvo: %s vtep : %s \n",nvo_name.c_str(), vtep_name.c_str()); - + return true; }