From cfc4b44572d9846e2dfc5738b0d51f1a5224af9b Mon Sep 17 00:00:00 2001 From: Shuotian Cheng Date: Fri, 6 May 2016 16:17:28 -0700 Subject: [PATCH] orchagent: Refacotring (#20) * orchagent: Refacotring - RouteOrch depends on NeighOrch - Move m_syncdNextHops to NeighOrch class * orchagent: Separating m_syncdNextHopGroups from m_syncdNextHops - NeighOrch class contains m_syncdNextHops that has next hop table - RouteOrch class contains m_syncdNextHopGruops that has next hop group table Separating m_syncdNextHops to next hops and next hop groups --- orchagent/neighorch.cpp | 117 +++++++++---- orchagent/neighorch.h | 33 +++- orchagent/orchdaemon.cpp | 4 +- orchagent/routeorch.cpp | 369 ++++++++++++++++++++------------------- orchagent/routeorch.h | 40 +++-- 5 files changed, 328 insertions(+), 235 deletions(-) diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index ebb701e1a7d5..f5fcc6772604 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -2,9 +2,89 @@ #include "logger.h" +#include "assert.h" + extern sai_neighbor_api_t* sai_neighbor_api; extern sai_next_hop_api_t* sai_next_hop_api; +bool NeighOrch::hasNextHop(IpAddress ipAddress) +{ + return m_syncdNextHops.find(ipAddress) != m_syncdNextHops.end(); +} + +bool NeighOrch::addNextHop(IpAddress ipAddress, Port port) +{ + SWSS_LOG_ENTER(); + + assert(!hasNextHop(ipAddress)); + + sai_attribute_t next_hop_attrs[3]; + next_hop_attrs[0].id = SAI_NEXT_HOP_ATTR_TYPE; + next_hop_attrs[0].value.s32 = SAI_NEXT_HOP_IP; + next_hop_attrs[1].id = SAI_NEXT_HOP_ATTR_IP; + next_hop_attrs[1].value.ipaddr.addr_family= SAI_IP_ADDR_FAMILY_IPV4; + next_hop_attrs[1].value.ipaddr.addr.ip4 = ipAddress.getV4Addr(); + next_hop_attrs[2].id = SAI_NEXT_HOP_ATTR_ROUTER_INTERFACE_ID; + next_hop_attrs[2].value.oid = port.m_rif_id; + + sai_object_id_t next_hop_id; + sai_status_t status = sai_next_hop_api->create_next_hop(&next_hop_id, 3, next_hop_attrs); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create next hop entry ip:%s rid%llx\n", + ipAddress.to_string().c_str(), port.m_rif_id); + return false; + } + + NextHopEntry next_hop_entry; + next_hop_entry.next_hop_id = next_hop_id; + next_hop_entry.ref_count = 0; + m_syncdNextHops[ipAddress] = next_hop_entry; + + return true; +} + +bool NeighOrch::removeNextHop(IpAddress ipAddress) +{ + SWSS_LOG_ENTER(); + + assert(hasNextHop(ipAddress)); + + if (m_syncdNextHops[ipAddress].ref_count > 0) + { + SWSS_LOG_ERROR("Failed to remove still referenced next hop entry ip:%s", + ipAddress.to_string().c_str()); + return false; + } + + m_syncdNextHops.erase(ipAddress); + return true; +} + +sai_object_id_t NeighOrch::getNextHopId(IpAddress ipAddress) +{ + assert(hasNextHop(ipAddress)); + return m_syncdNextHops[ipAddress].next_hop_id; +} + +int NeighOrch::getNextHopRefCount(IpAddress ipAddress) +{ + assert(hasNextHop(ipAddress)); + return m_syncdNextHops[ipAddress].ref_count; +} + +void NeighOrch::increaseNextHopRefCount(IpAddress ipAddress) +{ + assert(hasNextHop(ipAddress)); + m_syncdNextHops[ipAddress].ref_count ++; +} + +void NeighOrch::decreaseNextHopRefCount(IpAddress ipAddress) +{ + assert(hasNextHop(ipAddress)); + m_syncdNextHops[ipAddress].ref_count --; +} + void NeighOrch::doTask(Consumer &consumer) { SWSS_LOG_ENTER(); @@ -28,6 +108,7 @@ void NeighOrch::doTask(Consumer &consumer) it = consumer.m_toSync.erase(it); continue; } + string alias = key.substr(0, found); Port p; @@ -77,8 +158,8 @@ void NeighOrch::doTask(Consumer &consumer) else it++; } - /* Cannot locate the neighbor */ else + /* Cannot locate the neighbor */ it = consumer.m_toSync.erase(it); } else @@ -120,21 +201,8 @@ bool NeighOrch::addNeighbor(NeighborEntry neighborEntry, MacAddress macAddress) SWSS_LOG_NOTICE("Create neighbor entry rid:%llx alias:%s ip:%s\n", p.m_rif_id, alias.c_str(), ip_address.to_string().c_str()); - sai_attribute_t next_hop_attrs[3]; - next_hop_attrs[0].id = SAI_NEXT_HOP_ATTR_TYPE; - next_hop_attrs[0].value.s32 = SAI_NEXT_HOP_IP; - next_hop_attrs[1].id = SAI_NEXT_HOP_ATTR_IP; - next_hop_attrs[1].value.ipaddr.addr_family= SAI_IP_ADDR_FAMILY_IPV4; - next_hop_attrs[1].value.ipaddr.addr.ip4 = ip_address.getV4Addr(); - next_hop_attrs[2].id = SAI_NEXT_HOP_ATTR_ROUTER_INTERFACE_ID; - next_hop_attrs[2].value.oid = p.m_rif_id; - - sai_object_id_t next_hop_id; - status = sai_next_hop_api->create_next_hop(&next_hop_id, 3, next_hop_attrs); - if (status != SAI_STATUS_SUCCESS) + if (!addNextHop(ip_address, p)) { - SWSS_LOG_ERROR("Failed to create next hop ip:%s rid:%llx\n", ip_address.to_string().c_str(), p.m_rif_id); - status = sai_neighbor_api->remove_neighbor_entry(&neighbor_entry); if (status != SAI_STATUS_SUCCESS) { @@ -143,9 +211,6 @@ bool NeighOrch::addNeighbor(NeighborEntry neighborEntry, MacAddress macAddress) return false; } - SWSS_LOG_NOTICE("Create next hop ip:%s rid:%llx\n", ip_address.to_string().c_str(), p.m_rif_id); - m_routeOrch->createNextHopEntry(ip_address, next_hop_id); - m_syncdNeighbors[neighborEntry] = macAddress; } else @@ -168,7 +233,7 @@ bool NeighOrch::removeNeighbor(NeighborEntry neighborEntry) if (m_syncdNeighbors.find(neighborEntry) == m_syncdNeighbors.end()) return true; - if (m_routeOrch->getNextHopRefCount(ip_address)) + if (m_syncdNextHops[ip_address].ref_count > 0) { SWSS_LOG_ERROR("Neighbor is still referenced ip:%s\n", ip_address.to_string().c_str()); return false; @@ -186,7 +251,7 @@ bool NeighOrch::removeNeighbor(NeighborEntry neighborEntry) neighbor_entry.ip_address.addr_family = SAI_IP_ADDR_FAMILY_IPV4; neighbor_entry.ip_address.addr.ip4 = ip_address.getV4Addr(); - sai_object_id_t next_hop_id = m_routeOrch->getNextHopEntry(ip_address).next_hop_id; + sai_object_id_t next_hop_id = m_syncdNextHops[ip_address].next_hop_id; status = sai_next_hop_api->remove_next_hop(next_hop_id); if (status != SAI_STATUS_SUCCESS) { @@ -212,21 +277,11 @@ bool NeighOrch::removeNeighbor(NeighborEntry neighborEntry) } SWSS_LOG_ERROR("Failed to remove neighbor entry rid:%llx ip:%s\n", p.m_rif_id, ip_address.to_string().c_str()); - - sai_attribute_t attr; - attr.id = SAI_NEIGHBOR_ATTR_DST_MAC_ADDRESS; - memcpy(attr.value.mac, m_syncdNeighbors[neighborEntry].getMac(), 6); - - status = sai_neighbor_api->create_neighbor_entry(&neighbor_entry, 1, &attr); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to create neighbor entry mac:%s\n", m_syncdNeighbors[neighborEntry].to_string().c_str()); - } return false; } m_syncdNeighbors.erase(neighborEntry); - m_routeOrch->removeNextHopEntry(ip_address); + removeNextHop(ip_address); return true; } diff --git a/orchagent/neighorch.h b/orchagent/neighorch.h index f7601fdb23c3..ac129eef36eb 100644 --- a/orchagent/neighorch.h +++ b/orchagent/neighorch.h @@ -3,7 +3,8 @@ #include "orch.h" #include "portsorch.h" -#include "routeorch.h" + +#include "ipaddress.h" struct NeighborEntry { @@ -16,25 +17,45 @@ struct NeighborEntry } }; +struct NextHopEntry +{ + sai_object_id_t next_hop_id; // next hop id + int ref_count; // reference count +}; /* NeighborTable: NeighborEntry, neighbor MAC address */ typedef map NeighborTable; +/* NextHopTable: next hop IP address, NextHopEntry */ +typedef map NextHopTable; class NeighOrch : public Orch { public: - NeighOrch(DBConnector *db, string tableName, PortsOrch *portsOrch, RouteOrch *routeOrch) : - Orch(db, tableName), m_portsOrch(portsOrch), m_routeOrch(routeOrch) {}; + NeighOrch(DBConnector *db, string tableName, PortsOrch *portsOrch) : + Orch(db, tableName), + m_portsOrch(portsOrch) {}; + + bool hasNextHop(IpAddress); + + sai_object_id_t getNextHopId(IpAddress); + int getNextHopRefCount(IpAddress); + + void increaseNextHopRefCount(IpAddress); + void decreaseNextHopRefCount(IpAddress); + private: PortsOrch *m_portsOrch; - RouteOrch *m_routeOrch; - - void doTask(Consumer &consumer); NeighborTable m_syncdNeighbors; + NextHopTable m_syncdNextHops; + + bool addNextHop(IpAddress, Port); + bool removeNextHop(IpAddress); bool addNeighbor(NeighborEntry, MacAddress); bool removeNeighbor(NeighborEntry); + + void doTask(Consumer &consumer); }; #endif /* SWSS_NEIGHORCH_H */ diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 8a686e957856..8b4a3d6a9aaa 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -31,8 +31,8 @@ bool OrchDaemon::init() m_portsO = new PortsOrch(m_applDb, APP_PORT_TABLE_NAME); m_intfsO = new IntfsOrch(m_applDb, APP_INTF_TABLE_NAME, m_portsO); - m_routeO = new RouteOrch(m_applDb, APP_ROUTE_TABLE_NAME, m_portsO); - m_neighO = new NeighOrch(m_applDb, APP_NEIGH_TABLE_NAME, m_portsO, m_routeO); + m_neighO = new NeighOrch(m_applDb, APP_NEIGH_TABLE_NAME, m_portsO); + m_routeO = new RouteOrch(m_applDb, APP_ROUTE_TABLE_NAME, m_portsO, m_neighO); m_select = new Select(); return true; diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index 1e17f1fa8e3b..1beccd2a71c8 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -2,11 +2,18 @@ #include "logger.h" -extern sai_route_api_t* sai_route_api; -extern sai_next_hop_group_api_t* sai_next_hop_group_api; +#include "assert.h" + +extern sai_next_hop_group_api_t* sai_next_hop_group_api; +extern sai_route_api_t* sai_route_api; extern sai_object_id_t gVirtualRouterId; +bool RouteOrch::hasNextHopGroup(IpAddresses ipAddresses) +{ + return m_syncdNextHopGroups.find(ipAddresses) != m_syncdNextHopGroups.end(); +} + void RouteOrch::doTask(Consumer& consumer) { SWSS_LOG_ENTER(); @@ -64,8 +71,11 @@ void RouteOrch::doTask(Consumer& consumer) } IpPrefix ip_prefix = IpPrefix(key); + + /* Currently we don't support IPv6 */ if (!ip_prefix.isV4()) { + SWSS_LOG_WARN("Get unsupported IPv6 task ip:%s", ip_prefix.to_string().c_str()); it = consumer.m_toSync.erase(it); continue; } @@ -120,8 +130,8 @@ void RouteOrch::doTask(Consumer& consumer) else it++; } - /* Cannot locate the route */ else + /* Cannot locate the route */ it = consumer.m_toSync.erase(it); } else @@ -132,66 +142,113 @@ void RouteOrch::doTask(Consumer& consumer) } } -bool RouteOrch::createNextHopEntry(IpAddress ipAddress, sai_object_id_t nextHopId) +void RouteOrch::increaseNextHopRefCount(IpAddresses ipAddresses) { - SWSS_LOG_ENTER(); - IpAddresses ip_addresses(ipAddress.to_string()); - return createNextHopEntry(ip_addresses, nextHopId); + if (ipAddresses.getSize() == 1) + { + IpAddress ip_address(ipAddresses.to_string()); + m_neighOrch->increaseNextHopRefCount(ip_address); + } + else + { + m_syncdNextHopGroups[ipAddresses].ref_count ++; + } } - -bool RouteOrch::createNextHopEntry(IpAddresses ipAddresses, sai_object_id_t nextHopGroupId) +void RouteOrch::decreaseNextHopRefCount(IpAddresses ipAddresses) { - SWSS_LOG_ENTER(); - - if (m_syncdNextHops.find(ipAddresses) != m_syncdNextHops.end()) + if (ipAddresses.getSize() == 1) { - SWSS_LOG_ERROR("Failed to create existed next hop entry ip:%s nhid:%llx\n", ipAddresses.to_string().c_str(), nextHopGroupId); - return false; + IpAddress ip_address(ipAddresses.to_string()); + m_neighOrch->decreaseNextHopRefCount(ip_address); + } + else + { + m_syncdNextHopGroups[ipAddresses].ref_count --; } - - NextHopEntry next_hop_entry; - next_hop_entry.next_hop_id = nextHopGroupId; - next_hop_entry.ref_count = 0; - m_syncdNextHops[ipAddresses] = next_hop_entry; - return true; } -bool RouteOrch::removeNextHopEntry(IpAddress ipAddress) +bool RouteOrch::addNextHopGroup(IpAddresses ipAddresses) { SWSS_LOG_ENTER(); - IpAddresses ip_addresses(ipAddress.to_string()); + assert(!hasNextHopGroup(ipAddresses)); - if (m_syncdNextHops.find(ip_addresses) == m_syncdNextHops.end()) + if (m_nextHopGroupCount > NHGRP_MAX_SIZE) { - SWSS_LOG_ERROR("Failed to remove absent next hop entry ip:%s\n", ip_addresses.to_string().c_str()); + SWSS_LOG_DEBUG("Failed to create next hop group. Exceeding maximum number of next hop groups.\n"); return false; } - if (getNextHopRefCount(ip_addresses) != 0) + vector next_hop_ids; + set next_hop_set = ipAddresses.getIpAddresses(); + + /* Assert each IP address exists in m_syncdNextHops table, + * and add the corresponding next_hop_id to next_hop_ids. */ + for (auto it = next_hop_set.begin(); it != next_hop_set.end(); it++) + { + if (!m_neighOrch->hasNextHop(*it)) + { + SWSS_LOG_NOTICE("Failed to get next hop entry ip:%s", + (*it).to_string().c_str()); + return false; + } + + sai_object_id_t next_hop_id = m_neighOrch->getNextHopId(*it); + next_hop_ids.push_back(next_hop_id); + } + + sai_attribute_t nhg_attr; + vector nhg_attrs; + + nhg_attr.id = SAI_NEXT_HOP_GROUP_ATTR_TYPE; + nhg_attr.value.s32 = SAI_NEXT_HOP_GROUP_ECMP; + nhg_attrs.push_back(nhg_attr); + + nhg_attr.id = SAI_NEXT_HOP_GROUP_ATTR_NEXT_HOP_LIST; + nhg_attr.value.objlist.count = next_hop_ids.size(); + nhg_attr.value.objlist.list = next_hop_ids.data(); + nhg_attrs.push_back(nhg_attr); + + sai_object_id_t next_hop_group_id; + sai_status_t status = sai_next_hop_group_api-> + create_next_hop_group(&next_hop_group_id, nhg_attrs.size(), nhg_attrs.data()); + if (status != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to remove referenced next hop entry ip:%s", ip_addresses.to_string().c_str()); + SWSS_LOG_ERROR("Failed to create next hop group nh:%s\n", + ipAddresses.to_string().c_str()); return false; } - m_syncdNextHops.erase(ip_addresses); + m_nextHopGroupCount ++; + SWSS_LOG_NOTICE("Create next hop group nhgid:%llx nh:%s \n", + next_hop_group_id, ipAddresses.to_string().c_str()); + + /* Increate the ref_count for the next hops used by the next hop group. */ + for (auto it = next_hop_set.begin(); it != next_hop_set.end(); it++) + m_neighOrch->increaseNextHopRefCount(*it); + + /* + * Initialize the next hop gruop structure with ref_count as 0. This + * count will increase once the route is successfully syncd. + */ + NextHopGroupEntry next_hop_group_entry; + next_hop_group_entry.next_hop_group_id = next_hop_group_id; + next_hop_group_entry.ref_count = 0; + m_syncdNextHopGroups[ipAddresses] = next_hop_group_entry; + return true; } -bool RouteOrch::removeNextHopEntry(IpAddresses ipAddresses) +bool RouteOrch::removeNextHopGroup(IpAddresses ipAddresses) { SWSS_LOG_ENTER(); - if (m_syncdNextHops.find(ipAddresses) == m_syncdNextHops.end()) - { - SWSS_LOG_ERROR("Failed to remove absent next hop entry ip:%s\n", ipAddresses.to_string().c_str()); - return false; - } + assert(hasNextHopGroup(ipAddresses)); - if (ipAddresses.getSize() > 1 && getNextHopRefCount(ipAddresses) == 0) + if (m_syncdNextHopGroups[ipAddresses].ref_count == 0) { - sai_object_id_t next_hop_group_id = m_syncdNextHops[ipAddresses].next_hop_id; + sai_object_id_t next_hop_group_id = m_syncdNextHopGroups[ipAddresses].next_hop_group_id; sai_status_t status = sai_next_hop_group_api->remove_next_hop_group(next_hop_group_id); if (status != SAI_STATUS_SUCCESS) { @@ -203,162 +260,104 @@ bool RouteOrch::removeNextHopEntry(IpAddresses ipAddresses) set ip_address_set = ipAddresses.getIpAddresses(); for (auto it = ip_address_set.begin(); it != ip_address_set.end(); it++) - { - IpAddresses ip_address((*it).to_string()); - (m_syncdNextHops[ip_address]).ref_count --; - } + m_neighOrch->decreaseNextHopRefCount(*it); - m_syncdNextHops.erase(ipAddresses); + m_syncdNextHopGroups.erase(ipAddresses); } return true; } -int RouteOrch::getNextHopRefCount(IpAddress ipAddress) -{ - IpAddresses ip_addresses(ipAddress.to_string()); - return getNextHopRefCount(ip_addresses); -} - -int RouteOrch::getNextHopRefCount(IpAddresses ipAddresses) +void RouteOrch::addTempRoute(IpPrefix ipPrefix, IpAddresses nextHops) { - return m_syncdNextHops[ipAddresses].ref_count; -} - -NextHopEntry RouteOrch::getNextHopEntry(IpAddress ipAddress) -{ - IpAddresses ip_addresses(ipAddress.to_string()); - return getNextHopEntry(ip_addresses); -} - -NextHopEntry RouteOrch::getNextHopEntry(IpAddresses ipAddresses) -{ - return m_syncdNextHops[ipAddresses]; -} - -bool RouteOrch::addRoute(IpPrefix ipPrefix, IpAddresses nextHops) -{ - SWSS_LOG_ENTER(); - - /* nhid indicates the next hop id or next hop group id of this route */ - sai_object_id_t next_hop_id; + bool to_add = false; auto it_route = m_syncdRoutes.find(ipPrefix); - auto it_nxhop = m_syncdNextHops.find(nextHops); + auto next_hop_set = nextHops.getIpAddresses(); /* - * If next hop group cannot be found in m_syncdNextHops, - * then we need to add it. + * A temporary entry is added when route is not in m_syncdRoutes, + * or it is in m_syncdRoutes but the original next hop(s) is not a + * subset of the next hop group to be added. */ - if (it_nxhop == m_syncdNextHops.end()) + if (it_route != m_syncdRoutes.end()) { - NextHopEntry next_hop_entry; - vector next_hop_ids; - set next_hop_set = nextHops.getIpAddresses(); - - for (auto it = next_hop_set.begin(); it != next_hop_set.end(); it++) + auto tmp_set = m_syncdRoutes[ipPrefix].getIpAddresses(); + for (auto it = tmp_set.begin(); it != tmp_set.end(); it++) { - IpAddresses tmp_ip((*it).to_string()); - if (m_syncdNextHops.find(tmp_ip) == m_syncdNextHops.end()) - { - SWSS_LOG_ERROR("Failed to find next hop %s", (*it).to_string().c_str()); - return false; - } - - next_hop_id = m_syncdNextHops[tmp_ip].next_hop_id; - next_hop_ids.push_back(next_hop_id); + if (next_hop_set.find(*it) == next_hop_set.end()) + to_add = true; } + } + else + to_add = true; - /* - * If the current number of next hop groups exceeds the threshold, - * then we need to pick up a random next hop from the next hop group as - * the route's temporary next hop. - */ - if (m_nextHopGroupCount > NHGRP_MAX_SIZE) + if (to_add) + { + /* Remove next hops that are not in m_syncdNextHops */ + for (auto it = next_hop_set.begin(); it != next_hop_set.end();) { - bool to_add = false; - /* - * A temporary entry is added when route is not in m_syncdRoutes, - * or it is in m_syncdRoutes but the original next hop(s) is not a - * subset of the next hop group to be added. - */ - if (it_route != m_syncdRoutes.end()) + if (!m_neighOrch->hasNextHop(*it)) { - auto tmp_set = m_syncdRoutes[ipPrefix].getIpAddresses(); - for (auto it = tmp_set.begin(); it != tmp_set.end(); it++) - { - if (next_hop_set.find(*it) == next_hop_set.end()) - { - to_add = true; - } - } + SWSS_LOG_NOTICE("Failed to get next hop entry ip:%s", + (*it).to_string().c_str()); + it = next_hop_set.erase(it); } else - { - to_add = true; - } - - if (to_add) - { - /* Randomly pick an address from the address set. */ - auto it = next_hop_set.begin(); - advance(it, rand() % next_hop_set.size()); - - /* - * Set the route's temporary next hop to be the randomly picked one. - */ - IpAddresses tmp_nxhop((*it).to_string()); - addRoute(ipPrefix, tmp_nxhop); - } - - /* - * Return false since the original route is not successfully added. - */ - return false; + it++; } - sai_attribute_t nhg_attr; - vector nhg_attrs; + /* Return if next_hop_set is empty */ + if (next_hop_set.empty()) + return; - nhg_attr.id = SAI_NEXT_HOP_GROUP_ATTR_TYPE; - nhg_attr.value.s32 = SAI_NEXT_HOP_GROUP_ECMP; - nhg_attrs.push_back(nhg_attr); + /* Randomly pick an address from the set */ + auto it = next_hop_set.begin(); + advance(it, rand() % next_hop_set.size()); - nhg_attr.id = SAI_NEXT_HOP_GROUP_ATTR_NEXT_HOP_LIST; - nhg_attr.value.objlist.count = next_hop_ids.size(); - nhg_attr.value.objlist.list = next_hop_ids.data(); - nhg_attrs.push_back(nhg_attr); + /* Set the route's temporary next hop to be the randomly picked one */ + IpAddresses tmp_next_hop((*it).to_string()); + addRoute(ipPrefix, tmp_next_hop); + } +} - sai_status_t status = sai_next_hop_group_api-> - create_next_hop_group(&next_hop_id, nhg_attrs.size(), nhg_attrs.data()); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to create nex thop group nh:%s\n", nextHops.to_string().c_str()); - return false; - } +bool RouteOrch::addRoute(IpPrefix ipPrefix, IpAddresses nextHops) +{ + SWSS_LOG_ENTER(); - m_nextHopGroupCount ++; - SWSS_LOG_NOTICE("Create next hop group nhgid:%llx nh:%s \n", next_hop_id, nextHops.to_string().c_str()); + /* next_hop_id indicates the next hop id or next hop group id of this route */ + sai_object_id_t next_hop_id; + auto it_route = m_syncdRoutes.find(ipPrefix); - /* - * Increate the ref_count for the next hops used by the next hop group. - */ - for (auto it = next_hop_set.begin(); it != next_hop_set.end(); it++) + /* The route is pointing to a next hop */ + if (nextHops.getSize() == 1) + { + IpAddress ip_address(nextHops.to_string()); + if (m_neighOrch->hasNextHop(ip_address)) { - IpAddresses tmp_ip((*it).to_string().c_str()); - m_syncdNextHops[tmp_ip].ref_count ++; + next_hop_id = m_neighOrch->getNextHopId(ip_address); + } + else + { + SWSS_LOG_NOTICE("Failed to get next hop entry ip:%s", + nextHops.to_string().c_str()); + return false; } - - /* - * Initialize the next hop gruop structure with ref_count as 0. This - * count will increase once the route is successfully syncd. - */ - next_hop_entry.ref_count = 0; - next_hop_entry.next_hop_id = next_hop_id; - m_syncdNextHops[nextHops] = next_hop_entry; } + /* The route is pointing to a next hop group */ else { - next_hop_id = m_syncdNextHops[nextHops].next_hop_id; + if (!hasNextHopGroup(nextHops)) /* Create a new next hop group */ + { + if (!addNextHopGroup(nextHops)) + { + /* Add a temporary route when a next hop group cannot be added */ + addTempRoute(ipPrefix, nextHops); + /* Return false since the original route is not successfully added */ + return false; + } + } + + next_hop_id = m_syncdNextHopGroups[nextHops].next_hop_group_id; } /* Sync the route entry */ @@ -385,13 +384,16 @@ bool RouteOrch::addRoute(IpPrefix ipPrefix, IpAddresses nextHops) { SWSS_LOG_ERROR("Failed to create route %s with next hop(s) %s", ipPrefix.to_string().c_str(), nextHops.to_string().c_str()); - /* Clean up the newly created next hop (group) entry */ - removeNextHopEntry(nextHops); + /* Clean up the newly created next hop group entry */ + if (nextHops.getSize() > 1) + { + removeNextHopGroup(nextHops); + } return false; } /* Increase the ref_count for the next hop (group) entry */ - m_syncdNextHops[nextHops].ref_count ++; + increaseNextHopRefCount(nextHops); SWSS_LOG_INFO("Create route %s with next hop(s) %s", ipPrefix.to_string().c_str(), nextHops.to_string().c_str()); } @@ -400,18 +402,24 @@ bool RouteOrch::addRoute(IpPrefix ipPrefix, IpAddresses nextHops) sai_status_t status = sai_route_api->set_route_attribute(&route_entry, &route_attr); if (status != SAI_STATUS_SUCCESS) { + SWSS_LOG_ERROR("Failed to set route %s with next hop(s) %s", + ipPrefix.to_string().c_str(), nextHops.to_string().c_str()); return false; } - m_syncdNextHops[nextHops].ref_count ++; - auto it = m_syncdNextHops.find(it_route->second); - if (it == m_syncdNextHops.end()) + /* Increase the ref_count for the next hop (group) entry */ + increaseNextHopRefCount(nextHops); + + decreaseNextHopRefCount(it_route->second); + if (it_route->second.getSize() > 1 + && m_syncdNextHopGroups[it_route->second].ref_count == 0) { - return false; + removeNextHopGroup(it_route->second); } - it->second.ref_count --; - removeNextHopEntry(it->first); + SWSS_LOG_INFO("Set route %s with next hop(s) %s", + ipPrefix.to_string().c_str(), nextHops.to_string().c_str()); } + m_syncdRoutes[ipPrefix] = nextHops; return true; } @@ -437,19 +445,22 @@ bool RouteOrch::removeRoute(IpPrefix ipPrefix) auto it_route = m_syncdRoutes.find(ipPrefix); if (it_route != m_syncdRoutes.end()) { - auto it_next_hop = m_syncdNextHops.find(it_route->second); - if (it_next_hop == m_syncdNextHops.end()) - { - SWSS_LOG_ERROR("Failed to locate next hop entry \n"); - return false; - } - it_next_hop->second.ref_count --; - if (!removeNextHopEntry(it_next_hop->first)) + /* + * Decrease the reference count only when the route is pointing to a next hop. + * Decrease the reference count when the route is pointing to a next hop group, + * and check wheather the reference count decreases to zero. If yes, then we need + * to remove the next hop group. + */ + decreaseNextHopRefCount(it_route->second); + if (it_route->second.getSize() > 1 + && m_syncdNextHopGroups[it_route->second].ref_count == 0) { - SWSS_LOG_ERROR("Failed to remove next hop group\n"); - return false; + removeNextHopGroup(it_route->second); } } + SWSS_LOG_INFO("Remove route %s with next hop(s) %s", + ipPrefix.to_string().c_str(), it_route->second.to_string().c_str()); + m_syncdRoutes.erase(ipPrefix); return true; } diff --git a/orchagent/routeorch.h b/orchagent/routeorch.h index cd8a4ce5c0b3..e5b2d66133d8 100644 --- a/orchagent/routeorch.h +++ b/orchagent/routeorch.h @@ -3,6 +3,7 @@ #include "orch.h" #include "intfsorch.h" +#include "neighorch.h" #include "ipaddress.h" #include "ipaddresses.h" @@ -13,49 +14,54 @@ using namespace std; using namespace swss; -#define NHGRP_MAX_SIZE 16 +/* Maximum next hop group number */ +#define NHGRP_MAX_SIZE 128 -struct NextHopEntry +struct NextHopGroupEntry { - sai_object_id_t next_hop_id; // next hop id or next hop group id - int ref_count; // reference count + sai_object_id_t next_hop_group_id; // next hop group id + int ref_count; // reference count }; +/* NextHopGroupTable: next hop group IP addersses, NextHopGroupEntry */ +typedef map NextHopGroupTable; /* RouteTable: destination network, next hop IP address(es) */ typedef map RouteTable; -/* NextHopTable: next hop IP address(es), NextHopEntry */ -typedef map NextHopTable; class RouteOrch : public Orch { public: - RouteOrch(DBConnector *db, string tableName, PortsOrch *portsOrch) : + RouteOrch(DBConnector *db, string tableName, + PortsOrch *portsOrch, NeighOrch *neighOrch) : Orch(db, tableName), m_portsOrch(portsOrch), + m_neighOrch(neighOrch), m_nextHopGroupCount(0), m_resync(false) {}; - bool createNextHopEntry(IpAddress, sai_object_id_t); - bool createNextHopEntry(IpAddresses, sai_object_id_t); - bool removeNextHopEntry(IpAddress); - bool removeNextHopEntry(IpAddresses); - NextHopEntry getNextHopEntry(IpAddress); - NextHopEntry getNextHopEntry(IpAddresses); - int getNextHopRefCount(IpAddress); - int getNextHopRefCount(IpAddresses); + bool hasNextHopGroup(IpAddresses); private: PortsOrch *m_portsOrch; + NeighOrch *m_neighOrch; int m_nextHopGroupCount; bool m_resync; RouteTable m_syncdRoutes; - NextHopTable m_syncdNextHops; + NextHopGroupTable m_syncdNextHopGroups; - void doTask(Consumer& consumer); + void increaseNextHopRefCount(IpAddresses); + void decreaseNextHopRefCount(IpAddresses); + + bool addNextHopGroup(IpAddresses); + bool removeNextHopGroup(IpAddresses); + + void addTempRoute(IpPrefix, IpAddresses); bool addRoute(IpPrefix, IpAddresses); bool removeRoute(IpPrefix); + + void doTask(Consumer& consumer); }; #endif /* SWSS_ROUTEORCH_H */