Skip to content
15 changes: 6 additions & 9 deletions orchagent/high_frequency_telemetry/counternameupdater.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,15 @@ void CounterNameMapUpdater::setCounterNameMap(const std::vector<swss::FieldValue
{
SWSS_LOG_ENTER();

if (gHFTOrch)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pterosaur can you please review?

for (const auto& map : counter_name_maps)
{
for (const auto& map : counter_name_maps)
const std::string& counter_name = fvField(map);
sai_object_id_t oid = SAI_NULL_OBJECT_ID;
if (!fvValue(map).empty())
{
const std::string& counter_name = fvField(map);
sai_object_id_t oid = SAI_NULL_OBJECT_ID;
if (!fvValue(map).empty())
{
sai_deserialize_object_id(fvValue(map), oid);
}
setCounterNameMap(counter_name, oid);
sai_deserialize_object_id(fvValue(map), oid);
}
setCounterNameMap(counter_name, oid);
}
}

Expand Down
41 changes: 39 additions & 2 deletions orchagent/portsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4030,6 +4030,40 @@ void PortsOrch::registerPort(Port &p)
wred_port_stat_manager.setCounterIdList(p.m_port_id, CounterType::PORT, wred_port_stats);
}

// If queue-related flex counters are already enabled, generate queue maps
// for the newly added port so that usecases like dynamic port breakout works.
bool queueFcEnabled = flex_counters_orch->getQueueCountersState() ||
flex_counters_orch->getQueueWatermarkCountersState() ||
flex_counters_orch->getWredQueueCountersState();
if (queueFcEnabled && !p.m_queue_ids.empty())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rminnikanti could you please check the DPB sonic-mgmt tests? If this is not covered, could you please add them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kperumalbfn I don't see DPB test component in PTF. As far as I understand, DPB testing in sonic-mgmt can be performed on ports that are not part of the topology.

The mock_tests included in this PR verifies the regeneration of the NAME_MAP's. I’ve also validated this behavior on a device and shared the redis-dump output in the PR description for reference.

{
auto queuesStateVector = flex_counters_orch->getQueueConfigurations();

auto maxQueueNumber = getNumberOfPortSupportedQueueCounters(p.m_alias);
FlexCounterQueueStates flexCounterQueueState(maxQueueNumber);

if (queuesStateVector.count(p.m_alias))
{
flexCounterQueueState = queuesStateVector.at(p.m_alias);
}
else if (queuesStateVector.count(createAllAvailableBuffersStr))
{
if (maxQueueNumber > 0)
{
flexCounterQueueState.enableQueueCounters(0, maxQueueNumber - 1);
}
}
else
{
if (p.m_host_tx_queue_configured && p.m_host_tx_queue < maxQueueNumber)
{
flexCounterQueueState.enableQueueCounter(p.m_host_tx_queue);
}
}

generateQueueMapPerPort(p, flexCounterQueueState, false);
}

PortUpdate update = { p, true };
notify(SUBJECT_TYPE_PORT_CHANGE, static_cast<void *>(&update));

Expand All @@ -4053,9 +4087,12 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id)
return;
}

if (p.m_host_tx_queue_configured && p.m_queue_ids.size() > p.m_host_tx_queue)
if (!p.m_queue_ids.empty())
{
removePortBufferQueueCounters(p, to_string(p.m_host_tx_queue), false);
auto skip_host_tx_queue = p.m_host_tx_queue_configured && (p.m_queue_ids.size() > p.m_host_tx_queue);
// Remove all queue counters and mappings for this port to avoid stale entries
std::string range = "0-" + to_string(p.m_queue_ids.size() - 1);
removePortBufferQueueCounters(p, range, skip_host_tx_queue);
}

/* remove port from flex_counter_table for updating counters */
Expand Down
29 changes: 29 additions & 0 deletions tests/mock_tests/mock_table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,35 @@ namespace swss
table->second.erase(key);
}
}

void Table::hdel(const std::string &key, const std::string &field, const std::string &op, const std::string &prefix)
{
auto &table = gDB[m_pipe->getDbId()][getTableName()];
auto key_iter = table.find(key);
if (key_iter == table.end())
{
return;
}

auto &attrs = key_iter->second;
std::vector<FieldValueTuple> new_attrs;
for (const auto &attr : attrs)
{
if (attr.first != field)
{
new_attrs.push_back(attr);
}
}

if (new_attrs.empty())
{
table.erase(key);
}
else
{
table[key] = new_attrs;
}
}

void ProducerStateTable::set(const std::string &key,
const std::vector<FieldValueTuple> &values,
Expand Down
73 changes: 73 additions & 0 deletions tests/mock_tests/portsorch_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1475,6 +1475,79 @@ namespace portsorch_test
_unhook_sai_queue_api();
}

TEST_F(PortsOrchTest, PortDeleteQueueCountersCleanup)
{
Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);
std::deque<KeyOpFieldsValuesTuple> entries;

auto &ports = defaultPortList;
ASSERT_TRUE(!ports.empty());

// Create ports
for (const auto &it : ports)
{
portTable.set(it.first, it.second);
}

portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } });
portTable.set("PortInitDone", { { "lanes", "0" } });

gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();

Port port;
auto testQueueIdx = 0;
string portName = "Ethernet0";
ASSERT_TRUE(gPortsOrch->getPort(portName, port));
ASSERT_NE(port.m_port_id, SAI_NULL_OBJECT_ID);
ASSERT_GT(port.m_queue_ids.size(), 0u);

// Enable Queue flex counters
Table flexCounterCfg = Table(m_config_db.get(), CFG_FLEX_COUNTER_TABLE_NAME);
const std::vector<FieldValueTuple> enable({ {FLEX_COUNTER_STATUS_FIELD, "enable"} });
flexCounterCfg.set("QUEUE_WATERMARK", enable);
flexCounterCfg.set("QUEUE", enable);

auto flexCounterOrch = gDirectory.get<FlexCounterOrch*>();
flexCounterOrch->addExistingData(&flexCounterCfg);
static_cast<Orch *>(flexCounterOrch)->doTask();

sai_object_id_t targetQueueOid = port.m_queue_ids[testQueueIdx];

// Delete the port
entries.push_back({portName, "DEL", { { } }});
auto consumer = dynamic_cast<Consumer *>(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME));
consumer->addToSync(entries);
static_cast<Orch *>(gPortsOrch)->doTask();
entries.clear();

Table countersQueueNameMap(m_counters_db.get(), "COUNTERS_QUEUE_NAME_MAP");
Table countersQueuePortMap(m_counters_db.get(), "COUNTERS_QUEUE_PORT_MAP");

// Verify specific alias:idx field is gone from name maps
string dummy;
ASSERT_FALSE(countersQueueNameMap.hget("", portName + ":" + to_string(testQueueIdx), dummy));
ASSERT_FALSE(countersQueuePortMap.hget("", sai_serialize_object_id(targetQueueOid), dummy));

// Re-add the same port
auto it = ports.find(portName);
entries.push_back({portName, "SET", it->second});
consumer->addToSync(entries);
static_cast<Orch *>(gPortsOrch)->doTask();
entries.clear();

// Fetch the re-added port and verify COUNTERS_DB entries exist for the queue index
Port readded;
ASSERT_TRUE(gPortsOrch->getPort(portName, readded));
ASSERT_GT(readded.m_queue_ids.size(), 0u);
sai_object_id_t readdedQ1 = readded.m_queue_ids[testQueueIdx];

// Name-map should contain alias:idx as a field
ASSERT_TRUE(countersQueueNameMap.hget("", portName + ":" + to_string(testQueueIdx), dummy));
// Port-map should contain queue OID -> port OID mapping as a field
ASSERT_TRUE(countersQueuePortMap.hget("", sai_serialize_object_id(readdedQ1), dummy));
}

TEST_F(PortsOrchTest, PortPTConfigDefaultTimestampTemplate)
{
auto portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);
Expand Down
Loading