Skip to content
This repository was archived by the owner on Sep 14, 2022. It is now read-only.

Add negative_packet_loss_mode pass option for Mellanox nic as traffic… #172

Open
wanghekai wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add negative_packet_loss_mode pass option for Mellanox nic as traffic… #172

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e05b9a9
Add negative_packet_loss_mode pass option for Mellanox nic as traffic…
wanghekai Jun 30, 2021
f5d8651
increse sync and async timeout to 30 secs
wanghekai Oct 13, 2022
bba6d62
Merge branch 'atheurer:master' into master
wanghekai Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions binary-search.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@ def process_options ():
parser.add_argument('--negative-packet-loss',
dest='negative_packet_loss_mode',
help='What to do when negative packet loss is encountered',
default = 'quit',
choices = [ 'fail', 'quit', 'retry-to-fail', 'retry-to-quit' ]
default = 'pass',
Copy link
Collaborator

@k-rister k-rister Jun 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot change the default behavior for this result.

choices = [ 'pass', 'fail', 'quit', 'retry-to-fail', 'retry-to-quit' ]
)
parser.add_argument('--search-granularity',
dest='search_granularity',
Expand Down Expand Up @@ -1887,9 +1887,10 @@ def evaluate_trial(trial_params, trial_stats):
if trial_stats['directional'][direction]['active']:
if trial_stats['directional'][direction]['rx_lost_packets_pct'] < 0:
trial_result = trial_params['negative_packet_loss_mode']
bs_logger("\t(trial failed requirement, negative direction packet loss, direction: %s, trial result status: modified, trial result: %s)" %
(direction,
trial_result))
if trial_result != 'pass':
bs_logger("\t(trial failed requirement, negative direction packet loss, direction: %s, trial result status: modified, trial result: %s)" %
(direction,
trial_result))
Comment on lines 1889 to +1893
Copy link
Collaborator

@k-rister k-rister Jun 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way this is written it is possible that if other trial criteria would have already resulted in a failing trial result that encountering a negative packet loss "error" would negate that existing failure when --negative-packet-loss=pass was used because it does not factor in the previous state of trial_result.


requirement_msg = "passed"
result_msg = "unmodified"
Expand All @@ -1905,6 +1906,8 @@ def evaluate_trial(trial_params, trial_stats):
commify(trial_stats['directional'][direction]['rx_lost_packets']),
result_msg,
trial_result))
if trial_result == 'fail':
return trial_result
Comment on lines +1909 to +1910
Copy link
Collaborator

@k-rister k-rister Jun 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this prevents other code in the evaluate_trial function from running which is not something we want to do. We want all criteria to be evaluated so that the user has as clear a picture as possible to understand the behavior observed.

else:
if 'latency_device_pair' in trial_params and trial_params['latency_device_pair'] != '--':
# for the latency device pair, trial_params['loss_granularity'] == 'segment' == 'device'
Expand Down