From 27eb772610e5ed52deed8e57bbeac09088d16243 Mon Sep 17 00:00:00 2001 From: bingwang Date: Thu, 12 Nov 2020 01:48:48 -0800 Subject: [PATCH 1/5] Add new test case test_bgpmon Signed-off-by: bingwang --- tests/bgp/bgpmon.j2 | 15 +++++++ tests/bgp/test_bgpmon.py | 95 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 tests/bgp/bgpmon.j2 create mode 100644 tests/bgp/test_bgpmon.py diff --git a/tests/bgp/bgpmon.j2 b/tests/bgp/bgpmon.j2 new file mode 100644 index 00000000000..0bd74768bda --- /dev/null +++ b/tests/bgp/bgpmon.j2 @@ -0,0 +1,15 @@ +{ + "BGP_MONITORS": { + "{{ peer_addr }}": { + "admin_status": "up", + "asn": "{{ asn }}", + "holdtime": "10", + "keepalive": "3", + "local_addr": "{{ local_addr }}", + "name": "{{ peer_name }}", + "nhopself": "0", + "rrclient": "0" + } + } +} + diff --git a/tests/bgp/test_bgpmon.py b/tests/bgp/test_bgpmon.py new file mode 100644 index 00000000000..f971bcffb9a --- /dev/null +++ b/tests/bgp/test_bgpmon.py @@ -0,0 +1,95 @@ +import pytest +import logging +from netaddr import IPAddress, IPNetwork +import ptf.testutils as testutils +from jinja2 import Template +import ptf.packet as scapy +from ptf.mask import Mask +from tests.common.fixtures.ptfhost_utils import change_mac_addresses # lgtm[py/unused-import] +from tests.common.fixtures.ptfhost_utils import remove_ip_addresses # lgtm[py/unused-import] +from tests.common.helpers.generators import generate_ips as generate_ips + +pytestmark = [ + pytest.mark.topology('any'), +] + +BGPMON_TEMPLATE_FILE = 'bgp/bgpmon.j2' +BGPMON_CONFIG_FILE = '/tmp/bgpmon.json' +BGP_PORT = 179 +BGP_CONNECT_TIMEOUT = 120 + +logger = logging.getLogger(__name__) + +@pytest.fixture +def common_setup_teardown(duthost, ptfhost): + mg_facts = duthost.minigraph_facts(host=duthost.hostname)['ansible_facts'] + peer_addr = generate_ips(1, "%s/%s" % (mg_facts['minigraph_vlan_interfaces'][0]['addr'], + mg_facts['minigraph_vlan_interfaces'][0]['prefixlen']), + [IPAddress(mg_facts['minigraph_vlan_interfaces'][0]['addr'])])[0] + peer_addr = str(IPNetwork(peer_addr).ip) + peer_port = mg_facts['minigraph_port_indices'][mg_facts['minigraph_vlans'][mg_facts['minigraph_vlan_interfaces'][0]['attachto']]['members'][0]] + local_addr = mg_facts['minigraph_lo_interfaces'][0]['addr'] + # Assign peer addr to an interface on ptf + ptfhost.shell("ifconfig eth{} {}".format(peer_port, peer_addr)) + logger.info("Generated peer address {} and assigned to eth{} on ptf".format(peer_addr, peer_port)) + bgpmon_args = { + 'peer_addr': peer_addr, + 'asn': mg_facts['minigraph_bgp_asn'], + 'local_addr': local_addr, + 'peer_name': 'bgp_monitor' + } + bgpmon_template = Template(open(BGPMON_TEMPLATE_FILE).read()) + duthost.copy(content=bgpmon_template.render(**bgpmon_args), + dest=BGPMON_CONFIG_FILE) + yield local_addr, peer_addr, peer_port + # Cleanup bgp monitor + duthost.shell("redis-cli -n 4 -c DEL 'BGP_MONITORS|{}'".format(peer_addr)) + duthost.file(path=BGPMON_CONFIG_FILE, state='absent') + +def build_syn_pkt(local_addr, peer_addr, peer_port): + pkt = testutils.simple_tcp_packet( + pktlen=54, + ip_src=local_addr, + ip_dst=peer_addr, + tcp_dport=BGP_PORT, + tcp_flags="S" + ) + exp_packet = Mask(pkt) + exp_packet.set_do_not_care_scapy(scapy.Ether, "dst") + exp_packet.set_do_not_care_scapy(scapy.Ether, "src") + + exp_packet.set_do_not_care_scapy(scapy.IP, "version") + exp_packet.set_do_not_care_scapy(scapy.IP, "ihl") + exp_packet.set_do_not_care_scapy(scapy.IP, "tos") + exp_packet.set_do_not_care_scapy(scapy.IP, "len") + exp_packet.set_do_not_care_scapy(scapy.IP, "flags") + exp_packet.set_do_not_care_scapy(scapy.IP, "id") + exp_packet.set_do_not_care_scapy(scapy.IP, "frag") + exp_packet.set_do_not_care_scapy(scapy.IP, "ttl") + exp_packet.set_do_not_care_scapy(scapy.IP, "chksum") + exp_packet.set_do_not_care_scapy(scapy.IP, "options") + + exp_packet.set_do_not_care_scapy(scapy.TCP, "sport") + exp_packet.set_do_not_care_scapy(scapy.TCP, "seq") + exp_packet.set_do_not_care_scapy(scapy.TCP, "ack") + exp_packet.set_do_not_care_scapy(scapy.TCP, "reserved") + exp_packet.set_do_not_care_scapy(scapy.TCP, "dataofs") + exp_packet.set_do_not_care_scapy(scapy.TCP, "window") + exp_packet.set_do_not_care_scapy(scapy.TCP, "chksum") + exp_packet.set_do_not_care_scapy(scapy.TCP, "urgptr") + + exp_packet.set_ignore_extra_bytes() + return exp_packet + +def test_bgpmon(duthost, common_setup_teardown, ptfadapter): + """ + Add a bgp monitor on ptf and verify that DUT is attempting to establish connection to it + """ + local_addr, peer_addr, peer_port = common_setup_teardown + exp_packet = build_syn_pkt(local_addr, peer_addr, peer_port) + # Load bgp monitor config + logger.info("Configured bgpmon and verifying packet") + duthost.command("sonic-cfggen -j {} -w".format(BGPMON_CONFIG_FILE)) + # Verify syn packet on ptf + testutils.verify_packet(ptfadapter, exp_packet, peer_port, BGP_CONNECT_TIMEOUT) + From e19e5bd07a8c0a0257f1e2c8e6e8234701e2bfb8 Mon Sep 17 00:00:00 2001 From: bingwang Date: Thu, 12 Nov 2020 02:48:13 -0800 Subject: [PATCH 2/5] Add test_bgpmon to kvm test. Signed-off-by: bingwang --- tests/kvmtest.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/kvmtest.sh b/tests/kvmtest.sh index 675e7cc261c..7584b640d06 100755 --- a/tests/kvmtest.sh +++ b/tests/kvmtest.sh @@ -73,7 +73,8 @@ telemetry/test_telemetry.py \ test_features.py \ test_procdockerstatsd.py \ iface_namingmode/test_iface_namingmode.py \ -platform_tests/test_cpu_memory_usage.py" +platform_tests/test_cpu_memory_usage.py \ +bgp/test_bgpmon.py" # FIXME: The lldp test has been temporarily disabled for https://github.com/Azure/sonic-mgmt/pull/2413 # and https://github.com/Azure/sonic-buildimage/pull/5698. The reason is that these two PRs dependent on each other. From acfea3e3e6db0f15d3a76fb27d5e48779ba50942 Mon Sep 17 00:00:00 2001 From: bingwang Date: Fri, 13 Nov 2020 12:46:03 +0000 Subject: [PATCH 3/5] Update after review. 1.Move bgpmon.j2 to templates 2.Generate random IP address for test 3.Capture on all interfaces in default routes Signed-off-by: bingwang --- tests/bgp/{ => templates}/bgpmon.j2 | 0 tests/bgp/test_bgpmon.py | 68 ++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 15 deletions(-) rename tests/bgp/{ => templates}/bgpmon.j2 (100%) diff --git a/tests/bgp/bgpmon.j2 b/tests/bgp/templates/bgpmon.j2 similarity index 100% rename from tests/bgp/bgpmon.j2 rename to tests/bgp/templates/bgpmon.j2 diff --git a/tests/bgp/test_bgpmon.py b/tests/bgp/test_bgpmon.py index f971bcffb9a..95af78e9d8d 100644 --- a/tests/bgp/test_bgpmon.py +++ b/tests/bgp/test_bgpmon.py @@ -5,33 +5,70 @@ from jinja2 import Template import ptf.packet as scapy from ptf.mask import Mask +import json from tests.common.fixtures.ptfhost_utils import change_mac_addresses # lgtm[py/unused-import] from tests.common.fixtures.ptfhost_utils import remove_ip_addresses # lgtm[py/unused-import] from tests.common.helpers.generators import generate_ips as generate_ips +from tests.common.helpers.assertions import pytest_assert pytestmark = [ pytest.mark.topology('any'), ] -BGPMON_TEMPLATE_FILE = 'bgp/bgpmon.j2' +BGPMON_TEMPLATE_FILE = 'bgp/templates/bgpmon.j2' BGPMON_CONFIG_FILE = '/tmp/bgpmon.json' BGP_PORT = 179 -BGP_CONNECT_TIMEOUT = 120 +BGP_CONNECT_TIMEOUT = 121 +ZERO_ADDR = r'0.0.0.0/0' logger = logging.getLogger(__name__) +def route_through_default_routes(host, ip_addr): + output = host.shell("show ip route {} json".format(ip_addr))['stdout'] + routes_info = json.loads(output) + ret = True + for prefix in routes_info.keys(): + if prefix != ZERO_ADDR: + ret = False + break + return ret + +def generate_ip_through_default_route(host): + # Generate an IP address routed through default routes + ip_addrs = generate_ips(200, "200.0.0.1/24", []) + for ip_addr in ip_addrs: + if route_through_default_routes(host, ip_addr): + return ip_addr + return None + +def get_default_route_ports(host): + mg_facts = host.minigraph_facts(host=host.hostname)['ansible_facts'] + route_info = json.loads(host.shell("show ip route {} json".format(ZERO_ADDR))['stdout']) + ports = [] + for route in route_info[ZERO_ADDR]: + if route['protocol'] != 'bgp': + continue + for itfs in route['nexthops']: + ports.append(itfs['interfaceName']) + port_indices = [] + for port in ports: + if 'PortChannel' in port: + for member in mg_facts['minigraph_portchannels'][port]['members']: + port_indices.append(mg_facts['minigraph_port_indices'][member]) + else: + port_indices.append(mg_facts['minigraph_port_indices'][port]) + return port_indices + @pytest.fixture def common_setup_teardown(duthost, ptfhost): - mg_facts = duthost.minigraph_facts(host=duthost.hostname)['ansible_facts'] - peer_addr = generate_ips(1, "%s/%s" % (mg_facts['minigraph_vlan_interfaces'][0]['addr'], - mg_facts['minigraph_vlan_interfaces'][0]['prefixlen']), - [IPAddress(mg_facts['minigraph_vlan_interfaces'][0]['addr'])])[0] + peer_addr = generate_ip_through_default_route(duthost) + pytest_assert(peer_addr, "Failed to generate ip address for test") peer_addr = str(IPNetwork(peer_addr).ip) - peer_port = mg_facts['minigraph_port_indices'][mg_facts['minigraph_vlans'][mg_facts['minigraph_vlan_interfaces'][0]['attachto']]['members'][0]] + peer_ports = get_default_route_ports(duthost) + mg_facts = duthost.minigraph_facts(host=duthost.hostname)['ansible_facts'] local_addr = mg_facts['minigraph_lo_interfaces'][0]['addr'] # Assign peer addr to an interface on ptf - ptfhost.shell("ifconfig eth{} {}".format(peer_port, peer_addr)) - logger.info("Generated peer address {} and assigned to eth{} on ptf".format(peer_addr, peer_port)) + logger.info("Generated peer address {}".format(peer_addr)) bgpmon_args = { 'peer_addr': peer_addr, 'asn': mg_facts['minigraph_bgp_asn'], @@ -41,12 +78,13 @@ def common_setup_teardown(duthost, ptfhost): bgpmon_template = Template(open(BGPMON_TEMPLATE_FILE).read()) duthost.copy(content=bgpmon_template.render(**bgpmon_args), dest=BGPMON_CONFIG_FILE) - yield local_addr, peer_addr, peer_port + yield local_addr, peer_addr, peer_ports # Cleanup bgp monitor duthost.shell("redis-cli -n 4 -c DEL 'BGP_MONITORS|{}'".format(peer_addr)) duthost.file(path=BGPMON_CONFIG_FILE, state='absent') -def build_syn_pkt(local_addr, peer_addr, peer_port): + +def build_syn_pkt(local_addr, peer_addr): pkt = testutils.simple_tcp_packet( pktlen=54, ip_src=local_addr, @@ -85,11 +123,11 @@ def test_bgpmon(duthost, common_setup_teardown, ptfadapter): """ Add a bgp monitor on ptf and verify that DUT is attempting to establish connection to it """ - local_addr, peer_addr, peer_port = common_setup_teardown - exp_packet = build_syn_pkt(local_addr, peer_addr, peer_port) + local_addr, peer_addr, peer_ports = common_setup_teardown + exp_packet = build_syn_pkt(local_addr, peer_addr) # Load bgp monitor config - logger.info("Configured bgpmon and verifying packet") + logger.info("Configured bgpmon and verifying packet on {}".format(peer_ports)) duthost.command("sonic-cfggen -j {} -w".format(BGPMON_CONFIG_FILE)) # Verify syn packet on ptf - testutils.verify_packet(ptfadapter, exp_packet, peer_port, BGP_CONNECT_TIMEOUT) + testutils.verify_packet_any_port(test=ptfadapter, pkt=exp_packet, ports=peer_ports, timeout=BGP_CONNECT_TIMEOUT) From 7c1bd8f2bd868f6fa89988c64b3badc05ccc50d7 Mon Sep 17 00:00:00 2001 From: bingwang Date: Mon, 16 Nov 2020 06:37:51 +0000 Subject: [PATCH 4/5] Update veos_vtb for addressing ptf login issue The ptf hosts are placed under sonic group, and the ansible_user will be overwriten by that in ansible/group_vars/sonic/variables, which is ```admin``` in default. As a result, the login to ptf container will failed on vtestbed because of incorrect user. This commit address this issue. Signed-off-by: bingwang --- ansible/veos_vtb | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/ansible/veos_vtb b/ansible/veos_vtb index 393f6045348..59ca91aa2a5 100644 --- a/ansible/veos_vtb +++ b/ansible/veos_vtb @@ -33,6 +33,23 @@ all: vlab-04: vlab-05: vlab-06: + ptf: + hosts: + ptf-01: + ansible_host: 10.250.0.102 + ansible_hostv6: fec0::ffff:afa:2 + ptf-02: + ansible_host: 10.250.0.106 + ansible_hostv6: fec0::ffff:afa:6 + ptf-03: + ansible_host: 10.250.0.108 + ansible_hostv6: fec0::ffff:afa:8 + ptf-04: + ansible_host: 10.250.0.109 + ansible_hostv6: fec0::ffff:afa:9 + vars: + ansible_user: root + ansible_password: root sonic: hosts: vlab-01: @@ -93,27 +110,7 @@ all: ansible_hostv6: fec0::ffff:afa:4 type: simx hwsku: MSN3700 - ptf-01: - ansible_host: 10.250.0.102 - ansible_hostv6: fec0::ffff:afa:2 - ansible_user: root - ansible_password: root - ptf-02: - ansible_host: 10.250.0.106 - ansible_hostv6: fec0::ffff:afa:6 - ansible_user: root - ansible_password: root - ptf-03: - ansible_host: 10.250.0.108 - ansible_hostv6: fec0::ffff:afa:8 - ansible_user: root - ansible_password: root - ptf-04: - ansible_host: 10.250.0.109 - ansible_hostv6: fec0::ffff:afa:9 - ansible_user: root - ansible_password: root - + # The groups below are helpers to limit running playbooks to a specific server only server_1: vars: From 0454bddb4a42b4a8676964dacd5f5c796359d06f Mon Sep 17 00:00:00 2001 From: bingwang Date: Tue, 17 Nov 2020 02:58:04 +0000 Subject: [PATCH 5/5] Add a new case for verifying bgpmon when resolve-via-default is disabled Signed-off-by: bingwang --- ansible/veos_vtb | 39 +++++++++++++++++++++------------------ tests/bgp/test_bgpmon.py | 28 ++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/ansible/veos_vtb b/ansible/veos_vtb index 59ca91aa2a5..393f6045348 100644 --- a/ansible/veos_vtb +++ b/ansible/veos_vtb @@ -33,23 +33,6 @@ all: vlab-04: vlab-05: vlab-06: - ptf: - hosts: - ptf-01: - ansible_host: 10.250.0.102 - ansible_hostv6: fec0::ffff:afa:2 - ptf-02: - ansible_host: 10.250.0.106 - ansible_hostv6: fec0::ffff:afa:6 - ptf-03: - ansible_host: 10.250.0.108 - ansible_hostv6: fec0::ffff:afa:8 - ptf-04: - ansible_host: 10.250.0.109 - ansible_hostv6: fec0::ffff:afa:9 - vars: - ansible_user: root - ansible_password: root sonic: hosts: vlab-01: @@ -110,7 +93,27 @@ all: ansible_hostv6: fec0::ffff:afa:4 type: simx hwsku: MSN3700 - + ptf-01: + ansible_host: 10.250.0.102 + ansible_hostv6: fec0::ffff:afa:2 + ansible_user: root + ansible_password: root + ptf-02: + ansible_host: 10.250.0.106 + ansible_hostv6: fec0::ffff:afa:6 + ansible_user: root + ansible_password: root + ptf-03: + ansible_host: 10.250.0.108 + ansible_hostv6: fec0::ffff:afa:8 + ansible_user: root + ansible_password: root + ptf-04: + ansible_host: 10.250.0.109 + ansible_hostv6: fec0::ffff:afa:9 + ansible_user: root + ansible_password: root + # The groups below are helpers to limit running playbooks to a specific server only server_1: vars: diff --git a/tests/bgp/test_bgpmon.py b/tests/bgp/test_bgpmon.py index 95af78e9d8d..c484558b5cc 100644 --- a/tests/bgp/test_bgpmon.py +++ b/tests/bgp/test_bgpmon.py @@ -1,6 +1,6 @@ import pytest import logging -from netaddr import IPAddress, IPNetwork +from netaddr import IPNetwork import ptf.testutils as testutils from jinja2 import Template import ptf.packet as scapy @@ -35,8 +35,8 @@ def route_through_default_routes(host, ip_addr): def generate_ip_through_default_route(host): # Generate an IP address routed through default routes - ip_addrs = generate_ips(200, "200.0.0.1/24", []) - for ip_addr in ip_addrs: + for leading in range(11, 255): + ip_addr = generate_ips(1, "{}.0.0.1/24".format(leading), [])[0] if route_through_default_routes(host, ip_addr): return ip_addr return None @@ -83,7 +83,6 @@ def common_setup_teardown(duthost, ptfhost): duthost.shell("redis-cli -n 4 -c DEL 'BGP_MONITORS|{}'".format(peer_addr)) duthost.file(path=BGPMON_CONFIG_FILE, state='absent') - def build_syn_pkt(local_addr, peer_addr): pkt = testutils.simple_tcp_packet( pktlen=54, @@ -131,3 +130,24 @@ def test_bgpmon(duthost, common_setup_teardown, ptfadapter): # Verify syn packet on ptf testutils.verify_packet_any_port(test=ptfadapter, pkt=exp_packet, ports=peer_ports, timeout=BGP_CONNECT_TIMEOUT) +def test_bgpmon_no_resolve_via_default(duthost, common_setup_teardown, ptfadapter): + """ + Verify no syn for BGP is sent when 'ip nht resolve-via-default' is disabled. + """ + local_addr, peer_addr, peer_ports = common_setup_teardown + exp_packet = build_syn_pkt(local_addr, peer_addr) + # Load bgp monitor config + logger.info("Configured bgpmon and verifying no packet on {} when resolve-via-default is disabled".format(peer_ports)) + try: + # Disable resolve-via-default + duthost.command("vtysh -c \"configure terminal\" \ + -c \"no ip nht resolve-via-default\"") + duthost.command("sonic-cfggen -j {} -w".format(BGPMON_CONFIG_FILE)) + # Verify no syn packet is received + pytest_assert(0 == testutils.count_matched_packets_all_ports(test=ptfadapter, exp_packet=exp_packet, ports=peer_ports, timeout=BGP_CONNECT_TIMEOUT), + "Syn packets is captured when resolve-via-default is disabled") + finally: + # Re-enable resolve-via-default + duthost.command("vtysh -c \"configure terminal\" \ + -c \"ip nht resolve-via-default\"") +