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

Support both port name and alias in ACL table AttachTo attribute #13444

Merged
merged 5 commits into from
Jan 20, 2023

Conversation

bingwang-ms
Copy link
Contributor

@bingwang-ms bingwang-ms commented Jan 19, 2023

Why I did it

This PR is an enhancement of PR #13105
Because the input string of AttachTo for ACL table can appear in both port name group and port alias group, I added a logic to determine whether the string should be port name or port alias

  1. If all the input strings belong to port name group, then we treat all of them as port name
  2. If all the input strings belong to port alias, then we treat all of them as port alias
  3. If all the input string belongs to both port alias group and port name group, we prefer port alias. The behavior is as before.

How I did it

Walk through all port names/alias in the input to make a decision.

How to verify it

Verified by adding UT.

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

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

Description for the changelog

Support both port name and alias in ACL table AttachTo attribute.

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)

@bingwang-ms bingwang-ms force-pushed the acl_bind_to_port_name_enhance branch from 822cf42 to b10bf97 Compare January 20, 2023 06:44
@yxieca yxieca merged commit b03a65f into sonic-net:master Jan 20, 2023
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Jan 20, 2023
…onic-net#13444)

Why I did it
This PR is an enhancement of PR sonic-net#13105
Because the input string of AttachTo for ACL table can appear in both port name group and port alias group, I added a logic to determine whether the string should be port name or port alias

If all the input strings belong to port name group, then we treat all of them as port name
If all the input strings belong to port alias, then we treat all of them as port alias
If all the input string belongs to both port alias group and port name group, we prefer port alias. The behavior is as before.
How I did it
Walk through all port names/alias in the input to make a decision.

How to verify it
Verified by adding UT.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202205: #13451

bingwang-ms added a commit to bingwang-ms/sonic-buildimage that referenced this pull request Jan 20, 2023
…onic-net#13444)

Why I did it
This PR is an enhancement of PR sonic-net#13105
Because the input string of AttachTo for ACL table can appear in both port name group and port alias group, I added a logic to determine whether the string should be port name or port alias

If all the input strings belong to port name group, then we treat all of them as port name
If all the input strings belong to port alias, then we treat all of them as port alias
If all the input string belongs to both port alias group and port name group, we prefer port alias. The behavior is as before.
How I did it
Walk through all port names/alias in the input to make a decision.

How to verify it
Verified by adding UT.
mlok-nokia pushed a commit to mlok-nokia/sonic-buildimage that referenced this pull request Jan 26, 2023
…onic-net#13444)

Why I did it
This PR is an enhancement of PR sonic-net#13105
Because the input string of AttachTo for ACL table can appear in both port name group and port alias group, I added a logic to determine whether the string should be port name or port alias

If all the input strings belong to port name group, then we treat all of them as port name
If all the input strings belong to port alias, then we treat all of them as port alias
If all the input string belongs to both port alias group and port name group, we prefer port alias. The behavior is as before.
How I did it
Walk through all port names/alias in the input to make a decision.

How to verify it
Verified by adding UT.
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Feb 6, 2023
…onic-net#13444)

Why I did it
This PR is an enhancement of PR sonic-net#13105
Because the input string of AttachTo for ACL table can appear in both port name group and port alias group, I added a logic to determine whether the string should be port name or port alias

If all the input strings belong to port name group, then we treat all of them as port name
If all the input strings belong to port alias, then we treat all of them as port alias
If all the input string belongs to both port alias group and port name group, we prefer port alias. The behavior is as before.
How I did it
Walk through all port names/alias in the input to make a decision.

How to verify it
Verified by adding UT.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202211: #13678

mssonicbld pushed a commit that referenced this pull request Feb 7, 2023
…13444)

Why I did it
This PR is an enhancement of PR #13105
Because the input string of AttachTo for ACL table can appear in both port name group and port alias group, I added a logic to determine whether the string should be port name or port alias

If all the input strings belong to port name group, then we treat all of them as port name
If all the input strings belong to port alias, then we treat all of them as port alias
If all the input string belongs to both port alias group and port name group, we prefer port alias. The behavior is as before.
How I did it
Walk through all port names/alias in the input to make a decision.

How to verify it
Verified by adding UT.
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.

5 participants