Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions orchagent/high_frequency_telemetry/hftelorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,16 @@ HFTelOrch::HFTelOrch(
createNetlinkChannel("sonic_stel", "ipfix");
createTAM();

m_asic_notification_consumer = make_shared<NotificationConsumer>(&m_asic_db, "NOTIFICATIONS");
auto notifier = new Notifier(m_asic_notification_consumer.get(), this, "TAM_TEL_TYPE_STATE");
m_asic_notification_consumer = new NotificationConsumer(&m_asic_db, "NOTIFICATIONS");
auto notifier = new Notifier(m_asic_notification_consumer, this, "TAM_TEL_TYPE_STATE");
sai_attribute_t attr;
attr.id = SAI_SWITCH_ATTR_TAM_TEL_TYPE_CONFIG_CHANGE_NOTIFY;
attr.value.ptr = (void *)on_tam_tel_type_config_change;
if (sai_switch_api->set_switch_attribute(gSwitchId, &attr) != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to set SAI_SWITCH_ATTR_TAM_TEL_TYPE_CONFIG_CHANGE_NOTIFY");
delete notifier;
m_asic_notification_consumer = nullptr;
throw runtime_error("HFTelOrch initialization failure (failed to set tam tel type config change notify)");
}

Expand Down Expand Up @@ -490,7 +492,7 @@ void HFTelOrch::doTask(swss::NotificationConsumer &consumer)
std::string data;
std::vector<swss::FieldValueTuple> values;

if (&consumer != m_asic_notification_consumer.get())
if (&consumer != m_asic_notification_consumer)
{
SWSS_LOG_DEBUG("Is not TAM notification");
return;
Expand Down
2 changes: 1 addition & 1 deletion orchagent/high_frequency_telemetry/hftelorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class HFTelOrch : public Orch
private:
swss::Table m_state_telemetry_session;
swss::DBConnector m_asic_db;
std::shared_ptr<swss::NotificationConsumer> m_asic_notification_consumer;
swss::NotificationConsumer* m_asic_notification_consumer = nullptr;

std::unordered_map<std::string, std::shared_ptr<HFTelProfile>> m_name_profile_mapping;
std::unordered_map<sai_object_type_t, std::unordered_set<std::shared_ptr<HFTelProfile>>> m_type_profile_mapping;
Expand Down
142 changes: 142 additions & 0 deletions tests/mock_tests/hftelorch_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,49 @@
#include "ut_helper.h"
#include "mock_orchagent_main.h"
#include "high_frequency_telemetry/hftelorch.h"
#include "schema.h"
#include <gtest/gtest.h>
#include <memory>

extern sai_switch_api_t *sai_switch_api;

namespace hftelorch_test
{
using namespace std;
using hftel_is_supported_ut::SaiHookGuard;

namespace constructor_ut
{
sai_switch_api_t *pold_sai_switch_api = nullptr;
sai_switch_api_t ut_sai_switch_api{};

sai_status_t _ut_stub_sai_set_switch_attribute(
_In_ sai_object_id_t switch_id,
_In_ const sai_attribute_t *attr)
{
if (attr->id == SAI_SWITCH_ATTR_TAM_TEL_TYPE_CONFIG_CHANGE_NOTIFY)
{
return SAI_STATUS_FAILURE;
}

return pold_sai_switch_api->set_switch_attribute(switch_id, attr);
}

void hookSaiSwitchApi()
{
ut_sai_switch_api = *sai_switch_api;
pold_sai_switch_api = sai_switch_api;
ut_sai_switch_api.set_switch_attribute = _ut_stub_sai_set_switch_attribute;
sai_switch_api = &ut_sai_switch_api;
}

void unhookSaiSwitchApi()
{
sai_switch_api = pold_sai_switch_api;
pold_sai_switch_api = nullptr;
}
}

class HFTelOrchIsSupportedTest : public ::testing::Test
{
protected:
Expand Down Expand Up @@ -91,4 +127,110 @@ namespace hftelorch_test
SaiHookGuard guard(hftel_is_supported_ut::setSaiHookSwitchNotifySetNotImplemented);
EXPECT_FALSE(HFTelOrch::isSupportedHFTel(gSwitchId));
}

class HFTelOrchConstructorTest : public ::testing::Test
{
protected:
shared_ptr<swss::DBConnector> m_config_db;
shared_ptr<swss::DBConnector> m_state_db;

void SetUp() override
{
map<string, string> profile = {
{"SAI_VS_SWITCH_TYPE", "SAI_VS_SWITCH_TYPE_BCM56850"},
{"KV_DEVICE_MAC_ADDRESS", "20:03:04:05:06:00"},
};

ASSERT_EQ(ut_helper::initSaiApi(profile), SAI_STATUS_SUCCESS);

sai_attribute_t attr{};
attr.id = SAI_SWITCH_ATTR_INIT_SWITCH;
attr.value.booldata = true;

ASSERT_EQ(sai_switch_api->create_switch(&gSwitchId, 1, &attr), SAI_STATUS_SUCCESS);

m_config_db = make_shared<swss::DBConnector>("CONFIG_DB", 0);
m_state_db = make_shared<swss::DBConnector>("STATE_DB", 0);

constructor_ut::hookSaiSwitchApi();
}

void TearDown() override
{
constructor_ut::unhookSaiSwitchApi();

ASSERT_EQ(sai_switch_api->remove_switch(gSwitchId), SAI_STATUS_SUCCESS);
gSwitchId = SAI_NULL_OBJECT_ID;

ASSERT_EQ(ut_helper::uninitSaiApi(), SAI_STATUS_SUCCESS);
}
};

/*
* Forces set_switch_attribute for TAM_TEL_TYPE_CONFIG_CHANGE_NOTIFY to fail.
* Covers constructor error cleanup: delete notifier and nullptr consumer.
*/
TEST_F(HFTelOrchConstructorTest, ConstructorFailsWhenTamNotifySetFails)
{
const vector<string> stel_tables = {
CFG_HIGH_FREQUENCY_TELEMETRY_PROFILE_TABLE_NAME,
CFG_HIGH_FREQUENCY_TELEMETRY_GROUP_TABLE_NAME,
};

EXPECT_THROW(
{
HFTelOrch orch(m_config_db.get(), m_state_db.get(), stel_tables);
(void)orch;
},
runtime_error);
}

class HFTelOrchShutdownTest : public ::testing::Test
{
protected:
shared_ptr<swss::DBConnector> m_config_db;
shared_ptr<swss::DBConnector> m_state_db;

void SetUp() override
{
map<string, string> profile = {
{"SAI_VS_SWITCH_TYPE", "SAI_VS_SWITCH_TYPE_BCM56850"},
{"KV_DEVICE_MAC_ADDRESS", "20:03:04:05:06:00"},
};

ASSERT_EQ(ut_helper::initSaiApi(profile), SAI_STATUS_SUCCESS);

sai_attribute_t attr{};
attr.id = SAI_SWITCH_ATTR_INIT_SWITCH;
attr.value.booldata = true;

ASSERT_EQ(sai_switch_api->create_switch(&gSwitchId, 1, &attr), SAI_STATUS_SUCCESS);

m_config_db = make_shared<swss::DBConnector>("CONFIG_DB", 0);
m_state_db = make_shared<swss::DBConnector>("STATE_DB", 0);
}

void TearDown() override
{
ASSERT_EQ(sai_switch_api->remove_switch(gSwitchId), SAI_STATUS_SUCCESS);
gSwitchId = SAI_NULL_OBJECT_ID;

ASSERT_EQ(ut_helper::uninitSaiApi(), SAI_STATUS_SUCCESS);
}
};

/*
* Successful ctor then dtor: Notifier/Executor owns the ASIC NotificationConsumer.
* Regression for double-delete on shutdown (shared_ptr member + ~Executor).
*/
TEST_F(HFTelOrchShutdownTest, DestructorDoesNotDoubleDeleteNotificationConsumer)
{
const vector<string> stel_tables = {
CFG_HIGH_FREQUENCY_TELEMETRY_PROFILE_TABLE_NAME,
CFG_HIGH_FREQUENCY_TELEMETRY_GROUP_TABLE_NAME,
};

auto orch = make_unique<HFTelOrch>(m_config_db.get(), m_state_db.get(), stel_tables);
orch.reset();
}
}