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

[Dual-ToR] add default value for ACL rule for mellanox platform (Standby ToR) #13547

Merged
merged 1 commit into from
Feb 22, 2023
Merged

[Dual-ToR] add default value for ACL rule for mellanox platform (Standby ToR) #13547

merged 1 commit into from
Feb 22, 2023

Conversation

ayurkiv-nvda
Copy link
Contributor

@ayurkiv-nvda ayurkiv-nvda commented Jan 30, 2023 •
edited by liat-grozovik
Loading

Why I did it

Need to add the possibility to choose between dropping packets (using ACL) on ingress or egress in Dual ToR scenario

How I did it

Add new attribute "mux_tunnel_ingress_acl" to SYSTEM_DEFAULTS table

How to verify it

check that new attribute exists in redis:
admin@sonic:~$ redis-cli -n 4
127.0.0.1:6379[4]> HGETALL SYSTEM_DEFAULTS|mux_tunnel_ingress_acl
1."state"
2."false"

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)

@ayurkiv-nvda ayurkiv-nvda changed the title [Dual-ToR] add default value for ACL rule for mellanox platform [Dual-ToR] add default value for Dual-ToR Standby ACL rule for mellanox platform Jan 30, 2023
@ayurkiv-nvda ayurkiv-nvda changed the title [Dual-ToR] add default value for Dual-ToR Standby ACL rule for mellanox platform [Dual-ToR] add default value for ACL rule for mellanox platform (Standby ToR) Jan 30, 2023
bingwang-ms
bingwang-ms reviewed Jan 30, 2023
View reviewed changes
files/build_templates/init_cfg.json.j2
@@ -133,5 +133,16 @@
"digits_class": "true",
"special_class": "true"
}
},
"SYSTEM_DEFAULTS" : {
{%- if include_mux == "y" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it better to generate the SYSTEM_DEFAULTS table only when sonic_asic_platform == "mellanox", and include_mux == "y"? In this way, there will be no change for other platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your remark.
I thought about it previously. But I decided that it may look confusing that SYSTEM_DEFAULT table (that is generic table) is under "mellanox" and "mux" condition.
What do you think about it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Make sense. Please also consider when we are upgrading from previous version, the mux_tunnel_egress_acl is absent. Our code should be handle that exception.

@liat-grozovik
Copy link
Collaborator

@bingwang-ms can you please review and approve?

@liat-grozovik liat-grozovik merged commit 5ad78ab into sonic-net:master Feb 22, 2023
@liat-grozovik
Copy link
Collaborator

@ayurkiv-nvda please clarify to which branch the fix should be taken?
and if not cleanly cherry picked please have a separated PR against to requested branch.

@ayurkiv-nvda
Copy link
Contributor Author

@ayurkiv-nvda please clarify to which branch the fix should be taken? and if not cleanly cherry picked please have a separated PR against to requested branch.

It should be merged to 202205 also
But it can be merged only after #11117 will be merged to 202205

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Mar 7, 2023
@ayurkiv-nvda @mssonicbld
[Dual-ToR] add default value for ACL rule for mellanox platform (soni…
fcee8d4 
…c-net#13547)

- Why I did it
Need to add the possibility to choose between dropping packets (using ACL) on ingress or egress in Dual ToR scenario

- How I did it
Add new attribute "mux_tunnel_ingress_acl" to SYSTEM_DEFAULTS table

- How to verify it
check that new attribute exists in redis:
admin@sonic:~$ redis-cli -n 4
127.0.0.1:6379[4]> HGETALL SYSTEM_DEFAULTS|mux_tunnel_ingress_acl
1."state"
2."false"

Signed-off-by: Andriy Yurkiv <[email protected]>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202211: #14139

@mssonicbld mssonicbld mentioned this pull request Mar 7, 2023
Merged
mssonicbld pushed a commit that referenced this pull request Mar 10, 2023
@ayurkiv-nvda @mssonicbld
[Dual-ToR] add default value for ACL rule for mellanox platform (#13547)
c4e488c 
- Why I did it
Need to add the possibility to choose between dropping packets (using ACL) on ingress or egress in Dual ToR scenario

- How I did it
Add new attribute "mux_tunnel_ingress_acl" to SYSTEM_DEFAULTS table

- How to verify it
check that new attribute exists in redis:
admin@sonic:~$ redis-cli -n 4
127.0.0.1:6379[4]> HGETALL SYSTEM_DEFAULTS|mux_tunnel_ingress_acl
1."state"
2."false"

Signed-off-by: Andriy Yurkiv <[email protected]>
@ayurkiv-nvda ayurkiv-nvda mentioned this pull request Mar 14, 2023
Closed
StormLiangMS pushed a commit to StormLiangMS/sonic-buildimage that referenced this pull request Mar 28, 2023
@xumia @StormLiangMS
Merged PR 7734653: Merge code from public to internal
2a50b2d 
Related work items: sonic-net#276, sonic-net#305, sonic-net#332, sonic-net#338, sonic-net#339, sonic-net#1188, sonic-net#1192, sonic-net#1197, sonic-net#1206, sonic-net#1685, sonic-net#1690, sonic-net#1696, sonic-net#1699, sonic-net#1709, sonic-net#1727, sonic-net#1737, sonic-net#1741, sonic-net#1742, sonic-net#2511, sonic-net#2512, sonic-net#2532, sonic-net#2559, sonic-net#2626, sonic-net#2638, sonic-net#2645, sonic-net#2649, sonic-net#2660, sonic-net#2669, sonic-net#2670, sonic-net#2678, sonic-net#10084, sonic-net#11442, sonic-net#11873, sonic-net#12047, sonic-net#12110, sonic-net#12207, sonic-net#12529, sonic-net#12678, sonic-net#13235, sonic-net#13287, sonic-net#13372, sonic-net#13395, sonic-net#13456, sonic-net#13497, sonic-net#13522, sonic-net#13545, sonic-net#13547, sonic-net#13552, sonic-net#13569, sonic-net#13572, sonic-net#13578, sonic-net#13591, sonic-net#13611, sonic-net#13647, sonic-net#13649, sonic-net#13660, sonic-net#13710, sonic-net#13716, sonic-net#13724, sonic-net#13726, sonic-net#13732, sonic-net#13735, sonic-net#13739, sonic-net#13757, sonic-net#13786, sonic-net#13792, sonic-net#13800, sonic-net#13801, sonic-net#13802, sonic-net#13805, sonic-net#13806, sonic-net#13812, sonic-net#13814, sonic-net#13822, sonic-net#13831, sonic-net#13834, sonic-net#13847, sonic-net#13870, sonic-net#13882, sonic-net#13884, sonic-net#13885, sonic-net#13894, sonic-net#13895, sonic-net#13926, sonic-net#13932, sonic-net#13935, sonic-net#13942, sonic-net#13951, sonic-net#13953, sonic-net#13964
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@liat-grozovik liat-grozovik liat-grozovik approved these changes

@qiluo-msft qiluo-msft Awaiting requested review from qiluo-msft qiluo-msft is a code owner

@lguohan lguohan Awaiting requested review from lguohan lguohan is a code owner

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

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