Skip to content

Commit

Permalink
Add buffer pool logic and unittests (sonic-net#381)
Browse files Browse the repository at this point in the history
* Add cold discovered oids

* Add cold discovered oids

* Add buffer pool logic and unittests

* Bring back logging
  • Loading branch information
kcudnik authored and lguohan committed Nov 15, 2018
1 parent 4d5a7bb commit 08fccb0
Show file tree
Hide file tree
Showing 10 changed files with 59,207 additions and 9 deletions.
213 changes: 208 additions & 5 deletions syncd/syncd_applyview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2492,12 +2492,10 @@ std::shared_ptr<SaiObj> findCurrentBestMatchForLag(
return c.obj;
}
}

break;
}
}

SWSS_LOG_WARN("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: tmp %s", temporaryObj->str_object_id.c_str());

return nullptr;
}
Expand Down Expand Up @@ -3213,7 +3211,140 @@ std::shared_ptr<SaiObj> findCurrentBestMatchForHostifTrapGroup(
return nullptr;
}

std::shared_ptr<SaiObj> findCurrentBestMatchForGenericObjectUsingHeuristic(
std::shared_ptr<SaiObj> findCurrentBestMatchForBufferPool(
_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();

/*
* For buffer pool using buffer profile which could be set on ingress
* priority group or queue. Those two should be already matched.
*/

const auto tmpBufferProfiles = temporaryView.getNotProcessedObjectsByObjectType(SAI_OBJECT_TYPE_BUFFER_PROFILE);

for (auto tmpBufferProfile: tmpBufferProfiles)
{
auto tmpPoolIdAttr = tmpBufferProfile->tryGetSaiAttr(SAI_BUFFER_PROFILE_ATTR_POOL_ID);

if (tmpPoolIdAttr == nullptr)
continue;

if (tmpPoolIdAttr->getOid() != temporaryObj->getVid())
continue;

/*
* We have temporary buffer profile which uses this buffer pool, let's
* find ingress priority group or queue on which it could be set.
*/

auto tmpQueues = temporaryView.getObjectsByObjectType(SAI_OBJECT_TYPE_QUEUE);

for (auto tmpQueue: tmpQueues)
{
auto tmpBufferProfileIdAttr = tmpQueue->tryGetSaiAttr(SAI_QUEUE_ATTR_BUFFER_PROFILE_ID);

if (tmpBufferProfileIdAttr == nullptr)
continue;

if (tmpBufferProfileIdAttr->getOid() != tmpBufferProfile->getVid())
continue;

if (tmpQueue->getObjectStatus() != SAI_OBJECT_STATUS_MATCHED)
continue;

// we can use tmp VID since object is matched and both vids are the same
auto curQueue = currentView.oOids.at(tmpQueue->getVid());

auto curBufferProfileIdAttr = curQueue->tryGetSaiAttr(SAI_QUEUE_ATTR_BUFFER_PROFILE_ID);

if (curBufferProfileIdAttr == nullptr)
continue;

if (curBufferProfileIdAttr->getOid() == SAI_NULL_OBJECT_ID)
continue;

// we have buffer profile

auto curBufferProfile = currentView.oOids.at(curBufferProfileIdAttr->getOid());

auto curPoolIdAttr = curBufferProfile->tryGetSaiAttr(SAI_BUFFER_PROFILE_ATTR_POOL_ID);

if (curPoolIdAttr == nullptr)
continue;

for (auto c: candidateObjects)
{
if (c.obj->getVid() != curPoolIdAttr->getOid())
continue;

SWSS_LOG_INFO("found best BUFFER POOL based on buffer profile and queue %s", c.obj->str_object_id.c_str());

return c.obj;
}
}

/*
* Queues didn't worked, lets try to use ingress priority groups.
*/

auto tmpIngressPriorityGroups = temporaryView.getObjectsByObjectType(SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP);

for (auto tmpIngressPriorityGroup: tmpIngressPriorityGroups)
{
auto tmpBufferProfileIdAttr = tmpIngressPriorityGroup->tryGetSaiAttr(SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE);

if (tmpBufferProfileIdAttr == nullptr)
continue;

if (tmpBufferProfileIdAttr->getOid() != tmpBufferProfile->getVid())
continue;

if (tmpIngressPriorityGroup->getObjectStatus() != SAI_OBJECT_STATUS_MATCHED)
continue;

// we can use tmp VID since object is matched and both vids are the same
auto curIngressPriorityGroup = currentView.oOids.at(tmpIngressPriorityGroup->getVid());

auto curBufferProfileIdAttr = curIngressPriorityGroup->tryGetSaiAttr(SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE);

if (curBufferProfileIdAttr == nullptr)
continue;

if (curBufferProfileIdAttr->getOid() == SAI_NULL_OBJECT_ID)
continue;

// we have buffer profile

auto curBufferProfile = currentView.oOids.at(curBufferProfileIdAttr->getOid());

auto curPoolIdAttr = curBufferProfile->tryGetSaiAttr(SAI_BUFFER_PROFILE_ATTR_POOL_ID);

if (curPoolIdAttr == nullptr)
continue;

for (auto c: candidateObjects)
{
if (c.obj->getVid() != curPoolIdAttr->getOid())
continue;

SWSS_LOG_INFO("found best BUFFER POOL based on buffer profile and ingress priority group %s", c.obj->str_object_id.c_str());

return c.obj;
}
}

}

SWSS_LOG_NOTICE("failed to find best candidate for BUFFER POOL using buffer profile, ipg and queue");

return nullptr;
}

std::shared_ptr<SaiObj> findCurrentBestMatchForGenericObjectUsingGraph(
_In_ const AsicView &currentView,
_In_ const AsicView &temporaryView,
_In_ const std::shared_ptr<const SaiObj> &temporaryObj,
Expand Down Expand Up @@ -3253,10 +3384,31 @@ std::shared_ptr<SaiObj> findCurrentBestMatchForGenericObjectUsingHeuristic(
candidate = findCurrentBestMatchForAclTable(currentView, temporaryView, temporaryObj, candidateObjects);
break;

case SAI_OBJECT_TYPE_BUFFER_POOL:
candidate = findCurrentBestMatchForBufferPool(currentView, temporaryView, temporaryObj, candidateObjects);
break;

default:
break;
}

return candidate;
}

std::shared_ptr<SaiObj> findCurrentBestMatchForGenericObjectUsingHeuristic(
_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();

std::shared_ptr<SaiObj> candidate = findCurrentBestMatchForGenericObjectUsingGraph(
currentView,
temporaryView,
temporaryObj,
candidateObjects);

if (candidate != nullptr)
return candidate;

Expand Down Expand Up @@ -3613,6 +3765,23 @@ std::shared_ptr<SaiObj> findCurrentBestMatchForGenericObject(
* current view.
*/

/*
* But at this point also let's try find best candidate using graph paths,
* since if some attributes are missmatched (like for example more ACLs are
* created) this can lead to choose wrong LAG and have implications on
* router interface and so on. So matching by graph path here could be
* more precise.
*/

auto graphCandidate = findCurrentBestMatchForGenericObjectUsingGraph(
currentView,
temporaryView,
temporaryObj,
candidateObjects);

if (graphCandidate != nullptr)
return graphCandidate;

/*
* Sort candidate objects by equal attributes in descending order, we know
* here that we have at least 2 candidates.
Expand Down Expand Up @@ -6230,7 +6399,7 @@ void populateExistingObjects(
* objects on default existing objects, like for example buffer profile
* on ingress priority group. In this case buffer profile should not
* be considered as matched object and copied to temporary view, since
* this object was not decault existing object (on 1st cold boot) so in
* this object was not default existing object (on 1st cold boot) so in
* this case it must be processed by comparison logic and matched with
* possible new buffer profile created in temporary view. This may
* happen if OA will not care what was set previously on ingress
Expand Down Expand Up @@ -6372,6 +6541,38 @@ void updateRedisDatabase(
SWSS_LOG_NOTICE("updated redis database");
}

void logViewObjectCount(
_In_ const AsicView &currentView,
_In_ const AsicView &temporaryView)
{
SWSS_LOG_ENTER();

bool asic_changes = false;

for (int i = SAI_OBJECT_TYPE_NULL + 1; i < SAI_OBJECT_TYPE_MAX; i++)
{
sai_object_type_t ot = (sai_object_type_t)i;

size_t c = currentView.getObjectsByObjectType(ot).size();
size_t t = temporaryView.getObjectsByObjectType(ot).size();

if (c == t)
continue;

asic_changes = true;

SWSS_LOG_WARN("object count for %s on current view %zu is differnt than on temporary view: %zu",
sai_serialize_object_type(ot).c_str(),
c,
t);
}

if (asic_changes)
{
SWSS_LOG_WARN("object count is differnt on both view, there will be ASIC OPERATIONS!");
}
}

sai_status_t syncdApplyView()
{
SWSS_LOG_ENTER();
Expand Down Expand Up @@ -6512,6 +6713,8 @@ sai_status_t syncdApplyView()
temp.dumpRef("temp START");
}

logViewObjectCount(current, temp);

applyViewTransition(current, temp);

SWSS_LOG_NOTICE("ASIC operations to execute: %zu", current.asicGetOperationsCount());
Expand Down
4 changes: 2 additions & 2 deletions syncd/syncd_hard_reinit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -278,15 +278,15 @@ void checkAllIds()

/*
* If removing existing object, also make sure we remove it
* from DEFAULTVID map since this object will no longer exists.
* from COLDVIDS map since this object will no longer exists.
*/

if (g_ridToVidMap.find(rid) == g_ridToVidMap.end())
continue;

std::string strVid = sai_serialize_object_id(g_ridToVidMap.at(rid));

SWSS_LOG_INFO("removeing existing VID: %s", strVid.c_str());
SWSS_LOG_INFO("removing existing VID: %s", strVid.c_str());

g_redisClient->hdel(COLDVIDS, strVid);
}
Expand Down
2 changes: 1 addition & 1 deletion syncd/syncd_saiswitch.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class SaiSwitch
*
* @param rid Real ID to be examined.
*
* @return True if RID was discovered during cald boot init.
* @return True if RID was discovered during cold boot init.
*/
bool isColdBootDiscoveredRid(
_In_ sai_object_id_t rid) const;
Expand Down
90 changes: 90 additions & 0 deletions tests/brcm.pl
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,98 @@ sub test_brcm_acl_tables
play "acl_tables.rec", 0;
}

sub test_brcm_buffer_pool
{
fresh_start;

# we expect no operations on asic, and all buffer pools will be matched correctly

play "full_buffer.rec";
play "full_buffer_second.rec",0;
}

sub test_brcm_warm_boot_full
{
fresh_start;

play "full.rec";

request_warm_shutdown;
start_syncd_warm;

play "full_second.rec";

request_warm_shutdown;
}

sub test_brcm_warm_boot_empty
{
fresh_start;

play "empty_sw.rec";

request_warm_shutdown;
start_syncd_warm;

play "empty_sw.rec", 0;

request_warm_shutdown;
}

sub test_brcm_warm_boot_small_buffer
{
fresh_start;

play "small_buffer.rec";

request_warm_shutdown;
start_syncd_warm;

play "small_buffer.rec", 0;

request_warm_shutdown;
}

sub test_brcm_warm_boot_full_empty
{
fresh_start;

play "full.rec";

request_warm_shutdown;
start_syncd_warm;

play "empty_sw.rec";
play "empty_sw.rec", 0;
play "full_second.rec";
play "empty_sw.rec";

request_warm_shutdown;
}

sub test_brcm_config_acl
{
fresh_start;

play "config_acl.rec";
play "config_acl.rec", 0;

fresh_start;

play "config_acl2.rec";
play "config_acl2.rec", 0;
}

# RUN TESTS

test_brcm_config_acl;

test_brcm_warm_boot_full_empty;
test_brcm_warm_boot_small_buffer;
test_brcm_warm_boot_empty;
test_brcm_warm_boot_full;

test_brcm_buffer_pool;
test_brcm_acl_tables;
test_brcm_qos_map_order;
test_brcm_lag_no_members;
Expand Down
Loading

0 comments on commit 08fccb0

Please sign in to comment.