Skip to content

Fix multiple issues found during code review#6

Merged
hankbao merged 3 commits intomasterfrom
code-review-fix
Feb 3, 2026
Merged

Fix multiple issues found during code review#6
hankbao merged 3 commits intomasterfrom
code-review-fix

Conversation

@hankbao
Copy link
Copy Markdown
Owner

@hankbao hankbao commented Feb 3, 2026

Summary

  1. Fix ICMP checksum verification tautology (|| to && for IPv6 skip logic)
  2. Add parse_error_reply() to handle ICMP Destination Unreachable and Time Exceeded messages by extracting identifier/sequence from the embedded original echo request (IPv4 and IPv6)
  3. Make router failure persistent so all subsequent send() calls fail fast with a consistent error instead of consuming it once
  4. Register in the request registry before send_to() to prevent a race where fast loopback replies arrive before the entry exists
  5. Fix potential underflow in timeout RTT calculation using saturating_sub
  6. Read macOS IPv4 IP header length from IHL nibble instead of hardcoding 20 bytes
  7. Update dependencies: windows 0.62.2, socket2 0.6.2, tokio 1.49.0

- Add parse_error_reply() to handle ICMP Destination Unreachable and Time Exceeded messages by extracting identifier/sequence from the embedded original echo request (IPv4 and IPv6)
- Make router failure persistent so all subsequent send() calls fail fast with a consistent error instead of consuming it once
- Drain in-flight requests on router failure by clearing the registry, causing channel-closed handlers to check the stored error
- Register in the request registry before send_to to prevent a race where fast loopback replies arrive before the entry exists
- Fix verify_checksum() tautology (|| -> && for IPv6 skip logic)
- Fix potential underflow in timeout RTT calculation (saturating_sub)
- Read macOS IPv4 IP header length from IHL nibble instead of hardcoding 20 bytes
@hankbao hankbao merged commit 0824389 into master Feb 3, 2026
4 checks passed
@hankbao hankbao deleted the code-review-fix branch February 3, 2026 05:54
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