Support for platforms based on Barefoot Networks' device #452
Conversation
orchagent/portsorch.cpp
Outdated
| case 2: | ||
| attr.value.booldata = false; | ||
| break; | ||
| } |
There was a problem hiding this comment.
why not just bool? 0, 1 here?
There was a problem hiding this comment.
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.
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.
|
|
||
| AM_CONDITIONAL(sonic_asic_platform_barefoot, test x$CONFIGURED_PLATFORM = xbarefoot) | ||
| AM_COND_IF([sonic_asic_platform_barefoot], | ||
| [CFLAGS_COMMON+=" -I/opt/bfn/install/include/switchsai"]) |
There was a problem hiding this comment.
@rsunkad , why is this needed? I do not see it being used anywhere in your PR.
There was a problem hiding this comment.
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.cpp
main.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.
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.
| @@ -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.
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.
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.
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.
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.
sorry for the confusion, I am ok with this on 201712 branch. For master branch, we need to take a different approach.
orchagent/portsorch.cpp
Outdated
| attrs.push_back(attr); | ||
| } | ||
|
|
||
| if(fec_mode != "") { |
There was a problem hiding this comment.
can you make brace in a separate line and leave a space between if and parenthesis.
* initial barefoot support october 2017 * import changes from telemetry branch * Fix merge issues w.r.t rel_6_0 branch * add validate and get port speed APIs - cleanup deletes from earlier checkin * missed changes from earlier * missed integration delete * trying to get close to master version - and not break other vendor * set the port config in the ASIC_DB * update hostif oper status besides the DB update * Fix compilation issues * merge closer to master - including spaces and comments * fix typos enable mirror * cosmetic fix * Fix errors due to saiacl.h changes * Fix more errors due to sai header file changes * fix the order of nexthop/neighbor delete * Fix compilation issues and few issues seen in testing * Changes needed to add new Dtel api support to sairedis * Report session DTel table related fixes * Add more error handling code * Fix logic to read table name and keys from m_toSync map * Update value for dtel actions * Handle boolean attributes to accept true/false as well as presence/absence * Fix merge errors * Incorporate configdb related changes for dtel * fix code merge issue * Incorporate configdb related changes for dtel * add pfc_detect lua script * Changes to incorporate new DTel SAI changes * SONiC changes due to DTel experimental SAI changes * cleanup configure.ac to allow barefoot platform includes * fix typo * revert commented tunneldecap * test selective tunneldecap * revert too ambitious an attempt and just push_back into vector! * change fec mode to string (from integer) * cleanup based on review * closer to 201712 - cosmetic cleanup * address review comments * fix format * Support for platforms based on Barefoot Networks' device (#452) * Fix issue with "config save" followed by reboot * Temp remove crmorch Currently orchagent crashes if crm is running. Needs to be fixed * Initial code changes to address community design review comments * Temp remove crmorch Currently orchagent crashes if crm is running. Needs to be fixed P.S This is actual commit that removes crm. Previous commit was to correct a typo, and has wrong commit message * More changes to address review comments * Fix build errors * Fix for orchagent crash * Fix queue report deletion error * Bug fixes * Re-enable crm orchagent (#3) * Add support for new watchlist attribute to enable/disable tail drop reporting * Add new VS test for Dataplane Telemetry feature * Add support for AN and FEC to be specified in port_config.ini * do not set autoneg if it is already set * Address review comments * Fix compilation errors * set port adv speed for AN and add test Signed-off-by: Guohan Lu <gulv@microsoft.com> * Convert all the tabs to spaces * Bring in changes for port an/fec from azure master * support autoneg and fec in port config ini file * if autoneg is specified, along with speed; speed is set seperately as port attribute * Address review comments * Remove trailing whitespaces * Fix vs test indentation * Address review comments * Fix for VS test * Fix VS test failure * Fix for VS test * More VS test fixes * Fix for test_mirror VS test that was failing due to new DTel ACL tables
…ortchannel or portchannel members only when it is configured (sonic-net#277) (sonic-net#445)" (sonic-net#452) This reverts commit dc5d5c4.
Make binary-syncd the last target, so the non-rpc requisites will be available after the first build It will allow doing incremental builds later on for the main binary type
**- Incorporate support for platforms based on Barefoot Networks' device **
**- Added port config files and related scripts for the platforms **
**- Verified using SONiC tests **