mirror of
https://github.com/n0-computer/noq.git
synced 2026-10-23 20:56:24 +00:00
Philipp Krüger
89ed00c312
## Description Closes #565 ## Breaking Changes Only additions: - Adds `ConnectionStats::sent_packets`, `ConnectionStats::sent_bytes`, `PathStats::sent_packets`, `PathStast::sent_bytes` ## Notes & open questions The guiding principle behind this PR is a central use case of continually monitoring `sent_packets` and `lost_packets` as an indicator of "how good" our connection (or path) is doing and how healthy it is. Previously, this was not possible, because `lost_packets` can't really be seen relative to `UdpStats::datagrams` due to coalescing and generally some other inconsistencies. In this PR I'm taking great care to make sure `sent_packets` matches `lost_packets` once every single packet gets lost, and maintaining the invariant that `lost_packets <= sent_packets` always (I had added a `debug_assert!` for this at some point, not sure if people would prefer adding that back?). ~~Also, `lost_packets` only counts packets that we had "expected to make it", but got lost. I.e. it does *not* count off-path/NAT traversal challenges and responses nor MTUD probes. I'm matching this behavior with `sent_packets` now.~~ We decided against this after some discussion. `sent_packets` is just what it says - the packets sent, regardless whether those are probes or sent on a path that we expect to be alive. Detecting if the current packet is an MTUD probe is somewhat ugly... I decided to use `conn.path_data(path_id).mtud.in_flight_mtu_probe() == Some(packet_number)` for this, but could also have matched on `PadDatagram::ToSize`, as that's only used by MTUD probes, but the former seemed clearer. ## Change checklist - [x] Self-review. - [x] Documentation updates following the [style guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text), if relevant. - [x] Tests if relevant. - [x] All breaking changes documented.