From 4163834250c514f68c15752d0a557b441bc5a3b5 Mon Sep 17 00:00:00 2001 From: Danny Allen Date: Wed, 6 May 2020 11:06:50 -0700 Subject: [PATCH] [pytest] Fix ptf_runner issue introduced by COPP test conversion (#1644) * [copp] Convert COPP test to pytest (#1633) * Fix missing space in ptf_runner Signed-off-by: Danny Allen --- tests/common/devices.py | 10 ++ tests/copp/__init__.py | 0 tests/copp/conftest.py | 31 ++++ tests/copp/copp_utils.py | 118 +++++++++++++ tests/copp/scripts/update_copp_config.py | 53 ++++++ tests/copp/test_copp.py | 201 +++++++++++++++++++++++ tests/ptf_runner.py | 26 ++- tests/scripts/add_ip.sh | 8 + tests/scripts/remove_ip.sh | 11 +- tests/templates/ptf_nn_agent.conf.dut.j2 | 10 ++ tests/templates/ptf_nn_agent.conf.ptf.j2 | 10 ++ 11 files changed, 469 insertions(+), 9 deletions(-) create mode 100644 tests/copp/__init__.py create mode 100644 tests/copp/conftest.py create mode 100644 tests/copp/copp_utils.py create mode 100644 tests/copp/scripts/update_copp_config.py create mode 100644 tests/copp/test_copp.py create mode 100755 tests/scripts/add_ip.sh mode change 120000 => 100755 tests/scripts/remove_ip.sh create mode 100644 tests/templates/ptf_nn_agent.conf.dut.j2 create mode 100644 tests/templates/ptf_nn_agent.conf.ptf.j2 diff --git a/tests/common/devices.py b/tests/common/devices.py index 4c67738c7d..c0dbc7fa2f 100644 --- a/tests/common/devices.py +++ b/tests/common/devices.py @@ -544,6 +544,16 @@ def check_bgp_session_nsf(self, neighbor_ip): return True return False + def get_version(self): + """ + Gets the SONiC version this device is running. + + Returns: + str: the firmware version number (e.g. 20181130.31) + """ + output = dut.command("sonic-cfggen -y /etc/sonic/sonic_version.yml -v build_version") + return output["stdout_lines"][0].strip() + class EosHost(AnsibleHostBase): """ @summary: Class for Eos switch diff --git a/tests/copp/__init__.py b/tests/copp/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/copp/conftest.py b/tests/copp/conftest.py new file mode 100644 index 0000000000..57a746bdc1 --- /dev/null +++ b/tests/copp/conftest.py @@ -0,0 +1,31 @@ +""" + Pytest configuration used by the COPP tests. +""" + +def pytest_addoption(parser): + """ + Adds options to pytest that are used by the COPP tests. + """ + + parser.addoption( + "--nn_target_port", + action="store", + type=int, + default=3, + help="Which port to send traffic to", + ) + + parser.addoption( + "--pkt_tx_count", + action="store", + type=int, + default=100000, + help="How many packets to send to the DUT" + ) + + parser.addoption( + "--swap_syncd", + action="store_true", + default=False, + help="Install syncd RPC image for this test case" + ) diff --git a/tests/copp/copp_utils.py b/tests/copp/copp_utils.py new file mode 100644 index 0000000000..a0723f081f --- /dev/null +++ b/tests/copp/copp_utils.py @@ -0,0 +1,118 @@ +""" + Helpful utilities for writing tests for the COPP feature. + + Todo: + Refactor ptfadapter so it can be leveraged in these test cases. +""" + +DEFAULT_NN_TARGET_PORT = 3 + +_REMOVE_IP_SCRIPT = "scripts/remove_ip.sh" +_ADD_IP_SCRIPT = "scripts/add_ip.sh" +_UPDATE_COPP_SCRIPT = "copp/scripts/update_copp_config.py" + +_BASE_COPP_CONFIG = "/tmp/00-copp.config.json" +_SWSS_COPP_CONFIG = "swss:/etc/swss/config.d/00-copp.config.json" +_TEMP_COPP_CONFIG = "/tmp/copp_config.json" + +_PTF_NN_TEMPLATE = "templates/ptf_nn_agent.conf.ptf.j2" +_PTF_NN_DEST = "/etc/supervisor/conf.d/ptf_nn_agent.conf" + +_SYNCD_NN_TEMPLATE = "templates/ptf_nn_agent.conf.dut.j2" +_SYNCD_NN_DEST = "/tmp/ptf_nn_agent.conf" + +def limit_policer(dut, pps_limit): + """ + Updates the COPP configuration in the SWSS container to respect a given rate limit. + + Note: + The SWSS container must be restarted for the config change to take effect. + + Args: + dut (SonicHost): The target device. + pps_limit (int): The rate limit for COPP to enforce on ALL trap groups. + """ + + dut.command("docker cp {} {}".format(_SWSS_COPP_CONFIG, _BASE_COPP_CONFIG)) + dut.script(cmd="{} {} {} {}".format(_UPDATE_COPP_SCRIPT, pps_limit, + _BASE_COPP_CONFIG, _TEMP_COPP_CONFIG)) + dut.command("docker cp {} {}".format(_TEMP_COPP_CONFIG, _SWSS_COPP_CONFIG)) + +def restore_policer(dut): + """ + Reloads the default COPP configuration in the SWSS container. + + Notes: + This method should only be used after limit_policer. + + The SWSS container must be restarted for the config change to take effect. + """ + dut.command("docker cp {} {}".format(_BASE_COPP_CONFIG, _SWSS_COPP_CONFIG)) + +def configure_ptf(ptf, nn_target_port): + """ + Configures the PTF to run the NN agent on the specified port. + + Args: + ptf (PTFHost): The target PTF. + nn_target_port (int): The port to run NN agent on. + """ + + ptf.script(cmd=_REMOVE_IP_SCRIPT) + ptf.script(cmd=_ADD_IP_SCRIPT) + + facts = {"nn_target_port": nn_target_port} + ptf.host.options['variable_manager'].extra_vars.update(facts) + ptf.template(src=_PTF_NN_TEMPLATE, dest=_PTF_NN_DEST) + + ptf.supervisorctl(name="ptf_nn_agent", state="restarted") + + ptf.copy(src="ptftests", dest="/root") + +def restore_ptf(ptf): + """ + Restores the PTF and the NN agent to default settings. + + Args: + ptf (PTFHost): The target PTF. + """ + + ptf.script(cmd=_REMOVE_IP_SCRIPT) + + facts = {"nn_target_port": DEFAULT_NN_TARGET_PORT} + ptf.host.options['variable_manager'].extra_vars.update(facts) + + ptf.template(src=_PTF_NN_TEMPLATE, dest=_PTF_NN_DEST) + + ptf.supervisorctl(name="ptf_nn_agent", state="restarted") + +def configure_syncd(dut, nn_target_port): + """ + Configures syncd to run the NN agent on the specified port. + + Note: + The DUT must be running an RPC syncd image in order for the + NN agent to be available. + + Args: + dut (SonicHost): The target device. + nn_target_port (int): The port to run NN agent on. + """ + + facts = {"nn_target_port": nn_target_port, + "nn_target_interface": _map_port_number_to_interface(dut, nn_target_port)} + dut.host.options['variable_manager'].extra_vars.update(facts) + + dut.template(src=_SYNCD_NN_TEMPLATE, dest=_SYNCD_NN_DEST) + dut.command("docker cp {} syncd:/etc/supervisor/conf.d/".format(_SYNCD_NN_DEST)) + + dut.command("docker exec syncd supervisorctl reread") + dut.command("docker exec syncd supervisorctl update") + +def _map_port_number_to_interface(dut, nn_target_port): + """ + Retrieves the correct interface for a given port number. + """ + + interfaces = dut.command("portstat")["stdout_lines"][2:] + return interfaces[nn_target_port].split()[0] diff --git a/tests/copp/scripts/update_copp_config.py b/tests/copp/scripts/update_copp_config.py new file mode 100644 index 0000000000..007ac05198 --- /dev/null +++ b/tests/copp/scripts/update_copp_config.py @@ -0,0 +1,53 @@ +#!/usr/bin/python +""" + Script to modify a COPP configuration to follow a specified rate limit. + + This is used by the COPP tests to reduce the rate limit below that of the + PTF host's sending rate. + + Example:: + + $ python update_copp_config.py + $ python update_copp_config.py 600 /tmp/copp_config.json /tmp/new_copp_config.json +""" +import json +import sys + +def generate_limited_pps_config(pps_limit, input_config_file, output_config_file): + """ + Modifies a COPP config to use the specified rate limit. + + Notes: + This only affects COPP policies that enforce a rate limit. Other + policies are left alone. + + Args: + pps_limit (int): The rate limit to enforce expressed in PPS (packets-per-second) + input_config_file (str): The name of the file containing the initial config + output_config_file (str): The name of the file to output the modified config + """ + + with open(input_config_file) as input_stream: + copp_config = json.load(input_stream) + + for trap_group in copp_config: + for _, group_config in trap_group.items(): + # Notes: + # CIR (committed information rate) - bandwidth limit set by the policer + # CBS (committed burst size) - largest burst of packets allowed by the policer + # + # Setting these two values to pps_limit restricts the policer to allowing exactly + # that number of packets per second, which is what we want for our tests. + + if "cir" in group_config: + group_config["cir"] = pps_limit + + if "cbs" in group_config: + group_config["cbs"] = pps_limit + + with open(output_config_file, "w+") as output_stream: + json.dump(copp_config, output_stream) + +if __name__ == "__main__": + ARGS = sys.argv[1:] + generate_limited_pps_config(ARGS[0], ARGS[1], ARGS[2]) diff --git a/tests/copp/test_copp.py b/tests/copp/test_copp.py new file mode 100644 index 0000000000..186201f690 --- /dev/null +++ b/tests/copp/test_copp.py @@ -0,0 +1,201 @@ +""" + Tests the COPP feature in SONiC. + + Notes: + These test cases require that a special RPC syncd image is installed on the + DUT. You can either pre-install this image and run the test normally, or + specify the `--swap-syncd` flag from the command line to have the test fetch + the RPC image and install it before the test runs. + + These test cases limit the PPS of all trap groups to 600. This is done to ensure + that the PTF can send traffic fast enough to trigger the policer. In order to validate + higher rate limits, a physical traffic generator is needed, which is beyond the scope + of these test cases. + + Parameters: + --nn_target_port (int): Which port you want the test to send traffic + to. Default is 3. + + Note that this is not the same as the interface name. For example, Ethernet12 + may not be the 12th port in your system depending on the HWSKU under test. + + --pkt_tx_count (int): How many packets to send during each individual test case. + Default is 100000. + + --swap_syncd: Used to install the RPC syncd image before running the tests. Default + is disabled. +""" + +import time +from collections import namedtuple + +import pytest +from copp import copp_utils +from ptf_runner import ptf_runner +from common.system_utils import docker +from common.broadcom_data import is_broadcom_device + +_COPPTestParameters = namedtuple("_COPPTestParameters", + ["nn_target_port", + "pkt_tx_count", + "swap_syncd", + "topo"]) +_SUPPORTED_TOPOS = ["ptf32", "ptf64", "t1", "t1-lag"] +_TEST_RATE_LIMIT = 600 + +class TestCOPP(object): + """ + Tests basic COPP functionality in SONiC. + """ + + @pytest.mark.parametrize("protocol", ["ARP", + "IP2ME", + "SNMP", + "SSH"]) + def test_policer(self, protocol, duthost, ptfhost, _copp_testbed): + """ + Validates that rate-limited COPP groups work as expected. + + Checks that the policer enforces the rate limit for protocols + that have a set rate limit. + """ + + if protocol == "ARP" \ + and is_broadcom_device(duthost) \ + and "201811" not in duthost.get_version(): + pytest.xfail("ARP policy disabled on BRCM devices due to SAI bug") + + if protocol in ["IP2ME", "SNMP", "SSH"] and _copp_testbed.topo == "t1-lag": + pytest.xfail("Packets not received due to faulty DIP, see #1171") + + _copp_runner(duthost, + ptfhost, + protocol, + _copp_testbed) + + @pytest.mark.parametrize("protocol", ["BGP", + "DHCP", + "LACP", + "LLDP", + "UDLD"]) + def test_no_policer(self, protocol, duthost, ptfhost, _copp_testbed): + """ + Validates that non-rate-limited COPP groups work as expected. + + Checks that the policer does not enforce a rate limit for protocols + that do not have any set rate limit. + """ + + if protocol == "BGP" and _copp_testbed.topo == "t1-lag": + pytest.xfail("Packets not received due to faulty DIP, see #1171") + + _copp_runner(duthost, + ptfhost, + protocol, + _copp_testbed) + +@pytest.fixture(scope="class") +def _copp_testbed(duthost, ptfhost, testbed, request): + """ + Pytest fixture to handle setup and cleanup for the COPP tests. + """ + + test_params = _gather_test_params(testbed, request) + + if test_params.topo not in _SUPPORTED_TOPOS: + pytest.skip("Topology not supported by COPP tests") + + _setup_testbed(duthost, ptfhost, test_params) + yield test_params + _teardown_testbed(duthost, ptfhost, test_params) + +def _copp_runner(dut, ptf, protocol, test_params): + """ + Configures and runs the PTF test cases. + """ + + params = {"verbose": False, + "pkt_tx_count": test_params.pkt_tx_count, + "target_port": test_params.nn_target_port} + + dut_ip = dut.setup()["ansible_facts"]["ansible_eth0"]["ipv4"]["address"] + device_sockets = ["0-{}@tcp://127.0.0.1:10900".format(test_params.nn_target_port), + "1-{}@tcp://{}:10900".format(test_params.nn_target_port, dut_ip)] + + # NOTE: debug_level can actually slow the PTF down enough to fail the test cases + # that are not rate limited. Until this is addressed, do not use this flag as part of + # nightly test runs. + ptf_runner(host=ptf, + testdir="ptftests", + testname="copp_tests.{}Test".format(protocol), + platform="nn", + qlen=100000, + params=params, + relax=None, + debug_level=None, + device_sockets=device_sockets) + +def _gather_test_params(testbed, request): + """ + Fetches the test parameters from pytest. + """ + + nn_target_port = request.config.getoption("--nn_target_port") + pkt_tx_count = request.config.getoption("--pkt_tx_count") + swap_syncd = request.config.getoption("--swap_syncd") + topo = testbed["topo"]["name"] + + return _COPPTestParameters(nn_target_port=nn_target_port, + pkt_tx_count=pkt_tx_count, + swap_syncd=swap_syncd, + topo=topo) + +def _setup_testbed(dut, ptf, test_params): + """ + Sets up the testbed to run the COPP tests. + """ + + # We don't want LLDP to throw off our test results, so we disable it first. + dut.command("docker exec lldp supervisorctl stop lldp-syncd") + dut.command("docker exec lldp supervisorctl stop lldpd") + + copp_utils.configure_ptf(ptf, test_params.nn_target_port) + + copp_utils.limit_policer(dut, _TEST_RATE_LIMIT) + + if test_params.swap_syncd: + docker.swap_syncd(dut) + else: + # NOTE: Even if the rpc syncd image is already installed, we need to restart + # SWSS for the COPP changes to take effect. + _restart_swss(dut) + + copp_utils.configure_syncd(dut, test_params.nn_target_port) + +def _teardown_testbed(dut, ptf, test_params): + """ + Tears down the testbed, returning it to its initial state. + """ + + copp_utils.restore_ptf(ptf) + + copp_utils.restore_policer(dut) + + if test_params.swap_syncd: + docker.restore_default_syncd(dut) + else: + _restart_swss(dut) + + dut.command("docker exec lldp supervisorctl start lldpd") + dut.command("docker exec lldp supervisorctl start lldp-syncd") + +def _restart_swss(dut): + """ + Restarts SWSS and waits for the system to stabilize. + """ + + # The failure counter may be incremented by other test cases, so we clear it + # first to avoid crashing the testbed. + dut.command("systemctl reset-failed swss") + dut.command("systemctl restart swss") + time.sleep(60) diff --git a/tests/ptf_runner.py b/tests/ptf_runner.py index b1931f296d..9ca0333364 100644 --- a/tests/ptf_runner.py +++ b/tests/ptf_runner.py @@ -1,27 +1,37 @@ import pipes -def ptf_runner(host, testdir, testname, platform_dir, params={}, \ - platform="remote", qlen=0, relax=True, debug_level="info", \ - socket_recv_size=None, log_file=None): +def ptf_runner(host, testdir, testname, platform_dir=None, params={}, + platform="remote", qlen=0, relax=True, debug_level="info", + socket_recv_size=None, log_file=None, device_sockets=[]): - ptf_test_params = ";".join(["{}={}".format(k, repr(v)) for k, v in params.items()]) + cmd = "ptf --test-dir {} {}".format(testdir, testname) + + if platform_dir: + cmd += " --platform-dir {}".format(platform_dir) - cmd = "ptf --test-dir {} {} --platform-dir {}".format(testdir, testname, platform_dir) if qlen: cmd += " --qlen={}".format(qlen) + if platform: cmd += " --platform {}".format(platform) - if ptf_test_params: + + if params: + ptf_test_params = ";".join(["{}={}".format(k, repr(v)) for k, v in params.items()]) cmd += " -t {}".format(pipes.quote(ptf_test_params)) + if relax: cmd += " --relax" + if debug_level: cmd += " --debug {}".format(debug_level) + if log_file: cmd += " --log-file {}".format(log_file) + if socket_recv_size: cmd += " --socket-recv-size {}".format(socket_recv_size) - res = host.shell(cmd, chdir="/root") - + if device_sockets: + cmd += " ".join(map(" --device-socket {}".format, device_sockets)) + host.shell(cmd, chdir="/root") diff --git a/tests/scripts/add_ip.sh b/tests/scripts/add_ip.sh new file mode 100755 index 0000000000..e2fc433ada --- /dev/null +++ b/tests/scripts/add_ip.sh @@ -0,0 +1,8 @@ +#!/bin/bash + +set -e + +for i in `cat /proc/net/dev | grep eth | awk -F'eth|:' '{print $2}'`; do + last_el=$((1+i*2)) + ip address add 10.0.0.$last_el/31 dev eth$i +done diff --git a/tests/scripts/remove_ip.sh b/tests/scripts/remove_ip.sh deleted file mode 120000 index a03b98f404..0000000000 --- a/tests/scripts/remove_ip.sh +++ /dev/null @@ -1 +0,0 @@ -../../ansible/roles/test/files/helpers/remove_ip.sh \ No newline at end of file diff --git a/tests/scripts/remove_ip.sh b/tests/scripts/remove_ip.sh new file mode 100755 index 0000000000..abed39e4dc --- /dev/null +++ b/tests/scripts/remove_ip.sh @@ -0,0 +1,10 @@ +#!/bin/bash + +set -euo pipefail + +INTF_LIST=$(ls /sys/class/net | grep -E "^eth[0-9]+$") + +for INTF in ${INTF_LIST}; do + echo "Flush ${INTF} IP address" + ip addr flush dev ${INTF} +done diff --git a/tests/templates/ptf_nn_agent.conf.dut.j2 b/tests/templates/ptf_nn_agent.conf.dut.j2 new file mode 100644 index 0000000000..7e327fde45 --- /dev/null +++ b/tests/templates/ptf_nn_agent.conf.dut.j2 @@ -0,0 +1,10 @@ +[program:ptf_nn_agent] +command=/usr/bin/python /opt/ptf_nn_agent.py --device-socket 1@tcp://0.0.0.0:10900 -i 1-{{ nn_target_port }}@{{ nn_target_interface }} --set-nn-rcv-buffer=109430400 --set-iface-rcv-buffer=109430400 --set-nn-snd-buffer=109430400 --set-iface-snd-buffer=109430400 +process_name=ptf_nn_agent +stdout_logfile=/tmp/ptf_nn_agent.out.log +stderr_logfile=/tmp/ptf_nn_agent.err.log +redirect_stderr=false +autostart=true +autorestart=true +startsecs=1 +numprocs=1 diff --git a/tests/templates/ptf_nn_agent.conf.ptf.j2 b/tests/templates/ptf_nn_agent.conf.ptf.j2 new file mode 100644 index 0000000000..bb1282bc4a --- /dev/null +++ b/tests/templates/ptf_nn_agent.conf.ptf.j2 @@ -0,0 +1,10 @@ +[program:ptf_nn_agent] +command=/usr/bin/python /opt/ptf_nn_agent.py --device-socket 0@tcp://127.0.0.1:10900 -i 0-{{ nn_target_port }}@eth{{ nn_target_port }} +process_name=ptf_nn_agent +stdout_logfile=/tmp/ptf_nn_agent.out.log +stderr_logfile=/tmp/ptf_nn_agent.err.log +redirect_stderr=false +autostart=true +autorestart=true +startsecs=1 +numprocs=1