Skip to content

Commit

Permalink
Add next hop group specific comparison logic (sonic-net#353)
Browse files Browse the repository at this point in the history
  • Loading branch information
kcudnik authored and lguohan committed Oct 16, 2018
1 parent 05ef677 commit 6fdf6ff
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 2 deletions.
104 changes: 102 additions & 2 deletions syncd/syncd_applyview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2276,7 +2276,92 @@ std::shared_ptr<SaiObj> findCurrentBestMatchForLag(
}
}

SWSS_LOG_NOTICE("failed to find best candidate for LAG using LAG member and port id");
SWSS_LOG_WARN("failed to find best candidate for LAG using LAG member and port id");

return nullptr;
}

std::shared_ptr<SaiObj> findCurrentBestMatchForNextHopGroup(
_In_ const AsicView &currentView,
_In_ const AsicView &temporaryView,
_In_ const std::shared_ptr<const SaiObj> &temporaryObj,
_In_ const std::vector<sai_object_compare_info_t> &candidateObjects)
{
SWSS_LOG_ENTER();

/*
* First find route entries on which temporary NHG is assigned.
*/

const auto tmpRouteEntries = temporaryView.getNotProcessedObjectsByObjectType(SAI_OBJECT_TYPE_ROUTE_ENTRY);

std::shared_ptr<SaiObj> tmpRouteCandidate = nullptr;

for (auto tmpRoute: tmpRouteEntries)
{
if (!tmpRoute->hasAttr(SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID))
continue;

const auto routeNH = tmpRoute->getSaiAttr(SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID);

if (routeNH->getSaiAttr()->value.oid != temporaryObj->getVid())
continue;

tmpRouteCandidate = tmpRoute;

SWSS_LOG_NOTICE("Found route candidate for NHG: %s: %s",
temporaryObj->str_object_id.c_str(),
tmpRoute->str_object_id.c_str());

break;
}

if (tmpRouteCandidate == nullptr)
{
SWSS_LOG_NOTICE("failed to find route candidate for NHG: %s",
temporaryObj->str_object_id.c_str());

return false;
}

/*
* We found route candidate, then let's find the same route on the candidate side.
* But we will only compare prefix value.
*/

const auto curRouteEntries = currentView.getNotProcessedObjectsByObjectType(SAI_OBJECT_TYPE_ROUTE_ENTRY);

for (auto curRoute: curRouteEntries)
{
std::string tmpPrefix = sai_serialize_ip_prefix(tmpRouteCandidate->meta_key.objectkey.key.route_entry.destination);
std::string curPrefix = sai_serialize_ip_prefix(curRoute->meta_key.objectkey.key.route_entry.destination);

if (tmpPrefix != curPrefix)
continue;

/*
* Prefixes are equal, now find candidate NHG.
*/

if (!curRoute->hasAttr(SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID))
continue;

const auto routeNH = curRoute->getSaiAttr(SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID);

sai_object_id_t curNextHopId = routeNH->getSaiAttr()->value.oid;

for (auto candidate: candidateObjects)
{
if (curNextHopId != candidate.obj->getVid())
continue;

SWSS_LOG_NOTICE("Found NHG candidate %s", candidate.obj->str_object_id.c_str());

return candidate.obj;
}
}

SWSS_LOG_NOTICE("failed to find best candidate for next hop group using route_entry");

return nullptr;
}
Expand All @@ -2292,7 +2377,7 @@ std::shared_ptr<SaiObj> findCurrentBestMatchForGenericObjectUsingHeuristic(
if (temporaryObj->getObjectType() == SAI_OBJECT_TYPE_LAG)
{
/*
* For lat, let's try find matching LAG member which will be using the
* For lag, let's try find matching LAG member which will be using the
* same port, since we expect that port object can belong only to 1 lag.
*/

Expand All @@ -2302,6 +2387,21 @@ std::shared_ptr<SaiObj> findCurrentBestMatchForGenericObjectUsingHeuristic(
return candidateLag;
}

if (temporaryObj->getObjectType() == SAI_OBJECT_TYPE_NEXT_HOP_GROUP)
{
/*
* For next hop group, let's try find matching NHG which will be based on
* NHG set as SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID in route_entry.
* We assume that each class IPv4 and IPv6 will have different NGH, and
* each IP prefix will only be assigned to one NHG.
*/

std::shared_ptr<SaiObj> candidateNHG = findCurrentBestMatchForNextHopGroup(currentView, temporaryView, temporaryObj, candidateObjects);

if (candidateNHG != nullptr)
return candidateNHG;
}

/*
* Idea is to count all dependencies that uses this object. this may not
* be a good approach since logic may choose wrong candidate.
Expand Down
14 changes: 14 additions & 0 deletions tests/brcm.pl
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,22 @@ sub test_brcm_lag_comparison_logic
play "lag_comparison_logic.rec";
}

sub test_brcm_nhg_comparison_logic
{
fresh_start;

# we expect no asic operations on this test
# even do next hop groups looks the same, but are
# matched using route destination prefix

play "nhg_comparison_logic.rec";
play "nhg_comparison_logic.rec";
play "nhg_comparison_logic.rec";
}

# RUN TESTS

test_brcm_nhg_comparison_logic;
test_brcm_lag_comparison_logic;
test_brcm_speed_init_apply;
test_brcm_start_empty;
Expand Down
22 changes: 22 additions & 0 deletions tests/brcm/nhg_comparison_logic.rec
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
2018-10-08.16:48:52.023607|#|logrotate on: /var/log/swss/sairedis.rec
2018-10-08.16:48:52.023911|a|INIT_VIEW
2018-10-08.16:48:54.691593|A|SAI_STATUS_SUCCESS
2018-10-08.16:48:54.693204|c|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_INIT_SWITCH=true|SAI_SWITCH_ATTR_SRC_MAC_ADDRESS=90:B1:1C:F4:A8:53
2018-10-08.16:48:54.694576|g|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_DEFAULT_VIRTUAL_ROUTER_ID=oid:0x0
2018-10-08.16:49:09.027805|G|SAI_STATUS_SUCCESS|SAI_SWITCH_ATTR_DEFAULT_VIRTUAL_ROUTER_ID=oid:0x3000000000022
2018-10-08.16:49:09.428865|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"0.0.0.0/0","switch_id":"oid:0x21000000000000","vr":"oid:0x3000000000022"}|SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION=SAI_PACKET_ACTION_DROP
2018-10-08.16:49:09.431651|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"::/0","switch_id":"oid:0x21000000000000","vr":"oid:0x3000000000022"}|SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION=SAI_PACKET_ACTION_DROP
2018-10-08.16:50:00.546736|c|SAI_OBJECT_TYPE_NEXT_HOP_GROUP:oid:0x5000000000655|SAI_NEXT_HOP_GROUP_ATTR_TYPE=SAI_NEXT_HOP_GROUP_TYPE_ECMP
2018-10-08.16:50:00.546736|c|SAI_OBJECT_TYPE_NEXT_HOP_GROUP:oid:0x5000000000656|SAI_NEXT_HOP_GROUP_ATTR_TYPE=SAI_NEXT_HOP_GROUP_TYPE_ECMP
2018-10-08.16:49:48.551556|c|SAI_OBJECT_TYPE_NEXT_HOP_GROUP:oid:0x5000000000661|SAI_NEXT_HOP_GROUP_ATTR_TYPE=SAI_NEXT_HOP_GROUP_TYPE_ECMP
2018-10-08.16:49:48.551556|c|SAI_OBJECT_TYPE_NEXT_HOP_GROUP:oid:0x5000000000662|SAI_NEXT_HOP_GROUP_ATTR_TYPE=SAI_NEXT_HOP_GROUP_TYPE_ECMP
2018-10-08.16:49:24.413422|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"192.168.4.0/26","switch_id":"oid:0x21000000000000","vr":"oid:0x3000000000022"}|SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID=oid:0x5000000000655
2018-10-08.16:49:24.415747|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"192.168.4.64/26","switch_id":"oid:0x21000000000000","vr":"oid:0x3000000000022"}|SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID=oid:0x5000000000656
2018-10-08.16:49:24.417618|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"192.168.40.0/26","switch_id":"oid:0x21000000000000","vr":"oid:0x3000000000022"}|SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID=oid:0x5000000000656
2018-10-08.16:49:24.420139|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"192.168.40.64/26","switch_id":"oid:0x21000000000000","vr":"oid:0x3000000000022"}|SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID=oid:0x5000000000655
2018-10-08.16:50:06.093074|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"20c0:d9f0:0:40::/64","switch_id":"oid:0x21000000000000","vr":"oid:0x3000000000022"}|SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID=oid:0x5000000000661
2018-10-08.16:50:06.094240|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"20c0:d9f4::/64","switch_id":"oid:0x21000000000000","vr":"oid:0x3000000000022"}|SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID=oid:0x5000000000661
2018-10-08.16:50:06.095331|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"20c0:d9f8:0:40::/64","switch_id":"oid:0x21000000000000","vr":"oid:0x3000000000022"}|SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID=oid:0x5000000000662
2018-10-08.16:50:06.096585|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"20c0:d9f8::/64","switch_id":"oid:0x21000000000000","vr":"oid:0x3000000000022"}|SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID=oid:0x5000000000662
2018-10-08.16:49:09.568152|a|APPLY_VIEW
2018-10-08.16:49:09.573294|A|SAI_STATUS_SUCCESS

0 comments on commit 6fdf6ff

Please sign in to comment.