[fabric] Disable unnecessary processes in swss and the orchagent-portsyncd dependency for fabric asic #5569

ngoc-do · 2020-10-09T00:10:14Z

- Why I did it

Fabric asic just needs very few processes in swss to run. Many of them should be disabled. The following should be disabled.

program:portsyncd
program:neighsyncd
program:fdbsyncd
program:vlanmgrd
program:intfmgrd
program:portmgrd
program:buffermgrd
program:vrfmgrd
program:nbrmgrd
program:vxlanmgrd
program:coppmgrd
program:tunnelmgrd

Fabric asic doesn't run portsyncd to turn its ports on because it doesn't have any ports. But orchagent is currently configured to not start if portsyncd. We need to break this dependency.

- How I did it

Creating critical_processes.j2 and supervisord.conf.j2 for different asics. Also disabled dependency of orchagent and portsyncd for fabric asic by modifying /etc/supervisor/conf.d/supervisord.conf.j2. We now can know asic type from CONFIG_DB. So it's possible to have custom critical_processes and supervisord.conf based on chip type.

- How to verify it

Tested on a fixed system with NPUs (BCM88690) and fabric asics (BCM88790) - worked great.

anshuv-mfst · 2021-02-03T17:10:19Z

@srikanth (Nokia) could you please take a look, thanks.

skeesara-nokia · 2021-02-03T17:44:14Z

@srikanth (Nokia) could you please take a look, thanks.

Will do

prsunny · 2021-02-05T16:44:40Z

dockers/docker-orchagent/supervisord.conf.j2


+{% if is_fabric_asic == 0 %}


The concern with this approach is, if someone have to add a new process in the end, he may ignore this if is_fabric_asic. My suggestion is lets have one section for is_fabric_asic == 0 and else section for fabric processes (similar to critical_process.j2). This way priority can also be adjusted accordingly. What do you think?

Sure, that makes sense.

@prsunny I addressed your comments. Thanks for reviewing. The new commit 1 overrided the previous commit when I resolved conflicts. Hope that it's clear to you. Also updated the title + description.

I would prefer to have it properly listed in if-else. Currently the order/priority is not in order. This would be more confusing if we have to add new process later. please keep the original order for if not-fabric and add the fabric process in else part

Sounds reasonable. But some processes like orchagent is available in both fabric and not fabric part. So if we are doing that:

if not-fabric:
orchagent
portsyncd
else:
orchagent
end-if

orchagent will appear both, and seems redundant. Is this OK? All processes for fabric are also in not fabric

My main concern that having if and else requires to update orchagent at both parts if someone wants to update orchagent's configuration. That seems harder to manage and redundant. So having if may be better.

prsunny · 2021-02-05T16:46:49Z

Can you also modify the description and title. From the changes, it doesn't look like only the portsyncd dependency changes but also some process are not even started in fabric.

ngoc-do · 2021-02-05T19:16:34Z

Can you also modify the description and title. From the changes, it doesn't look like only the portsyncd dependency changes but also some process are not even started in fabric.

That makes sense too.

anshuv-mfst · 2021-02-24T17:08:49Z

@skeesara-nokia - please review/approve the PR, thanks.

ngoc-do · 2021-04-07T16:58:41Z

Could we please merge this PR? It could cause conflicts if we keep it here for too long.

ngoc-do · 2021-04-07T17:00:15Z

@prsunny Please review it.

ngoc-do · 2021-04-22T16:07:26Z

Hello, could some one please approve and merge this PR?

anshuv-mfst · 2021-04-27T22:20:40Z

Hi @prsunny , @skeesara-nokia - Could you please review and approve.

prsunny · 2021-04-27T23:10:31Z

dockers/docker-orchagent/docker-init.sh

@@ -13,6 +13,12 @@ CFGGEN_PARAMS=" \
 "
 VLAN=$(sonic-cfggen $CFGGEN_PARAMS)

+mkdir -p /etc/supervisor/
+sonic-cfggen -d -t /usr/share/sonic/templates/critical_processes.j2 > /etc/supervisor/critical_processes


@tahmed-dev for feedback here.

Thanks @prsunny. Can you please combine the above calls into a single call to sonic-cggen.

mkdir -p /etc/supervisor/ mkdir -p /etc/supervisor/conf.d/ sonic-cfggen -d -t /usr/share/sonic/templates/critical_processes.j2,/etc/supervisor/critical_processes \ -t /usr/share/sonic/templates/supervisord.conf.j2,/etc/supervisor/conf.d/supervisord.conf

Indeed, those new sonic-cfggen calls need to be part of the above calls at line 12 above.

CFGGEN_PARAMS=" \ -d \ -y /etc/sonic/constants.yml \ -t /usr/share/sonic/templates/switch.json.j2,/etc/swss/config.d/switch.json \ -t /usr/share/sonic/templates/ipinip.json.j2,/etc/swss/config.d/ipinip.json \ -t /usr/share/sonic/templates/ports.json.j2,/etc/swss/config.d/ports.json \ -t /usr/share/sonic/templates/vlan_vars.j2 \ -t /usr/share/sonic/templates/ndppd.conf.j2,/etc/ndppd.conf \ -t /usr/share/sonic/templates/critical_processes.j2,/etc/supervisor/critical_processes \ -t /usr/share/sonic/templates/supervisord.conf.j2,/etc/supervisor/conf.d/supervisord.conf "

Thanks @tahmed-dev. That's a great suggestion. I updated with a new commit.

prsunny · 2021-04-27T23:14:18Z

dockers/docker-orchagent/supervisord.conf.j2

+{%- endif %}
+{%- endif %}
+
+[program:orchagent]


My concern is, priorities are all jumbled up here. IMO, its harder to add a process. If you dont prefer to have duplicate, please have it in the order of priority and add the skip the ones for fabric within "if" case

Sorting based on priority makes sense.

For clarification, I will add the processes which do NOT run on fabric in "if" case.

Disable the following for fabric asic: program:portsyncd program:neighsyncd program:fdbsyncd program:vlanmgrd program:intfmgrd program:portmgrd program:buffermgrd program:vrfmgrd program:nbrmgrd program:vxlanmgrd program:coppmgrd program:tunnelmgrd Also, this removes orchagent-portsyncd dependency where orchagent will start if portsyncd doesn't run. Signed-off-by: ngocdo <[email protected]>

ngoc-do · 2021-05-17T16:39:16Z

@prsunny Could you please review the updated code? Fabric code in swss merged, and so we need this one merge asap. Thanks.

prsunny · 2021-05-17T16:44:21Z

@prsunny Could you please review the updated code? Fabric code in swss merged, and so we need this one merge asap. Thanks.

@ngoc-do , Sure will review. Kindly avoid force-push, thereby deleting commit history.

tahmed-dev · 2021-05-18T23:24:22Z

dockers/docker-orchagent/docker-init.sh

@@ -13,6 +13,12 @@ CFGGEN_PARAMS=" \
 "
 VLAN=$(sonic-cfggen $CFGGEN_PARAMS)

+mkdir -p /etc/supervisor/
+sonic-cfggen -d -t /usr/share/sonic/templates/critical_processes.j2 > /etc/supervisor/critical_processes


Thanks @prsunny. Can you please combine the above calls into a single call to sonic-cggen.

mkdir -p /etc/supervisor/ mkdir -p /etc/supervisor/conf.d/ sonic-cfggen -d -t /usr/share/sonic/templates/critical_processes.j2,/etc/supervisor/critical_processes \ -t /usr/share/sonic/templates/supervisord.conf.j2,/etc/supervisor/conf.d/supervisord.conf

ngoc-do · 2021-05-24T23:09:42Z

Addressed reviewer comments with latest commit.

anshuv-mfst · 2021-05-26T16:10:12Z

@tahmed-dev - can you please take a look at the earliest, review comments addressed.

ngoc-do · 2021-06-04T23:16:03Z

Could we merge this PR? Seems it got approved and no conflicts.

anshuv-mfst · 2021-06-09T15:57:11Z

@tahmed-dev, @prsunny - Can you please help with merge.

saravanansv · 2021-06-15T01:31:20Z

on a supervisor dockers/docker-orchagent/base_image_files/monit_swss needs to be a smaller version of what it is right ?

…syncd dependency for fabric asic (sonic-net#5569) * Disable unnecessary processes in swss for fabric asic Signed-off-by: ngocdo <[email protected]>

ngoc-do mentioned this pull request Oct 9, 2020

[fabricportsorch] Add fabric support sonic-net/sonic-swss#1459

Merged

ngoc-do force-pushed the fabric branch from 5968ea8 to 0c3107d Compare October 19, 2020 18:59

lguohan force-pushed the master branch from 3690c1a to 512eb6b Compare December 25, 2020 18:34

lguohan requested a review from prsunny February 3, 2021 17:13

prsunny reviewed Feb 5, 2021

View reviewed changes

ngoc-do requested a review from lguohan as a code owner February 6, 2021 20:29

ngoc-do force-pushed the fabric branch from 0c3107d to d6799e3 Compare February 8, 2021 07:25

ngoc-do changed the title ~~[fabric] Disable orchagent-portsyncd dependency for fabric asic~~ [fabric] Disable unnecessary processes in swss and the orchagent-portsyncd dependency for fabric asic Feb 8, 2021

skeesara-nokia previously approved these changes Feb 24, 2021

View reviewed changes

ngoc-do dismissed skeesara-nokia’s stale review via 848d1e7 April 22, 2021 19:03

ngoc-do force-pushed the fabric branch from 963aa7b to 848d1e7 Compare April 22, 2021 19:03

prsunny reviewed Apr 27, 2021

View reviewed changes

prsunny requested a review from tahmed-dev April 27, 2021 23:14

ngoc-do force-pushed the fabric branch from 848d1e7 to 409aa3c Compare May 3, 2021 16:44

prsunny previously approved these changes May 18, 2021

View reviewed changes

tahmed-dev suggested changes May 18, 2021

View reviewed changes

Combine new cfggen statements into the existing ones

9056c3e

ngoc-do dismissed prsunny’s stale review via 9056c3e May 24, 2021 23:07

prsunny approved these changes May 25, 2021

View reviewed changes

tahmed-dev approved these changes Jun 2, 2021

View reviewed changes

prsunny merged commit 710563f into sonic-net:master Jun 9, 2021

judyjoseph added the Request for 202106 Branch label Jun 22, 2021

judyjoseph added the Included in 202106 Branch label Jul 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fabric] Disable unnecessary processes in swss and the orchagent-portsyncd dependency for fabric asic #5569

[fabric] Disable unnecessary processes in swss and the orchagent-portsyncd dependency for fabric asic #5569

ngoc-do commented Oct 9, 2020 •

edited

Loading

anshuv-mfst commented Feb 3, 2021

skeesara-nokia commented Feb 3, 2021

prsunny Feb 5, 2021

ngoc-do Feb 5, 2021

ngoc-do Feb 8, 2021

prsunny Feb 8, 2021

ngoc-do Feb 8, 2021

ngoc-do Feb 9, 2021

prsunny commented Feb 5, 2021

ngoc-do commented Feb 5, 2021

anshuv-mfst commented Feb 24, 2021

ngoc-do commented Apr 7, 2021

ngoc-do commented Apr 7, 2021

ngoc-do commented Apr 22, 2021

anshuv-mfst commented Apr 27, 2021

prsunny Apr 27, 2021

tahmed-dev May 18, 2021

tahmed-dev May 19, 2021

ngoc-do May 24, 2021

prsunny Apr 27, 2021

ngoc-do Apr 28, 2021

ngoc-do commented May 17, 2021

prsunny commented May 17, 2021

tahmed-dev May 18, 2021

ngoc-do commented May 24, 2021

anshuv-mfst commented May 26, 2021

ngoc-do commented Jun 4, 2021

anshuv-mfst commented Jun 9, 2021

saravanansv commented Jun 15, 2021

[fabric] Disable unnecessary processes in swss and the orchagent-portsyncd dependency for fabric asic #5569

[fabric] Disable unnecessary processes in swss and the orchagent-portsyncd dependency for fabric asic #5569

Conversation

ngoc-do commented Oct 9, 2020 • edited Loading

anshuv-mfst commented Feb 3, 2021

skeesara-nokia commented Feb 3, 2021

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prsunny commented Feb 5, 2021

ngoc-do commented Feb 5, 2021

anshuv-mfst commented Feb 24, 2021

ngoc-do commented Apr 7, 2021

ngoc-do commented Apr 7, 2021

ngoc-do commented Apr 22, 2021

anshuv-mfst commented Apr 27, 2021

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngoc-do commented May 17, 2021

prsunny commented May 17, 2021

Choose a reason for hiding this comment

ngoc-do commented May 24, 2021

anshuv-mfst commented May 26, 2021

ngoc-do commented Jun 4, 2021

anshuv-mfst commented Jun 9, 2021

saravanansv commented Jun 15, 2021

ngoc-do commented Oct 9, 2020 •

edited

Loading