Skip to content

Commit

Permalink
[acl] Fix some issues in acl test (#5115)
Browse files Browse the repository at this point in the history 
* [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 <[email protected]>

* Wrap logic to get_dst_ports() function

Signed-off-by: Kevin(Shengkai) Wang <[email protected]>
  • Loading branch information
@kevinskwang
kevinskwang authored Feb 11, 2022
1 parent 377a9e5 commit adca5c7
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 19 deletions.
4 changes: 2 additions & 2 deletions tests/acl/templates/acltb_test_rules.j2
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
},
"ip": {
"config": {
"destination-ip-address": "192.168.0.4/32"
"destination-ip-address": "192.168.0.252/32"
}
}
},
Expand Down Expand Up @@ -228,7 +228,7 @@
},
"ip": {
"config": {
"destination-ip-address": "192.168.0.8/32"
"destination-ip-address": "192.168.0.251/32"
}
}
},
Expand Down
4 changes: 2 additions & 2 deletions tests/acl/templates/acltb_test_rules_part_2.j2
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
},
"ip": {
"config": {
"destination-ip-address": "192.168.0.4/32"
"destination-ip-address": "192.168.0.252/32"
}
}
},
Expand Down Expand Up @@ -228,7 +228,7 @@
},
"ip": {
"config": {
"destination-ip-address": "192.168.0.8/32"
"destination-ip-address": "192.168.0.251/32"
}
}
},
Expand Down
47 changes: 32 additions & 15 deletions tests/acl/test_acl.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand All @@ -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()

Expand Down Expand Up @@ -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."""
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit adca5c7

Please sign in to comment.