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.

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

[DASH] Add support for ENI counters #30

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
125 changes: 124 additions & 1 deletion orchagent/dash/dashorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "tokenize.h"
#include "crmorch.h"
#include "saihelper.h"
#include "flex_counter_manager.h"

#include "taskworker.h"
#include "pbutils.h"
Expand All @@ -31,10 +32,38 @@ extern sai_dash_eni_api_t* sai_dash_eni_api;
extern sai_object_id_t gSwitchId;
extern size_t gMaxBulkSize;
extern CrmOrch *gCrmOrch;
extern bool gTraditionalFlexCounter;

DashOrch::DashOrch(DBConnector *db, vector<string> &tableName, ZmqServer *zmqServer) : ZmqOrch(db, tableName, zmqServer)
#define FLEX_COUNTER_UPD_INTERVAL 1

Choose a reason for hiding this comment

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

Why there are two defines for the update interval? What is the difference comparing to ENI_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS?

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's redundant, will remove

Copy link
Owner Author

Choose a reason for hiding this comment

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

Actually i'm wrong sorry. FLEX_COUNTER_UPD_INTERVAL is the period for SelectableTimer. ENI_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS is the default polling interval for FlexCounter i.e. to collect and populate the COUNTERS_DB


DashOrch::DashOrch(DBConnector *db, vector<string> &tableName, ZmqServer *zmqServer) :
ZmqOrch(db, tableName, zmqServer),
m_eni_stat_manager(ENI_STAT_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, ENI_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS, false)
{
SWSS_LOG_ENTER();

m_asic_db = std::shared_ptr<DBConnector>(new DBConnector("ASIC_DB", 0));
m_counter_db = std::shared_ptr<DBConnector>(new DBConnector("COUNTERS_DB", 0));
m_eni_name_table = std::unique_ptr<Table>(new Table(m_counter_db.get(), COUNTERS_ENI_NAME_MAP));

if (gTraditionalFlexCounter)
{
m_vid_to_rid_table = std::make_unique<Table>(m_asic_db.get(), "VIDTORID");
}

auto intervT = timespec { .tv_sec = FLEX_COUNTER_UPD_INTERVAL , .tv_nsec = 0 };
m_fc_update_timer = new SelectableTimer(intervT);
auto executorT = new ExecutableTimer(m_fc_update_timer, this, "FLEX_COUNTER_UPD_TIMER");
Orch::addExecutor(executorT);

/* Fetch the available counter Ids */
m_counter_stats.clear();
auto stat_enum_list = queryAvailableCounterStats((sai_object_type_t)SAI_OBJECT_TYPE_ENI);
for (auto &stat_enum: stat_enum_list)
{
auto counter_id = static_cast<sai_eni_stat_t>(stat_enum);
m_counter_stats.insert(sai_serialize_eni_stat(counter_id));
}
}

bool DashOrch::addApplianceEntry(const string& appliance_id, const dash::appliance::Appliance &entry)
Expand Down Expand Up @@ -370,6 +399,8 @@ bool DashOrch::addEniObject(const string& eni, EniEntry& entry)
}
}

addEniToFC(eni_id, eni);

gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_DASH_ENI);

SWSS_LOG_NOTICE("Created ENI object for %s", eni.c_str());
Expand Down Expand Up @@ -468,6 +499,8 @@ bool DashOrch::removeEniObject(const string& eni)
}
}

removeEniFromFC(entry.eni_id, eni);

gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_DASH_ENI);

SWSS_LOG_NOTICE("Removed ENI object for %s", eni.c_str());
Expand Down Expand Up @@ -682,3 +715,93 @@ void DashOrch::doTask(ConsumerBase& consumer)
SWSS_LOG_ERROR("Unknown table: %s", tn.c_str());
}
}

void DashOrch::removeEniFromFC(sai_object_id_t oid, const string &name)
{
SWSS_LOG_ENTER();

if (oid == SAI_NULL_OBJECT_ID)
{
SWSS_LOG_WARN("Cannot remove counter on NULL OID for eni %s", name.c_str());
return;
}

if (m_eni_stat_work_queue.find(oid) != m_eni_stat_work_queue.end())
{
m_eni_stat_work_queue.erase(oid);
return;
}

m_eni_name_table->hdel("", name);
m_eni_stat_manager.clearCounterIdList(oid);
SWSS_LOG_DEBUG("Unregistered eni %s to Flex counter", name.c_str());
}

void DashOrch::clearEniFCStats()
{
for (auto it = eni_entries_.begin(); it != eni_entries_.end(); it++)
{
removeEniFromFC(it->second.eni_id, it->first);
}
}

void DashOrch::handleFCStatusUpdate(bool enabled)
{
if (!enabled && m_eni_fc_status)
{
m_fc_update_timer->stop();
clearEniFCStats();
}
else if (enabled && !m_eni_fc_status)
{
m_fc_update_timer->start();
}
m_eni_fc_status = enabled;
}

void DashOrch::addEniToFC(sai_object_id_t oid, const string &name)
{
auto was_empty = m_eni_stat_work_queue.empty();
m_eni_stat_work_queue[oid] = name;
if (was_empty)
{
m_fc_update_timer->start();
}
}

void DashOrch::doTask(SelectableTimer &timer)
{
SWSS_LOG_ENTER();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to start this at all in case if we are not using traditional model? All it does in non traditional approach is to ++it.

Copy link
Owner Author

Choose a reason for hiding this comment

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

The update logic will run regardless of the model. If it's asic_db approach, we don't wait for vidtorid table to be populated (cause since it's through ASIC channel, this op is guarenteed to be run after the actual object creation.

In the case of traditional counter, we are waiting until the vidtorid object is created before population FLEX_COUNTER_DB


if (!m_eni_fc_status)
{
m_fc_update_timer->stop();
return ;
}

for (auto it = m_eni_stat_work_queue.begin(); it != m_eni_stat_work_queue.end(); )
{
string value;
const auto id = sai_serialize_object_id(it->first);

if (!gTraditionalFlexCounter || m_vid_to_rid_table->hget("", id, value))
{
SWSS_LOG_INFO("Registering %s, id %s", it->second.c_str(), id.c_str());
std::vector<FieldValueTuple> eniNameFvs;
eniNameFvs.emplace_back(it->second, id);
m_eni_name_table->set("", eniNameFvs);

m_eni_stat_manager.setCounterIdList(it->first, CounterType::ENI, m_counter_stats);
it = m_eni_stat_work_queue.erase(it);
}
else
{
++it;
}
}

if (m_eni_stat_work_queue.empty())
Copy link
Collaborator

Choose a reason for hiding this comment

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

In case of ! gTraditionalFlexCounter since the queue may not be empty this may loop for ever on creation of single eni. This may be unwarranted.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Same comment as above m_eni_stat_work_queue will be empty eventually

{
m_fc_update_timer->stop();
}
}
22 changes: 22 additions & 0 deletions orchagent/dash/dashorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@
#include "timer.h"
#include "zmqorch.h"
#include "zmqserver.h"
#include "flex_counter_manager.h"

#include "dash_api/appliance.pb.h"
#include "dash_api/route_type.pb.h"
#include "dash_api/eni.pb.h"
#include "dash_api/qos.pb.h"

#define ENI_STAT_COUNTER_FLEX_COUNTER_GROUP "ENI_STAT_COUNTER"
#define ENI_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS 10000
Copy link
Collaborator

Choose a reason for hiding this comment

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

How did we decide the default polling interval as 10 sec. Is it based on some spec?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Every other object other than PORT has a default of 10 sec. you think i should update this?


struct EniEntry
{
sai_object_id_t eni_id;
Expand All @@ -39,12 +43,14 @@ class DashOrch : public ZmqOrch
public:
DashOrch(swss::DBConnector *db, std::vector<std::string> &tables, swss::ZmqServer *zmqServer);
const EniEntry *getEni(const std::string &eni) const;
void handleFCStatusUpdate(bool is_enabled);

private:
ApplianceTable appliance_entries_;
RoutingTypeTable routing_type_entries_;
EniTable eni_entries_;
QosTable qos_entries_;

void doTask(ConsumerBase &consumer);
void doTaskApplianceTable(ConsumerBase &consumer);
void doTaskRoutingTypeTable(ConsumerBase &consumer);
Expand All @@ -63,4 +69,20 @@ class DashOrch : public ZmqOrch
bool setEniAdminState(const std::string& eni, const EniEntry& entry);
bool addQosEntry(const std::string& qos_name, const dash::qos::Qos &entry);
bool removeQosEntry(const std::string& qos_name);

private:
std::map<sai_object_id_t, std::string> m_eni_stat_work_queue;
FlexCounterManager m_eni_stat_manager;
bool m_eni_fc_status = false;
std::unordered_set<std::string> m_counter_stats;
std::unique_ptr<swss::Table> m_eni_name_table;
std::unique_ptr<swss::Table> m_vid_to_rid_table;
std::shared_ptr<swss::DBConnector> m_counter_db;
std::shared_ptr<swss::DBConnector> m_asic_db;
swss::SelectableTimer* m_fc_update_timer = nullptr;

void doTask(swss::SelectableTimer&);
void addEniToFC(sai_object_id_t oid, const std::string& name);
void removeEniFromFC(sai_object_id_t oid, const std::string& name);
void clearEniFCStats();
};
1 change: 1 addition & 0 deletions orchagent/flex_counter/flex_counter_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const unordered_map<CounterType, string> FlexCounterManager::counter_id_field_lo
{ CounterType::TUNNEL, TUNNEL_COUNTER_ID_LIST },
{ CounterType::HOSTIF_TRAP, FLOW_COUNTER_ID_LIST },
{ CounterType::ROUTE, FLOW_COUNTER_ID_LIST },
{ CounterType::ENI, ENI_COUNTER_ID_LIST },
};

FlexManagerDirectory g_FlexManagerDirectory;
Expand Down
1 change: 1 addition & 0 deletions orchagent/flex_counter/flex_counter_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ enum class CounterType
TUNNEL,
HOSTIF_TRAP,
ROUTE,
ENI
};

// FlexCounterManager allows users to manage a group of flex counters.
Expand Down
8 changes: 8 additions & 0 deletions orchagent/flexcounterorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <swss/tokenize.h>
#include "routeorch.h"
#include "macsecorch.h"
#include "dash/dashorch.h"
#include "flowcounterrouteorch.h"

extern sai_port_api_t *sai_port_api;
Expand All @@ -39,6 +40,7 @@ extern sai_object_id_t gSwitchId;
#define TUNNEL_KEY "TUNNEL"
#define FLOW_CNT_TRAP_KEY "FLOW_CNT_TRAP"
#define FLOW_CNT_ROUTE_KEY "FLOW_CNT_ROUTE"
#define ENI_KEY "ENI"

unordered_map<string, string> flexCounterGroupMap =
{
Expand All @@ -61,6 +63,7 @@ unordered_map<string, string> flexCounterGroupMap =
{"MACSEC_SA", COUNTERS_MACSEC_SA_GROUP},
{"MACSEC_SA_ATTR", COUNTERS_MACSEC_SA_ATTR_GROUP},
{"MACSEC_FLOW", COUNTERS_MACSEC_FLOW_GROUP},
{"ENI", ENI_STAT_COUNTER_FLEX_COUNTER_GROUP}
};


Expand All @@ -84,6 +87,7 @@ void FlexCounterOrch::doTask(Consumer &consumer)
SWSS_LOG_ENTER();

VxlanTunnelOrch* vxlan_tunnel_orch = gDirectory.get<VxlanTunnelOrch*>();
DashOrch* dash_orch = gDirectory.get<DashOrch*>();
if (gPortsOrch && !gPortsOrch->allPortsReady())
{
return;
Expand Down Expand Up @@ -200,6 +204,10 @@ void FlexCounterOrch::doTask(Consumer &consumer)
{
vxlan_tunnel_orch->generateTunnelCounterMap();
}
if (dash_orch && (key == ENI_KEY))
{
dash_orch->handleFCStatusUpdate((value == "enable"));
}
if (gCoppOrch && (key == FLOW_CNT_TRAP_KEY))
{
if (value == "enable")
Expand Down
23 changes: 23 additions & 0 deletions orchagent/saihelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1100,3 +1100,26 @@ void stopFlexCounterPolling(sai_object_id_t switch_oid,

sai_switch_api->set_switch_attribute(switch_oid, &attr);
}

/*
Use metadata info of the SAI object to infer all the available stats
Syncd already has logic to filter out the supported stats
*/
std::vector<sai_stat_id_t> queryAvailableCounterStats(const sai_object_type_t object_type)
{
std::vector<sai_stat_id_t> stat_list;
auto info = sai_metadata_get_object_type_info(object_type);

SWSS_LOG_NOTICE("SAI object %s supports stat type %s",
sai_serialize_object_type(object_type).c_str(),
info->statenum->name);

auto statenumlist = info->statenum->values;
auto statnumcount = (uint32_t)info->statenum->valuescount;

for (uint32_t i = 0; i < statnumcount; i++)
{
stat_list.push_back(static_cast<sai_stat_id_t>(statenumlist[i]));
}
return stat_list;
}
2 changes: 2 additions & 0 deletions orchagent/saihelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,5 @@ void startFlexCounterPolling(sai_object_id_t switch_oid,
const std::string &stats_mode="");
void stopFlexCounterPolling(sai_object_id_t switch_oid,
const std::string &key);

std::vector<sai_stat_id_t> queryAvailableCounterStats(const sai_object_type_t);
Loading
Loading