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.

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

[storage_backend] Add backend acl service #14229

Merged
merged 3 commits into from
Mar 16, 2023

Conversation

neethajohn
Copy link
Contributor

@neethajohn neethajohn commented Mar 13, 2023

Change in sonic-net/sonic-utilities#2236 was done to load acls on storage backend device. During conversion, it was observed that some rules were getting skipped with the following error
Mar 7 19:33:53 sonic config-setup[999]: Running command: acl-loader update incremental /etc/sonic/backend_acl.json
Mar 7 19:33:53 sonic config-setup[999]: Running command: acl-loader update incremental /etc/sonic/backend_acl.json
Mar 7 19:33:54 sonic python3: :- operator(): Key '{SWITCH_CAPABILITY|switch}' unavailable in database '{STATE_DB}'
Mar 7 19:33:54 sonic python3: :- operator(): Key '{SWITCH_CAPABILITY|switch}' unavailable in database '{STATE_DB}'
Mar 7 19:33:54 sonic /acl-loader: Error processing rule 1: Rule action ACCEPT is not supported in table DATAACL, rule 1. Skipped.
Mar 7 19:33:54 sonic /acl-loader: Error processing rule 1: Rule action ACCEPT is not supported in table DATAACL, rule 1. Skipped.

The reason for the above failure was because acl loader does some sanity checks before writing the rules into config db and depends on SWITCH_CAPABILITY|switch table in state db (https://github.com/sonic-net/sonic-utilities/blob/master/acl_loader/main.py#L451) which is not present as soon as the switch boots up resulting in the rule being invalid.

Why I did it

This PR addresses the issue mentioned above by loading the acl config as a service on a storage backend device

How I did it

The new acl service is a oneshot service which will start after swss and does some retries to ensure that the SWITCH_CAPABILITY info is present before attempting to load the acl rules. The service is also bound to sonic targets which ensures that it gets restarted during minigraph reload and config reload

How to verify it

Build an image with the following changes and did the following tests

  1. Verified that acl is loaded successfully on a storage backend device after a switch boot up
  2. Verified that acl is loaded successfully on a storage backend ToR after minigraph load and config reload
  3. Verified that acl is not loaded if the device is not a storage backend ToR or the device does not have a DATAACL table

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211

Description for the changelog

Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Neetha John <nejo@microsoft.com>
Signed-off-by: Neetha John <nejo@microsoft.com>
@neethajohn neethajohn marked this pull request as ready for review March 14, 2023 01:11
Copy link
Contributor

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

overall lgtm

Signed-off-by: Neetha John <nejo@microsoft.com>
@neethajohn
Copy link
Contributor Author

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 14229 in repo sonic-net/sonic-buildimage

Copy link
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi left a comment

Choose a reason for hiding this comment

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

This change looks good for single asic backend-ToR, which is good for current scenario.
In order to keep the change generic for single and multi-asic, change should be done to get SWITCH_CAPABILITY from first front-end namespace like https://github.com/sonic-net/sonic-utilities/blob/master/acl_loader/main.py#L443

@yxieca yxieca merged commit f30fb6e into sonic-net:master Mar 16, 2023
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Mar 16, 2023
Why I did it
This PR addresses the issue mentioned above by loading the acl config as a service on a storage backend device

How I did it
The new acl service is a oneshot service which will start after swss and does some retries to ensure that the SWITCH_CAPABILITY info is present before attempting to load the acl rules. The service is also bound to sonic targets which ensures that it gets restarted during minigraph reload and config reload

How to verify it
Build an image with the following changes and did the following tests

Verified that acl is loaded successfully on a storage backend device after a switch boot up
Verified that acl is loaded successfully on a storage backend ToR after minigraph load and config reload
Verified that acl is not loaded if the device is not a storage backend ToR or the device does not have a DATAACL table

Signed-off-by: Neetha John <nejo@microsoft.com>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202205: #14281

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Mar 19, 2023
Why I did it
This PR addresses the issue mentioned above by loading the acl config as a service on a storage backend device

How I did it
The new acl service is a oneshot service which will start after swss and does some retries to ensure that the SWITCH_CAPABILITY info is present before attempting to load the acl rules. The service is also bound to sonic targets which ensures that it gets restarted during minigraph reload and config reload

How to verify it
Build an image with the following changes and did the following tests

Verified that acl is loaded successfully on a storage backend device after a switch boot up
Verified that acl is loaded successfully on a storage backend ToR after minigraph load and config reload
Verified that acl is not loaded if the device is not a storage backend ToR or the device does not have a DATAACL table

Signed-off-by: Neetha John <nejo@microsoft.com>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202211: #14339

mssonicbld pushed a commit that referenced this pull request Mar 19, 2023
Why I did it
This PR addresses the issue mentioned above by loading the acl config as a service on a storage backend device

How I did it
The new acl service is a oneshot service which will start after swss and does some retries to ensure that the SWITCH_CAPABILITY info is present before attempting to load the acl rules. The service is also bound to sonic targets which ensures that it gets restarted during minigraph reload and config reload

How to verify it
Build an image with the following changes and did the following tests

Verified that acl is loaded successfully on a storage backend device after a switch boot up
Verified that acl is loaded successfully on a storage backend ToR after minigraph load and config reload
Verified that acl is not loaded if the device is not a storage backend ToR or the device does not have a DATAACL table

Signed-off-by: Neetha John <nejo@microsoft.com>
qiluo-msft pushed a commit that referenced this pull request Mar 20, 2023
Why I did it
This PR addresses the issue mentioned above by loading the acl config as a service on a storage backend device

How I did it
The new acl service is a oneshot service which will start after swss and does some retries to ensure that the SWITCH_CAPABILITY info is present before attempting to load the acl rules. The service is also bound to sonic targets which ensures that it gets restarted during minigraph reload and config reload

How to verify it
Build an image with the following changes and did the following tests

Verified that acl is loaded successfully on a storage backend device after a switch boot up
Verified that acl is loaded successfully on a storage backend ToR after minigraph load and config reload
Verified that acl is not loaded if the device is not a storage backend ToR or the device does not have a DATAACL table

Signed-off-by: Neetha John <nejo@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants