Skip to content

hftelorch: minor improvements and cleanups for high frequency telemetry #4315

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

Open
Pterosaur wants to merge 10 commits into
base: master
Choose a base branch
from
Open

hftelorch: minor improvements and cleanups for high frequency telemetry #4315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1be47b7
hftelorch: remove unused macro and add SAI return value checks
Pterosaur Mar 9, 2026
3d69007
hftelprofile: fix missing push_back for TAM report interval unit
Pterosaur Mar 9, 2026
c705671
hftelprofile: fix clearGroup using operator[] on unordered_map
Pterosaur Mar 9, 2026
a1753f6
counternameupdater: fix potential dangling pointer in Message
Pterosaur Mar 9, 2026
b67cef0
hftelgroup: use move semantics for updateStatsIDs
Pterosaur Mar 9, 2026
7efaa82
hftelprofile: use u32 for TAM counter subscription stat id
Pterosaur Mar 10, 2026
90d7028
hftelprofile: delete counter subscriptions before tel type
Pterosaur Mar 10, 2026
c470f11
hftelorch: harden message ownership and partial create cleanup
Pterosaur Mar 12, 2026
a9cdd81
Merge branch 'master' into fix/hft-orchagent-bugs
Pterosaur Mar 12, 2026
cf1b7d1
Merge branch 'master' into fix/hft-orchagent-bugs
prsunny Mar 18, 2026
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
26 changes: 9 additions & 17 deletions orchagent/high_frequency_telemetry/counternameupdater.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,11 @@ void CounterNameMapUpdater::setCounterNameMap(const std::string &counter_name, s

if (gHFTOrch)
{
std::string unified_counter_name = unify_counter_name(counter_name);
Message msg{
.m_table_name = m_table_name.c_str(),
.m_operation = OPERATION::SET,
.m_set{
.m_counter_name = unified_counter_name.c_str(),
.m_oid = oid,
},
};
Message msg;
msg.m_table_name = m_table_name;
msg.m_operation = OPERATION::SET;
msg.m_counter_name = unify_counter_name(counter_name);
msg.m_oid = oid;
gHFTOrch->locallyNotify(msg);
}

Expand Down Expand Up @@ -58,14 +54,10 @@ void CounterNameMapUpdater::delCounterNameMap(const std::string &counter_name)

if (gHFTOrch)
{
std::string unified_counter_name = unify_counter_name(counter_name);
Message msg{
.m_table_name = m_table_name.c_str(),
.m_operation = OPERATION::DEL,
.m_del{
.m_counter_name = unified_counter_name.c_str(),
},
};
Message msg;
msg.m_table_name = m_table_name;
msg.m_operation = OPERATION::DEL;
msg.m_counter_name = unify_counter_name(counter_name);
gHFTOrch->locallyNotify(msg);
}

Expand Down
22 changes: 5 additions & 17 deletions orchagent/high_frequency_telemetry/counternameupdater.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,14 @@ class CounterNameMapUpdater
DEL,
};

struct SetPayload
{
const char* m_counter_name;
sai_object_id_t m_oid;
};

struct DelPayload
{
const char* m_counter_name;
};

struct Message
{
const char* m_table_name;
std::string m_table_name;
OPERATION m_operation;
union
{
SetPayload m_set;
DelPayload m_del;
};
// Use a string to own the counter name, avoiding dangling pointers
// when the caller's local string goes out of scope.
std::string m_counter_name;
sai_object_id_t m_oid = SAI_NULL_OBJECT_ID; // Only valid for SET operation
};

CounterNameMapUpdater(const std::string &db_name, const std::string &table_name);
Expand Down
4 changes: 2 additions & 2 deletions orchagent/high_frequency_telemetry/hftelgroup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ void HFTelGroup::updateObjects(const set<string> &object_names)
}
}

void HFTelGroup::updateStatsIDs(const std::set<sai_stat_id_t> &stats_ids)
void HFTelGroup::updateStatsIDs(std::set<sai_stat_id_t> &&stats_ids)
{
SWSS_LOG_ENTER();

m_stats_ids = move(stats_ids);
m_stats_ids = std::move(stats_ids);
}

bool HFTelGroup::isSameObjects(const std::set<std::string> &object_names) const
Expand Down
2 changes: 1 addition & 1 deletion orchagent/high_frequency_telemetry/hftelgroup.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class HFTelGroup
HFTelGroup(const std::string& group_name);
~HFTelGroup() = default;
void updateObjects(const std::set<std::string> &object_names);
void updateStatsIDs(const std::set<sai_stat_id_t> &stats_ids);
void updateStatsIDs(std::set<sai_stat_id_t> &&stats_ids);
bool isSameObjects(const std::set<std::string> &object_names) const;
bool isObjectInGroup(const std::string &object_name) const;
const std::unordered_map<std::string, sai_uint16_t>& getObjects() const { return m_objects; }
Expand Down
51 changes: 30 additions & 21 deletions orchagent/high_frequency_telemetry/hftelorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
using namespace std;
using namespace swss;

#define CONSTANTS_FILE "/et/sonic/constants.yml"

const unordered_map<string, sai_object_type_t> HFTelOrch::SUPPORT_COUNTER_TABLES = {
{COUNTERS_PORT_NAME_MAP, SAI_OBJECT_TYPE_PORT},
{COUNTERS_BUFFER_POOL_NAME_MAP, SAI_OBJECT_TYPE_BUFFER_POOL},
Expand Down Expand Up @@ -110,23 +108,23 @@ void HFTelOrch::locallyNotify(const CounterNameMapUpdater::Message &msg)
auto counter_itr = HFTelOrch::SUPPORT_COUNTER_TABLES.find(msg.m_table_name);
if (counter_itr == HFTelOrch::SUPPORT_COUNTER_TABLES.end())
{
SWSS_LOG_WARN("The counter table %s is not supported by high frequency telemetry", msg.m_table_name);
SWSS_LOG_WARN("The counter table %s is not supported by high frequency telemetry", msg.m_table_name.c_str());
return;
}

SWSS_LOG_NOTICE("The counter table %s is updated, operation %d, object %s",
msg.m_table_name,
msg.m_table_name.c_str(),
msg.m_operation,
msg.m_operation == CounterNameMapUpdater::SET ? msg.m_set.m_counter_name : msg.m_del.m_counter_name);
msg.m_counter_name.c_str());

// Update the local cache
if (msg.m_operation == CounterNameMapUpdater::SET)
{
m_counter_name_cache[counter_itr->second][msg.m_set.m_counter_name] = msg.m_set.m_oid;
m_counter_name_cache[counter_itr->second][msg.m_counter_name] = msg.m_oid;
}
else if (msg.m_operation == CounterNameMapUpdater::DEL)
{
m_counter_name_cache[counter_itr->second].erase(msg.m_del.m_counter_name);
m_counter_name_cache[counter_itr->second].erase(msg.m_counter_name);
}

// Update the profile
Expand All @@ -138,24 +136,24 @@ void HFTelOrch::locallyNotify(const CounterNameMapUpdater::Message &msg)
for (auto profile_itr = type_itr->second.begin(); profile_itr != type_itr->second.end(); profile_itr++)
{
auto profile = *profile_itr;
const char *counter_name = msg.m_operation == CounterNameMapUpdater::SET ? msg.m_set.m_counter_name : msg.m_del.m_counter_name;
const auto &counter_name = msg.m_counter_name;

if (!profile->canBeUpdated(counter_itr->second))
{
// TODO: Here is a potential issue, we might need to retry the task.
// Because the Syncd is generating the configuration(template),
// we cannot update the monitor objects at this time.
SWSS_LOG_WARN("The high frequency telemetry profile %s is not ready to be updated, but the object %s want to be updated", profile->getProfileName().c_str(), counter_name);
SWSS_LOG_WARN("The high frequency telemetry profile %s is not ready to be updated, but the object %s want to be updated", profile->getProfileName().c_str(), counter_name.c_str());
continue;
}

if (msg.m_operation == CounterNameMapUpdater::SET)
{
profile->setObjectSAIID(counter_itr->second, counter_name, msg.m_set.m_oid);
profile->setObjectSAIID(counter_itr->second, counter_name.c_str(), msg.m_oid);
}
else if (msg.m_operation == CounterNameMapUpdater::DEL)
{
profile->delObjectSAIID(counter_itr->second, counter_name);
profile->delObjectSAIID(counter_itr->second, counter_name.c_str());
}
else
{
Expand Down Expand Up @@ -583,10 +581,13 @@ void HFTelOrch::createNetlinkChannel(const string &genl_family, const string &ge
strncpy(attr.value.chardata, genl_group.c_str(), sizeof(attr.value.chardata));
attrs.push_back(attr);

sai_hostif_api->create_hostif(&m_sai_hostif_obj, gSwitchId, static_cast<uint32_t>(attrs.size()), attrs.data());

// // Create hostif trap group object
// sai_hostif_api->create_hostif_trap_group(&m_sai_hostif_trap_group_obj, gSwitchId, 0, nullptr);
if (handleSaiCreateStatus(
SAI_API_HOSTIF,
sai_hostif_api->create_hostif(&m_sai_hostif_obj, gSwitchId, static_cast<uint32_t>(attrs.size()), attrs.data())) != task_success)
{
deleteNetlinkChannel();
return;
}

// Create hostif user defined trap object
attrs.clear();
Expand All @@ -595,11 +596,13 @@ void HFTelOrch::createNetlinkChannel(const string &genl_family, const string &ge
attr.value.s32 = SAI_HOSTIF_USER_DEFINED_TRAP_TYPE_TAM;
attrs.push_back(attr);

// attr.id = SAI_HOSTIF_USER_DEFINED_TRAP_ATTR_TRAP_GROUP;
// attr.value.oid = m_sai_hostif_trap_group_obj;
// attrs.push_back(attr);

sai_hostif_api->create_hostif_user_defined_trap(&m_sai_hostif_user_defined_trap_obj, gSwitchId, static_cast<uint32_t>(attrs.size()), attrs.data());
if (handleSaiCreateStatus(
SAI_API_HOSTIF,
sai_hostif_api->create_hostif_user_defined_trap(&m_sai_hostif_user_defined_trap_obj, gSwitchId, static_cast<uint32_t>(attrs.size()), attrs.data())) != task_success)
{
deleteNetlinkChannel();
return;
}

// Create hostif table entry object
attrs.clear();
Expand All @@ -620,7 +623,13 @@ void HFTelOrch::createNetlinkChannel(const string &genl_family, const string &ge
attr.value.oid = m_sai_hostif_obj;
attrs.push_back(attr);

sai_hostif_api->create_hostif_table_entry(&m_sai_hostif_table_entry_obj, gSwitchId, static_cast<uint32_t>(attrs.size()), attrs.data());
if (handleSaiCreateStatus(
SAI_API_HOSTIF,
sai_hostif_api->create_hostif_table_entry(&m_sai_hostif_table_entry_obj, gSwitchId, static_cast<uint32_t>(attrs.size()), attrs.data())) != task_success)
{
deleteNetlinkChannel();
return;
}
}

void HFTelOrch::deleteNetlinkChannel()
Expand Down
17 changes: 11 additions & 6 deletions orchagent/high_frequency_telemetry/hftelprofile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ void HFTelProfile::setStatsIDs(const string &group_name, const set<string> &obje
if (itr == m_groups.end() || itr->first != sai_object_type)
{
HFTelGroup group(group_name);
group.updateStatsIDs(stats_ids_set);
group.updateStatsIDs(std::move(stats_ids_set));
m_groups.insert(itr, {sai_object_type, move(group)});
}
else
Expand All @@ -324,7 +324,7 @@ void HFTelProfile::setStatsIDs(const string &group_name, const set<string> &obje
{
return;
}
itr->second.updateStatsIDs(stats_ids_set);
itr->second.updateStatsIDs(std::move(stats_ids_set));
}

// TODO: In the phase 2, we don't need to stop the stream before update the stats
Expand Down Expand Up @@ -451,10 +451,14 @@ void HFTelProfile::clearGroup(const std::string &group_name)
m_groups.erase(itr);
}
m_sai_tam_tel_type_templates.erase(sai_object_type);
m_sai_tam_tel_type_states.erase(m_sai_tam_tel_type_objs[sai_object_type]);
m_sai_tam_tel_type_objs.erase(sai_object_type);
m_sai_tam_report_objs.erase(sai_object_type);
m_sai_tam_counter_subscription_objs.erase(sai_object_type);
auto tel_type_itr = m_sai_tam_tel_type_objs.find(sai_object_type);
if (tel_type_itr != m_sai_tam_tel_type_objs.end())
{
m_sai_tam_tel_type_states.erase(tel_type_itr->second);
m_sai_tam_tel_type_objs.erase(tel_type_itr);
}
m_sai_tam_report_objs.erase(sai_object_type);
m_name_sai_map.erase(sai_object_type);

SWSS_LOG_NOTICE("Cleared high frequency telemetry group %s with no objects", group_name.c_str());
Expand Down Expand Up @@ -663,6 +667,7 @@ sai_object_id_t HFTelProfile::getTAMReportObjID(sai_object_type_t object_type)

attr.id = SAI_TAM_REPORT_ATTR_REPORT_INTERVAL_UNIT;
attr.value.s32 = SAI_TAM_REPORT_INTERVAL_UNIT_USEC;
attrs.push_back(attr);

handleSaiCreateStatus(
SAI_API_TAM,
Expand Down Expand Up @@ -862,7 +867,7 @@ void HFTelProfile::deployCounterSubscription(sai_object_type_t object_type, sai_
attrs.push_back(attr);

attr.id = SAI_TAM_COUNTER_SUBSCRIPTION_ATTR_STAT_ID;
attr.value.oid = stat_id;
attr.value.u32 = static_cast<uint32_t>(stat_id);
attrs.push_back(attr);

attr.id = SAI_TAM_COUNTER_SUBSCRIPTION_ATTR_LABEL;
Expand Down
Loading