From adca5c78191827ca5b27918f206580138c3e9d72 Mon Sep 17 00:00:00 2001 From: Kevin Wang <65380078+kevinskwang@users.noreply.github.com> Date: Fri, 11 Feb 2022 09:44:10 +0800 Subject: [PATCH] [acl] Fix some issues in acl test (#5115) * [acl] Fix some issues in acl test What is the motivation for this PR? 1.Current the preset downstream dest IP "192.168.0.2", "192.168.0.4" and "192.168.0.8" are duplicated in Garp service. It will trigger ARP flapping during the test if garp and arp work together. 2.This case will send ping pkts to set up the arp table at the beginning of the case. But on dual-tor system, the ping pkts will be dropped if we randomly pick up the standby tor. 3.As we already know the dest port for downstream traffic, so we just need to poll that dedicated interface rather than poll all interfaces, to make the test more efficient and accurate. How did you do it? 1.Use "192.168.0.253", "192.168.0.252" and "192.168.0.251" as the downstream dest IP to avoid IP confliction with garp service. 2.Send the ping pkts from both tors. 3.Use verify_packet() rather than verify_packet_any_port() How did you verify/test it? Run the test_acl.py Signed-off-by: Kevin(Shengkai) Wang * Wrap logic to get_dst_ports() function Signed-off-by: Kevin(Shengkai) Wang --- tests/acl/templates/acltb_test_rules.j2 | 4 +- .../acl/templates/acltb_test_rules_part_2.j2 | 4 +- tests/acl/test_acl.py | 47 +++++++++++++------ 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/tests/acl/templates/acltb_test_rules.j2 b/tests/acl/templates/acltb_test_rules.j2 index d0f11dff0cd..cc6435bcf41 100644 --- a/tests/acl/templates/acltb_test_rules.j2 +++ b/tests/acl/templates/acltb_test_rules.j2 @@ -31,7 +31,7 @@ }, "ip": { "config": { - "destination-ip-address": "192.168.0.4/32" + "destination-ip-address": "192.168.0.252/32" } } }, @@ -228,7 +228,7 @@ }, "ip": { "config": { - "destination-ip-address": "192.168.0.8/32" + "destination-ip-address": "192.168.0.251/32" } } }, diff --git a/tests/acl/templates/acltb_test_rules_part_2.j2 b/tests/acl/templates/acltb_test_rules_part_2.j2 index d0f11dff0cd..cc6435bcf41 100644 --- a/tests/acl/templates/acltb_test_rules_part_2.j2 +++ b/tests/acl/templates/acltb_test_rules_part_2.j2 @@ -31,7 +31,7 @@ }, "ip": { "config": { - "destination-ip-address": "192.168.0.4/32" + "destination-ip-address": "192.168.0.252/32" } } }, @@ -228,7 +228,7 @@ }, "ip": { "config": { - "destination-ip-address": "192.168.0.8/32" + "destination-ip-address": "192.168.0.251/32" } } }, diff --git a/tests/acl/test_acl.py b/tests/acl/test_acl.py index 5261ee13448..177c937e279 100644 --- a/tests/acl/test_acl.py +++ b/tests/acl/test_acl.py @@ -21,6 +21,7 @@ from tests.common.fixtures.ptfhost_utils import copy_arp_responder_py, run_garp_service, change_mac_addresses from tests.common.utilities import wait_until from tests.common.dualtor.dual_tor_mock import mock_server_base_ip_addr +from tests.common.helpers.assertions import pytest_assert logger = logging.getLogger(__name__) @@ -58,18 +59,20 @@ # TODO: These routes don't match the VLAN interface from the T0 topology. # This needs to be addressed before we can enable the v6 tests for T0 DOWNSTREAM_DST_IP = { - "ipv4": "192.168.0.2", + "ipv4": "192.168.0.253", "ipv6": "20c0:a800::2" } DOWNSTREAM_IP_TO_ALLOW = { - "ipv4": "192.168.0.4", + "ipv4": "192.168.0.252", "ipv6": "20c0:a800::4" } DOWNSTREAM_IP_TO_BLOCK = { - "ipv4": "192.168.0.8", + "ipv4": "192.168.0.251", "ipv6": "20c0:a800::8" } +DOWNSTREAM_IP_PORT_MAP = {} + UPSTREAM_DST_IP = { "ipv4": "192.168.128.1", "ipv6": "40c0:a800::2" @@ -244,6 +247,7 @@ def noop(): port = random.choice(setup["vlan_ports"]) addr = addr_list[i] vlan_host_map[port][str(addr)] = mac + DOWNSTREAM_IP_PORT_MAP[addr] = port arp_responder_conf = {} for port in vlan_host_map: @@ -260,12 +264,13 @@ def noop(): ptfhost.shell("supervisorctl restart arp_responder") def populate_arp_table(): - duthost.command("sonic-clear fdb all") - duthost.command("sonic-clear arp") - # Wait some time to ensure the async call of clear is completed - time.sleep(20) - for addr in addr_list: - duthost.command("ping {} -c 3".format(addr), module_ignore_errors=True) + for dut in duthosts: + dut.command("sonic-clear fdb all") + dut.command("sonic-clear arp") + # Wait some time to ensure the async call of clear is completed + time.sleep(20) + for addr in addr_list: + dut.command("ping {} -c 3".format(addr), module_ignore_errors=True) populate_arp_table() @@ -554,9 +559,15 @@ def get_src_port(self, setup, direction): logger.info("Selected source port {}".format(src_port)) self.src_port = src_port - def get_dst_ports(self, setup, direction): + def get_dst_ports(self, setup, direction, dst_ip=None): """Get the set of possible destination ports for the current test.""" - return setup["upstream_port_ids"] if direction == "downlink->uplink" else setup["downstream_port_ids"] + if direction == "downlink->uplink": + return setup["upstream_port_ids"] + else: + dst_port = DOWNSTREAM_IP_PORT_MAP.get(dst_ip) + pytest_assert(dst_port is not None, + "Can't find dst port for IP {}".format(dst_ip)) + return dst_port def get_dst_ip(self, direction, ip_version): """Get the default destination IP for the current test.""" @@ -860,11 +871,17 @@ def _verify_acl_traffic(self, setup, direction, ptfadapter, pkt, dropped, ip_ver ptfadapter.dataplane.flush() testutils.send(ptfadapter, self.src_port, pkt) - - if dropped: - testutils.verify_no_packet_any(ptfadapter, exp_pkt, ports=self.get_dst_ports(setup, direction)) + if direction == "uplink->downlink": + if dropped: + testutils.verify_no_packet(ptfadapter, exp_pkt, self.get_dst_ports(setup, direction, pkt[packet.IP].dst)) + else: + testutils.verify_packet(ptfadapter, exp_pkt, self.get_dst_ports(setup, direction, pkt[packet.IP].dst)) else: - testutils.verify_packet_any_port(ptfadapter, exp_pkt, ports=self.get_dst_ports(setup, direction), timeout=20) + if dropped: + testutils.verify_no_packet_any(ptfadapter, exp_pkt, ports=self.get_dst_ports(setup, direction)) + else: + testutils.verify_packet_any_port(ptfadapter, exp_pkt, ports=self.get_dst_ports(setup, direction), + timeout=20) class TestBasicAcl(BaseAclTest):