Skip to content

Conversation

@benhillis
Copy link
Member

complete_tx_packet used .unwrap() on the pending packet lookup, which would panic if the backend returned a TxId that was out of range or already completed. Use get_mut().and_then() and log an error instead.

complete_tx_packet used .unwrap() on the pending packet lookup, which
would panic if the backend returned a TxId that was out of range or
already completed. Use get_mut().and_then() and log an error instead.
@benhillis benhillis requested a review from a team as a code owner February 10, 2026 16:21
Copilot AI review requested due to automatic review settings February 10, 2026 16:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates virtio_net TX completion handling to avoid panics when the network backend reports an unexpected TxId, improving robustness at the backend/device boundary.

Changes:

  • Replace unwrap() on pending_tx_packets lookup with a safe get_mut(...).and_then(...) pattern.
  • Log and ignore unexpected TX completions instead of panicking.

Comment on lines +998 to +1005
let Some(mut tx_packet) = state
.pending_tx_packets
.get_mut(id.0 as usize)
.and_then(|p| p.take())
else {
tracing::error!(tx_id = id.0, "unexpected tx completion for unknown packet");
return Ok(());
};
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The log message here says "unknown packet", but this branch also triggers when the TxId is in range and the slot is already None (e.g., duplicate completion). Consider distinguishing the cases (out-of-range vs already-completed) and/or including the pending_tx_packets length in the log fields to make debugging misbehaving backends easier.

Suggested change
let Some(mut tx_packet) = state
.pending_tx_packets
.get_mut(id.0 as usize)
.and_then(|p| p.take())
else {
tracing::error!(tx_id = id.0, "unexpected tx completion for unknown packet");
return Ok(());
};
let idx = id.0 as usize;
let pending_len = state.pending_tx_packets.len();
let Some(slot) = state.pending_tx_packets.get_mut(idx) else {
tracing::error!(
tx_id = id.0,
tx_index = idx,
pending_tx_packets_len = pending_len,
"unexpected tx completion for out-of-range packet"
);
return Ok(());
};
let Some(mut tx_packet) = slot.take() else {
tracing::error!(
tx_id = id.0,
tx_index = idx,
pending_tx_packets_len = pending_len,
"unexpected tx completion for already-completed packet"
);
return Ok(());
};

Copilot uses AI. Check for mistakes.
@benhillis benhillis marked this pull request as draft February 10, 2026 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant