Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[ACL] Write ACL table/rule creation status into STATE_DB #2662

Merged
merged 7 commits into from
Mar 8, 2023
Merged

[ACL] Write ACL table/rule creation status into STATE_DB #2662

merged 7 commits into from
Mar 8, 2023

Conversation

bingwang-ms
Copy link
Contributor

What I did
HLD sonic-net/SONiC#1261
This PR is to update orchagent to write ACL table/rule creation status into STATE_DB.
Currently, show acl table and show acl rule commands read ACL table/rule configuration from CONFIG_DB directly. We don't know whether the ACL table or rule is created successfully.
We improved orchagent to write the status of ACL table/rule into a STATE_DB table.

Why I did it
Add the status of ACL table and ACL rule into STATE_DB so that user can tell whether the table or rule is created successfully.

How I verified it
Verified by copying the updated orchagent to a testbed and run.

Details if related
HLD sonic-net/SONiC#1261

@bingwang-ms bingwang-ms requested a review from prsunny as a code owner February 10, 2023 01:38
@bingwang-ms bingwang-ms mentioned this pull request Feb 10, 2023
Merged
@prsunny
Copy link
Collaborator

prsunny commented Feb 13, 2023

You may want to clear the STATE_DB tables during swss restart, otherwise it can show an old data

prsunny
prsunny reviewed Feb 13, 2023
View reviewed changes
orchagent/aclorch.cpp
it = consumer.m_toSync.erase(it);
}
else
it++;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to handle here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I added a Pending removal status for this case.

Copy link
Contributor Author

@bingwang-ms bingwang-ms Mar 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I added a Pending removal status for the ACL rules that are pending to be removed.

@bingwang-ms
Copy link
Contributor Author

You may want to clear the STATE_DB tables during swss restart, otherwise it can show an old data

But even if swss restart, the ACL table/rule is still programmed in ASIC, right? So I think we can keep the status in STATE_DB.
There is one scenario I need to handle, that is clear the STATE_DB entries at config_reload. Otherwise there can be stale entries in STATE_DB.

@bingwang-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bingwang-ms
Copy link
Contributor Author

You may want to clear the STATE_DB tables during swss restart, otherwise it can show an old data

But even if swss restart, the ACL table/rule is still programmed in ASIC, right? So I think we can keep the status in STATE_DB. There is one scenario I need to handle, that is clear the STATE_DB entries at config_reload. Otherwise there can be stale entries in STATE_DB.

I added two functions at aclorch startup to clear the status from STATE_DB. This can address both config reload and swss restart scenarios.

prsunny
prsunny previously approved these changes Mar 2, 2023
View reviewed changes
@bingwang-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bingwang-ms bingwang-ms merged commit 98a16cf into sonic-net:master Mar 8, 2023
yxieca pushed a commit that referenced this pull request Mar 8, 2023
@bingwang-ms @yxieca
[ACL] Write ACL table/rule creation status into STATE_DB (#2662)
95d3f6d 
* Add status for ACL_TABLE and ACL_RULE in STATE_DB
@dgsudharsan dgsudharsan mentioned this pull request Mar 9, 2023
Merged
8 tasks
dgsudharsan added a commit to dgsudharsan/sonic-buildimage that referenced this pull request Mar 14, 2023
@dgsudharsan
[submodule] Update sonic-swss submodule
c8bb11d 
Update sonic-swss submodule pointer to include the following:
* 98a16cf [ACL] Write ACL table/rule creation status into STATE_DB ([sonic-net#2662](sonic-net/sonic-swss#2662))
* a2c9a61 [EVPN]Handling error scenarios during route programming and IMR add ([sonic-net#2670](sonic-net/sonic-swss#2670))
* 115efe8 [bfdorch] add default TOS value for BFD session ([sonic-net#2689](sonic-net/sonic-swss#2689))
* a198289 [orchagent, SRv6]: create seglist support to set sid list type ([sonic-net#2406](sonic-net/sonic-swss#2406))

Signed-off-by: dgsudharsan <sudharsand@nvidia.com>
prsunny pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Mar 14, 2023
@dgsudharsan
[submodule] Update sonic-swss submodule (#14177)
43af6b9 
Update sonic-swss submodule pointer to include the following:
* 98a16cf [ACL] Write ACL table/rule creation status into STATE_DB ([#2662](sonic-net/sonic-swss#2662))
* a2c9a61 [EVPN]Handling error scenarios during route programming and IMR add ([#2670](sonic-net/sonic-swss#2670))
* 115efe8 [bfdorch] add default TOS value for BFD session ([#2689](sonic-net/sonic-swss#2689))
* a198289 [orchagent, SRv6]: create seglist support to set sid list type ([#2406](sonic-net/sonic-swss#2406))
StormLiangMS pushed a commit that referenced this pull request Mar 19, 2023
@bingwang-ms @StormLiangMS
[ACL] Write ACL table/rule creation status into STATE_DB (#2662)
9d38fbc 
* Add status for ACL_TABLE and ACL_RULE in STATE_DB
keboliu added a commit to keboliu/sonic-swss that referenced this pull request Apr 1, 2023
@keboliu
Revert "[ACL] Write ACL table/rule creation status into STATE_DB (son…
e0aa24f 
…ic-net#2662)"

This reverts commit 9d38fbc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prsunny prsunny prsunny approved these changes

Assignees
No one assigned
Labels
Included in 202205 Branch Included in 202211 Branch Request for 202205 Branch Request for 202211 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bingwang-ms @prsunny @yxieca @StormLiangMS