Skip to content

[CoPP] Add always_enabled field to coppmgr logic #2147

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

Merged
merged 1 commit into from
Feb 23, 2022

Conversation

noaOrMlnx
Copy link
Collaborator

  • What I did
    Add the new design for copp manager - always_enabled field in copp_cfg.json file.

  • Why I did it
    A change was done for traps needs to be installed but doesn't have feature (arp, udld, lacp, ip2me),
    in the new implementation, coppmgr will not automatically install traps which has no entry in features table, but will check first if the trap has "always_enabled":"true" field.

  • How I verified it
    run tests for making sure traps are installed and not when expecting them to.

  • Details if related
    Related to sonic-buildimage change - [CoPP] Add always_enabled field sonic-buildimage#9302
    SONiC mgmt test plan can be found in Copp manager redesign test plan SONiC#903.

This commit is a cherry-pick of #2034

- What I did
Add the new design for copp manager - always_enabled field in copp_cfg.json file.

- Why I did it
A change was done for traps needs to be installed but doesn't have feature (arp, udld, lacp, ip2me),
in the new implementation, coppmgr will not automatically install traps which has no entry in features table, but will check first if the trap has "always_enabled":"true" field.

- How I verified it
run tests for making sure traps are installed and not when expecting them to.

- Details if related
Related to sonic-buildimage change - sonic-net/sonic-buildimage#9302
SONiC mgmt test plan can be found in sonic-net/SONiC#903.
@lgtm-com
Copy link

lgtm-com bot commented Feb 16, 2022

This pull request introduces 2 alerts when merging bb833c4 into 081dd01 - view on LGTM.com

new alerts:

  • 2 for Variable defined multiple times

@prsunny
Copy link
Collaborator

prsunny commented Feb 16, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Collaborator

prsunny commented Feb 17, 2022

Isn't this dependent on sonic-net/sonic-buildimage#9999 ?

@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Collaborator

prsunny commented Feb 22, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny prsunny merged commit 6ac0b9b into sonic-net:202012 Feb 23, 2022
@prsunny
Copy link
Collaborator

prsunny commented Feb 23, 2022

@noaOrMlnx , could you please raise a submodule update?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants