Skip to content

Commit

Permalink
[NAT] Fix fails in NAT TC (sonic-net#5832)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@vmorokhx @kellyyeh
vmorokhx authored and kellyyeh committed Mar 31, 2023
1 parent ef99f08 commit bf71677
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 11 deletions.
1 change: 0 additions & 1 deletion tests/nat/conftest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")


Expand Down
10 changes: 10 additions & 0 deletions tests/nat/test_dynamic_nat.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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)
Expand Down
20 changes: 10 additions & 10 deletions tests/nat/test_static_nat.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)]
}
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"],
Expand All @@ -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,
Expand All @@ -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"],
Expand All @@ -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,
Expand Down Expand Up @@ -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"],
Expand Down

0 comments on commit bf71677

Please sign in to comment.