Skip to content

[ACL] Enable ACL multi-binding for Dual-ToR setups #1552

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
Merged
merged 1 commit into from
Apr 19, 2025
Merged

[ACL] Enable ACL multi-binding for Dual-ToR setups #1552

merged 1 commit into from
Apr 19, 2025

Conversation

ayurkiv-nvda
Copy link
Contributor

Enable ACL multi-binding for Dual-ToR scenario

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@bingwang-ms
Copy link
Contributor

Can you point to me that where is DUAL_TOR set to true? Is it in sai.profile?

@ayurkiv-nvda
Copy link
Contributor Author

Can you point to me that where is DUAL_TOR set to true? Is it in sai.profile?

Hello @bingwang-ms

So we are taking "DUAL_TOR" status from https://github.com/sonic-net/sonic-buildimage/blob/master/files/build_templates/swss_vars.j2

"dual_tor" from swss.vars is taken from config_db.json:
"dual_tor": {% if DEVICE_METADATA.localhost.type == "ToRRouter" and DEVICE_METADATA.localhost.subtype == "DualToR" %}"enable"{% else %}"disable"{% endif %},

@bingwang-ms
Copy link
Contributor

/azp run Azure.sonic-sairedis

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ayurkiv-nvda ayurkiv-nvda marked this pull request as ready for review March 20, 2025 08:57
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ayurkiv-nvda
Copy link
Contributor Author

/azpw run Azure.sonic-sairedis

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-sairedis

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ayurkiv-nvda
Copy link
Contributor Author

Hello @bingwang-ms
Can you please approve PR if it is ok for you?
Thanks

@bingwang-ms
Copy link
Contributor

LGTM. Thanks @ayurkiv-nvda
A sonic-mgmt test is needed to cover this change, please link to this PR if you already have. Thanks

@ayurkiv-nvda
Copy link
Contributor Author

LGTM. Thanks @ayurkiv-nvda A sonic-mgmt test is needed to cover this change, please link to this PR if you already have. Thanks

Thanks @bingwang-ms
Currently, we do not have sonic-mgmt test to cover this change. Existing ACL tests have to be updated. Thanks

@bingwang-ms
Copy link
Contributor

@kcudnik Can you help merge?

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 24, 2025

sure after success build

@ayurkiv-nvda
Copy link
Contributor Author

/azpw run Azure.sonic-sairedis

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-sairedis

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

@ayurkiv-nvda please check the PR checker. Is the fail is general or related to this PR. If general please ping the maintainer so he can take further look.

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bingwang-ms
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ayurkiv-nvda
Copy link
Contributor Author

/azpw run CodeQL

@mssonicbld
Copy link
Collaborator

/AzurePipelines run CodeQL

@azure-pipelines Azure Pipelines
Copy link

No pipelines are associated with this pull request.

@bingwang-ms
Copy link
Contributor

Hi @ayurkiv-nvda Could you please sync up code?
@kcudnik, could you help merge this PR once PR test passed?

@kcudnik kcudnik merged commit 80eb929 into sonic-net:master Apr 19, 2025
13 of 14 checks passed
@mssonicbld mssonicbld mentioned this pull request Apr 21, 2025
Merged
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202411: #1585

@bingwang-ms
Copy link
Contributor

CC @kperumalbfn for 202411 cherry-pick

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kcudnik kcudnik kcudnik approved these changes

@bingwang-ms bingwang-ms bingwang-ms approved these changes

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

Successfully merging this pull request may close these issues.
6 participants
@ayurkiv-nvda @mssonicbld @bingwang-ms @kcudnik @liat-grozovik @volodymyrsamotiy