-
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
[COPP:Yang-changes] Fix for config replace issues seen with COPP configurations #19817
base: master
Are you sure you want to change the base?
[COPP:Yang-changes] Fix for config replace issues seen with COPP configurations #19817
Conversation
==> Root cause: • COPP's device initial config copp_cfg.json has missing yang mandatory leaf "trap_action" for "default" COPP_GROUP. • "genetlink_name" & "genetlink_mcgrp_name" nodes are not added in sonic-copp.yang, which are used for "queue2_group1" in copp_cfg.json ==> Fix: • In copp_cfg.json, mandatory leaf "trap_action" is added for "default" COPP_GROUP. "COPP_GROUP": { "default": { "trap_action":"trap", <<< this node needs to be added "queue": "0", "meter_type":"packets", • "genetlink_name" and "genetlink_mcgrp_name" leaves are added in sonic-copp.yang. • DB Migration script is updated to upgrade the DB config automatically. ==> Tests : Config replace -> i. Config load copp_cfg.json ii. Config save iii. Config replace -> Ensure no error seen Upgrade -> i. Save config in build without the fix ii. upgrade to build with changes and ensure configurations are reflected in new config_db.json ==> PRs : Sonic-buildimage : sonic-net/sonic-buildimage#19817 Sonic-utilities : #3473 Sonic (docs) :
@qiluo-msft @lguohan @ganglyu @prsunny @dgsudharsan Please help in reviewing these PRs- |
@@ -133,6 +138,16 @@ module sonic-copp { | |||
default "forward"; | |||
description "Red action"; | |||
} | |||
|
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.
These are internal fields and shouldn't be user configurable. Please check the comment here #7199 (comment)
@@ -1,6 +1,7 @@ | |||
{ | |||
"COPP_GROUP": { | |||
"default": { | |||
"trap_action":"trap", |
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 file is not validated against yang. The trap action for default is hardcoded due to legacy code present in https://github.com/sonic-net/sonic-swss/blob/465391d1825ea3906d27a1285d2da5876f763cd7/orchagent/copporch.cpp#L185
By default the copp config doesn't go to config_db and thus we don't require yang validation on this file. However if the user need to override the copp config, user needs to set the appropriate fields along with correct yang validation.
==> Issue:
config replace/yang-validation fails for COPP configurations.
==> Root cause:
• COPP's device initial config copp_cfg.json has missing yang mandatory leaf "trap_action" for "default" COPP_GROUP.
• "genetlink_name" & "genetlink_mcgrp_name" nodes are not added in sonic-copp.yang, which are used for
"queue2_group1" in copp_cfg.json
==> Fix:
• In copp_cfg.json, mandatory leaf "trap_action" is added for "default" COPP_GROUP.
"COPP_GROUP": {
"default": {
+ "trap_action":"trap", <<< this node needs to be added
"queue": "0",
"meter_type":"packets",
• "genetlink_name" and "genetlink_mcgrp_name" leaves are added in sonic-copp.yang.
• DB Migration script is updated to upgrade the DB config automatically.
==> Tests :
Config replace ->
i. Config load copp_cfg.json
ii. Config save
iii. Config replace -> Ensure no error seen
Upgrade ->
i. Save config in build without the fix
ii. upgrade to build with changes and ensure configurations are reflected in new config_db.json
==> PRs :
Sonic-buildimage : #19817
Sonic-utilities : sonic-net/sonic-utilities#3473
SONiC (docs) : sonic-net/SONiC#1773
Signed-off-by: Anukul Verma anukverm@cisco.com