Changes for LLDP docker to support multi-npu platforms #4530

abdosi · 2020-05-04T19:12:47Z

- Why I did it
To support lldp for multi-npu platforms following changes are done:
a) Enable LLDP for Host namespace for Management Port
b) Make sure Management IP is available in per asic namespace
needed for LLDP Chassis configuration
c) Make sure chassis mac-address is correct in per asic namespace
d) Do not run lldp on eth0 of per asic namespace and avoid chassis
configuration for same
e) Use Linux hostname instead from Device Metadata for lldp chassis
configuration since in multi-npu platforms device metadata hostname
will be different

- How I did it
Updated the lldp docker templates to use device metadata sub_role which will be specified
for multi-npu platforms

- How to verify it
Verified LLDP Neighbors and chassis configuration for both single npu and multi-npu platforms.

pavel-shirshov

I have some questions. Can you please respond on them?

pavel-shirshov · 2020-05-04T22:25:22Z

dockers/docker-lldp-sv2/start.sh

+# update the hostname. Not using device metadata as in multi-npu platform each namespace
+# has its own device metadata hostname. Using the hostname set by docker using host enviroment
+
+sed -e "s/\${hostname}/$(hostname)/" -i /etc/lldpd.conf 


Where hostname is defined?

@pavel-shirshov this is linux hostname based of /etc/host

@pavel-shirshov this change is no more applicable and we will continue to use Device Metadata hostname.

pavel-shirshov · 2020-05-04T22:26:38Z

dockers/docker-lldp-sv2/supervisord.conf.j2

@@ -31,7 +31,11 @@ stderr_logfile=syslog
 # - `-dd` means to stay in foreground, log warnings to console
 # - `-ddd` means to stay in foreground, log warnings and info to console
 # - `-dddd` means to stay in foreground, log all to console
+{% if DEVICE_METADATA['localhost']['sub_role'] is defined and DEVICE_METADATA['localhost']['sub_role']|length %}
+command=/usr/sbin/lldpd -d -I Ethernet* -C Ethernet*
+{% else %}
 command=/usr/sbin/lldpd -d -I Ethernet*,eth0 -C eth0


what about rewritong this command line using sed,inside of supervisord.conf? And don't use jinja2 template for that?
We can change supervisor config in start.sh

i suggest to make this code generic, detect if there is eth0, if there is then use eth0, if there is no, do not add eth0 into the command line. it is better than templating based on the sub_role.

@pavel-shirshov : I tried couple of things to avoid jinja2 template
a) supervisor environment variable
b) sed command
However both does not seems to work. supervisos.conf gets loaded whens supervisor comes up and seems there is some dependency we can't change run time from start.sh

@lguohan: eth0 is present both in container and host. So we cannot distinguish that, Do we need to check if container is using --net= host or not ?

@pavel-shirshov , what is the concern for using jinja2 template for supervisord.conf? if we can first generate and start supervisord, then we do not need to reload.

@pavel-shirshov Based on above comment will be using jinja template for suerpvisor.conf

@lguohan It looks like we're going to have another snowflake in SONiC. Everywhere we start with "start.sh" script. Here the "start.sh" is not run on the start (So the name is misleading). And we have another start script, with another name. This is confusing.

dockers/docker-lldp-sv2/lldpd.conf.j2

dockers/docker-lldp-sv2/start.sh

a) Enable LLDP for Host namespace for Management Port b) Make sure Management IP is avaliable in per asic namespace needed for LLDP Chassis configuration c) Make sure chassis mac-address is correct in per asic namespace d) Do not run lldp on eth0 of per asic namespace and avoid chassis configuration for same e) Use Linux hostname instead from Device Metadata for lldp chassis configuration since in multi-npu platforms device metadata hostname will be differnt Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>

a) Use Device Metadata hostname even in per namespace conatiner. updated minigraph parsing for same to have hostname as system hostname and add new key for asic name b) Minigraph changes to have MGMT_INTERFACE Key in per asic/namespace config also as needed for LLDP for setting chassis management IP. Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>

abdosi · 2020-05-07T17:54:28Z

@pavel-shirshov can you please review/approve the PR.

pavel-shirshov

Can you please address the comment?

pavel-shirshov · 2020-05-09T01:10:35Z

dockers/docker-lldp-sv2/Dockerfile.j2

@@ -35,12 +35,13 @@ RUN apt-get purge -y python-pip && \
           /python-wheels          \
           ~/.cache

+COPY ["docker-lldp-init.sh", "/usr/local/bin/"]


can you please put the script into /usr/bin?
We never use /usr/local for anything (but agree it is more appropriate to put our own code).

@pavel-shirshov Done

abdosi · 2020-05-10T23:10:07Z

retest vsimage please

* Changes for LLDP for Multi NPU Platoforms:- a) Enable LLDP for Host namespace for Management Port b) Make sure Management IP is avaliable in per asic namespace needed for LLDP Chassis configuration c) Make sure chassis mac-address is correct in per asic namespace d) Do not run lldp on eth0 of per asic namespace and avoid chassis configuration for same e) Use Linux hostname instead from Device Metadata for lldp chassis configuration since in multi-npu platforms device metadata hostname will be differnt Signed-off-by: Abhishek Dosi <abdosi@microsoft.com> * Address Review Comment with following changes: a) Use Device Metadata hostname even in per namespace conatiner. updated minigraph parsing for same to have hostname as system hostname and add new key for asic name b) Minigraph changes to have MGMT_INTERFACE Key in per asic/namespace config also as needed for LLDP for setting chassis management IP. Signed-off-by: Abhishek Dosi <abdosi@microsoft.com> * Address Review Comments

* [201911][devices] skip_fancontrol for wedge 100 barefoot platforms (sonic-net#4528) * [device] DellEMC s5232f 50G hwsku support (sonic-net#4525) * [device] DellEmc S5232 support for new hwsku C8D48 8 100G ports and 48 50G ports * 10G ports update for S5232 hwsku-C8D48 Signed-off-by: Srideep Devireddy <srideep_devireddy@dell.com> * DellEMC S6000 updated sensors.conf (sonic-net#4568) Change PSU MAX temperature to 80 degree Change tmp75 sensors default temperature value from 25/50 to 70/80 degree. * [sonic-slave-stretch]: install same version for docker-ce and docker-ce-cli difference versions can cause compatibility issue between the server and client Signed-off-by: Guohan Lu <lguohan@gmail.com> * [baseimage]: install same version for docker-ce and docker-ce-cli Signed-off-by: Guohan Lu <lguohan@gmail.com> * [FRR]: Update frr to latest 7.2.1-s3 (sonic-net#4294) - Updated to latest frr 7.2.1 from the master. - Updated patches accordingly * [sonic-buildimage] updated minigraph for ACL Table data and ACL Interface Binding for Multi-NPU platforms (sonic-net#4491) * [sonic-buildimage] updated minigraph for ACL Table data and ACL Interface binding update for multu-npu platform based on subrole as "Frontend" or "Backend". For backend npu no ACL table is associated. For frontend npu only front-panel interface are associated. Updated with test case and fix typo in sample-mingraph for npu Address Review comments Signed-off-by: Abhishek Dosi <abdosi@microsoft.com> * Fixed the logic as per preview comment. Interface Filter logic only applies to Everflow/Mirror tables. * Address Review Comments. * Changes for LLDP docker to support multi-npu platforms (sonic-net#4530) * Changes for LLDP for Multi NPU Platoforms:- a) Enable LLDP for Host namespace for Management Port b) Make sure Management IP is avaliable in per asic namespace needed for LLDP Chassis configuration c) Make sure chassis mac-address is correct in per asic namespace d) Do not run lldp on eth0 of per asic namespace and avoid chassis configuration for same e) Use Linux hostname instead from Device Metadata for lldp chassis configuration since in multi-npu platforms device metadata hostname will be differnt Signed-off-by: Abhishek Dosi <abdosi@microsoft.com> * Address Review Comment with following changes: a) Use Device Metadata hostname even in per namespace conatiner. updated minigraph parsing for same to have hostname as system hostname and add new key for asic name b) Minigraph changes to have MGMT_INTERFACE Key in per asic/namespace config also as needed for LLDP for setting chassis management IP. Signed-off-by: Abhishek Dosi <abdosi@microsoft.com> * Address Review Comments * Moved utility functions for multi-npu platforms from sonic-utilities to sonic_device_util.py (sonic-net#4559) * Moved utility functions for multi-npu platforms from sonic-utilities config/main.py to here so that they can be used any module * Fix the issue with test run during compilation with acl-uploader PR#908 of sonic-utilities. * Fix get_num_npu as it was retuning string and not int * Address Review Comments * Address Review Comments * Fix for issue where image is compile with flag ENABLE_DHCP_GRAPH_SERVICE (sonic-net#4573) and then we load image and reboot even if there was existing config_db.json we will look for DHCP Service. we should disbale update_graph in such cases. This behaviour is silimar to what we have in 201811 image. * Change to enable redistribute connected on Frontend asics instead of backend asics (sonic-net#4588) Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com> * [DellEMC] S6000 Disable Low power mode by default (sonic-net#4592) * [BFN] Updated Barefoot SDK to 2020-05-07 (sonic-net#4566) Signed-off-by: Andriy Kokhan <akokhan@barefootnetworks.com> * [minigraph] Add tags for egress mirror tables (sonic-net#4526) Signed-off-by: Danny Allen <daall@microsoft.com> * [Submodule update] sonic-utlities with PR's [201911][show] Fix abbreviations for 'show ip bgp ...' commands (sonic-net#909) Changes to support acl-loader and mirror-session config commands for multi-npu platforms. (sonic-net#908) Changes to commands config reload/load-minigraph (sonic-net#919) Stop/Start restapi server upon config reload (sonic-net#911) [config] Add 'interface transceiver' subgroup with 'lpmode' and 'reset' subcommands (sonic-net#904) * [minigraph] Support FECDisabled in minigraph parser (sonic-net#4556) (sonic-net#4624) Signed-off-by: Qi Luo <qiluo-msft@users.noreply.github.com> * [ntp] enable/disable NTP long jump according to reboot type (sonic-net#4577) * [ntp] enable/disable NTP long jump according to reboot type - Enable NTP long jump after cold reboot. - Disable NTP long jump after warrm/fast reboot. Signed-off-by: Ying Xie <ying.xie@microsoft.com> * fix typo * further refactoring * use sonic-db-cli instead * [arista]: remove the soc property disabling sram scan (sonic-net#4623) * Changes to support config-setup service for multi-npu (sonic-net#4609) * Changes to support config-setup service for multi-npu platforms. For Multi-npu we are not supporting as of now config initializtion and ZTP. It will support creating config db from minigraph or using config db from previous file system Signed-off-by: Abhishek Dosi <abdosi@microsoft.com> * Address Review Comments. * Address Review comments * Address Review Comments of using pyhton based config load_minigraph/ config save/config reload from shell scripts so that we don't duplicate code. Also while running from shell we will skip stop/start services done by those commands. * Updated to use python command so no code duplication. * [config]: Fix the device type and internal bgp session status for multi NPU platforms (sonic-net#4600) * The following changes for multi-npu platforms are done - Set the type in device_metadata for asic configuration to be same as host - Set the admin-status of internal bgp sessions as up Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com> * Adding new BGP peer groups PEER_V4_INT and PEER_V6_INT. (sonic-net#4620) * Adding new BGP peer groups PEER_V4_INT and PEER_V6_INT. The internal BGP sessions will be added to this peer group while the external BGP sessions will be added to the exising PEER_V4 and PEER_V6 peer group. * Check for "ASIC" keyword in the hostname to identify the internal neighbors. * [submodule update] sonic-swss with PR [vnet] Fix IP2ME route creation logic for BITMAP VNET interface (sonic-net#1284) * [submodule update] sonic-util Revert "[config] Add 'interface transceiver' subgroup with 'lpmode' and 'reset' subcommands (sonic-net#904)" Multi-asic changes for config bgp commands and utilities. (sonic-net#910) * [submodule update] sonic-rest API's PR#39 Setup module versioning Add support for get all Vlans (#37) * Update golang version for 1.11.5 to 1.14.2 (sonic-net#4520) Co-authored-by: Myron Sosyak <49795530+msosyak@users.noreply.github.com> Co-authored-by: Srideep <srideep_devireddy@dell.com> Co-authored-by: paavaanan <paavaanan_t_n@dell.com> Co-authored-by: Guohan Lu <lguohan@gmail.com> Co-authored-by: pavel-shirshov <pavelsh@microsoft.com> Co-authored-by: abdosi <58047199+abdosi@users.noreply.github.com> Co-authored-by: arlakshm <55814491+arlakshm@users.noreply.github.com> Co-authored-by: Santhosh Kumar T <53558409+santhosh-kt@users.noreply.github.com> Co-authored-by: Andriy Kokhan <43479230+akokhan@users.noreply.github.com> Co-authored-by: Danny Allen <daall@microsoft.com> Co-authored-by: Abhishek Dosi <abdosi@microsoft.com> Co-authored-by: Qi Luo <qiluo-msft@users.noreply.github.com> Co-authored-by: Ying Xie <yxieca@users.noreply.github.com> Co-authored-by: Samuel Angebault <staphylo@arista.com> Co-authored-by: judyjoseph <53951155+judyjoseph@users.noreply.github.com>

make sure only syncd is match on pgrep

abdosi added Request for 201911 Branch Multi-NPU 🔢 labels May 4, 2020

abdosi requested review from lguohan, jleveque and pavel-shirshov May 4, 2020 19:12

abdosi force-pushed the multinpu_lldp branch from 0cd49bc to c346914 Compare May 4, 2020 22:20

pavel-shirshov reviewed May 4, 2020

View reviewed changes

lguohan reviewed May 5, 2020

View reviewed changes

dockers/docker-lldp-sv2/start.sh Outdated Show resolved Hide resolved

abdosi force-pushed the multinpu_lldp branch 2 times, most recently from d9f1b54 to 16da822 Compare May 6, 2020 07:12

abdosi force-pushed the multinpu_lldp branch from 16da822 to f1bc3eb Compare May 6, 2020 20:49

lguohan previously approved these changes May 7, 2020

View reviewed changes

pavel-shirshov suggested changes May 9, 2020

View reviewed changes

abdosi dismissed lguohan’s stale review via 1feb5ae May 10, 2020 05:51

Address Review Comments

358b2e4

abdosi force-pushed the multinpu_lldp branch from 1feb5ae to 358b2e4 Compare May 10, 2020 05:55

pavel-shirshov approved these changes May 11, 2020

View reviewed changes

abdosi merged commit a96f9ec into sonic-net:master May 11, 2020

abdosi deleted the multinpu_lldp branch May 11, 2020 18:08

abdosi added the Included in 201911 Branch label May 20, 2020

abdosi added a commit to abdosi/sonic-mgmt that referenced this pull request May 29, 2020

Fix for Issue sonic-net/sonic-buildimage#4530

c3a3bd0

make sure only syncd is match on pgrep

abdosi mentioned this pull request May 29, 2020

make sure only syncd is match on pgrep sonic-net/sonic-mgmt#1708

Merged

abdosi added a commit to sonic-net/sonic-mgmt that referenced this pull request May 31, 2020

Fix for Issue sonic-net/sonic-buildimage#4530 (#1708)

90351bb

make sure only syncd is match on pgrep

yxieca pushed a commit to sonic-net/sonic-mgmt that referenced this pull request Jun 2, 2020

Fix for Issue sonic-net/sonic-buildimage#4530 (#1708)

486047c

make sure only syncd is match on pgrep

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes for LLDP docker to support multi-npu platforms #4530

Changes for LLDP docker to support multi-npu platforms #4530

abdosi commented May 4, 2020

pavel-shirshov left a comment

pavel-shirshov May 4, 2020

abdosi May 5, 2020 •

edited

Loading

abdosi May 6, 2020

pavel-shirshov May 4, 2020

lguohan May 5, 2020

abdosi May 5, 2020 •

edited

Loading

lguohan May 6, 2020

abdosi May 6, 2020

pavel-shirshov May 9, 2020

abdosi commented May 7, 2020

pavel-shirshov left a comment

pavel-shirshov May 9, 2020

abdosi May 10, 2020

abdosi commented May 10, 2020

Changes for LLDP docker to support multi-npu platforms #4530

Changes for LLDP docker to support multi-npu platforms #4530

Conversation

abdosi commented May 4, 2020

pavel-shirshov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abdosi May 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abdosi May 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abdosi commented May 7, 2020

pavel-shirshov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abdosi commented May 10, 2020

abdosi May 5, 2020 •

edited

Loading

abdosi May 5, 2020 •

edited

Loading