Skip to content

Commit

Permalink
Increase tolerance for 'ip-proto' hash_key on t2 topos
Browse files Browse the repository at this point in the history 
'ip-proto' hash_key only has 8-bits of entropy which won't result in
good distributions on systems with large ECMP groups
  • Loading branch information
@arista-nwolfe
arista-nwolfe committed Dec 20, 2024
1 parent fd0b20c commit c53e2c0
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 14 deletions.
26 changes: 17 additions & 9 deletions ansible/roles/test/files/ptftests/py3/hash_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class HashTest(BaseTest):
# Class variables
# ---------------------------------------------------------------------
DEFAULT_BALANCING_RANGE = 0.25
RELAXED_BALANCING_RANGE = 0.80
BALANCING_TEST_TIMES = 250
DEFAULT_SWITCH_TYPE = 'voq'

Expand Down Expand Up @@ -109,6 +110,8 @@ def setUp(self):
self.ipver = self.test_params.get('ipver', 'ipv4')
self.is_active_active_dualtor = self.test_params.get("is_active_active_dualtor", False)

self.topo_name = self.test_params.get('topo_name', '')

# set the base mac here to make it persistent across calls of check_ip_route
self.base_mac = self.dataplane.get_mac(
*random.choice(list(self.dataplane.ports.keys())))
Expand Down Expand Up @@ -223,7 +226,7 @@ def check_hash(self, hash_key):
hash_key, hit_count_map))

for next_hop in next_hops:
self.check_balancing(next_hop.get_next_hop(), hit_count_map, src_port)
self.check_balancing(next_hop.get_next_hop(), hit_count_map, src_port, hash_key)

def check_ip_route(self, hash_key, src_port, dst_ip, dst_port_lists):
if ip_network(six.text_type(dst_ip)).version == 4:
Expand Down Expand Up @@ -457,15 +460,20 @@ def check_ipv6_route(self, hash_key, src_port, dst_port_lists):
format(ip_src, ip_dst, src_port, rcvd_port, exp_src_mac, actual_src_mac))
return (rcvd_port, rcvd_pkt)

def check_within_expected_range(self, actual, expected):
def check_within_expected_range(self, actual, expected, hash_key):
'''
@summary: Check if the actual number is within the accepted range of the expected number
@param actual : acutal number of recieved packets
@param expected : expected number of recieved packets
@return (percentage, bool)
'''
percentage = (actual - expected) / float(expected)
return (percentage, abs(percentage) <= self.balancing_range)
balancing_range = self.balancing_range
if hash_key == 'ip-proto' and self.topo_name == 't2':
# ip-protocol only has 8-bits of entropy which results in poor hashing distributions on topologies with
# a large number of ecmp paths so relax the hashing requirements
balancing_range = self.RELAXED_BALANCING_RANGE
return (percentage, abs(percentage) <= balancing_range)

def check_same_asic(self, src_port, exp_port_list):
updated_exp_port_list = list()
Expand Down Expand Up @@ -494,7 +502,7 @@ def check_same_asic(self, src_port, exp_port_list):
exp_port_list = updated_exp_port_list
return exp_port_list

def check_balancing(self, dest_port_list, port_hit_cnt, src_port):
def check_balancing(self, dest_port_list, port_hit_cnt, src_port, hash_key):
'''
@summary: Check if the traffic is balanced across the ECMP groups and the LAG members
@param dest_port_list : a list of ECMP entries and in each ECMP entry a list of ports
Expand Down Expand Up @@ -543,7 +551,7 @@ def check_balancing(self, dest_port_list, port_hit_cnt, src_port):
for member in ecmp_entry:
total_entry_hit_cnt += port_hit_cnt.get(member, 0)
(p, r) = self.check_within_expected_range(
total_entry_hit_cnt, float(total_hit_cnt)/len(asic_member))
total_entry_hit_cnt, float(total_hit_cnt)/len(asic_member), hash_key)
logging.info("%-10s \t %-10s \t %10d \t %10d \t %10s"
% ("ECMP", str(ecmp_entry), total_hit_cnt//len(asic_member),
total_entry_hit_cnt, str(round(p, 4)*100) + '%'))
Expand All @@ -552,7 +560,7 @@ def check_balancing(self, dest_port_list, port_hit_cnt, src_port):
continue
for member in ecmp_entry:
(p, r) = self.check_within_expected_range(port_hit_cnt.get(
member, 0), float(total_entry_hit_cnt)/len(ecmp_entry))
member, 0), float(total_entry_hit_cnt)/len(ecmp_entry), hash_key)
logging.info("%-10s \t %-10s \t %10d \t %10d \t %10s"
% ("LAG", str(member), total_entry_hit_cnt//len(ecmp_entry),
port_hit_cnt.get(member, 0), str(round(p, 4)*100) + '%'))
Expand Down Expand Up @@ -846,7 +854,7 @@ def check_hash(self, hash_key):
hash_key, hit_count_map))

for next_hop in next_hops:
self.check_balancing(next_hop.get_next_hop(), hit_count_map, src_port)
self.check_balancing(next_hop.get_next_hop(), hit_count_map, src_port, hash_key)

def runTest(self):
"""
Expand Down Expand Up @@ -1133,7 +1141,7 @@ def check_hash(self, hash_key):
hash_key, hit_count_map))

for next_hop in next_hops:
self.check_balancing(next_hop.get_next_hop(), hit_count_map, src_port)
self.check_balancing(next_hop.get_next_hop(), hit_count_map, src_port, hash_key)

def runTest(self):
"""
Expand Down Expand Up @@ -1450,7 +1458,7 @@ def check_hash(self, hash_key):
hash_key, hit_count_map))

for next_hop in next_hops:
self.check_balancing(next_hop.get_next_hop(), hit_count_map, src_port)
self.check_balancing(next_hop.get_next_hop(), hit_count_map, src_port, hash_key)

def runTest(self):
"""
Expand Down
15 changes: 10 additions & 5 deletions tests/fib/test_fib.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,8 @@ def test_hash(add_default_route_to_dut, duthosts, fib_info_files_per_function, s
"ignore_ttl": ignore_ttl,
"single_fib_for_duts": single_fib_for_duts,
"switch_type": switch_type,
"is_active_active_dualtor": is_active_active_dualtor
"is_active_active_dualtor": is_active_active_dualtor,
"topo_name": updated_tbinfo['topo']['name']
},
log_file=log_file,
qlen=PTF_QLEN,
Expand Down Expand Up @@ -392,7 +393,8 @@ def test_ipinip_hash(add_default_route_to_dut, duthost, duthosts, fib_info_files
"vlan_ids": VLANIDS,
"ignore_ttl": ignore_ttl,
"single_fib_for_duts": single_fib_for_duts,
"ipver": ipver
"ipver": ipver,
"topo_name": tbinfo['topo']['name']
},
log_file=log_file,
qlen=PTF_QLEN,
Expand Down Expand Up @@ -433,7 +435,8 @@ def test_ipinip_hash_negative(add_default_route_to_dut, duthosts, fib_info_files
"vlan_ids": VLANIDS,
"ignore_ttl": ignore_ttl,
"single_fib_for_duts": single_fib_for_duts,
"ipver": ipver
"ipver": ipver,
"topo_name": tbinfo['topo']['name']
},
log_file=log_file,
qlen=PTF_QLEN,
Expand Down Expand Up @@ -480,7 +483,8 @@ def test_vxlan_hash(add_default_route_to_dut, duthost, duthosts, fib_info_files_
"vlan_ids": VLANIDS,
"ignore_ttl": ignore_ttl,
"single_fib_for_duts": single_fib_for_duts,
"ipver": vxlan_ipver
"ipver": vxlan_ipver,
"topo_name": tbinfo['topo']['name']
},
log_file=log_file,
qlen=PTF_QLEN,
Expand Down Expand Up @@ -530,7 +534,8 @@ def test_nvgre_hash(add_default_route_to_dut, duthost, duthosts, fib_info_files_
"vlan_ids": VLANIDS,
"ignore_ttl": ignore_ttl,
"single_fib_for_duts": single_fib_for_duts,
"ipver": nvgre_ipver
"ipver": nvgre_ipver,
"topo_name": tbinfo['topo']['name']
},
log_file=log_file,
qlen=PTF_QLEN,
Expand Down

0 comments on commit c53e2c0

Please sign in to comment.