From 3765ca2dcf635eed834c0bfac264e6a4beea78ff Mon Sep 17 00:00:00 2001 From: Tianrong Zhang Date: Fri, 26 Feb 2021 02:07:36 +0000 Subject: [PATCH 1/7] dhcp dual tor testing --- .../test/files/ptftests/dhcp_relay_test.py | 28 ++++-- tests/dhcp_relay/test_dhcp_relay.py | 90 ++++++++++++++++--- 2 files changed, 100 insertions(+), 18 deletions(-) diff --git a/ansible/roles/test/files/ptftests/dhcp_relay_test.py b/ansible/roles/test/files/ptftests/dhcp_relay_test.py index 1d1ca62066c..31fd8415a59 100644 --- a/ansible/roles/test/files/ptftests/dhcp_relay_test.py +++ b/ansible/roles/test/files/ptftests/dhcp_relay_test.py @@ -118,6 +118,12 @@ def setUp(self): self.client_port_index = int(self.test_params['client_port_index']) self.client_mac = self.dataplane.get_mac(0, self.client_port_index) + self.switch_loopback_ip = self.test_params['switch_loopback_ip'] + + # 'dual' for dual tor testing + # 'default' for regular single tor testing + self.dual_tor = (self.test_params['testing_mode'] == 'dual') + # option82 is a byte string created by the relay agent. It contains the circuit_id and remote_id fields. # circuit_id is stored as suboption 1 of option 82. # It consists of the following: @@ -139,6 +145,12 @@ def setUp(self): self.option82 += struct.pack('BB', 2, len(remote_id_string)) self.option82 += remote_id_string + # In 'dual' testing mode, vlan ip is stored as suboption 5 of option 82. + if self.dual_tor: + link_selection = ''.join([chr(int(byte)) for byte in self.relay_iface_ip.split('.')]) + self.option82 += struct.pack('BB', 5, 4) + self.option82 += link_selection + # We'll assign our client the IP address 1 greater than our relay interface (i.e., gateway) IP self.client_ip = incrementIpAddress(self.relay_iface_ip, 1) self.client_subnet = self.test_params['relay_iface_netmask'] @@ -195,7 +207,7 @@ def create_dhcp_discover_relayed_packet(self): ciaddr=self.DEFAULT_ROUTE_IP, yiaddr=self.DEFAULT_ROUTE_IP, siaddr=self.DEFAULT_ROUTE_IP, - giaddr=self.relay_iface_ip, + giaddr=self.relay_iface_ip if not self.dual_tor else self.switch_loopback_ip, chaddr=my_chaddr) bootp /= scapy.DHCP(options=[('message-type', 'discover'), ('relay_agent_Information', self.option82), @@ -214,10 +226,10 @@ def create_dhcp_offer_packet(self): eth_dst=self.relay_iface_mac, eth_client=self.client_mac, ip_server=self.server_ip, - ip_dst=self.relay_iface_ip, + ip_dst=self.relay_iface_ip if not self.dual_tor else self.switch_loopback_ip, ip_offered=self.client_ip, port_dst=self.DHCP_SERVER_PORT, - ip_gateway=self.relay_iface_ip, + ip_gateway=self.relay_iface_ip if not self.dual_tor else self.switch_loopback_ip, netmask_client=self.client_subnet, dhcp_lease=self.LEASE_TIME, padding_bytes=0, @@ -246,7 +258,7 @@ def create_dhcp_offer_relayed_packet(self): ciaddr=self.DEFAULT_ROUTE_IP, yiaddr=self.client_ip, siaddr=self.server_ip, - giaddr=self.relay_iface_ip, + giaddr=self.relay_iface_ip if not self.dual_tor else self.switch_loopback_ip, chaddr=my_chaddr) bootp /= scapy.DHCP(options=[('message-type', 'offer'), ('server_id', self.server_ip), @@ -300,7 +312,7 @@ def create_dhcp_request_relayed_packet(self): ciaddr=self.DEFAULT_ROUTE_IP, yiaddr=self.DEFAULT_ROUTE_IP, siaddr=self.DEFAULT_ROUTE_IP, - giaddr=self.relay_iface_ip, + giaddr=self.relay_iface_ip if not self.dual_tor else self.switch_loopback_ip, chaddr=my_chaddr) bootp /= scapy.DHCP(options=[('message-type', 'request'), ('requested_addr', self.client_ip), @@ -321,10 +333,10 @@ def create_dhcp_ack_packet(self): eth_dst=self.relay_iface_mac, eth_client=self.client_mac, ip_server=self.server_ip, - ip_dst=self.relay_iface_ip, + ip_dst=self.relay_iface_ip if not self.dual_tor else self.switch_loopback_ip, ip_offered=self.client_ip, port_dst=self.DHCP_SERVER_PORT, - ip_gateway=self.relay_iface_ip, + ip_gateway=self.relay_iface_ip if not self.dual_tor else self.switch_loopback_ip, netmask_client=self.client_subnet, dhcp_lease=self.LEASE_TIME, padding_bytes=0, @@ -353,7 +365,7 @@ def create_dhcp_ack_relayed_packet(self): ciaddr=self.DEFAULT_ROUTE_IP, yiaddr=self.client_ip, siaddr=self.server_ip, - giaddr=self.relay_iface_ip, + giaddr=self.relay_iface_ip if not self.dual_tor else self.switch_loopback_ip, chaddr=my_chaddr) bootp /= scapy.DHCP(options=[('message-type', 'ack'), ('server_id', self.server_ip), diff --git a/tests/dhcp_relay/test_dhcp_relay.py b/tests/dhcp_relay/test_dhcp_relay.py index f5537b2ae56..426f96b98cb 100644 --- a/tests/dhcp_relay/test_dhcp_relay.py +++ b/tests/dhcp_relay/test_dhcp_relay.py @@ -15,6 +15,7 @@ BROADCAST_MAC = 'ff:ff:ff:ff:ff:ff' DEFAULT_DHCP_CLIENT_PORT = 68 + @pytest.fixture(autouse=True) def ignore_expected_loganalyzer_exceptions(rand_one_dut_hostname, loganalyzer): """Ignore expected failures logs during test execution.""" @@ -42,6 +43,8 @@ def dut_dhcp_relay_data(duthosts, rand_one_dut_hostname, ptfhost, tbinfo): mg_facts = duthost.get_extended_minigraph_facts(tbinfo) host_facts = duthost.setup()['ansible_facts'] + switch_loopback_ip = mg_facts['minigraph_lo_interfaces'][0]['addr'] + # SONiC spawns one DHCP relay agent per VLAN interface configured on the DUT vlan_dict = mg_facts['minigraph_vlans'] for vlan_iface_name, vlan_info_dict in vlan_dict.items(): @@ -94,10 +97,13 @@ def dut_dhcp_relay_data(duthosts, rand_one_dut_hostname, ptfhost, tbinfo): dhcp_relay_data['client_iface'] = client_iface dhcp_relay_data['uplink_interfaces'] = uplink_interfaces dhcp_relay_data['uplink_port_indices'] = uplink_port_indices + dhcp_relay_data['switch_loopback_ip'] = str(switch_loopback_ip) + dhcp_relay_data_list.append(dhcp_relay_data) return dhcp_relay_data_list + @pytest.fixture(scope="module") def validate_dut_routes_exist(duthosts, rand_one_dut_hostname, dut_dhcp_relay_data): """Fixture to valid a route to each DHCP server exist @@ -112,12 +118,31 @@ def validate_dut_routes_exist(duthosts, rand_one_dut_hostname, dut_dhcp_relay_da assert len(rtInfo["nexthops"]) > 0, "Failed to find route to DHCP server '{0}'".format(dhcp_server) -def test_dhcp_relay_default(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist): +def set_dual_tor(duthost): + duthost.shell('redis-cli -n 4 HSET "DEVICE_METADATA|localhost" "subtype" "DualToR"') + duthost.shell('systemctl restart dhcp_relay') + duthost.shell('systemctl reset-failed dhcp_relay') + time.sleep(30) + + +def reset_dual_tor(duthost): + duthost.shell('redis-cli -n 4 HDEL "DEVICE_METADATA|localhost" "subtype"') + duthost.shell('systemctl restart dhcp_relay') + duthost.shell('systemctl reset-failed dhcp_relay') + time.sleep(30) + + +@pytest.mark.parametrize("testing_mode", ['single', 'dual']) +def test_dhcp_relay_default(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): """Test DHCP relay functionality on T0 topology. For each DHCP relay agent running on the DuT, verify DHCP packets are relayed properly """ duthost = duthosts[rand_one_dut_hostname] + + if testing_mode == 'dual': + set_dual_tor(duthost) + for dhcp_relay in dut_dhcp_relay_data: # Run the DHCP relay test on the PTF host ptf_runner(ptfhost, @@ -134,17 +159,27 @@ def test_dhcp_relay_default(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_r "relay_iface_mac": str(dhcp_relay['downlink_vlan_iface']['mac']), "relay_iface_netmask": str(dhcp_relay['downlink_vlan_iface']['mask']), "dest_mac_address": BROADCAST_MAC, - "client_udp_src_port": DEFAULT_DHCP_CLIENT_PORT}, + "client_udp_src_port": DEFAULT_DHCP_CLIENT_PORT, + "switch_loopback_ip": dhcp_relay['switch_loopback_ip'], + "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") + if testing_mode == 'dual': + reset_dual_tor(duthost) + -def test_dhcp_relay_after_link_flap(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist): +@pytest.mark.parametrize("testing_mode", ['single', 'dual']) +def test_dhcp_relay_after_link_flap(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): """Test DHCP relay functionality on T0 topology after uplinks flap For each DHCP relay agent running on the DuT, with relay agent running, flap the uplinks, then test whether the DHCP relay agent relays packets properly. """ duthost = duthosts[rand_one_dut_hostname] + + if testing_mode == 'dual': + set_dual_tor(duthost) + for dhcp_relay in dut_dhcp_relay_data: # Bring all uplink interfaces down for iface in dhcp_relay['uplink_interfaces']: @@ -175,11 +210,17 @@ def test_dhcp_relay_after_link_flap(duthosts, rand_one_dut_hostname, ptfhost, du "relay_iface_mac": str(dhcp_relay['downlink_vlan_iface']['mac']), "relay_iface_netmask": str(dhcp_relay['downlink_vlan_iface']['mask']), "dest_mac_address": BROADCAST_MAC, - "client_udp_src_port": DEFAULT_DHCP_CLIENT_PORT}, + "client_udp_src_port": DEFAULT_DHCP_CLIENT_PORT, + "switch_loopback_ip": dhcp_relay['switch_loopback_ip'], + "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") + if testing_mode == 'dual': + reset_dual_tor(duthost) + -def test_dhcp_relay_start_with_uplinks_down(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist): +@pytest.mark.parametrize("testing_mode", ['single', 'dual']) +def test_dhcp_relay_start_with_uplinks_down(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): """Test DHCP relay functionality on T0 topology when relay agent starts with uplinks down For each DHCP relay agent running on the DuT, bring the uplinks down, then restart the @@ -187,6 +228,10 @@ def test_dhcp_relay_start_with_uplinks_down(duthosts, rand_one_dut_hostname, ptf relays packets properly. """ duthost = duthosts[rand_one_dut_hostname] + + if testing_mode == 'dual': + set_dual_tor(duthost) + for dhcp_relay in dut_dhcp_relay_data: # Bring all uplink interfaces down for iface in dhcp_relay['uplink_interfaces']: @@ -224,16 +269,26 @@ def test_dhcp_relay_start_with_uplinks_down(duthosts, rand_one_dut_hostname, ptf "relay_iface_mac": str(dhcp_relay['downlink_vlan_iface']['mac']), "relay_iface_netmask": str(dhcp_relay['downlink_vlan_iface']['mask']), "dest_mac_address": BROADCAST_MAC, - "client_udp_src_port": DEFAULT_DHCP_CLIENT_PORT}, + "client_udp_src_port": DEFAULT_DHCP_CLIENT_PORT, + "switch_loopback_ip": dhcp_relay['switch_loopback_ip'], + "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") + if testing_mode == 'dual': + reset_dual_tor(duthost) + -def test_dhcp_relay_unicast_mac(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist): +@pytest.mark.parametrize("testing_mode", ['single', 'dual']) +def test_dhcp_relay_unicast_mac(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): """Test DHCP relay functionality on T0 topology with unicast mac Instead of using broadcast MAC, use unicast MAC of DUT and verify that DHCP relay functionality is entact. """ duthost = duthosts[rand_one_dut_hostname] + + if testing_mode == 'dual': + set_dual_tor(duthost) + for dhcp_relay in dut_dhcp_relay_data: # Run the DHCP relay test on the PTF host ptf_runner(ptfhost, @@ -250,17 +305,27 @@ def test_dhcp_relay_unicast_mac(duthosts, rand_one_dut_hostname, ptfhost, dut_dh "relay_iface_mac": str(dhcp_relay['downlink_vlan_iface']['mac']), "relay_iface_netmask": str(dhcp_relay['downlink_vlan_iface']['mask']), "dest_mac_address": duthost.facts["router_mac"], - "client_udp_src_port": DEFAULT_DHCP_CLIENT_PORT}, + "client_udp_src_port": DEFAULT_DHCP_CLIENT_PORT, + "switch_loopback_ip": dhcp_relay['switch_loopback_ip'], + "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") + if testing_mode == 'dual': + reset_dual_tor(duthost) + -def test_dhcp_relay_random_sport(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist): +@pytest.mark.parametrize("testing_mode", ['single', 'dual']) +def test_dhcp_relay_random_sport(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): """Test DHCP relay functionality on T0 topology with random source port (sport) If the client is SNAT'd, the source port could be changed to a non-standard port (i.e., not 68). Verify that DHCP relay works with random high sport. """ duthost = duthosts[rand_one_dut_hostname] + + if testing_mode == 'dual': + set_dual_tor(duthost) + RANDOM_CLIENT_PORT = random.choice(range(1000, 65535)) for dhcp_relay in dut_dhcp_relay_data: # Run the DHCP relay test on the PTF host @@ -278,5 +343,10 @@ def test_dhcp_relay_random_sport(duthosts, rand_one_dut_hostname, ptfhost, dut_d "relay_iface_mac": str(dhcp_relay['downlink_vlan_iface']['mac']), "relay_iface_netmask": str(dhcp_relay['downlink_vlan_iface']['mask']), "dest_mac_address": BROADCAST_MAC, - "client_udp_src_port": RANDOM_CLIENT_PORT}, + "client_udp_src_port": RANDOM_CLIENT_PORT, + "switch_loopback_ip": dhcp_relay['switch_loopback_ip'], + "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") + + if testing_mode == 'dual': + reset_dual_tor(duthost) \ No newline at end of file From 92bc71222abd4e722d3d24ade71a14e2f2143fa5 Mon Sep 17 00:00:00 2001 From: Tianrong Zhang Date: Fri, 26 Feb 2021 02:27:52 +0000 Subject: [PATCH 2/7] minor update on comment --- ansible/roles/test/files/ptftests/dhcp_relay_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible/roles/test/files/ptftests/dhcp_relay_test.py b/ansible/roles/test/files/ptftests/dhcp_relay_test.py index 31fd8415a59..674fc4e9458 100644 --- a/ansible/roles/test/files/ptftests/dhcp_relay_test.py +++ b/ansible/roles/test/files/ptftests/dhcp_relay_test.py @@ -121,7 +121,7 @@ def setUp(self): self.switch_loopback_ip = self.test_params['switch_loopback_ip'] # 'dual' for dual tor testing - # 'default' for regular single tor testing + # 'single' for regular single tor testing self.dual_tor = (self.test_params['testing_mode'] == 'dual') # option82 is a byte string created by the relay agent. It contains the circuit_id and remote_id fields. From 5b8ef8c9166ae56154a045c13a3a9994948f90aa Mon Sep 17 00:00:00 2001 From: Tianrong Zhang Date: Fri, 26 Feb 2021 02:29:20 +0000 Subject: [PATCH 3/7] minor update --- tests/dhcp_relay/test_dhcp_relay.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/dhcp_relay/test_dhcp_relay.py b/tests/dhcp_relay/test_dhcp_relay.py index 426f96b98cb..3ff835a8750 100644 --- a/tests/dhcp_relay/test_dhcp_relay.py +++ b/tests/dhcp_relay/test_dhcp_relay.py @@ -349,4 +349,5 @@ def test_dhcp_relay_random_sport(duthosts, rand_one_dut_hostname, ptfhost, dut_d log_file="/tmp/dhcp_relay_test.DHCPTest.log") if testing_mode == 'dual': - reset_dual_tor(duthost) \ No newline at end of file + reset_dual_tor(duthost) + From d0c14283249d6e1c1e4d4afdd4d9cfdeace73f0b Mon Sep 17 00:00:00 2001 From: Tianrong Zhang Date: Sat, 27 Feb 2021 02:58:18 +0000 Subject: [PATCH 4/7] always check state before each test starts --- .../test/files/ptftests/dhcp_relay_test.py | 4 + tests/dhcp_relay/test_dhcp_relay.py | 83 ++++++++++--------- 2 files changed, 49 insertions(+), 38 deletions(-) diff --git a/ansible/roles/test/files/ptftests/dhcp_relay_test.py b/ansible/roles/test/files/ptftests/dhcp_relay_test.py index 674fc4e9458..1db53a4af9e 100644 --- a/ansible/roles/test/files/ptftests/dhcp_relay_test.py +++ b/ansible/roles/test/files/ptftests/dhcp_relay_test.py @@ -146,6 +146,10 @@ def setUp(self): self.option82 += remote_id_string # In 'dual' testing mode, vlan ip is stored as suboption 5 of option 82. + # It consists of the following: + # Byte 0: Suboption number, always set to 5 + # Byte 1: Length of suboption data in bytes, always set to 4 (ipv4 addr has 4 bytes) + # Bytes 2+: vlan ip addr if self.dual_tor: link_selection = ''.join([chr(int(byte)) for byte in self.relay_iface_ip.split('.')]) self.option82 += struct.pack('BB', 5, 4) diff --git a/tests/dhcp_relay/test_dhcp_relay.py b/tests/dhcp_relay/test_dhcp_relay.py index 3ff835a8750..5f41adba29a 100644 --- a/tests/dhcp_relay/test_dhcp_relay.py +++ b/tests/dhcp_relay/test_dhcp_relay.py @@ -14,6 +14,8 @@ BROADCAST_MAC = 'ff:ff:ff:ff:ff:ff' DEFAULT_DHCP_CLIENT_PORT = 68 +SINGLE_TOR_MODE = 'single' +DUAL_TOR_MODE = 'dual' @pytest.fixture(autouse=True) @@ -118,30 +120,49 @@ def validate_dut_routes_exist(duthosts, rand_one_dut_hostname, dut_dhcp_relay_da assert len(rtInfo["nexthops"]) > 0, "Failed to find route to DHCP server '{0}'".format(dhcp_server) -def set_dual_tor(duthost): - duthost.shell('redis-cli -n 4 HSET "DEVICE_METADATA|localhost" "subtype" "DualToR"') +def restart_dhcp_service(duthost): duthost.shell('systemctl restart dhcp_relay') duthost.shell('systemctl reset-failed dhcp_relay') time.sleep(30) -def reset_dual_tor(duthost): - duthost.shell('redis-cli -n 4 HDEL "DEVICE_METADATA|localhost" "subtype"') - duthost.shell('systemctl restart dhcp_relay') - duthost.shell('systemctl reset-failed dhcp_relay') - time.sleep(30) +def get_subtype_from_configdb(duthost): + # HEXISTS returns 1 if the key exists, otherwise 0 + subtype_exist = duthost.shell('redis-cli -n 4 HEXISTS "DEVICE_METADATA|localhost" "subtype"')["stdout"] + subtype_value = "" + if subtype_exist: + subtype_value = duthost.shell('redis-cli -n 4 HGET "DEVICE_METADATA|localhost" "subtype"')["stdout"] + return subtype_exist, subtype_value + + +def set_tor_testing_mode(duthost, testing_mode): + subtype_exist, subtype_value = get_subtype_from_configdb(duthost) + + if testing_mode == SINGLE_TOR_MODE: + if subtype_exist: + duthost.shell('redis-cli -n 4 HDEL "DEVICE_METADATA|localhost" "subtype"') + restart_dhcp_service(duthost) + + if testing_mode == DUAL_TOR_MODE: + if not subtype_exist or subtype_value != 'DualToR': + duthost.shell('redis-cli -n 4 HSET "DEVICE_METADATA|localhost" "subtype" "DualToR"') + restart_dhcp_service(duthost) + +def clear_tor_testing_mode(duthost, testing_mode): + if testing_mode == DUAL_TOR_MODE: + duthost.shell('redis-cli -n 4 HDEL "DEVICE_METADATA|localhost" "subtype"') + restart_dhcp_service(duthost) -@pytest.mark.parametrize("testing_mode", ['single', 'dual']) + +@pytest.mark.parametrize("testing_mode", [SINGLE_TOR_MODE, DUAL_TOR_MODE]) def test_dhcp_relay_default(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): """Test DHCP relay functionality on T0 topology. For each DHCP relay agent running on the DuT, verify DHCP packets are relayed properly """ duthost = duthosts[rand_one_dut_hostname] - - if testing_mode == 'dual': - set_dual_tor(duthost) + set_tor_testing_mode(duthost, testing_mode) for dhcp_relay in dut_dhcp_relay_data: # Run the DHCP relay test on the PTF host @@ -164,11 +185,10 @@ def test_dhcp_relay_default(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_r "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") - if testing_mode == 'dual': - reset_dual_tor(duthost) + clear_tor_testing_mode(duthost, testing_mode) -@pytest.mark.parametrize("testing_mode", ['single', 'dual']) +@pytest.mark.parametrize("testing_mode", [SINGLE_TOR_MODE, DUAL_TOR_MODE]) def test_dhcp_relay_after_link_flap(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): """Test DHCP relay functionality on T0 topology after uplinks flap @@ -176,9 +196,7 @@ def test_dhcp_relay_after_link_flap(duthosts, rand_one_dut_hostname, ptfhost, du then test whether the DHCP relay agent relays packets properly. """ duthost = duthosts[rand_one_dut_hostname] - - if testing_mode == 'dual': - set_dual_tor(duthost) + set_tor_testing_mode(duthost, testing_mode) for dhcp_relay in dut_dhcp_relay_data: # Bring all uplink interfaces down @@ -215,11 +233,10 @@ def test_dhcp_relay_after_link_flap(duthosts, rand_one_dut_hostname, ptfhost, du "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") - if testing_mode == 'dual': - reset_dual_tor(duthost) + clear_tor_testing_mode(duthost, testing_mode) -@pytest.mark.parametrize("testing_mode", ['single', 'dual']) +@pytest.mark.parametrize("testing_mode", [SINGLE_TOR_MODE, DUAL_TOR_MODE]) def test_dhcp_relay_start_with_uplinks_down(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): """Test DHCP relay functionality on T0 topology when relay agent starts with uplinks down @@ -228,9 +245,7 @@ def test_dhcp_relay_start_with_uplinks_down(duthosts, rand_one_dut_hostname, ptf relays packets properly. """ duthost = duthosts[rand_one_dut_hostname] - - if testing_mode == 'dual': - set_dual_tor(duthost) + set_tor_testing_mode(duthost, testing_mode) for dhcp_relay in dut_dhcp_relay_data: # Bring all uplink interfaces down @@ -274,20 +289,17 @@ def test_dhcp_relay_start_with_uplinks_down(duthosts, rand_one_dut_hostname, ptf "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") - if testing_mode == 'dual': - reset_dual_tor(duthost) + clear_tor_testing_mode(duthost, testing_mode) -@pytest.mark.parametrize("testing_mode", ['single', 'dual']) +@pytest.mark.parametrize("testing_mode", [SINGLE_TOR_MODE, DUAL_TOR_MODE]) def test_dhcp_relay_unicast_mac(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): """Test DHCP relay functionality on T0 topology with unicast mac Instead of using broadcast MAC, use unicast MAC of DUT and verify that DHCP relay functionality is entact. """ duthost = duthosts[rand_one_dut_hostname] - - if testing_mode == 'dual': - set_dual_tor(duthost) + set_tor_testing_mode(duthost, testing_mode) for dhcp_relay in dut_dhcp_relay_data: # Run the DHCP relay test on the PTF host @@ -310,11 +322,10 @@ def test_dhcp_relay_unicast_mac(duthosts, rand_one_dut_hostname, ptfhost, dut_dh "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") - if testing_mode == 'dual': - reset_dual_tor(duthost) + clear_tor_testing_mode(duthost, testing_mode) -@pytest.mark.parametrize("testing_mode", ['single', 'dual']) +@pytest.mark.parametrize("testing_mode", [SINGLE_TOR_MODE, DUAL_TOR_MODE]) def test_dhcp_relay_random_sport(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): """Test DHCP relay functionality on T0 topology with random source port (sport) @@ -322,9 +333,7 @@ def test_dhcp_relay_random_sport(duthosts, rand_one_dut_hostname, ptfhost, dut_d Verify that DHCP relay works with random high sport. """ duthost = duthosts[rand_one_dut_hostname] - - if testing_mode == 'dual': - set_dual_tor(duthost) + set_tor_testing_mode(duthost, testing_mode) RANDOM_CLIENT_PORT = random.choice(range(1000, 65535)) for dhcp_relay in dut_dhcp_relay_data: @@ -348,6 +357,4 @@ def test_dhcp_relay_random_sport(duthosts, rand_one_dut_hostname, ptfhost, dut_d "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") - if testing_mode == 'dual': - reset_dual_tor(duthost) - + clear_tor_testing_mode(duthost, testing_mode) From a2a0625d48b495a9793148a94fca0c5bf8808dc3 Mon Sep 17 00:00:00 2001 From: Tianrong Zhang Date: Tue, 2 Mar 2021 06:19:18 +0000 Subject: [PATCH 5/7] move setup and teardown into a fixture function --- tests/dhcp_relay/test_dhcp_relay.py | 56 +++++++++++++---------------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/tests/dhcp_relay/test_dhcp_relay.py b/tests/dhcp_relay/test_dhcp_relay.py index 5f41adba29a..733db288bdf 100644 --- a/tests/dhcp_relay/test_dhcp_relay.py +++ b/tests/dhcp_relay/test_dhcp_relay.py @@ -123,6 +123,15 @@ def validate_dut_routes_exist(duthosts, rand_one_dut_hostname, dut_dhcp_relay_da def restart_dhcp_service(duthost): duthost.shell('systemctl restart dhcp_relay') duthost.shell('systemctl reset-failed dhcp_relay') + + for retry in range(5): + time.sleep(30) + dhcp_status = duthost.shell('docker container top dhcp_relay | grep dhcrelay | cat')["stdout"] + if dhcp_status != "": + break + else: + assert False, "Failed to restart dhcp docker" + time.sleep(30) @@ -135,7 +144,10 @@ def get_subtype_from_configdb(duthost): return subtype_exist, subtype_value -def set_tor_testing_mode(duthost, testing_mode): +@pytest.fixture(scope="module", params=[SINGLE_TOR_MODE, DUAL_TOR_MODE]) +def testing_config(request, duthosts, rand_one_dut_hostname): + testing_mode = request.param + duthost = duthosts[rand_one_dut_hostname] subtype_exist, subtype_value = get_subtype_from_configdb(duthost) if testing_mode == SINGLE_TOR_MODE: @@ -148,21 +160,19 @@ def set_tor_testing_mode(duthost, testing_mode): duthost.shell('redis-cli -n 4 HSET "DEVICE_METADATA|localhost" "subtype" "DualToR"') restart_dhcp_service(duthost) + yield testing_mode, duthost -def clear_tor_testing_mode(duthost, testing_mode): if testing_mode == DUAL_TOR_MODE: duthost.shell('redis-cli -n 4 HDEL "DEVICE_METADATA|localhost" "subtype"') restart_dhcp_service(duthost) -@pytest.mark.parametrize("testing_mode", [SINGLE_TOR_MODE, DUAL_TOR_MODE]) -def test_dhcp_relay_default(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): +def test_dhcp_relay_default(ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_config): """Test DHCP relay functionality on T0 topology. For each DHCP relay agent running on the DuT, verify DHCP packets are relayed properly """ - duthost = duthosts[rand_one_dut_hostname] - set_tor_testing_mode(duthost, testing_mode) + testing_mode, duthost = testing_config for dhcp_relay in dut_dhcp_relay_data: # Run the DHCP relay test on the PTF host @@ -185,18 +195,14 @@ def test_dhcp_relay_default(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_r "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") - clear_tor_testing_mode(duthost, testing_mode) - -@pytest.mark.parametrize("testing_mode", [SINGLE_TOR_MODE, DUAL_TOR_MODE]) -def test_dhcp_relay_after_link_flap(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): +def test_dhcp_relay_after_link_flap(ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_config): """Test DHCP relay functionality on T0 topology after uplinks flap For each DHCP relay agent running on the DuT, with relay agent running, flap the uplinks, then test whether the DHCP relay agent relays packets properly. """ - duthost = duthosts[rand_one_dut_hostname] - set_tor_testing_mode(duthost, testing_mode) + testing_mode, duthost = testing_config for dhcp_relay in dut_dhcp_relay_data: # Bring all uplink interfaces down @@ -233,19 +239,15 @@ def test_dhcp_relay_after_link_flap(duthosts, rand_one_dut_hostname, ptfhost, du "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") - clear_tor_testing_mode(duthost, testing_mode) - -@pytest.mark.parametrize("testing_mode", [SINGLE_TOR_MODE, DUAL_TOR_MODE]) -def test_dhcp_relay_start_with_uplinks_down(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): +def test_dhcp_relay_start_with_uplinks_down(ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_config): """Test DHCP relay functionality on T0 topology when relay agent starts with uplinks down For each DHCP relay agent running on the DuT, bring the uplinks down, then restart the relay agent while the uplinks are still down. Then test whether the DHCP relay agent relays packets properly. """ - duthost = duthosts[rand_one_dut_hostname] - set_tor_testing_mode(duthost, testing_mode) + testing_mode, duthost = testing_config for dhcp_relay in dut_dhcp_relay_data: # Bring all uplink interfaces down @@ -289,17 +291,13 @@ def test_dhcp_relay_start_with_uplinks_down(duthosts, rand_one_dut_hostname, ptf "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") - clear_tor_testing_mode(duthost, testing_mode) - -@pytest.mark.parametrize("testing_mode", [SINGLE_TOR_MODE, DUAL_TOR_MODE]) -def test_dhcp_relay_unicast_mac(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): +def test_dhcp_relay_unicast_mac(ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_config): """Test DHCP relay functionality on T0 topology with unicast mac Instead of using broadcast MAC, use unicast MAC of DUT and verify that DHCP relay functionality is entact. """ - duthost = duthosts[rand_one_dut_hostname] - set_tor_testing_mode(duthost, testing_mode) + testing_mode, duthost = testing_config for dhcp_relay in dut_dhcp_relay_data: # Run the DHCP relay test on the PTF host @@ -322,18 +320,14 @@ def test_dhcp_relay_unicast_mac(duthosts, rand_one_dut_hostname, ptfhost, dut_dh "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") - clear_tor_testing_mode(duthost, testing_mode) - -@pytest.mark.parametrize("testing_mode", [SINGLE_TOR_MODE, DUAL_TOR_MODE]) -def test_dhcp_relay_random_sport(duthosts, rand_one_dut_hostname, ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_mode): +def test_dhcp_relay_random_sport(ptfhost, dut_dhcp_relay_data, validate_dut_routes_exist, testing_config): """Test DHCP relay functionality on T0 topology with random source port (sport) If the client is SNAT'd, the source port could be changed to a non-standard port (i.e., not 68). Verify that DHCP relay works with random high sport. """ - duthost = duthosts[rand_one_dut_hostname] - set_tor_testing_mode(duthost, testing_mode) + testing_mode, duthost = testing_config RANDOM_CLIENT_PORT = random.choice(range(1000, 65535)) for dhcp_relay in dut_dhcp_relay_data: @@ -356,5 +350,3 @@ def test_dhcp_relay_random_sport(duthosts, rand_one_dut_hostname, ptfhost, dut_d "switch_loopback_ip": dhcp_relay['switch_loopback_ip'], "testing_mode": testing_mode}, log_file="/tmp/dhcp_relay_test.DHCPTest.log") - - clear_tor_testing_mode(duthost, testing_mode) From 0ec19898e27038a65e63e25a29434b576fc57528 Mon Sep 17 00:00:00 2001 From: Tianrong Zhang Date: Tue, 2 Mar 2021 23:08:17 +0000 Subject: [PATCH 6/7] fix output type --- tests/dhcp_relay/test_dhcp_relay.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/dhcp_relay/test_dhcp_relay.py b/tests/dhcp_relay/test_dhcp_relay.py index 733db288bdf..75176d8527f 100644 --- a/tests/dhcp_relay/test_dhcp_relay.py +++ b/tests/dhcp_relay/test_dhcp_relay.py @@ -137,7 +137,7 @@ def restart_dhcp_service(duthost): def get_subtype_from_configdb(duthost): # HEXISTS returns 1 if the key exists, otherwise 0 - subtype_exist = duthost.shell('redis-cli -n 4 HEXISTS "DEVICE_METADATA|localhost" "subtype"')["stdout"] + subtype_exist = int(duthost.shell('redis-cli -n 4 HEXISTS "DEVICE_METADATA|localhost" "subtype"')["stdout"]) subtype_value = "" if subtype_exist: subtype_value = duthost.shell('redis-cli -n 4 HGET "DEVICE_METADATA|localhost" "subtype"')["stdout"] From 10af7d7070b2c70ef866b45f88e90439a176f776 Mon Sep 17 00:00:00 2001 From: Tianrong Zhang Date: Wed, 3 Mar 2021 05:01:50 +0000 Subject: [PATCH 7/7] reset failed before and after restart --- tests/dhcp_relay/test_dhcp_relay.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/dhcp_relay/test_dhcp_relay.py b/tests/dhcp_relay/test_dhcp_relay.py index 75176d8527f..8d167df818e 100644 --- a/tests/dhcp_relay/test_dhcp_relay.py +++ b/tests/dhcp_relay/test_dhcp_relay.py @@ -121,6 +121,7 @@ def validate_dut_routes_exist(duthosts, rand_one_dut_hostname, dut_dhcp_relay_da def restart_dhcp_service(duthost): + duthost.shell('systemctl reset-failed dhcp_relay') duthost.shell('systemctl restart dhcp_relay') duthost.shell('systemctl reset-failed dhcp_relay')