-
Notifications
You must be signed in to change notification settings - Fork 522
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
[intfsorch] add RIF flex counter group #765
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
#include <map> | ||
#include <net/if.h> | ||
|
||
#include "sai_serialize.h" | ||
#include "intfsorch.h" | ||
#include "ipprefix.h" | ||
#include "logger.h" | ||
|
@@ -30,10 +31,49 @@ extern BufferOrch *gBufferOrch; | |
|
||
const int intfsorch_pri = 35; | ||
|
||
#define RIF_FLEX_STAT_COUNTER_POLL_MSECS "1000" | ||
#define UPDATE_MAPS_SEC 1 | ||
|
||
static const vector<sai_router_interface_stat_t> rifStatIds = | ||
{ | ||
SAI_ROUTER_INTERFACE_STAT_IN_PACKETS, | ||
SAI_ROUTER_INTERFACE_STAT_IN_OCTETS, | ||
SAI_ROUTER_INTERFACE_STAT_IN_ERROR_PACKETS, | ||
SAI_ROUTER_INTERFACE_STAT_IN_ERROR_OCTETS, | ||
SAI_ROUTER_INTERFACE_STAT_OUT_PACKETS, | ||
SAI_ROUTER_INTERFACE_STAT_OUT_OCTETS, | ||
SAI_ROUTER_INTERFACE_STAT_OUT_ERROR_PACKETS, | ||
SAI_ROUTER_INTERFACE_STAT_OUT_ERROR_OCTETS, | ||
}; | ||
|
||
IntfsOrch::IntfsOrch(DBConnector *db, string tableName, VRFOrch *vrf_orch) : | ||
Orch(db, tableName, intfsorch_pri), m_vrfOrch(vrf_orch) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
/* Initialize DB connectors */ | ||
m_counter_db = shared_ptr<DBConnector>(new DBConnector(COUNTERS_DB, DBConnector::DEFAULT_UNIXSOCKET, 0)); | ||
m_flex_db = shared_ptr<DBConnector>(new DBConnector(FLEX_COUNTER_DB, DBConnector::DEFAULT_UNIXSOCKET, 0)); | ||
// m_state_db = shared_ptr<DBConnector>(new DBConnector(STATE_DB, DBConnector::DEFAULT_UNIXSOCKET, 0)); | ||
m_asic_db = shared_ptr<DBConnector>(new DBConnector(ASIC_DB, DBConnector::DEFAULT_UNIXSOCKET, 0)); | ||
/* Initialize COUNTER_DB tables */ | ||
m_rifNameTable = unique_ptr<Table>(new Table(m_counter_db.get(), COUNTERS_RIF_NAME_MAP)); | ||
m_rifTypeTable = unique_ptr<Table>(new Table(m_counter_db.get(), COUNTERS_RIF_TYPE_MAP)); | ||
|
||
// m_stateInterfaceTable = unique_ptr<Table>(new Table(m_state_db.get(), STATE_INTERFACE_TABLE_NAME)); | ||
m_vidToRidTable = unique_ptr<Table>(new Table(m_asic_db.get(), "VIDTORID")); | ||
auto intervT = timespec { .tv_sec = UPDATE_MAPS_SEC , .tv_nsec = 0 }; | ||
m_updateMapsTimer = new SelectableTimer(intervT); | ||
auto executorT = new ExecutableTimer(m_updateMapsTimer, this, "UPDATE_MAPS_TIMER"); | ||
Orch::addExecutor(executorT); | ||
/* Initialize FLEX_COUNTER_DB tables */ | ||
m_flexCounterTable = unique_ptr<ProducerTable>(new ProducerTable(m_flex_db.get(), FLEX_COUNTER_TABLE)); | ||
m_flexCounterGroupTable = unique_ptr<ProducerTable>(new ProducerTable(m_flex_db.get(), FLEX_COUNTER_GROUP_TABLE)); | ||
|
||
vector<FieldValueTuple> fieldValues; | ||
fieldValues.emplace_back(POLL_INTERVAL_FIELD, RIF_FLEX_STAT_COUNTER_POLL_MSECS); | ||
fieldValues.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ); | ||
m_flexCounterGroupTable->set(RIF_STAT_COUNTER_FLEX_COUNTER_GROUP, fieldValues); | ||
} | ||
|
||
sai_object_id_t IntfsOrch::getRouterIntfsId(const string &alias) | ||
|
@@ -431,6 +471,7 @@ bool IntfsOrch::addRouterIntfs(sai_object_id_t vrf_id, Port &port) | |
port.m_vr_id = vrf_id; | ||
|
||
gPortsOrch->setPort(port.m_alias, port); | ||
m_rifsToAdd.push_back(port); | ||
|
||
SWSS_LOG_NOTICE("Create router interface %s MTU %u", port.m_alias.c_str(), port.m_mtu); | ||
|
||
|
@@ -447,6 +488,9 @@ bool IntfsOrch::removeRouterIntfs(Port &port) | |
return false; | ||
} | ||
|
||
const auto id = sai_serialize_object_id(port.m_rif_id); | ||
removeRifFromFlexCounter(id, port.m_alias); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this called directly whereas There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is due to FC logic. If there will be some issues with this logic, I believe the FC flow should be changed. Right now it's tricky to dynamically add/remove objects that need to be polled. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How to ensure deleting flex counter is before deleting rif in syncd? If deleting rif happened before deleting flex counter, rid cannot be found in the VIDTOIRID. |
||
|
||
sai_status_t status = sai_router_intfs_api->remove_router_interface(port.m_rif_id); | ||
if (status != SAI_STATUS_SUCCESS) | ||
{ | ||
|
@@ -652,3 +696,104 @@ void IntfsOrch::removeDirectedBroadcast(const Port &port, const IpAddress &ip_ad | |
|
||
SWSS_LOG_NOTICE("Remove broadcast route ip:%s", ip_addr.to_string().c_str()); | ||
} | ||
|
||
void IntfsOrch::addRifToFlexCounter(const string &id, const string &name, const string &type) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
/* update RIF maps in COUNTERS_DB */ | ||
vector<FieldValueTuple> rifNameVector; | ||
vector<FieldValueTuple> rifTypeVector; | ||
|
||
rifNameVector.emplace_back(name, id); | ||
rifTypeVector.emplace_back(id, type); | ||
|
||
m_rifNameTable->set("", rifNameVector); | ||
m_rifTypeTable->set("", rifTypeVector); | ||
|
||
/* update RIF in FLEX_COUNTER_DB */ | ||
string key = getRifFlexCounterTableKey(id); | ||
|
||
|
||
std::ostringstream counters_stream; | ||
for (const auto& it: rifStatIds) | ||
{ | ||
counters_stream << sai_serialize_router_interface_stat(it) << comma; | ||
} | ||
|
||
/* check the state of intf, if registering the intf to FC will result in runtime error */ | ||
vector<FieldValueTuple> fvt; | ||
vector<FieldValueTuple> fieldValues; | ||
fieldValues.emplace_back(RIF_COUNTER_ID_LIST, counters_stream.str()); | ||
|
||
m_flexCounterTable->set(key, fieldValues); | ||
SWSS_LOG_DEBUG("Registered interface %s to Flex counter", name.c_str()); | ||
} | ||
|
||
void IntfsOrch::removeRifFromFlexCounter(const string &id, const string &name) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
/* remove it from COUNTERS_DB maps */ | ||
m_rifNameTable->hdel("", name); | ||
m_rifTypeTable->hdel("", id); | ||
|
||
/* remove it from FLEX_COUNTER_DB */ | ||
string key = getRifFlexCounterTableKey(id); | ||
|
||
vector<FieldValueTuple> fieldValues; | ||
fieldValues.emplace_back(RIF_COUNTER_ID_LIST, ""); | ||
|
||
m_flexCounterTable->set(key, fieldValues); | ||
SWSS_LOG_DEBUG("Unregistered interface %s from Flex counter", name.c_str()); | ||
} | ||
|
||
string IntfsOrch::getRifFlexCounterTableKey(string key) | ||
{ | ||
return string(RIF_STAT_COUNTER_FLEX_COUNTER_GROUP) + ":" + key; | ||
} | ||
|
||
void IntfsOrch::generateInterfaceMap() | ||
{ | ||
m_updateMapsTimer->start(); | ||
} | ||
|
||
void IntfsOrch::doTask(SelectableTimer &timer) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
SWSS_LOG_NOTICE("Registering %ld new intfs, deleting %ld ", m_rifsToAdd.size(), m_rifsToRemove.size()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest to change to INFO/DEBUG level |
||
string value; | ||
for (auto it = m_rifsToAdd.begin(); it != m_rifsToAdd.end(); ++it) | ||
{ | ||
SWSS_LOG_NOTICE("Registering begin -> %s ", it->m_alias.c_str()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we have a separate for-loop for this log? With the log in below for-loop, this appears to be redundant. |
||
} | ||
for (auto it = m_rifsToAdd.begin(); it != m_rifsToAdd.end(); ) | ||
{ | ||
const auto id = sai_serialize_object_id(it->m_rif_id); | ||
SWSS_LOG_NOTICE("Registering %s, id %s", it->m_alias.c_str(), id.c_str()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest to change this to INFO level |
||
std::string type; | ||
switch(it->m_type) | ||
{ | ||
case Port::PHY: | ||
case Port::LAG: | ||
type = "SAI_ROUTER_INTERFACE_TYPE_PORT"; | ||
break; | ||
case Port::VLAN: | ||
type = "SAI_ROUTER_INTERFACE_TYPE_VLAN"; | ||
break; | ||
default: | ||
SWSS_LOG_ERROR("Unsupported port type: %d", it->m_type); | ||
type = ""; | ||
break; | ||
} | ||
if (m_vidToRidTable->hget("", id, value)) | ||
{ | ||
SWSS_LOG_NOTICE("Registering %s it is ready", it->m_alias.c_str()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest INFO level |
||
addRifToFlexCounter(id, it->m_alias, type); | ||
it = m_rifsToAdd.erase(it); | ||
} | ||
else | ||
{ | ||
++it; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
#include "orch.h" | ||
#include "portsorch.h" | ||
#include "vrforch.h" | ||
#include "timer.h" | ||
|
||
#include "ipaddresses.h" | ||
#include "ipprefix.h" | ||
|
@@ -15,6 +16,8 @@ | |
extern sai_object_id_t gVirtualRouterId; | ||
extern MacAddress gMacAddress; | ||
|
||
#define RIF_STAT_COUNTER_FLEX_COUNTER_GROUP "RIF_STAT_COUNTER" | ||
|
||
struct IntfsEntry | ||
{ | ||
std::set<IpPrefix> ip_addresses; | ||
|
@@ -35,10 +38,32 @@ class IntfsOrch : public Orch | |
|
||
bool setRouterIntfsMtu(Port &port); | ||
std::set<IpPrefix> getSubnetRoutes(); | ||
|
||
void generateInterfaceMap(); | ||
void addRifToFlexCounter(const string&, const string&, const string&); | ||
void removeRifFromFlexCounter(const string&, const string&); | ||
|
||
private: | ||
|
||
SelectableTimer* m_updateMapsTimer = nullptr; | ||
std::vector<Port> m_rifsToAdd; | ||
std::vector<Port> m_rifsToRemove; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see this updated any where. Is it missed? |
||
|
||
VRFOrch *m_vrfOrch; | ||
IntfsTable m_syncdIntfses; | ||
void doTask(Consumer &consumer); | ||
void doTask(SelectableTimer &timer); | ||
|
||
shared_ptr<DBConnector> m_counter_db; | ||
shared_ptr<DBConnector> m_flex_db; | ||
shared_ptr<DBConnector> m_asic_db; | ||
unique_ptr<Table> m_rifNameTable; | ||
unique_ptr<Table> m_rifTypeTable; | ||
unique_ptr<Table> m_vidToRidTable; | ||
unique_ptr<ProducerTable> m_flexCounterTable; | ||
unique_ptr<ProducerTable> m_flexCounterGroupTable; | ||
|
||
std::string getRifFlexCounterTableKey(std::string s); | ||
|
||
int getRouterIntfsRefCount(const string&); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove commented code