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-loader] Only add default deny rule when table is L3 or L3V6 #2796

Merged
merged 8 commits into from
Apr 20, 2023
Merged

[acl-loader] Only add default deny rule when table is L3 or L3V6 #2796

merged 8 commits into from
Apr 20, 2023

Conversation

lizhijianrd
Copy link
Contributor

@lizhijianrd lizhijianrd commented Apr 17, 2023 •
edited
Loading

What I did

  1. Update acl-loader to only add default deny rule when table is L3 or L3V6.
  2. Update unittest to cover it.

How I did it

Update function deny_rule and return {} if table is not L3 or L3V6.

How to verify it

  1. Update unittest and run all testcases to verify:
----------- coverage: platform linux, python 3.9.2-final-0 -----------
Name                                                   Stmts   Miss Branch BrPart  Cover
----------------------------------------------------------------------------------------
acl_loader/__init__.py                                     0      0      0      0   100%
acl_loader/main.py                                       640    152    284     50    71%  ...
=================================================================== short test summary info ====================================================================
FAILED tests/disk_check_test.py::TestDiskCheck::test_readonly - assert 1 == 0
FAILED tests/drops_group_test.py::TestDropCounters::test_show_counts - AssertionError: assert ('    IFACE    STATE    RX_ERR    RX_DROPS    TX_ERR    TX_DROP...
FAILED tests/drops_group_test.py::TestDropCounters::test_show_counts_with_group - AssertionError: assert ('\n'\n '          DEVICE    SWITCH_DROPS\n'\n '----...
FAILED tests/drops_group_test.py::TestDropCounters::test_show_counts_with_type - AssertionError: assert ('    IFACE    STATE    RX_ERR    RX_DROPS    DEBUG_0...
============================================== 4 failed, 2300 passed, 3 skipped, 16 warnings in 672.50s (0:11:12) ==============================================
  1. Built the package and installed on DUT to verify.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

@Blueve
Copy link
Contributor

Blueve commented Apr 18, 2023

@bingwang-ms can you help review this PR?

Blueve
Blueve previously approved these changes Apr 18, 2023
View reviewed changes
@bingwang-ms
Copy link
Contributor

The change looks good. Can you please also add UT code to cover the change? Thanks

@lizhijianrd lizhijianrd changed the title [acl-loader] Only add default deny rule when table is L3, L3V6, MIRROR or MIRRORV6 [acl-loader] Only add default deny rule when table is L3 or L3V6 Apr 19, 2023
@lizhijianrd lizhijianrd requested a review from Blueve April 19, 2023 12:22
@lizhijianrd lizhijianrd merged commit b547bb4 into sonic-net:master Apr 20, 2023
StormLiangMS pushed a commit that referenced this pull request Apr 20, 2023
@lizhijianrd @StormLiangMS
[acl-loader] Only add default deny rule when table is L3 or L3V6 (#2796)
e0bafc2 
What I did
1. Update acl-loader to only add default deny rule when table is L3 or L3V6.
2. Update unittest to cover it.

How I did it
Update function deny_rule and return {} if table is not L3 or L3V6.

How to verify it
1. Update unittest and run all testcases to verify.
2. Built the package and installed on DUT to verify.

Signed-off-by: Zhijian Li <[email protected]>
dprital added a commit to dprital/sonic-buildimage that referenced this pull request May 1, 2023
@dprital
[submodule] Advance sonic-utilities pointer
6877859 
Update sonic-utilities submodule pointer to include the following:
* 88ffb167 [config]config reload should generate sysinfo if missing ([sonic-net#2778](sonic-net/sonic-utilities#2778))
* 7443b9e5 [sonic-package-manager] support extension with multiple YANG modules ([sonic-net#2752](sonic-net/sonic-utilities#2752))
* 522c3a9e [sonic-package-manager] add support for multiple CLI plugin files ([sonic-net#2753](sonic-net/sonic-utilities#2753))
* b38fcfd1 [show][muxcable] fix  RC ([sonic-net#2812](sonic-net/sonic-utilities#2812))
* 7e24463f [chassis]: remote cli commands infra for sonic chassis ([sonic-net#2701](sonic-net/sonic-utilities#2701))
* bee593e4 [DPB]Fixing typo in config breakout output ([sonic-net#2802](sonic-net/sonic-utilities#2802))
* ada603c5 [config]Support multi-asic  Golden Config override ([sonic-net#2738](sonic-net/sonic-utilities#2738))
* 88a7daa8 [show][barefoot] replace shell=True ([sonic-net#2699](sonic-net/sonic-utilities#2699))
* 5e99edb5 [sonic_package_manager] replace shell=True ([sonic-net#2726](sonic-net/sonic-utilities#2726))
* b547bb45 [acl-loader] Only add default deny rule when table is L3 or L3V6 ([sonic-net#2796](sonic-net/sonic-utilities#2796))

Signed-off-by: dprital <[email protected]>
@dprital dprital mentioned this pull request May 1, 2023
Closed
10 tasks
lizhijianrd added a commit to lizhijianrd/sonic-utilities that referenced this pull request May 4, 2023
@lizhijianrd
[acl-loader] Only add default deny rule when table is L3 or L3V6 (son…
5a03b0d 
…ic-net#2796)

What I did
1. Update acl-loader to only add default deny rule when table is L3 or L3V6.
2. Update unittest to cover it.

How I did it
Update function deny_rule and return {} if table is not L3 or L3V6.

How to verify it
1. Update unittest and run all testcases to verify.
2. Built the package and installed on DUT to verify.

Signed-off-by: Zhijian Li <[email protected]>
@lizhijianrd lizhijianrd mentioned this pull request May 4, 2023
Merged
yxieca pushed a commit that referenced this pull request May 4, 2023
@lizhijianrd
[acl-loader] Only add default deny rule when table is L3 or L3V6 (#2796…
f6359bc 
…) (#2826)

What I did
1. Update acl-loader to only add default deny rule when table is L3 or L3V6.
2. Update unittest to cover it.

How I did it
Update function deny_rule and return {} if table is not L3 or L3V6.

How to verify it
1. Update unittest and run all testcases to verify.
2. Built the package and installed on DUT to verify.

Signed-off-by: Zhijian Li <[email protected]>
@lizhijianrd
Copy link
Contributor Author

Opened PR #2826 to backport to 202205 branch and solved the conflict.

@lizhijianrd lizhijianrd deleted the fix-acl-loader branch October 25, 2023 10:34
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

@Blueve Blueve Blueve 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.
5 participants
@lizhijianrd @Blueve @bingwang-ms @yxieca @StormLiangMS