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

test_acl.py checks for wrong counters and also misreports errors #5576

Open
rck-innovium opened this issue Apr 28, 2022 · 0 comments
Open

test_acl.py checks for wrong counters and also misreports errors #5576

rck-innovium opened this issue Apr 28, 2022 · 0 comments

Comments

@rck-innovium
Copy link

Description
#1 Some subtests check for the wrong ACL Rule counters

For example: subtest test_acl.py::test_tcp_flags_match_dropped() checks for ACL counter 5.
However, the rule for matching TCP flags and dropping the packet is rule#19.
https://github.com/Azure/sonic-mgmt/blob/c41fff2017a05107ae20375537a51cccc4f55dca/tests/acl/templates/acltb_test_rules.j2#L280

    def test_tcp_flags_match_dropped(self, setup, direction, ptfadapter, counters_sanity_check, ip_version):
        """Verify that we can match and drop on the TCP flags."""
        pkt = self.tcp_packet(setup, direction, ptfadapter, ip_version, flags=0x24)

        self._verify_acl_traffic(setup, direction, ptfadapter, pkt, True, ip_version)
-        counters_sanity_check.append(5)
+        counters_sanity_check.append(19)

Similarly test_l4_dport_match_forwarded checks for Rule 5.

These testcases happen to pass if subtest test_ip_proto_match_forwarded is executed and updates the counters for Rule 5. The testcases do not check the correct ACL counters.

#2 A more serious debuggability problem with the script is its error reporting

The script reports whether a testcase passes or fails by checking the expected traffic forwarding behaviour.
The actual check for the ACL rule counters happens as part of test teardown. Now, say if the rule counters of subtest#1 fails, the script reports the error for the last subtest!!
For example, if rule#1 (test_source_ip_match_forwarded) fails, the script reports an error in test_tcp_flags_match_dropped.

------------------------------ live log teardown -------------------------------
05:45:52 INFO test_acl.py:counters_sanity_check:477: Counters for ACL rule "RULE_1" before traffic:
{u'PACKET_ACTION': u'FORWARD',
u'PRIORITY': u'9999',
u'SRC_IP': u'20.0.0.2/32',
u'bytes_count': 0,
u'packets_count': 0}
05:45:52 INFO test_acl.py:counters_sanity_check:481: Counters for ACL rule "**RULE_1**" after traffic:
{u'PACKET_ACTION': u'FORWARD',
u'PRIORITY': u'9999',
u'SRC_IP': u'20.0.0.2/32',
u'bytes_count': 0,
u'packets_count': 0}
05:45:53 INFO test_acl.py:acl_rules:435: Removing ACL rules
05:45:53 INFO test_acl.py:teardown_rules:388: Finished with tests, removing all ACL rules...
05:45:54 INFO test_acl.py:teardown_rules:395: Applying "acl_test_dir/acl_rules_del.json"
 
acl/test_acl.py::TestBasicAcl::**test_tcp_flags_match_dropped[**ipv4-egress-uplink->downlink] **ERROR** [ 31%]

Steps to reproduce the issue:

  1. Run pytests test_acl.py in T0 or T1 topology
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant