-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Copp Manager Changes #4861
Copp Manager Changes #4861
Conversation
Copp changes vs changes
@@ -53,7 +53,7 @@ if [[ "$SYSTEM_WARM_START" == "true" ]] || [[ "$SWSS_WARM_START" == "true" ]]; t | |||
fi | |||
fi | |||
|
|||
SWSSCONFIG_ARGS="00-copp.config.json ipinip.json ports.json switch.json " |
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.
Please remove this file from the source location also.
Hi, @dgsudharsan! Could you please tell me when do you plan to resolve conflicts? |
@dgsudharsan , could you please resolve conflicts? |
…to copp_changes
@@ -71,6 +71,18 @@ stderr_logfile=syslog | |||
dependent_startup=true | |||
dependent_startup_wait_for=orchagent:running | |||
|
|||
[program:coppmgrd] | |||
command=/usr/bin/coppmgrd |
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.
Based on HLD, coppmgrd shall be started before other processes. Is there any reason for starting at priority 6?
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.
Its starting immediately after the orchagent. The initial idea was to start coppmgrd at the same priority as start.sh which was earlier programming copp
sudo cp $IMAGE_CONFIGS/copp/copp-config.service $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM | ||
sudo cp $IMAGE_CONFIGS/copp/copp-config.sh $FILESYSTEM_ROOT/usr/bin/ | ||
sudo cp $IMAGE_CONFIGS/copp/copp_cfg.j2 $FILESYSTEM_ROOT_USR_SHARE_SONIC_TEMPLATES/ | ||
echo "copp-config.service" | sudo tee -a $GENERATED_SERVICE_FILE |
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.
Instead of having a service to generate copp_cfg.json, can we do it as part of docker-orchagent/docker-init.sh?
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.
The file is in the host and not inside the docker and hence we need it to be created through a one shot task.
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.
ok
files/image_config/copp/copp_cfg.j2
Outdated
"meter_type":"packets", | ||
"mode":"sr_tcm", | ||
"cir":"6000", | ||
"cbs":"6000", |
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.
Please align these two lines
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.
Done
…to copp_changes
…to copp_changes
retest this please |
retest this please |
retest vsimage please |
4 similar comments
retest vsimage please |
retest vsimage please |
retest vsimage please |
retest vsimage please |
…to copp_changes
sonic-swss: [fea7ade] [orchagent][port] In case of successful port creation set log level to NOTICE instead of ERR (sonic-net#1500) [7b76d2e] Copp Manager Changes (sonic-net#1333) [bed7970] [orchagent] Arm 32-bit arch compilation warning Fixes (sonic-net#1488) [b9084a7] Revert: swss: flush g_asicState after each event is done sonic-net#570 (sonic-net#1478) [d6e15e9] [dvs] Clean-up conftest.py (sonic-net#1406)
retest vs please |
2 similar comments
retest vs please |
retest vs please |
…to copp_changes
retest vs please |
2 similar comments
retest vs please |
retest vs please |
retest vsimage please |
…to copp_changes
retest mellanox please |
command=/usr/bin/coppmgrd | ||
priority=6 | ||
autostart=false | ||
autorestart=unexpected |
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.
@dgsudharsan For the process coppmgrd
, the autorestart
field was set to unexpected
. That means if it exited unexpectedly, this process will be restarted automatically. So I am wondering whether we should put this process in the critical_processes
or not since the processes in that file can not be restarted if they exited?
*Introduce CoPP Manager infrastructure Copp service to generate initial copp config template file Co-authored-by: dgsudharsan <[email protected]>
- Why I did it
Made Changes to Introduce CoPP Manager infrastructure
- How I did it
Added Logic to include copp manager
- How to verify it
Verify using pytest and manual testing
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)