Skip to content

Commit

Permalink
[drop_packets]: Skip counter check for some tests (#4132)
Browse files Browse the repository at this point in the history
What is the motivation for this PR?
Recent SAI changes on some platforms cause some dropped packets to not be added to the RX_DRP counter

How did you do it?
If the DUT used for the test has the SAI change applied (sai_adjust_acl_drop_in_rx_drop), do not check the RX_DRP counter for the following test cases:

IP packet with TTL=0
IP packet with broken header
IP packet with missing header
Instead, for these cases only look at traffic forwarding behavior to determine if the test passes (packets generated by these tests should all be dropped and not forwarded by the DUT).

How did you verify/test it?
Run the tests on platforms with the new SAI change and verify they pass

Signed-off-by: Lawrence Lee <[email protected]>
  • Loading branch information
theasianpianist authored Sep 1, 2021
1 parent f6586d6 commit 1965a7d
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 10 deletions.
37 changes: 30 additions & 7 deletions tests/drop_packets/drop_packets.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import ptf.mask as mask
import ptf.packet as packet

from tests.common.errors import RunAnsibleModuleFail
from tests.common.helpers.assertions import pytest_assert, pytest_require
from tests.common.platform.device_utils import fanout_switch_port_lookup
from tests.common.helpers.constants import DEFAULT_NAMESPACE
Expand All @@ -28,6 +29,28 @@

logger = logging.getLogger(__name__)

@pytest.fixture(scope='module')
def sai_acl_drop_adj_enabled(rand_selected_dut):
"""
Determines if the `sai_adjust_acl_drop_in_rx_drop` property is enabled
Note that this property is specific to BRCM platforms
Since this leads to an undpredictable number of packets getting
counted as RX_DRP (in certain test cases), if it is enabled we need to
skip checking the drop counters for certain test cases
"""
check_cmd = "which bcmcmd > /dev/null && bcmcmd 'config show' | grep 'sai_adjust_acl_drop_in_rx_drop=1'"
try:
rand_selected_dut.shell(check_cmd)
except RunAnsibleModuleFail:
# If the above command fails, we can assume that either
# we are not on a BRCM platform or the specified property
# is not enabled/available
return False

return True


@pytest.fixture
def fanouthost(request, duthosts, rand_one_dut_hostname, localhost):
Expand Down Expand Up @@ -586,13 +609,13 @@ def test_loopback_filter(do_test, ptfadapter, setup, tx_dut_ports, pkt_fields, p
do_test("L3", pkt, ptfadapter, ports_info, setup["neighbor_sniff_ports"], tx_dut_ports)


def test_ip_pkt_with_expired_ttl(do_test, ptfadapter, setup, tx_dut_ports, pkt_fields, ports_info):
def test_ip_pkt_with_expired_ttl(do_test, ptfadapter, setup, tx_dut_ports, pkt_fields, ports_info, sai_acl_drop_adj_enabled):
"""
@summary: Create an IP packet with TTL=0.
"""
log_pkt_params(ports_info["dut_iface"], ports_info["dst_mac"], ports_info["src_mac"], pkt_fields["ipv4_dst"],
pkt_fields["ipv4_src"])

pkt = testutils.simple_tcp_packet(
eth_dst=ports_info["dst_mac"], # DUT port
eth_src=ports_info["src_mac"], # PTF port
Expand All @@ -602,11 +625,11 @@ def test_ip_pkt_with_expired_ttl(do_test, ptfadapter, setup, tx_dut_ports, pkt_f
tcp_dport=pkt_fields["tcp_dport"],
ip_ttl=0)

do_test("L3", pkt, ptfadapter, ports_info, setup["neighbor_sniff_ports"], tx_dut_ports)
do_test("L3", pkt, ptfadapter, ports_info, setup["neighbor_sniff_ports"], tx_dut_ports, skip_counter_check=sai_acl_drop_adj_enabled)


@pytest.mark.parametrize("pkt_field, value", [("version", 1), ("chksum", 10), ("ihl", 1)])
def test_broken_ip_header(do_test, ptfadapter, setup, tx_dut_ports, pkt_fields, pkt_field, value, ports_info):
def test_broken_ip_header(do_test, ptfadapter, setup, tx_dut_ports, pkt_fields, pkt_field, value, ports_info, sai_acl_drop_adj_enabled):
"""
@summary: Create a packet with broken IP header.
"""
Expand All @@ -622,10 +645,10 @@ def test_broken_ip_header(do_test, ptfadapter, setup, tx_dut_ports, pkt_fields,
)
setattr(pkt[testutils.scapy.scapy.all.IP], pkt_field, value)

do_test("L3", pkt, ptfadapter, ports_info, setup["neighbor_sniff_ports"], tx_dut_ports)
do_test("L3", pkt, ptfadapter, ports_info, setup["neighbor_sniff_ports"], tx_dut_ports, skip_counter_check=sai_acl_drop_adj_enabled)


def test_absent_ip_header(do_test, ptfadapter, setup, tx_dut_ports, pkt_fields, ports_info):
def test_absent_ip_header(do_test, ptfadapter, setup, tx_dut_ports, pkt_fields, ports_info, sai_acl_drop_adj_enabled):
"""
@summary: Create packets with absent IP header.
"""
Expand All @@ -645,7 +668,7 @@ def test_absent_ip_header(do_test, ptfadapter, setup, tx_dut_ports, pkt_fields,
pkt.type = 0x800
pkt = pkt/tcp

do_test("L3", pkt, ptfadapter, ports_info, setup["neighbor_sniff_ports"], tx_dut_ports)
do_test("L3", pkt, ptfadapter, ports_info, setup["neighbor_sniff_ports"], tx_dut_ports, skip_counter_check=sai_acl_drop_adj_enabled)


@pytest.mark.parametrize("eth_dst", ["01:00:5e:00:01:02", "ff:ff:ff:ff:ff:ff"])
Expand Down
12 changes: 9 additions & 3 deletions tests/drop_packets/test_drop_counters.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def acl_setup(duthosts, loganalyzer):
time.sleep(ACL_COUNTERS_UPDATE_INTERVAL)


def base_verification(discard_group, pkt, ptfadapter, duthosts, asic_index, ports_info, tx_dut_ports=None):
def base_verification(discard_group, pkt, ptfadapter, duthosts, asic_index, ports_info, tx_dut_ports=None, skip_counter_check=False):
"""
Base test function for verification of L2 or L3 packet drops. Verification type depends on 'discard_group' value.
Supported 'discard_group' values: 'L2', 'L3', 'ACL', 'NO_DROPS'
Expand All @@ -148,6 +148,12 @@ def base_verification(discard_group, pkt, ptfadapter, duthosts, asic_index, port
duthost.command(CMD_PREFIX+"sonic-clear rifcounters")

send_packets(pkt, ptfadapter, ports_info["ptf_tx_port_id"], PKT_NUMBER)

# Some test cases will not increase the drop counter consistently on certain platforms
if skip_counter_check:
logger.info("Skipping counter check")
return None

if discard_group == "L2":
verify_drop_counters(duthosts, asic_index, ports_info["dut_iface"], GET_L2_COUNTERS, L2_COL_KEY, packets_count=PKT_NUMBER)
for duthost in duthosts:
Expand Down Expand Up @@ -248,7 +254,7 @@ def check_if_skip():

@pytest.fixture(scope='module')
def do_test(duthosts):
def do_counters_test(discard_group, pkt, ptfadapter, ports_info, sniff_ports, tx_dut_ports=None, comparable_pkt=None):
def do_counters_test(discard_group, pkt, ptfadapter, ports_info, sniff_ports, tx_dut_ports=None, comparable_pkt=None, skip_counter_check=False):
"""
Execute test - send packet, check that expected discard counters were incremented and packet was dropped
@param discard_group: Supported 'discard_group' values: 'L2', 'L3', 'ACL', 'NO_DROPS'
Expand All @@ -260,7 +266,7 @@ def do_counters_test(discard_group, pkt, ptfadapter, ports_info, sniff_ports, tx
"""
check_if_skip()
asic_index = ports_info["asic_index"]
base_verification(discard_group, pkt, ptfadapter, duthosts, asic_index, ports_info, tx_dut_ports)
base_verification(discard_group, pkt, ptfadapter, duthosts, asic_index, ports_info, tx_dut_ports, skip_counter_check=skip_counter_check)

# Verify packets were not egresed the DUT
if discard_group != "NO_DROPS":
Expand Down

0 comments on commit 1965a7d

Please sign in to comment.