From 882ccc6d464eb4a2acbb0afbed55d14af670287f Mon Sep 17 00:00:00 2001 From: Marian Pritsak Date: Thu, 24 Jan 2019 23:10:12 +0200 Subject: [PATCH] [vnetorch] Change logic for adding VNet interface (#761) * [vnetorch] Change logic for adding VNet interface Signed-off-by: Marian Pritsak * Insert ipprefix only if exists Signed-off-by: Marian Pritsak --- orchagent/intfsorch.cpp | 137 +++++++++++++++++++++++----------------- orchagent/intfsorch.h | 1 + orchagent/vnetorch.cpp | 25 +++++++- orchagent/vnetorch.h | 28 ++------ 4 files changed, 107 insertions(+), 84 deletions(-) diff --git a/orchagent/intfsorch.cpp b/orchagent/intfsorch.cpp index a7d42272f65b..4006d4851140 100644 --- a/orchagent/intfsorch.cpp +++ b/orchagent/intfsorch.cpp @@ -100,6 +100,73 @@ set IntfsOrch:: getSubnetRoutes() return subnet_routes; } +bool IntfsOrch::setIntf(const string& alias, sai_object_id_t vrf_id, const IpPrefix *ip_prefix) +{ + SWSS_LOG_ENTER(); + + Port port; + gPortsOrch->getPort(alias, port); + + auto it_intfs = m_syncdIntfses.find(alias); + if (it_intfs == m_syncdIntfses.end()) + { + if (addRouterIntfs(vrf_id, port)) + { + IntfsEntry intfs_entry; + intfs_entry.ref_count = 0; + m_syncdIntfses[alias] = intfs_entry; + } + else + { + return false; + } + } + + if (!ip_prefix || m_syncdIntfses[alias].ip_addresses.count(*ip_prefix)) + { + /* Request to create router interface, no prefix present or Duplicate entry */ + return true; + } + + /* NOTE: Overlap checking is required to handle ifconfig weird behavior. + * When set IP address using ifconfig command it applies it in two stages. + * On stage one it sets IP address with netmask /8. On stage two it + * changes netmask to specified in command. As DB is async event to + * add IP address with original netmask may come before event to + * delete IP with netmask /8. To handle this we in case of overlap + * we should wait until entry with /8 netmask will be removed. + * Time frame between those event is quite small.*/ + bool overlaps = false; + for (const auto &prefixIt: m_syncdIntfses[alias].ip_addresses) + { + if (prefixIt.isAddressInSubnet(ip_prefix->getIp()) || + ip_prefix->isAddressInSubnet(prefixIt.getIp())) + { + overlaps = true; + SWSS_LOG_NOTICE("Router interface %s IP %s overlaps with %s.", port.m_alias.c_str(), + prefixIt.to_string().c_str(), ip_prefix->to_string().c_str()); + break; + } + } + + if (overlaps) + { + /* Overlap of IP address network */ + return false; + } + + addSubnetRoute(port, *ip_prefix); + addIp2MeRoute(vrf_id, *ip_prefix); + + if (port.m_type == Port::VLAN && ip_prefix->isV4()) + { + addDirectedBroadcast(port, ip_prefix->getBroadcastIp()); + } + + m_syncdIntfses[alias].ip_addresses.insert(*ip_prefix); + return true; +} + void IntfsOrch::doTask(Consumer &consumer) { SWSS_LOG_ENTER(); @@ -149,17 +216,7 @@ void IntfsOrch::doTask(Consumer &consumer) } sai_object_id_t vrf_id = gVirtualRouterId; - if (!vnet_name.empty()) - { - VNetOrch* vnet_orch = gDirectory.get(); - if (!vnet_orch->isVnetExists(vnet_name)) - { - it++; - continue; - } - vrf_id = vnet_orch->getVRid(vnet_name); - } - else if (!vrf_name.empty()) + if (!vrf_name.empty()) { if (m_vrfOrch->isVRFexists(vrf_name)) { @@ -225,67 +282,29 @@ void IntfsOrch::doTask(Consumer &consumer) continue; } - auto it_intfs = m_syncdIntfses.find(alias); - if (it_intfs == m_syncdIntfses.end()) + if (!vnet_name.empty()) { - if (addRouterIntfs(vrf_id, port)) + VNetOrch* vnet_orch = gDirectory.get(); + if (!vnet_orch->isVnetExists(vnet_name)) { - IntfsEntry intfs_entry; - intfs_entry.ref_count = 0; - m_syncdIntfses[alias] = intfs_entry; + it++; + continue; } - else + if (!vnet_orch->setIntf(alias, vnet_name, ip_prefix_in_key ? &ip_prefix : nullptr)) { it++; continue; } } - - vrf_id = port.m_vr_id; - if (!ip_prefix_in_key || m_syncdIntfses[alias].ip_addresses.count(ip_prefix)) - { - /* Request to create router interface, no prefix present or Duplicate entry */ - it = consumer.m_toSync.erase(it); - continue; - } - - /* NOTE: Overlap checking is required to handle ifconfig weird behavior. - * When set IP address using ifconfig command it applies it in two stages. - * On stage one it sets IP address with netmask /8. On stage two it - * changes netmask to specified in command. As DB is async event to - * add IP address with original netmask may come before event to - * delete IP with netmask /8. To handle this we in case of overlap - * we should wait until entry with /8 netmask will be removed. - * Time frame between those event is quite small.*/ - bool overlaps = false; - for (const auto &prefixIt: m_syncdIntfses[alias].ip_addresses) + else { - if (prefixIt.isAddressInSubnet(ip_prefix.getIp()) || - ip_prefix.isAddressInSubnet(prefixIt.getIp())) + if (!setIntf(alias, vrf_id, ip_prefix_in_key ? &ip_prefix : nullptr)) { - overlaps = true; - SWSS_LOG_NOTICE("Router interface %s IP %s overlaps with %s.", port.m_alias.c_str(), - prefixIt.to_string().c_str(), ip_prefix.to_string().c_str()); - break; + it++; + continue; } } - if (overlaps) - { - /* Overlap of IP address network */ - ++it; - continue; - } - - addSubnetRoute(port, ip_prefix); - addIp2MeRoute(vrf_id, ip_prefix); - - if (port.m_type == Port::VLAN && ip_prefix.isV4()) - { - addDirectedBroadcast(port, ip_prefix.getBroadcastIp()); - } - - m_syncdIntfses[alias].ip_addresses.insert(ip_prefix); it = consumer.m_toSync.erase(it); } else if (op == DEL_COMMAND) diff --git a/orchagent/intfsorch.h b/orchagent/intfsorch.h index 6273a44c3a95..33ad4a6f51db 100644 --- a/orchagent/intfsorch.h +++ b/orchagent/intfsorch.h @@ -35,6 +35,7 @@ class IntfsOrch : public Orch bool setRouterIntfsMtu(Port &port); std::set getSubnetRoutes(); + bool setIntf(const string& alias, sai_object_id_t vrf_id = gVirtualRouterId, const IpPrefix *ip_prefix = nullptr); private: VRFOrch *m_vrfOrch; IntfsTable m_syncdIntfses; diff --git a/orchagent/vnetorch.cpp b/orchagent/vnetorch.cpp index 955eb243034b..06cde55f9a3f 100644 --- a/orchagent/vnetorch.cpp +++ b/orchagent/vnetorch.cpp @@ -14,12 +14,14 @@ #include "vxlanorch.h" #include "directory.h" #include "swssnet.h" +#include "intfsorch.h" extern sai_virtual_router_api_t* sai_virtual_router_api; extern sai_route_api_t* sai_route_api; extern sai_object_id_t gSwitchId; extern Directory gDirectory; extern PortsOrch *gPortsOrch; +extern IntfsOrch *gIntfsOrch; /* * VRF Modeling and VNetVrf class definitions @@ -163,6 +165,26 @@ VNetOrch::VNetOrch(DBConnector *db, const std::string& tableName, VNET_EXEC op) } } +bool VNetOrch::setIntf(const string& alias, const string vnet_name, const IpPrefix *prefix) +{ + SWSS_LOG_ENTER(); + + if (isVnetExecVrf()) + { + if (!isVnetExists(vnet_name)) + { + SWSS_LOG_WARN("VNET %s doesn't exist", vnet_name.c_str()); + return false; + } + + auto *vnet_obj = getTypePtr(vnet_name); + sai_object_id_t vrf_id = vnet_obj->getVRidIngress(); + + return gIntfsOrch->setIntf(alias, vrf_id, prefix); + } + + return false; +} bool VNetOrch::addOperation(const Request& request) { SWSS_LOG_ENTER(); @@ -226,8 +248,9 @@ bool VNetOrch::addOperation(const Request& request) create = true; } + VNetVrfObject *vrfObj = dynamic_cast(obj.get()); if (!vxlan_orch->createVxlanTunnelMap(tunnel, TUNNEL_MAP_T_VIRTUAL_ROUTER, vni, - obj->getEncapMapId(), obj->getDecapMapId())) + vrfObj->getEncapMapId(), vrfObj->getDecapMapId())) { SWSS_LOG_ERROR("VNET '%s', tunnel '%s', map create failed", vnet_name.c_str(), tunnel.c_str()); diff --git a/orchagent/vnetorch.h b/orchagent/vnetorch.h index b6939d37704a..367616529c90 100644 --- a/orchagent/vnetorch.h +++ b/orchagent/vnetorch.h @@ -49,10 +49,6 @@ class VNetObject public: VNetObject(string& tunName, set& peer) : tunnel_(tunName), peer_list_(peer) { } - virtual sai_object_id_t getEncapMapId() const = 0; - - virtual sai_object_id_t getDecapMapId() const = 0; - virtual bool updateObj(vector&) = 0; void setPeerList(set& p_list) @@ -60,8 +56,6 @@ class VNetObject peer_list_ = p_list; } - virtual sai_object_id_t getVRid() const = 0; - const set& getPeerList() const { return peer_list_; @@ -90,17 +84,17 @@ class VNetVrfObject : public VNetObject set getVRids() const; - virtual sai_object_id_t getEncapMapId() const + sai_object_id_t getEncapMapId() const { return getVRidIngress(); } - virtual sai_object_id_t getDecapMapId() const + sai_object_id_t getDecapMapId() const { return getVRidEgress(); } - virtual sai_object_id_t getVRid() const + sai_object_id_t getVRid() const { return getVRidIngress(); } @@ -123,6 +117,7 @@ class VNetOrch : public Orch2 { public: VNetOrch(DBConnector *db, const std::string&, VNET_EXEC op = VNET_EXEC::VNET_EXEC_VRF); + bool setIntf(const string& alias, const string vnet_name, const IpPrefix *prefix = nullptr); bool isVnetExists(const std::string& name) const { @@ -135,26 +130,11 @@ class VNetOrch : public Orch2 return static_cast(vnet_table_.at(name).get()); } - sai_object_id_t getEncapMapId(const std::string& name) const - { - return vnet_table_.at(name)->getEncapMapId(); - } - - sai_object_id_t getDecapMapId(const std::string& name) const - { - return vnet_table_.at(name)->getDecapMapId(); - } - const set& getPeerList(const std::string& name) const { return vnet_table_.at(name)->getPeerList(); } - sai_object_id_t getVRid(const std::string& name) const - { - return vnet_table_.at(name)->getVRid(); - } - string getTunnelName(const std::string& name) const { return vnet_table_.at(name)->getTunnelName();