Skip to content

Ignore received packets that weren't sent during examine_flow #13203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
StormLiangMS merged 4 commits into from
Jul 8, 2024
Merged

Ignore received packets that weren't sent during examine_flow #13203

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
54aa8c2
Handle received packets that weren't sent during examine_flow
Ryangwaite Jun 10, 2024
a4cbabb
Improved logging of packets that were both sent and received
Ryangwaite Jun 24, 2024
5056c12
Initialize prev_payload to -1 since this packet doesnt exist
Ryangwaite Jun 24, 2024
6467d01
Replaced try-catch with if-else + key in list check
Ryangwaite Jul 3, 2024
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
80 changes: 59 additions & 21 deletions ansible/roles/test/files/ptftests/py3/advanced-reboot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,7 @@ def sad_revert(self):

def setUp(self):
self.fails['dut'] = set()
self.fails['infrastructure'] = set()
self.dut_mac = self.test_params['dut_mac']
self.vlan_mac = self.test_params['vlan_mac']
self.lo_prefix = self.test_params['lo_prefix']
Expand Down Expand Up @@ -1943,14 +1944,17 @@ def examine_flow(self, filename=None):
self.lost_packets = dict()
self.max_disrupt, self.total_disruption = 0, 0
sent_packets = dict()
# Track packet id's that were neither sent or received
missing_sent_and_received_packet_id_sequences = []
self.fails['dut'].add("Sniffer failed to capture any traffic")
self.assertTrue(packets, "Sniffer failed to capture any traffic")
self.fails['dut'].clear()
prev_payload = None
if packets:
prev_payload, prev_time = 0, 0
prev_payload, prev_time = -1, 0
Copy link
Contributor Author

@Ryangwaite Ryangwaite Jun 25, 2024

Choose a reason for hiding this comment

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

There was a problem where if the packet with packet id 0 (i.e. the first packet) was absent, it wouldn't be included in the analysis below. Changed to a non-packet ID by default to resolve this.

sent_payload = 0
received_counter = 0 # Counts packets from dut.
received_but_not_sent_packets = set()
sent_counter = 0
received_t1_to_vlan = 0
received_vlan_to_t1 = 0
Expand Down Expand Up @@ -1985,31 +1989,60 @@ def examine_flow(self, filename=None):
prev_time = received_time
continue
if received_payload - prev_payload > 1:
# Packets in a row are missing, a disruption.
if received_payload not in sent_packets:
self.log("Ignoring received packet with payload {}, as it was not sent".format(
received_payload))
received_but_not_sent_packets.add(received_payload)
continue
Copy link
Contributor

@vaibhavhd vaibhavhd Jun 18, 2024

Choose a reason for hiding this comment

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

This will miss the failure case - when there is a received packet drop and also a sent packet drop. We may not care if sent packets are not captured in pcap. But any drop in received packet should be reported.

Do not fail immediately upon noticing a received packet drop. We want to measure all received packet drops before concluding a failed state.

Copy link
Contributor Author

@Ryangwaite Ryangwaite Jun 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

For the branch highlighted here, it handles the case where a received packet was present but no corresponding send packet in which case it logs it out, takes note of the received but not sent packet and carries on processing.

Further below, in the try-except while-loop it determines if packet IDs were missed for both send and receive from the pcaps and if so tracks them and continues processing.

Once all packets have been processed, if there were any that were missed from both send and received then these are logged out as a failure and the test will fail.

# Packets in a row are missing, a potential disruption.
self.log("received_payload: {}, prev_payload: {}, sent_counter: {}, received_counter: {}".format(
received_payload, prev_payload, sent_counter, received_counter))
# How many packets lost in a row.
lost_id = (received_payload - 1) - prev_payload
# How long disrupt lasted.
disrupt = (
sent_packets[received_payload] - sent_packets[prev_payload + 1])
# Add disrupt to the dict:
self.lost_packets[prev_payload] = (
lost_id, disrupt, received_time - disrupt, received_time)
self.log("Disruption between packet ID %d and %d. For %.4f " % (
prev_payload, received_payload, disrupt))
for lost_index in range(prev_payload + 1, received_payload):
# lost received for packet sent from vlan to T1.
if (lost_index % 5) == 0:
missed_vlan_to_t1 += 1

# Find previous sequential sent packet that was captured
missing_sent_and_received_pkt_count = 0
prev_pkt_pt = prev_payload + 1
prev_sent_packet_time = None
while prev_pkt_pt < received_payload:
if prev_pkt_pt in sent_packets:
prev_sent_packet_time = sent_packets[prev_pkt_pt]
break # Found it
else:
missed_t1_to_vlan += 1
self.log("")
if not self.disruption_start:
self.disruption_start = datetime.datetime.fromtimestamp(
prev_time)
self.disruption_stop = datetime.datetime.fromtimestamp(
received_time)
if prev_pkt_pt not in received_but_not_sent_packets:
missing_sent_and_received_pkt_count += 1
prev_pkt_pt += 1
if missing_sent_and_received_pkt_count > 0:
missing_sent_and_received_packet_id_sequences_fmtd = \
str(prev_payload + 1) if missing_sent_and_received_pkt_count == 1\
else "{}-{}".format(prev_payload + 1, received_payload - 1)
missing_sent_and_received_packet_id_sequences.append(
missing_sent_and_received_packet_id_sequences_fmtd)
if prev_sent_packet_time is not None:
# Disruption occurred - some sent packets were not received

# How long disrupt lasted.
this_sent_packet_time = sent_packets[received_payload]
disrupt = this_sent_packet_time - prev_sent_packet_time

# Add disrupt to the dict:
self.lost_packets[prev_payload] = (
lost_id, disrupt, received_time - disrupt, received_time)
self.log("Disruption between packet ID %d and %d. For %.4f " % (
prev_payload, received_payload, disrupt))
for lost_index in range(prev_payload + 1, received_payload):
# lost received for packet sent from vlan to T1.
if lost_index in sent_packets:
if (lost_index % 5) == 0:
missed_vlan_to_t1 += 1
else:
missed_t1_to_vlan += 1
self.log("")
if not self.disruption_start:
self.disruption_start = datetime.datetime.fromtimestamp(
prev_time)
self.disruption_stop = datetime.datetime.fromtimestamp(
received_time)
prev_payload = received_payload
prev_time = received_time
self.log(
Expand Down Expand Up @@ -2043,6 +2076,11 @@ def examine_flow(self, filename=None):
self.total_disrupt_time = 0
self.log("Gaps in forwarding not found.")

if missing_sent_and_received_packet_id_sequences:
self.fails["infrastructure"].add(
"Missing sent and received packets: {}"
.format(missing_sent_and_received_packet_id_sequences))

self.dataplane_loss_checked_successfully = True

if self.reboot_type == "fast-reboot" and not self.lost_packets:
Expand Down