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.

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

Jump to bottom

[portsorch,intfsorch] add port, rif rates FC groups #1201

Merged
merged 7 commits into from
Jul 14, 2020
Merged

[portsorch,intfsorch] add port, rif rates FC groups #1201

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4b1e0c7
[portsorch,intfsorch] add port, rif rates FC groups
Jan 28, 2020
a2e95d2
fix comments
Feb 17, 2020
7290ee2
fix comments
Apr 9, 2020
6ea64bd
fix comments
Apr 21, 2020
0c22b13
fix comments
Apr 23, 2020
57613ce
remove speed calc from lua scripts
Jun 5, 2020
c7fdbac
trigger lgtm
Jun 10, 2020
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
2 changes: 2 additions & 0 deletions orchagent/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ CFLAGS_SAI = -I /usr/include/sai
swssdir = $(datadir)/swss

dist_swss_DATA = \
rif_rates.lua \
pfc_detect_innovium.lua \
pfc_detect_mellanox.lua \
pfc_detect_broadcom.lua \
pfc_detect_barefoot.lua \
pfc_detect_nephos.lua \
pfc_restore.lua \
port_rates.lua \
watermark_queue.lua \
watermark_pg.lua \
watermark_bufferpool.lua
Expand Down
2 changes: 2 additions & 0 deletions orchagent/flexcounterorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ extern BufferOrch *gBufferOrch;
unordered_map<string, string> flexCounterGroupMap =
{
{"PORT", PORT_STAT_COUNTER_FLEX_COUNTER_GROUP},
{"PORT_RATES", PORT_RATE_COUNTER_FLEX_COUNTER_GROUP},
{"QUEUE", QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP},
{"PFCWD", PFC_WD_FLEX_COUNTER_GROUP},
{"QUEUE_WATERMARK", QUEUE_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP},
{"PG_WATERMARK", PG_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP},
{BUFFER_POOL_WATERMARK_KEY, BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP},
{"RIF", RIF_STAT_COUNTER_FLEX_COUNTER_GROUP},
{"RIF_RATES", RIF_RATE_COUNTER_FLEX_COUNTER_GROUP},
{"DEBUG_COUNTER", DEBUG_COUNTER_FLEX_COUNTER_GROUP},
};

Expand Down
20 changes: 18 additions & 2 deletions orchagent/intfsorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,24 @@ IntfsOrch::IntfsOrch(DBConnector *db, string tableName, VRFOrch *vrf_orch) :
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);

string rifRatePluginName = "rif_rates.lua";

try
{
string rifRateLuaScript = swss::loadLuaScript(rifRatePluginName);
string rifRateSha = swss::loadRedisScript(m_counter_db.get(), rifRateLuaScript);

vector<FieldValueTuple> fieldValues;
fieldValues.emplace_back(RIF_PLUGIN_FIELD, rifRateSha);
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);
}
catch (const runtime_error &e)
{
SWSS_LOG_WARN("RIF flex counter group plugins was not set successfully: %s", e.what());
}
}

sai_object_id_t IntfsOrch::getRouterIntfsId(const string &alias)
Expand Down Expand Up @@ -1013,7 +1031,6 @@ void IntfsOrch::addRifToFlexCounter(const string &id, const string &name, const
/* update RIF in FLEX_COUNTER_DB */
string key = getRifFlexCounterTableKey(id);


std::ostringstream counters_stream;
for (const auto& it: rifStatIds)
{
Expand All @@ -1023,7 +1040,6 @@ void IntfsOrch::addRifToFlexCounter(const string &id, const string &name, const
/* check the state of intf, if registering the intf to FC will result in runtime error */
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());
}
Expand Down
3 changes: 2 additions & 1 deletion orchagent/intfsorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ extern sai_object_id_t gVirtualRouterId;
extern MacAddress gMacAddress;

#define RIF_STAT_COUNTER_FLEX_COUNTER_GROUP "RIF_STAT_COUNTER"
#define RIF_RATE_COUNTER_FLEX_COUNTER_GROUP "RIF_RATE_COUNTER"

struct IntfsEntry
{
Expand All @@ -35,7 +36,7 @@ class IntfsOrch : public Orch
sai_object_id_t getRouterIntfsId(const string&);
bool isPrefixSubnet(const IpPrefix&, const string&);
string getRouterIntfsAlias(const IpAddress &ip, const string &vrf_name = "");

string getRifRateFlexCounterTableKey(string key);
Copy link
Contributor

@qiluo-msft qiluo-msft Apr 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

getRifRateFlexCounterTableKey [](start = 11, length = 29)

above line uses RouterIntfs, here uses Rif. Are they different? #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The latter refers to RIF_RATE Flex Counter group instead of Router Interface.

void increaseRouterIntfsRefCount(const string&);
void decreaseRouterIntfsRefCount(const string&);

Expand Down
88 changes: 88 additions & 0 deletions orchagent/port_rates.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
-- KEYS - port IDs
-- ARGV[1] - counters db index
-- ARGV[2] - counters table name
-- ARGV[3] - poll time interval
-- return log

local logtable = {}

local function logit(msg)
logtable[#logtable+1] = tostring(msg)
end

local counters_db = ARGV[1]
local counters_table_name = ARGV[2]
local rates_table_name = "RATES"

-- Get configuration
redis.call('SELECT', counters_db)
local smooth_interval = redis.call('HGET', rates_table_name .. ':' .. 'PORT', 'PORT_SMOOTH_INTERVAL')
Copy link
Contributor

Choose a reason for hiding this comment

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

: [](start = 64, length = 1)

The table separator are from database_config.json. Do not hardcode.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not trivial to access the database config from the lua script. Should we pass the separator as one of the parameters?
I see the separator hardcoded in a lot of other places, like this. If there is a need to avoid such things in future, I suggest separate PR and refactor it in all the lua scripts.

Some scripts have other dbs/table names hardcoded, and ATM receive the same ARGV. Need to suggest broad solution.

Copy link
Contributor

Choose a reason for hiding this comment

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

Lua parameter is good enough.
Could you please add a TODO comment or Github issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@qiluo-msft Created issue #1319

local alpha = redis.call('HGET', rates_table_name .. ':' .. 'PORT', 'PORT_ALPHA')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this changes causes the following issue on latest SONiC master

Jul 22 11:17:22.139228 dut ERR syncd[25]: :- runRedisScript: Caught exception while running Redis lua script: ERR Error running script (call to f_fd0ea76fc13f9fcc7910e4b1fd8c9018ea197cf4): @user_script:21: user_script:21: attempt to perform arithmetic on local 'alpha' (a boolean value) : Input/output error
Jul 22 11:17:22.786235 dut ERR syncd[25]: :- guard: RedisReply catches system_error: command: *39#015#012$7#015#012EVALSHA#015#012$40#015#01231fc701ca9b1b9f968f501c92b639f50f6346a9c#015#012$2#015#01232#015#012$19#015#012oid:0x100000000004f#015#012$19#015#012oid:0x1000000000067#015#012$19#015#012oid:0x100000000007f#015#012$19#015#012oid:0x1000000000097#015#012$19#015#012oid:0x10000000000af#015#012$19#015#012oid:0x10000000000c7#015#012$19#015#012oid:0x10000000000df#015#012$19#015#012oid:0x10000000000f7#015#012$19#015#012oid:0x100000000010f#015#012$19#015#012oid:0x1000000000127#015#012$19#015#012oid:0x100000000013f#015#012$19#015#012oid:0x1000000000157#015#012$19#015#012oid:0x100000000016f#015#012$19#015#012oid:0x1000000000187#015#012$19#015#012oid:0x100000000019f#015#012$19#015#012oid:0x10000000001b7#015#012$19#015#012oid:0x10000000001cf#015#012$19#015#012oid:0x10000000001e7#015#012$19#015#012oid:0x10000000001ff#015#012$19#015#012oid:0x1000000000217#015#012$19#015#012oid:0x100000000022f#015#012$19#015#012oid:0x1000000000247#015#012$19#015#012oid:0x100000000025f#015#012$19#015#012oid:0x1000000000277#015#012$19#015#012oid:0x100000000028f#015#012$19#015#012oid:0x10000000002a7#015#012$19#015#012oid:0x10000000002bf#015#012$19#015#012oid:0x10000000002d7#015#012$19#015#012oid:0x10000000002ef#015#012$19#015#012oid:0x1000000000307#015#012$19#015#012oid:0x100000000031f#015#012$19#015#012oid:0x1000000000337#015#012$1#015#0122#015#012$8#015#012COUNTERS#015#012$7#015#0121000000#015#012$2#015#012''#015#012, reason: ERR Error running script (call to f_31fc701ca9b1b9f968f501c92b639f50f6346a9c): @user_script:21: user_script:21: attempt to perform arithmetic on local 'alpha' (a boolean value) : Input/output error
Jul 22 11:17:22.786235 dut ERR syncd[25]: :- runRedisScript: Caught exception while running Redis lua script: ERR Error running script (call to f_31fc701ca9b1b9f968f501c92b639f50f6346a9c): @user_script:21: user_script:21: attempt to perform arithmetic on local 'alpha' (a boolean value) : Input/output error
Jul 22 11:17:23.141016 dut ERR syncd[25]: :- guard: RedisReply catches system_error: command: *12#015#012$7#015#012EVALSHA#015#012$40#015#012fd0ea76fc13f9fcc7910e4b1fd8c9018ea197cf4#015#012$1#015#0125#015#012$19#015#012oid:0x6000000000392#015#012$19#015#012oid:0x6000000000393#015#012$19#015#012oid:0x6000000000394#015#012$19#015#012oid:0x6000000000395#015#012$19#015#012oid:0x6000000000396#015#012$1#015#0122#015#012$8#015#012COUNTERS#015#012$7#015#0121000000#015#012$2#015#012''#015#012, reason: ERR Error running script (call to f_fd0ea76fc13f9fcc7910e4b1fd8c9018ea197cf4): @user_script:21: user_script:21: attempt to perform arithmetic on local 'alpha' (a boolean value) : Input/output error
Jul 22 11:17:23.141016 dut ERR syncd[25]: :- runRedisScript: Caught exception while running Redis lua script: ERR Error running script (call to f_fd0ea76fc13f9fcc7910e4b1fd8c9018ea197cf4): @user_script:21: user_script:21: attempt to perform arithmetic on local 'alpha' (a boolean value) : Input/output error
Jul 22 11:17:23.337583 dut NOTICE syncd[25]: :- processBulkQuadEvent: bulk SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER execute with 2 items
Jul 22 11:17:23.735421 dut ERR syncd[25]: :- guard: RedisReply catches system_error: command: *39#015#012$7#015#012EVALSHA#015#012$40#015#01231fc701ca9b1b9f968f501c92b639f50f6346a9c#015#012$2#015#01232#015#012$19#015#012oid:0x100000000004f#015#012$19#015#012oid:0x1000000000067#015#012$19#015#012oid:0x100000000007f#015#012$19#015#012oid:0x1000000000097#015#012$19#015#012oid:0x10000000000af#015#012$19#015#012oid:0x10000000000c7#015#012$19#015#012oid:0x10000000000df#015#012$19#015#012oid:0x10000000000f7#015#012$19#015#012oid:0x100000000010f#015#012$19#015#012oid:0x1000000000127#015#012$19#015#012oid:0x100000000013f#015#012$19#015#012oid:0x1000000000157#015#012$19#015#012oid:0x100000000016f#015#012$19#015#012oid:0x1000000000187#015#012$19#015#012oid:0x100000000019f#015#012$19#015#012oid:0x10000000001b7#015#012$19#015#012oid:0x10000000001cf#015#012$19#015#012oid:0x10000000001e7#015#012$19#015#012oid:0x10000000001ff#015#012$19#015#012oid:0x1000000000217#015#012$19#015#012oid:0x100000000022f#015#012$19#015#012oid:0x1000000000247#015#012$19#015#012oid:0x100000000025f#015#012$19#015#012oid:0x1000000000277#015#012$19#015#012oid:0x100000000028f#015#012$19#015#012oid:0x10000000002a7#015#012$19#015#012oid:0x10000000002bf#015#012$19#015#012oid:0x10000000002d7#015#012$19#015#012oid:0x10000000002ef#015#012$19#015#012oid:0x1000000000307#015#012$19#015#012oid:0x100000000031f#015#012$19#015#012oid:0x1000000000337#015#012$1#015#0122#015#012$8#015#012COUNTERS#015#012$7#015#0121000000#015#012$2#015#012''#015#012, reason: ERR Error running script (call to f_31fc701ca9b1b9f968f501c92b639f50f6346a9c): @user_script:21: user_script:21: attempt to perform arithmetic on local 'alpha' (a boolean value) : Input/output error

Please double check.

local one_minus_alpha = 1.0 - alpha
local delta = tonumber(ARGV[3])

logit(alpha)
logit(one_minus_alpha)
logit(delta)

local initialized = redis.call('HGET', rates_table_name, 'INIT_DONE')

logit(initialized)

for i = 1, n do
-- Get new COUNTERS values
local in_ucast_pkts = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_IN_UCAST_PKTS')
local in_non_ucast_pkts = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_IN_NON_UCAST_PKTS')
local out_ucast_pkts = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_OUT_UCAST_PKTS')
local out_non_ucast_pkts = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_OUT_NON_UCAST_PKTS')
local in_octets = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_IN_OCTETS')
local out_octets = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_OUT_OCTETS')

if initialized == 'DONE' or initialized == 'COUNTERS_LAST' then
-- Get old COUNTERS values
local in_ucast_pkts_last = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_IN_UCAST_PKTS_last')
local in_non_ucast_pkts_last = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_IN_NON_UCAST_PKTS_last')
local out_ucast_pkts_last = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_OUT_UCAST_PKTS_last')
local out_non_ucast_pkts_last = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_OUT_NON_UCAST_PKTS_last')
local in_octets_last = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_IN_OCTETS_last')
local out_octets_last = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_OUT_OCTETS_last')

-- Calculate new rates values
local rx_bps_new = (in_octets - in_octets_last)/delta
local tx_bps_new = (out_octets - out_octets_last)/delta
local rx_pps_new = ((in_ucast_pkts + in_non_ucast_pkts) - (in_ucast_pkts_last + in_non_ucast_pkts_last))/delta
local tx_pps_new = ((out_ucast_pkts + out_non_ucast_pkts) - (out_ucast_pkts_last + out_non_ucast_pkts_last))/delta

if initialized == "DONE" then
-- Get old rates values
local rx_bps_old = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'RX_BPS')
local rx_pps_old = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'RX_PPS')
local tx_bps_old = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'TX_BPS')
local tx_pps_old = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'TX_PPS')

-- Smooth the rates values and store them in DB
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'RX_BPS', alpha*rx_bps_new + one_minus_alpha*rx_bps_old)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'RX_PPS', alpha*rx_pps_new + one_minus_alpha*rx_pps_old)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'TX_BPS', alpha*tx_bps_new + one_minus_alpha*tx_bps_old)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'TX_PPS', alpha*tx_pps_new + one_minus_alpha*tx_pps_old)
else
-- Store unsmoothed initial rates values in DB
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'RX_BPS', rx_bps_new)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'RX_PPS', rx_pps_new)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'TX_BPS', tx_bps_new)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'TX_PPS', tx_pps_new)
redis.call('HSET', rates_table_name, 'INIT_DONE', 'DONE')
end
else
-- Set old COUNTERS values
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_IN_UCAST_PKTS_last', in_ucast_pkts)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_IN_NON_UCAST_PKTS_last', in_non_ucast_pkts)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_OUT_UCAST_PKTS_last', out_ucast_pkts)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_OUT_NON_UCAST_PKTS_last', out_non_ucast_pkts)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_IN_OCTETS_last', in_octets)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'SAI_PORT_STAT_IF_OUT_OCTETS_last', out_octets)
redis.call('HSET', rates_table_name, 'INIT_DONE', 'COUNTERS_LAST')
end
end

return logtable
15 changes: 12 additions & 3 deletions orchagent/portsorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ extern BufferOrch *gBufferOrch;
#define QUEUE_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS 10000
#define QUEUE_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS "10000"
#define PG_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS "10000"
#define PORT_RATE_FLEX_COUNTER_POLLING_INTERVAL_MS "1000"


static map<string, sai_port_fec_mode_t> fec_mode_map =
Expand Down Expand Up @@ -185,6 +186,7 @@ PortsOrch::PortsOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames)
string queueWmSha, pgWmSha;
string queueWmPluginName = "watermark_queue.lua";
string pgWmPluginName = "watermark_pg.lua";
string portRatePluginName = "port_rates.lua";

try
{
Expand All @@ -194,6 +196,9 @@ PortsOrch::PortsOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames)
string pgLuaScript = swss::loadLuaScript(pgWmPluginName);
pgWmSha = swss::loadRedisScript(m_counter_db.get(), pgLuaScript);

string portRateLuaScript = swss::loadLuaScript(portRatePluginName);
string portRateSha = swss::loadRedisScript(m_counter_db.get(), portRateLuaScript);

vector<FieldValueTuple> fieldValues;
fieldValues.emplace_back(QUEUE_PLUGIN_FIELD, queueWmSha);
fieldValues.emplace_back(POLL_INTERVAL_FIELD, QUEUE_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS);
Expand All @@ -205,10 +210,16 @@ PortsOrch::PortsOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames)
fieldValues.emplace_back(POLL_INTERVAL_FIELD, PG_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS);
fieldValues.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ_AND_CLEAR);
m_flexCounterGroupTable->set(PG_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP, fieldValues);

fieldValues.clear();
fieldValues.emplace_back(PORT_PLUGIN_FIELD, portRateSha);
fieldValues.emplace_back(POLL_INTERVAL_FIELD, PORT_RATE_FLEX_COUNTER_POLLING_INTERVAL_MS);
fieldValues.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ);
m_flexCounterGroupTable->set(PORT_STAT_COUNTER_FLEX_COUNTER_GROUP, fieldValues);
}
catch (const runtime_error &e)
{
SWSS_LOG_ERROR("Watermark flex counter groups were not set successfully: %s", e.what());
SWSS_LOG_ERROR("Port flex counter groups were not set successfully: %s", e.what());
}
Copy link
Contributor

@qiluo-msft qiluo-msft Apr 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

duplicated code #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Combined into one try block

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for misleading. I mean the code change is almost the same as in IntfsOrch::IntfsOrch(). Is it possible to reuse code?

In reply to: 412570352 [](ancestors = 412570352)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suggest that I revert this particular change,
So we can go forward with the review.
The flex counter plugin registering procedure can be refactored in later PR. There is the FlexCounterManager class, we can extend to manage plugins also, but as separate effort.


uint32_t i, j;
Expand Down Expand Up @@ -1503,15 +1514,13 @@ bool PortsOrch::initPort(const string &alias, const set<int> &lane_set)
vector<FieldValueTuple> fields;
fields.push_back(tuple);
m_counterTable->set("", fields);

// Install a flex counter for this port to track stats
std::unordered_set<std::string> counter_stats;
for (const auto& it: port_stat_ids)
{
counter_stats.emplace(sai_serialize_port_stat(it));
}
port_stat_manager.setCounterIdList(p.m_port_id, CounterType::PORT, counter_stats);

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

Expand Down
3 changes: 3 additions & 0 deletions orchagent/portsorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#define FCS_LEN 4
#define VLAN_TAG_LEN 4
#define PORT_STAT_COUNTER_FLEX_COUNTER_GROUP "PORT_STAT_COUNTER"
#define PORT_RATE_COUNTER_FLEX_COUNTER_GROUP "PORT_RATE_COUNTER"
#define QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP "QUEUE_STAT_COUNTER"
#define QUEUE_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP "QUEUE_WATERMARK_STAT_COUNTER"
#define PG_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP "PG_WATERMARK_STAT_COUNTER"
Expand Down Expand Up @@ -93,6 +94,7 @@ class PortsOrch : public Orch, public Subject
bool addSubPort(Port &port, const string &alias, const bool &adminUp = true, const uint32_t &mtu = 0);
bool removeSubPort(const string &alias);
private:

unique_ptr<Table> m_counterTable;
unique_ptr<Table> m_portTable;
unique_ptr<Table> m_queueTable;
Expand All @@ -107,6 +109,7 @@ class PortsOrch : public Orch, public Subject

std::string getQueueWatermarkFlexCounterTableKey(std::string s);
std::string getPriorityGroupWatermarkFlexCounterTableKey(std::string s);
std::string getPortRateFlexCounterTableKey(std::string s);

shared_ptr<DBConnector> m_counter_db;
shared_ptr<DBConnector> m_flex_db;
Expand Down
77 changes: 77 additions & 0 deletions orchagent/rif_rates.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
-- KEYS - rif IDs
-- ARGV[1] - counters db index
-- ARGV[2] - counters table name
-- ARGV[3] - poll time interval
-- return log

local logtable = {}

local function logit(msg)
logtable[#logtable+1] = tostring(msg)
end

local counters_db = ARGV[1]
local counters_table_name = ARGV[2]
local rates_table_name = "RATES"

-- Get configuration
redis.call('SELECT', counters_db)
local smooth_interval = redis.call('HGET', rates_table_name .. ':' .. 'RIF', 'RIF_SMOOTH_INTERVAL')
local alpha = redis.call('HGET', rates_table_name .. ':' .. 'RIF', 'RIF_ALPHA')
local one_minus_alpha = 1.0 - alpha
local delta = tonumber(ARGV[3])

local initialized = redis.call('HGET', rates_table_name, 'INIT_DONE')
logit(initialized)

for i = 1, n do
-- Get new COUNTERS values
local in_octets = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_ROUTER_INTERFACE_STAT_IN_OCTETS')
local in_pkts = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_ROUTER_INTERFACE_STAT_IN_PACKETS')
local out_octets = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_ROUTER_INTERFACE_STAT_OUT_OCTETS')
local out_pkts = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_ROUTER_INTERFACE_STAT_OUT_PACKETS')

if initialized == "DONE" or initialized == "COUNTERS_LAST" then
-- Get old COUNTERS values
local in_octets_pkts_last = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'SAI_ROUTER_INTERFACE_STAT_IN_OCTETS_last')
local in_pkts_last = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'SAI_ROUTER_INTERFACE_STAT_IN_PACKETS_last')
local out_octets_last = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'SAI_ROUTER_INTERFACE_STAT_OUT_OCTETS_last')
local out_pkts_last = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'SAI_ROUTER_INTERFACE_STAT_OUT_PACKETS_last')

-- Calculate new rates values
local rx_bps_new = (in_octets - in_octets_last)/delta
local tx_bps_new = (out_octets - out_octets_last)/delta
local rx_pps_new = (in_pkts - in_pkts_last)/delta
local tx_pps_new = (out_pkts - out_pkts_last)/delta

if initialized == "DONE" then
-- Get old rates values
local rx_bps_old = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'RX_BPS')
local rx_pps_old = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'RX_PPS')
local tx_bps_old = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'TX_BPS')
local tx_pps_old = redis.call('HGET', rates_table_name .. ':' .. KEYS[i], 'TX_PPS')

-- Smooth the rates values and store them in DB
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'RX_BPS', alpha*rx_bps_new + one_minus_alpha*rx_bps_old)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'RX_PPS', alpha*rx_pps_new + one_minus_alpha*rx_pps_old)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'TX_BPS', alpha*tx_bps_new + one_minus_alpha*tx_bps_old)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'TX_PPS', alpha*tx_pps_new + one_minus_alpha*tx_pps_old)
else
-- Store unsmoothed initial rates values in DB
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'RX_BPS', rx_bps_new)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'RX_PPS', rx_pps_new)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'TX_BPS', tx_bps_new)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'TX_PPS', tx_pps_new)
redis.call('HSET', rates_table_name, 'INIT_DONE', 'DONE')
end
else
-- Set old COUNTERS values
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'SAI_ROUTER_INTERFACE_STAT_IN_OCTETS_last', in_octets)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'SAI_ROUTER_INTERFACE_STAT_IN_PACKETS_last', in_pkts)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'SAI_ROUTER_INTERFACE_STAT_OUT_OCTETS_last', out_octets)
redis.call('HSET', rates_table_name .. ':' .. KEYS[i], 'SAI_ROUTER_INTERFACE_STAT_OUT_PACKETS_last', out_pkts)
redis.call('HSET', rates_table_name, 'INIT_DONE', 'COUNTERS_LAST')
end
end

return logtable