From 4ca6ae7a47cf8a31c19810159b01c03575021656 Mon Sep 17 00:00:00 2001 From: Vladyslav Morokhovych Date: Wed, 8 Feb 2023 03:29:24 +0200 Subject: [PATCH] [NAT] Fix fails in NAT TC (#5832) What is the motivation for this PR? Failed NAT TC's due to "fullcone" in iptables verification and test_nat_dynamic_pool_threshold fails. How did you do it? Define dynamic source port for expected packet before verification for test_nat_dynamic_pool_threshold. Remove "fullcone" in iptables verification. How did you verify/test it? Run tests with iptables verification and run test_nat_dynamic_pool_threshold. --- tests/nat/conftest.py | 1 - tests/nat/test_dynamic_nat.py | 10 ++++++++++ tests/nat/test_static_nat.py | 20 ++++++++++---------- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/tests/nat/conftest.py b/tests/nat/conftest.py index bdf868442c2..a593c027741 100644 --- a/tests/nat/conftest.py +++ b/tests/nat/conftest.py @@ -65,7 +65,6 @@ def teardown(duthost): shutdown_cmds = ["sudo config nat remove {}" .format(cmd) for cmd in ["static all", "bindings", "pools", "interfaces"]] exec_command(duthost, shutdown_cmds) - # Clear all enries duthost.command("sudo sonic-clear nat translations") diff --git a/tests/nat/test_dynamic_nat.py b/tests/nat/test_dynamic_nat.py index d971d798970..2c1a724d559 100644 --- a/tests/nat/test_dynamic_nat.py +++ b/tests/nat/test_dynamic_nat.py @@ -36,6 +36,7 @@ from nat_helpers import generate_and_verify_traffic_dropped from nat_helpers import get_cli_show_nat_config_output from nat_helpers import write_json +from nat_helpers import check_peers_by_ping import ptf.testutils as testutils from tests.common.helpers.assertions import pytest_assert @@ -396,6 +397,13 @@ def test_nat_dynamic_pool_threshold(self, ptfhost, tbinfo, duthost, ptfadapter, network_data.ip_dst, dst_port, network_data.ip_src, src_port + 2, network_data.public_ip) + # Define dynamic source port for expected packet + network_data = get_network_data(ptfadapter, setup_data, direction, interface_type, nat_type=nat_type) + l4_ports = get_dynamic_l4_ports(duthost, protocol_type, direction, network_data.public_ip) + if l4_ports.exp_src_port != POOL_RANGE_START_PORT: + first_exp_src_port = POOL_RANGE_START_PORT + 1 + else: + first_exp_src_port = POOL_RANGE_START_PORT # Send TCP/UDP bidirectional traffic(host-tor -> leaf-tor and vice versa) and check # Check new translation entry generate_and_verify_traffic(duthost, ptfadapter, setup_data, interface_type, 'host-tor', @@ -727,6 +735,8 @@ def test_nat_interfaces_flap_dynamic(self, ptfhost, tbinfo, duthost, ptfadapter, .format(iptables_output, iptables_rules)) # Enable outer interface dut_interface_control(duthost, "enable", setup_data["config_portchannels"][ifname_to_disable]['members'][0]) + # update arp entries + check_peers_by_ping(duthost) # Send traffic generate_and_verify_traffic(duthost, ptfadapter, setup_data, interface_type, direction, protocol_type, nat_type=nat_type) diff --git a/tests/nat/test_static_nat.py b/tests/nat/test_static_nat.py index 333808004f8..3429fd42713 100644 --- a/tests/nat/test_static_nat.py +++ b/tests/nat/test_static_nat.py @@ -689,7 +689,7 @@ def test_nat_static_iptables_add_remove(self, ptfhost, tbinfo, duthost, ptfadapt entries_table = {} # Check that NAT entries are NOT present in iptables before adding iptables_output = dut_nat_iptables_status(duthost) - iptables_rules = {"prerouting": ['DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1 fullcone'], + iptables_rules = {"prerouting": ['DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1'], "postrouting": [] } pytest_assert(iptables_rules == iptables_output, @@ -712,7 +712,7 @@ def test_nat_static_iptables_add_remove(self, ptfhost, tbinfo, duthost, ptfadapt iptables_rules = { "prerouting": [ "DNAT all -- 0.0.0.0/0 {} mark match 0x2 to:{}".format(network_data.public_ip, network_data.private_ip), - "DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1 fullcone"], + "DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1"], "postrouting": [ "SNAT all -- {} 0.0.0.0/0 mark match 0x2 to:{}".format(network_data.private_ip, network_data.public_ip)] } @@ -728,7 +728,7 @@ def test_nat_static_iptables_add_remove(self, ptfhost, tbinfo, duthost, ptfadapt protocol_type, nat_type=nat_type) # Check that NAT entries are not present in iptables after removal iptables_output = dut_nat_iptables_status(duthost) - iptables_rules = {"prerouting": ['DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1 fullcone'], + iptables_rules = {"prerouting": ['DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1'], "postrouting": [] } pytest_assert(iptables_rules == iptables_output, @@ -745,7 +745,7 @@ def test_nat_static_global_double_add(self, ptfhost, tbinfo, duthost, ptfadapter entries_table = {} # Check that NAT entries are NOT present in iptables before adding iptables_output = dut_nat_iptables_status(duthost) - iptables_rules = {"prerouting": ['DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1 fullcone'], + iptables_rules = {"prerouting": ['DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1'], "postrouting": [] } pytest_assert(iptables_rules == iptables_output, @@ -782,7 +782,7 @@ def test_nat_static_interface_add_remove_interface_ip(self, ptfhost, tbinfo, dut nat_type = 'static_nat' # Check that NAT entries are NOT present in iptables before adding iptables_output = dut_nat_iptables_status(duthost) - iptables_rules = {"prerouting": ['DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1 fullcone'], + iptables_rules = {"prerouting": ['DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1'], "postrouting": [] } pytest_assert(iptables_rules == iptables_output, @@ -821,7 +821,7 @@ def test_nat_static_interface_add_remove_interface_ip(self, ptfhost, tbinfo, dut "DNAT all -- 0.0.0.0/0 {} mark match {} to:{}".format(network_data.public_ip, tested_zones[zone]["exp_zone"], network_data.private_ip), - "DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1 fullcone"], + "DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1"], "postrouting": [ "SNAT all -- {} 0.0.0.0/0 mark match {} to:{}".format(network_data.private_ip, tested_zones[zone]["exp_zone"], @@ -839,7 +839,7 @@ def test_nat_static_interface_add_remove_interface_ip(self, ptfhost, tbinfo, dut setup_data["config_portchannels"][ifname_to_disable]['members'][0], interface_ip) # Check that NAT entries are not present in iptables after removing interface IP iptables_output = dut_nat_iptables_status(duthost) - iptables_rules = {"prerouting": ['DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1 fullcone'], + iptables_rules = {"prerouting": ['DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1'], "postrouting": [] } pytest_assert(iptables_rules == iptables_output, @@ -856,7 +856,7 @@ def test_nat_static_interface_add_remove_interface_ip(self, ptfhost, tbinfo, dut "DNAT all -- 0.0.0.0/0 {} mark match {} to:{}".format(network_data.public_ip, tested_zones[zone]["exp_zone"], network_data.private_ip), - "DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1 fullcone"], + "DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1"], "postrouting": [ "SNAT all -- {} 0.0.0.0/0 mark match {} to:{}".format(network_data.private_ip, tested_zones[zone]["exp_zone"], @@ -883,7 +883,7 @@ def test_nat_static_interface_add_remove_interface(self, ptfhost, tbinfo, duthos nat_type = 'static_nat' # Check that NAT entries are NOT present in iptables before adding iptables_output = dut_nat_iptables_status(duthost) - iptables_rules = {"prerouting": ['DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1 fullcone'], + iptables_rules = {"prerouting": ['DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1'], "postrouting": [] } pytest_assert(iptables_rules == iptables_output, @@ -922,7 +922,7 @@ def test_nat_static_interface_add_remove_interface(self, ptfhost, tbinfo, duthos "DNAT all -- 0.0.0.0/0 {} mark match {} to:{}".format(network_data.public_ip, tested_zones[zone]["exp_zone"], network_data.private_ip), - "DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1 fullcone"], + "DNAT all -- 0.0.0.0/0 0.0.0.0/0 to:1.1.1.1"], "postrouting": [ "SNAT all -- {} 0.0.0.0/0 mark match {} to:{}".format(network_data.private_ip, tested_zones[zone]["exp_zone"],