Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix adding flex counter to wrong context #1421

Merged
merged 8 commits into from
Oct 23, 2024
Merged

Fix adding flex counter to wrong context #1421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c8da61f
Fix adding flex counter to wrong context
byu343 Sep 18, 2024
016c654
Fix formatting
byu343 Oct 2, 2024
0ece9dc
Fix to pass the test SyncdMlnxTest.portBulkAddRemove
byu343 Sep 26, 2024
2281261
Apply context check if objectId is not NULL
byu343 Oct 7, 2024
9fa064f
Merge branch 'master' into flex-counter-context
byu343 Oct 16, 2024
5a753e1
Merge branch 'master' into flex-counter-context
yaqiangz Oct 17, 2024
fab2187
Merge branch 'master' into flex-counter-context
byu343 Oct 21, 2024
bf353b4
Merge branch 'master' into flex-counter-context
byu343 Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions lib/RedisRemoteSaiInterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2049,6 +2049,21 @@ sai_switch_notifications_t RedisRemoteSaiInterface::syncProcessNotification(
return { };
}

bool RedisRemoteSaiInterface::containsSwitch(
_In_ sai_object_id_t switchId) const
{
SWSS_LOG_ENTER();

if (!m_switchContainer->contains(switchId))
{
SWSS_LOG_INFO("context %s failed to find switch %s",
m_contextConfig->m_name.c_str(), sai_serialize_object_id(switchId).c_str());
return false;
}

return true;
kcudnik marked this conversation as resolved.
Show resolved Hide resolved
}

const std::map<sai_object_id_t, swss::TableDump>& RedisRemoteSaiInterface::getTableDump() const
{
SWSS_LOG_ENTER();
Expand Down
3 changes: 3 additions & 0 deletions lib/RedisRemoteSaiInterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,9 @@ namespace sairedis

const std::map<sai_object_id_t, swss::TableDump>& getTableDump() const;

bool containsSwitch(
_In_ sai_object_id_t switchId) const;

private: // QUAD API helpers

sai_status_t create(
Expand Down
12 changes: 9 additions & 3 deletions lib/Sai.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,19 @@ sai_status_t Sai::set(

// skip metadata if attribute is redis extension attribute

// TODO this is setting on all contexts, but maybe we want one specific?
// and do set on all if objectId == NULL

bool success = true;

// Setting on all contexts if objectType != SAI_OBJECT_TYPE_SWITCH or objectId == NULL
for (auto& kvp: m_contextMap)
{
if (objectType == SAI_OBJECT_TYPE_SWITCH && objectId != SAI_NULL_OBJECT_ID)
{
if (!kvp.second->m_redisSai->containsSwitch(objectId))
{
continue;
}
}

sai_status_t status = kvp.second->m_redisSai->set(objectType, objectId, attr);

success &= (status == SAI_STATUS_SUCCESS);
Expand Down
16 changes: 10 additions & 6 deletions syncd/tests/TestSyncdMlnx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,10 +284,14 @@ TEST_F(SyncdMlnxTest, portBulkAddRemove)
attr.id = SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER_GROUP;
attr.value.ptr = (void*)&flexCounterGroupParam;

status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, &attr);
// Failed to create if the switchId is invalid for the context
status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, m_switchId + 1, &attr);
std::vector<swss::FieldValueTuple> fvVector, fvVectorExpected;
ASSERT_FALSE(m_flexCounterGroupTable->get("PORT_STAT_COUNTER", fvVector));

status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, m_switchId, &attr);
ASSERT_EQ(status, SAI_STATUS_SUCCESS);

std::vector<swss::FieldValueTuple> fvVector, fvVectorExpected;
ASSERT_TRUE(m_flexCounterGroupTable->get("PORT_STAT_COUNTER", fvVector));
fvVectorExpected.emplace_back(POLL_INTERVAL_FIELD, poll_interval);
fvVectorExpected.emplace_back(STATS_MODE_FIELD, stats_mode);
Expand Down Expand Up @@ -315,15 +319,15 @@ TEST_F(SyncdMlnxTest, portBulkAddRemove)
attr.id = SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER;
attr.value.ptr = (void*)&flexCounterParam;

status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, &attr);
status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, m_switchId, &attr);
ASSERT_EQ(status, SAI_STATUS_FAILURE);
ASSERT_FALSE(m_flexCounterTable->get(key, fvVector));

// Try with a good key
key = "PORT_STAT_COUNTER:" + sai_serialize_object_id(oidList[0]);
flexCounterParam.counter_key.list = (int8_t*)const_cast<char *>(key.c_str());
flexCounterParam.counter_key.count = (uint32_t)key.length();
status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, &attr);
status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, m_switchId, &attr);
ASSERT_EQ(status, SAI_STATUS_SUCCESS);

ASSERT_TRUE(m_flexCounterTable->get(key, fvVector));
Expand All @@ -336,7 +340,7 @@ TEST_F(SyncdMlnxTest, portBulkAddRemove)
flexCounterParam.counter_field_name.count = 0;

// 3. Stop counter polling for the port
status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, &attr);
status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, m_switchId, &attr);
ASSERT_EQ(status, SAI_STATUS_SUCCESS);
ASSERT_FALSE(m_flexCounterTable->get(key, fvVector));

Expand All @@ -351,7 +355,7 @@ TEST_F(SyncdMlnxTest, portBulkAddRemove)
attr.id = SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER_GROUP;
attr.value.ptr = (void*)&flexCounterGroupParam;

status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, &attr);
status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, m_switchId, &attr);
ASSERT_EQ(status, SAI_STATUS_SUCCESS);
ASSERT_FALSE(m_flexCounterTable->get(key, fvVector));

Expand Down
Loading