-
Notifications
You must be signed in to change notification settings - Fork 558
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
Support for platforms based on Barefoot Networks' device #452
Changes from 30 commits
475e2da
4a02a6f
9aa8377
c78f25f
56d480e
e0cad33
1531771
0011df4
b5f5856
1b347a4
41cff68
8c1d488
c944692
f9d4999
2505270
5a1db21
5aabe44
34f5fdc
0b921c6
78a4866
3e7243b
6ad7ece
5374ea1
7bf3da3
870cf49
041d82d
75aed6e
f7df97f
a12a2e5
5d049e8
933960f
bc95cca
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 |
---|---|---|
@@ -0,0 +1,91 @@ | ||
-- KEYS - queue IDs | ||
-- ARGV[1] - counters db index | ||
-- ARGV[2] - counters table name | ||
-- ARGV[3] - poll time interval | ||
-- return queue Ids that satisfy criteria | ||
|
||
local counters_db = ARGV[1] | ||
local counters_table_name = ARGV[2] | ||
local poll_time = tonumber(ARGV[3]) | ||
|
||
local rets = {} | ||
|
||
redis.call('SELECT', counters_db) | ||
|
||
-- Iterate through each queue | ||
local n = table.getn(KEYS) | ||
for i = n, 1, -1 do | ||
local counter_keys = redis.call('HKEYS', counters_table_name .. ':' .. KEYS[i]) | ||
local counter_num = 0 | ||
local old_counter_num = 0 | ||
local is_deadlock = false | ||
local pfc_wd_status = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'PFC_WD_STATUS') | ||
local pfc_wd_action = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'PFC_WD_ACTION') | ||
if pfc_wd_status == 'operational' or pfc_wd_action == 'alert' then | ||
local detection_time = tonumber(redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'PFC_WD_DETECTION_TIME')) | ||
local time_left = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'PFC_WD_DETECTION_TIME_LEFT') | ||
if not time_left then | ||
time_left = detection_time | ||
else | ||
time_left = tonumber(time_left) | ||
end | ||
|
||
local queue_index = redis.call('HGET', 'COUNTERS_QUEUE_INDEX_MAP', KEYS[i]) | ||
local port_id = redis.call('HGET', 'COUNTERS_QUEUE_PORT_MAP', KEYS[i]) | ||
local pfc_rx_pkt_key = 'SAI_PORT_STAT_PFC_' .. queue_index .. '_RX_PKTS' | ||
local pfc_on2off_key = 'SAI_PORT_STAT_PFC_' .. queue_index .. '_ON2OFF_RX_PKTS' | ||
|
||
|
||
-- Get all counters | ||
local occupancy_bytes = tonumber(redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_QUEUE_STAT_CURR_OCCUPANCY_BYTES')) | ||
local packets = tonumber(redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_QUEUE_STAT_PACKETS')) | ||
local pfc_rx_packets = tonumber(redis.call('HGET', counters_table_name .. ':' .. port_id, pfc_rx_pkt_key)) | ||
local pfc_on2off = tonumber(redis.call('HGET', counters_table_name .. ':' .. port_id, pfc_on2off_key)) | ||
local queue_pause_status = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_QUEUE_ATTR_PAUSE_STATUS') | ||
|
||
local packets_last = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_QUEUE_STAT_PACKETS_last') | ||
local pfc_rx_packets_last = redis.call('HGET', counters_table_name .. ':' .. port_id, pfc_rx_pkt_key .. '_last') | ||
local pfc_on2off_last = redis.call('HGET', counters_table_name .. ':' .. port_id, pfc_on2off_key .. '_last') | ||
local queue_pause_status_last = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_QUEUE_ATTR_PAUSE_STATUS_last') | ||
|
||
-- DEBUG CODE START. Uncomment to enable | ||
local debug_storm = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'DEBUG_STORM') | ||
-- DEBUG CODE END. | ||
|
||
-- If this is not a first run, then we have last values available | ||
if packets_last and pfc_rx_packets_last and pfc_on2off_last and queue_pause_status_last then | ||
packets_last = tonumber(packets_last) | ||
pfc_rx_packets_last = tonumber(pfc_rx_packets_last) | ||
pfc_on2off_last = tonumber(pfc_on2off_last) | ||
|
||
-- Check actual condition of queue being in PFC storm | ||
if (occupancy_bytes > 0 and packets - packets_last == 0 and pfc_rx_packets - pfc_rx_packets_last > 0) or | ||
-- DEBUG CODE START. Uncomment to enable | ||
(debug_storm == "enabled") or | ||
-- DEBUG CODE END. | ||
(occupancy_bytes == 0 and pfc_rx_packets - pfc_rx_packets_last > 0 and pfc_on2off - pfc_on2off_last == 0 and queue_pause_status_last == 'true' and queue_pause_status == 'true') then | ||
if time_left <= poll_time then | ||
redis.call('PUBLISH', 'PFC_WD', '["' .. KEYS[i] .. '","storm"]') | ||
is_deadlock = true | ||
time_left = detection_time | ||
else | ||
time_left = time_left - poll_time | ||
end | ||
else | ||
if pfc_wd_action == 'alert' and pfc_wd_status ~= 'operational' then | ||
redis.call('PUBLISH', 'PFC_WD', '["' .. KEYS[i] .. '","restore"]') | ||
end | ||
time_left = detection_time | ||
end | ||
end | ||
|
||
-- Save values for next run | ||
redis.call('HSET', counters_table_name .. ':' .. KEYS[i], 'SAI_QUEUE_ATTR_PAUSE_STATUS_last', queue_pause_status) | ||
redis.call('HSET', counters_table_name .. ':' .. KEYS[i], 'SAI_QUEUE_STAT_PACKETS_last', packets) | ||
redis.call('HSET', counters_table_name .. ':' .. KEYS[i], 'PFC_WD_DETECTION_TIME_LEFT', time_left) | ||
redis.call('HSET', counters_table_name .. ':' .. port_id, pfc_rx_pkt_key .. '_last', pfc_rx_packets) | ||
redis.call('HSET', counters_table_name .. ':' .. port_id, pfc_on2off_key .. '_last', pfc_on2off) | ||
end | ||
end | ||
|
||
return rets |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,6 @@ | |
#include "converter.h" | ||
#include "saiserialize.h" | ||
|
||
#include "saiserialize.h" | ||
|
||
extern sai_switch_api_t *sai_switch_api; | ||
extern sai_bridge_api_t *sai_bridge_api; | ||
|
@@ -665,7 +664,6 @@ bool PortsOrch::validatePortSpeed(sai_object_id_t port_id, sai_uint32_t speed) | |
bool PortsOrch::setPortSpeed(sai_object_id_t port_id, sai_uint32_t speed) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
sai_attribute_t attr; | ||
sai_status_t status; | ||
|
||
|
@@ -728,6 +726,33 @@ bool PortsOrch::getQueueType(sai_object_id_t queue_id, string &type) | |
return true; | ||
} | ||
|
||
bool PortsOrch::setPortAutoNeg(sai_object_id_t id, int an) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
sai_attribute_t attr; | ||
attr.id = SAI_PORT_ATTR_AUTO_NEG_MODE; | ||
switch(an) { | ||
case 0: // leave it as is? | ||
return true; | ||
case 1: | ||
attr.value.booldata = true; | ||
break; | ||
case 2: | ||
attr.value.booldata = false; | ||
break; | ||
} | ||
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 not just bool? 0, 1 here? 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. The original idea was to support default/on/off. Default would imply SDK defaults. But SAI does not support that. The use case is to default to different settings based on the type of cable/transcievers/etc. I can revert this back to boolean, if you feel it won't be supported in the near future. 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. according to SAI, it seems is bool value, and the default value is false. If you feel that the default value is incorrect, then we should change in the sai spec. I think it will be clear here to follow SAI standard. |
||
|
||
sai_status_t status = sai_port_api->set_port_attribute(id, &attr); | ||
if (status != SAI_STATUS_SUCCESS) | ||
{ | ||
SWSS_LOG_ERROR("Failed to set AN %u to port pid:%lx", attr.value.u32, id); | ||
return false; | ||
} | ||
SWSS_LOG_INFO("Set AN %u to port pid:%lx", attr.value.u32, id); | ||
return true; | ||
} | ||
|
||
bool PortsOrch::setHostIntfsOperStatus(sai_object_id_t port_id, bool up) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
@@ -771,7 +796,7 @@ void PortsOrch::updateDbPortOperStatus(sai_object_id_t id, sai_port_oper_status_ | |
} | ||
} | ||
|
||
bool PortsOrch::addPort(const set<int> &lane_set, uint32_t speed) | ||
bool PortsOrch::addPort(const set<int> &lane_set, uint32_t speed, int an, string fec_mode) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
|
@@ -789,6 +814,18 @@ bool PortsOrch::addPort(const set<int> &lane_set, uint32_t speed) | |
attr.value.u32list.count = static_cast<uint32_t>(lanes.size()); | ||
attrs.push_back(attr); | ||
|
||
if(an == true) { | ||
attr.id = SAI_PORT_ATTR_AUTO_NEG_MODE; | ||
attr.value.booldata = true; | ||
attrs.push_back(attr); | ||
} | ||
|
||
if(fec_mode != "") { | ||
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. can you make brace in a separate line and leave a space between if and parenthesis. |
||
attr.id = SAI_PORT_ATTR_FEC_MODE; | ||
attr.value.u32 = fec_mode_map[fec_mode]; | ||
attrs.push_back(attr); | ||
} | ||
|
||
sai_object_id_t port_id; | ||
sai_status_t status = sai_port_api->create_port(&port_id, gSwitchId, static_cast<uint32_t>(attrs.size()), attrs.data()); | ||
if (status != SAI_STATUS_SUCCESS) | ||
|
@@ -939,6 +976,7 @@ void PortsOrch::doPortTask(Consumer &consumer) | |
string fec_mode; | ||
uint32_t mtu = 0; | ||
uint32_t speed = 0; | ||
int an = -1; | ||
|
||
for (auto i : kfvFieldsValues(t)) | ||
{ | ||
|
@@ -953,6 +991,7 @@ void PortsOrch::doPortTask(Consumer &consumer) | |
int lane = stoi(lane_str); | ||
lane_set.insert(lane); | ||
} | ||
|
||
} | ||
|
||
/* Set port admin status */ | ||
|
@@ -970,12 +1009,16 @@ void PortsOrch::doPortTask(Consumer &consumer) | |
/* Set port fec */ | ||
if (fvField(i) == "fec") | ||
fec_mode = fvValue(i); | ||
|
||
/* Set autoneg */ | ||
if (fvField(i) == "autoneg") | ||
an = (int)stoul(fvValue(i)); | ||
} | ||
|
||
/* Collect information about all received ports */ | ||
if (lane_set.size()) | ||
{ | ||
m_lanesAliasSpeedMap[lane_set] = make_tuple(alias, speed); | ||
m_lanesAliasSpeedMap[lane_set] = make_tuple(alias, speed, an, fec_mode); | ||
} | ||
|
||
/* Once all ports received, go through the each port and perform appropriate actions: | ||
|
@@ -990,7 +1033,7 @@ void PortsOrch::doPortTask(Consumer &consumer) | |
if (m_lanesAliasSpeedMap.find(it->first) == m_lanesAliasSpeedMap.end()) | ||
{ | ||
char *platform = getenv("platform"); | ||
if (platform && strstr(platform, MLNX_PLATFORM_SUBSTRING)) | ||
if (platform && (strstr(platform, BFN_PLATFORM_SUBSTRING) || strstr(platform, MLNX_PLATFORM_SUBSTRING))) | ||
{ | ||
if (!removePort(it->second)) | ||
{ | ||
|
@@ -1019,9 +1062,9 @@ void PortsOrch::doPortTask(Consumer &consumer) | |
// work around to avoid syncd termination on SAI error due missing create_port SAI API | ||
// can be removed when SAI redis return NotImplemented error | ||
char *platform = getenv("platform"); | ||
if (platform && strstr(platform, MLNX_PLATFORM_SUBSTRING)) | ||
if (platform && (strstr(platform, BFN_PLATFORM_SUBSTRING) || strstr(platform, MLNX_PLATFORM_SUBSTRING))) | ||
{ | ||
if (!addPort(it->first, get<1>(it->second))) | ||
if (!addPort(it->first, get<1>(it->second), get<2>(it->second), get<3>(it->second))) | ||
{ | ||
throw runtime_error("PortsOrch initialization failure."); | ||
} | ||
|
@@ -1138,6 +1181,18 @@ void PortsOrch::doPortTask(Consumer &consumer) | |
} | ||
} | ||
|
||
|
||
if(an != -1) { | ||
if (setPortAutoNeg(p.m_port_id, an)) | ||
SWSS_LOG_NOTICE("Set port %s AN to %u", alias.c_str(), an); | ||
else | ||
{ | ||
SWSS_LOG_ERROR("Failed to set port %s AN to %u", alias.c_str(), an); | ||
it++; | ||
continue; | ||
} | ||
} | ||
|
||
if (fec_mode != "") | ||
{ | ||
if (fec_mode_map.find(fec_mode) != fec_mode_map.end()) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -199,7 +199,7 @@ void handlePortConfigFile(ProducerStateTable &p, string file) | |
throw "Port configuration file not found!"; | ||
} | ||
|
||
list<string> header = {"name", "lanes", "alias", "speed"}; | ||
list<string> header = {"name", "lanes", "alias", "speed", "autoneg", "fec"}; | ||
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. in the master branch, fec is coming from config. If we are going to support autoneg in master, it should also come from config db. 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. Isn't there a problem with config DB for initial port states - ports will come up as default and then potentially "flap" when this configuration from DB is applied? I thought the portconfig.ini is first played into the SAI library and then modified when the config DB is evaluated and it notices a change. I may be wrong on this. 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. ultimately, all the configurations should come from one place. we plan to consolidate all configurations into config db. I think since we are using create port API with the attributes coming from config db, there should be no flap. 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. So, we can remove the portconfig.ini file and get the breakout, speeds and hostinterface name, index associations from the configuration DB. The problem we had was that at boot up - we come up with default configurations - if the default matches the portconfig.ini no changes are observed. If changes come later it causes the remote nodes to see another flap. If we can eliminate this it will be great. Currently, you can see that some of the device configurations use breakout, fec and AN in the default config files. If you want us to remove fec and autoneg in the 201712 branch, we can do it. But the devices will need to apply a patch to build for their platforms. Some of them have fixed breakout configuration and default to fec and AN settings (sonic-buildimage pull request). Let me know if you want me to revert the changes and I will do so. 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. sorry for the confusion, I am ok with this on 201712 branch. For master branch, we need to take a different approach. |
||
string line; | ||
while (getline(infile, line)) | ||
{ | ||
|
@@ -254,6 +254,18 @@ void handlePortConfigFile(ProducerStateTable &p, string file) | |
attrs.push_back(speed_attr); | ||
} | ||
|
||
if ((entry.find("autoneg") != entry.end()) && (entry["autoneg"] != "")) | ||
{ | ||
FieldValueTuple autoneg_attr("autoneg", entry["autoneg"]); | ||
attrs.push_back(autoneg_attr); | ||
} | ||
|
||
if ((entry.find("fec") != entry.end()) && (entry["fec"] != "")) | ||
{ | ||
FieldValueTuple fec_attr("fec", entry["fec"]); | ||
attrs.push_back(fec_attr); | ||
} | ||
|
||
p.set(entry["name"], attrs); | ||
|
||
g_portSet.insert(entry["name"]); | ||
|
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.
@rsunkad , why is this needed? I do not see it being used anywhere in your PR.
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.
Below is the compilation without the change: sai,h is always included from the install dir in our current debs.
libtool: link: g++ -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wl,-z -Wl,relro -o portsyncd portsyncd-portsyncd.o portsyncd-linksync.o -lnl-3 -lnl-route-3 -lswsscommon -lnl-genl-3 -lhiredis
make[4]: Leaving directory '/sonic/src/sonic-swss/portsyncd'
Making all in orchagent
make[4]: Entering directory '/sonic/src/sonic-swss/orchagent'
g++ -DHAVE_CONFIG_H -I. -I.. -I .. -g -DNDEBUG -std=c++11 -Wall -fPIC -Wno-write-strings -I/usr/include/libnl3 -I/usr/include/swss -Werror -Wno-reorder -Wcast-align -Wcast-qual -Wconversion -Wdisabled-optimization -Wextra -Wfloat-equal -Wformat=2 -Wformat-nonliteral -Wformat-security -Wformat-y2k -Wimport -Winit-self -Winvalid-pch -Wlong-long -Wmissing-field-initializers -Wmissing-format-attribute -Wno-aggregate-return -Wno-padded -Wno-switch-enum -Wno-unused-parameter -Wpacked -Wpointer-arith -Wredundant-decls -Wstack-protector -Wstrict-aliasing=3 -Wswitch -Wswitch-default -Wunreachable-code -Wunused -Wvariadic-macros -Wno-switch-default -Wno-long-long -Wno-redundant-decls -I /usr/include/sai -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -c -o orchagent-main.o
test -f 'main.cpp' || echo './'
main.cppmain.cpp:2:17: fatal error: sai.h: No such file or directory
#include "sai.h"
^
compilation terminated.
Makefile:550: recipe for target 'orchagent-main.o' failed
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.
I see, I think it is because on other platforms, they have a libsaixxx-dev packages which installs SAI headers under /usr/include/sai. They install the package before compile swss, so they do not need this.