fixed test_null_route_helper: changed a default ACL rule #4163
Conversation
| @@ -149,6 +149,11 @@ def apply_pre_defined_rules(rand_selected_dut, create_acl_table): | |||
| rand_selected_dut.shell("acl-loader update full " + ACL_JSON_FILE_DEST) | |||
| # Wait 5 seconds for ACL rule creation | |||
| time.sleep(5) | |||
| # remove default DROP rule to make ACCEPT by default | |||
There was a problem hiding this comment.
Would it be better to change default rule action from DROP to ACCEPT in case if adding this to config and than deleting?
There was a problem hiding this comment.
not sure how to do that. The applied config file does not contain default rule, it seems to be added implicitly at the moment when we apply the config
There was a problem hiding this comment.
Thanks for the update. Actually, the default DROP rule is expected. It's added automatically in INGRESS ACL. To defy that behavior, we will add a default FORWARD rule in the pre-loaded ACL table.
https://github.com/Azure/sonic-mgmt/blob/34b3766a4259853a71bde90a94ecbed5921e758b/tests/acl/null_route/acl.json#L115-L128
There was a problem hiding this comment.
Could you please provide more details about the test failure? I can do a debug locally. I didn't see any failure from the last update. Thanks so much~
There was a problem hiding this comment.
@bingwang-ms the test fails with the first dataset
("1.2.3.4", "", FORWARD), # Should be forwared in default
It is expected to be forwarded but seems like default DROP applies and a packet is dropped.
There was a problem hiding this comment.
@bingwang-ms could you please take a look?
There was a problem hiding this comment.
@bingwang-ms could you please take a look?
Sorry for missing the comment before.
The packet with src_ip 1.2.3.4 is forwarded because we have this rule in acl.json
"9998": {
"actions": {
"config": {
"forwarding-action": "ACCEPT"
}
},
"config": {
"sequence-id": 9998
},
"ip": {
"config": {
"source-ip-address": "0.0.0.0/0",
"destination-ip-address": "0.0.0.0/0"
}
}
}
}
},
So there is no need to delete the default DROP rule since the above rule has a higher priority.
There was a problem hiding this comment.
seems you're right, the mentioned rule should match before a default drop rule. Thanks !
Description of PR
Summary: fixed test_null_route_helper: changed a default ACL rule
Fixes # (issue)
test_null_route_helper preconfigures some ACL rules before the test and then checks packets pass/drop while issuing corresponding block/unblock requests using null_route_helper. Test fails because an expected packet is not forwarded. The issue appeared to be a misconfigured default ACL action for the preconfigured table - author intended it to be ACCEPT but it is DROP...
Type of change
Back port request
Approach
What is the motivation for this PR?
Fix test issue
How did you do it?
Made a change based on info provided in the PR
How did you verify/test it?
py.test --inventory=../ansible/lab,../ansible/veos --testbed_file=../ansible/testbed.csv --module-path=../ansible/library -v -rA --topology=t0,any acl/null_route/test_null_route_helper.py
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation