-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[pfcwd]: add flag to enable pfc wd on start #1429
Conversation
Signed-off-by: Sihui Han <sihan@microsoft.com>
Makefile
Outdated
@@ -8,6 +8,7 @@ | |||
# * ENABLE_DHCP_GRAPH_SERVICE: Enables get-graph service to fetch minigraph files | |||
# through http. | |||
# * SHUTDOWN_BGP_ON_START: Sets admin-down state for all bgp peerings after restart. | |||
# * ENABLE_PFC_WD_ON_START: Enable PFCWD on server-facing ports when starts. |
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.
use PFCWD consistently throughout the CR?
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.
Enable PFC WatchDog (PFCWD) on server-facing ports by default for TOR switch.
@@ -168,6 +168,10 @@ sudo bash -c "echo enabled=false > $FILESYSTEM_ROOT/etc/sonic/updategraph.conf" | |||
{% if shutdown_bgp_on_start == "y" %} | |||
sudo bash -c "echo '{ \"DEVICE_METADATA\": { \"localhost\": { \"default_bgp_status\": \"down\" } } }' >> $FILESYSTEM_ROOT/etc/sonic/init_cfg.json" | |||
{% endif %} | |||
{% if enable_pfc_wd_on_start == "y" %} | |||
sudo bash -c "echo '{ \"DEVICE_METADATA\": { \"localhost\": { \"default_pfc_wd_status\": \"enable\" } } }' >> $FILESYSTEM_ROOT/etc/sonic/init_cfg.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.
which agent is reading this DEVICE_METADATA default_pfc_wd_status, where is that code?
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.
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.
if the PR is related/depended to other PR, can you mark this information in the PR comments?
Makefile
Outdated
@@ -8,6 +8,7 @@ | |||
# * ENABLE_DHCP_GRAPH_SERVICE: Enables get-graph service to fetch minigraph files | |||
# through http. | |||
# * SHUTDOWN_BGP_ON_START: Sets admin-down state for all bgp peerings after restart. | |||
# * ENABLE_PFC_WD_ON_START: Enable PFCWD on server-facing ports when starts. |
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.
Suggest change to ENABLE_PFC_WD_ON_START: Enable PFC Watchdog on server-facing ports when SONiC starts.
for clarity.
rules/config
Outdated
@@ -36,6 +36,10 @@ DEFAULT_PASSWORD = YourPaSsWoRd | |||
# bgp service starts. | |||
# SHUTDOWN_BGP_ON_START = y | |||
|
|||
# ENABLE_PFC_WD_ON_START - if set to y PFC WD will be enabled all server-facing ports when |
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.
Suggest changing WD
to Watchdog
for clarity.
Signed-off-by: Sihui Han <sihan@microsoft.com>
@@ -168,6 +168,10 @@ sudo bash -c "echo enabled=false > $FILESYSTEM_ROOT/etc/sonic/updategraph.conf" | |||
{% if shutdown_bgp_on_start == "y" %} | |||
sudo bash -c "echo '{ \"DEVICE_METADATA\": { \"localhost\": { \"default_bgp_status\": \"down\" } } }' >> $FILESYSTEM_ROOT/etc/sonic/init_cfg.json" | |||
{% endif %} | |||
{% if enable_pfcwd_on_start == "y" %} | |||
sudo bash -c "echo '{ \"DEVICE_METADATA\": { \"localhost\": { \"default_pfc_wd_status\": \"enable\" } } }' >> $FILESYSTEM_ROOT/etc/sonic/init_cfg.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.
to be consistent, this should also be default_pfcwd_status
Signed-off-by: Sihui Han <sihan@microsoft.com>
Makefile
Outdated
@@ -63,7 +64,7 @@ SONIC_BUILD_INSTRUCTION := make \ | |||
BUILD_NUMBER=$(BUILD_NUMBER) \ | |||
ENABLE_DHCP_GRAPH_SERVICE=$(ENABLE_DHCP_GRAPH_SERVICE) \ | |||
SHUTDOWN_BGP_ON_START=$(SHUTDOWN_BGP_ON_START) \ | |||
ENABLE_PFC_WD_ON_START=$(ENABLE_PFC_WD_ON_START) \ | |||
ENABLE_PFCWD_ON_START=$(ENABLE_PFCWD_ON_START) \ |
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.
This will always rewrite value in config 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.
Warn user while deleting VLAN if it has IP addresses. Signed-off-by: d-dashkov <Dmytro_Dashkov@Jabil.com>
Signed-off-by: Sihui Han sihan@microsoft.com
- What I did
Add flag to enable pfc wd on start
- How I did it
Add the option to enable pfc wd on start
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)