* fix: Do not send off-path data when closing
When we are closing we're only supposed to send packets containing
CONNECTION_CLOSE and (PATH_)ACK frames. There is no need to continue
sending any off-path data.
* maybe fix
* revert that. it was wrong
* Apply suggestion from @matheus23
Co-authored-by: Philipp Krüger <philipp.krueger1@gmail.com>
---------
Co-authored-by: Philipp Krüger <philipp.krueger1@gmail.com>
* fix(proto): Don't close paths from previous HP round we're still interested in
* Use local/remote instead of src/dst in 4-tuple logs
* Don't clear all path timers in `close_path`.
* Also check for `!self.abandoned_paths.contains(&path_id)`
* refactor(proto): Canonicalize IP addrs early for HP state
* Spellcheck
* Revert canonicalization to the right places again.
We don't want to canonicalize when adding local addresses, since there might be NAT46 between us and the peer.
We also don't want to canonicalize right when we receive ADD_ADDRESS frames, since that has an effect on maximum number of addresses calculations.
* Preserve address mapping invariant in `iroh_hp::NatTraversalRound`
* Update doc comments
* Map ipv6-mapped IPv4 addresses to IPv4 if possible/necessary
* Don't convert `::1` to `127.0.0.1` in `map_to_local_socket_family`
Also: add a test case
* Prefer `Ipv4Addr::to_ipv4_mapped`
* split up poll_transmit per path and per space
* refactor: return last_packet_number
* cleanup status enums
* refactor: streamline mtu probe building
* refactor: extract remote cid exhaustion handling into function
* refactor: move remote_cid check per path
* refactor: simplify send space check
* fixup
* docs(proto): describe enums
* wip
* friday was too short. make it work, maybe
* Do coalescing right
* tweak comments, logs
* remove redundant size-setting
* fix path scheduling description, add bug description
* fixup path stats
* wording, wording
* weird off path enum variants are no longer needed!
* fix fmt
* Make TransmitBuf in each loop and collate off-path work
This make the construction of the TransmitBuf a little less weird, but
still a bit weird.
* Consistent naming for now
* missing rename
* fix merge
* Pass around a Vec while no transmit has been started yet
* expand on what is meant here, link to new issue
* pointless return
---------
Co-authored-by: dignifiedquire <me@dignifiedquire.com>
Co-authored-by: Diva Martínez <26765164+divagant-martian@users.noreply.github.com>
Co-authored-by: Diva Martínez <git@divma.net>
With multipath there is no guarantee that CIDs exist. Since this
previous PathData is currently only used to send a single
fire-and-forget PATH_CHALLENGE there is no value to store it as the
previous path, since it would not be able to be sent. And this path
would still have to be path-validated from scratch if it is the valid
one.
Additionally fix a bug that only stored this path if the previous path
was *currently* validating. This was a bug introduced due to
refactors. The intention is to store the path only if it was
path-validated. Not to store it only if it was currently not being
validated.
Fixes#310
Also, removes "log" from list of features for `iroh-quinn-proto` to run with `docs-rs`, since it is not a feature `iroh-quinn-proto` has
Co-authored-by: “ramfox” <“kasey@n0.computer”>
* fix(multipath): handle missing remote CIDs better
QUIC-MULTIPATH § 4.7 says that sending PATH_CIDS_BLOCKED is to be done
when opening the path. For locally opened paths we were already doing
this but for remotely opened paths we did not yet do that.
Instead it was deferred to poll_transmit to send this, but that is
troublesome:
- Primarily it means that when checking paths suddenly more data is
available to be sent and should have been sent on an earlier
path. Making the logic of poll_transmit very difficult.
- It meant precaution should be taken against sending this too much,
which wasn't being done.
So instead we schedule PATH_CIDS_BLOCKED when the paths are actually
being opened. Simplifying things significantly.
* needless map
Though the compiler should have optimised that away
* actually read the review text
* remove path_challenge comment
* explicit false
* Apply suggestions from code review
Co-authored-by: Diva Martínez <26765164+divagant-martian@users.noreply.github.com>
* Do not log when an abandoned path has retired CIDs
That's the normal situation, it can get fairly verbose, and you should
be able to deduce this from other logs.
* fix broken merge
---------
Co-authored-by: Diva Martínez <26765164+divagant-martian@users.noreply.github.com>
* Implement encode-decode tests for ack
And some DRY
* Add path_ack as well
* Get rid of TestFrame
* clippy
* fmt
* Remove more explicit strategies
* Use StreamMeta for encoding stream frames instead of duplicating the logic
* Make the arb for RangeSet also create the empty set
Also exchange unwrap to expect in the arbs that use it
* PR feedback
- Derive arbs unless they are really complex
- Test remaining is empty in encode/decode proptest
* fix(proto): Don't close paths from previous HP round we're still interested in
* Use local/remote instead of src/dst in 4-tuple logs
* Don't clear all path timers in `close_path`.
* Also check for `!self.abandoned_paths.contains(&path_id)`
* Add proptests for encode/decode
* Add proptest arbitrary for VarInt and StreamId
Also fix the StreamId arbitrary::Arbitrary generation
* Add proptest arbitraries for various quinn-proto types and make use of them in the random interaction test
* Add more arbitraries.
* Remove unneeded derives
* Use MaybeFrame arb
* fmt
* Use cfg(test) for the arbitraries.
Also move the encode_decode tests into a separate file.
* Add more cases to TestFrame
* Get rid of the last remaining record enum cases.
* More arbitraries. Now almost every frame type is roundrrip tested.
* remove dead code
* clippy
* Add proptest and test-strategy even in wasm
(we don't *run* proptests, but need them to compile)
* Use a feature flag after all...
going in circles...
* Remove claude settings and DRY the proptest attribute.
* Revert "Use a feature flag after all..."
This reverts commit b27add632b.
* Try to get proptest to work in wasm after all
It doesn't need to run tests, just compile.
* Use the right hmac import
* sort dev deps
* Remove some feature flag gated imports
they are not worth it in terms of DRY
* Use test_strategy::Arbitrary by default in frame. It gets used way more often.
* refactor: Update to rust edition 2025
* bump to 1.88
That's what I need for let chains.
* i asked for let chain, clippy looooooves the let chains!
* more let chains in the workspace
* fix(proto): Some frames were sent on non-validated path
We were accidentally sending frames on non-validated paths. And also
on PATH_STATUS_BACKUP paths.
With the current way this is controlled when path_exclusive_only is
set we should not be sending frames that can be sent on other
paths. Respecting this for more frame types will make sure they are
not sent on non-validated and backup paths.
Fixes#314.
* refactor: Update to rust edition 2025
* bump to 1.88
That's what I need for let chains.
* i asked for let chain, clippy looooooves the let chains!
* more let chains in the workspace
* fix(proto): Use most relevant PTOs for PATH_ABANDON calculations
Instead of always using max PTO.
* refactor: Use a nice enum
* fix merge
* cw: Use proper `TransportError`s, improve comments
* cw: Clear up confusing comment
Before they were decoded lazily, but that work as done many times over and over again, when they where ever logged in debug mode. This decodes these immediately and stores them in an `ArrayRangeSet`
Also moves the debug impl to the `ArrayRangeSet` making that print more easily
* fix getting frame types
* make stat recording readable
* create the base to what we want to achieve
* add path_ack to Builder::encode
* add Ack and Close
* fix finish and finish and track
* add path response
* add HandshakeDone and ReahOut
* add ObservedAddr
* add Ping and ImmediateAck
* add more frames
* add moreeee frames
* sip
* wip
* wip
* wip
* wip
* restore qlog to work on a reduced diff
* compiles
* wip
* Fix PacketBuilder::simple_data_buf usage
- do not request 100 bytes padding by default
- do not reseve space for the AEAD tag
- when using this make sure the datagram is started and has the
correct size
* cleanup
* spelling
* make diff more obvious
* reduce LOC
* apparently these lifetimes can be elided?
* 1 LOC less
* more cleanup
* reverse change
* reduce LOCs
* Encode impl no longer needed
* remove comment
* clippy
* remove unnecesary lifetimes
* remove nice but deprecated code :(
* remove files that shouldn't be there
* leverage enum_assoc
* rename `PacketBuilder`'s `encode` to `write_frame`
* add display impls for frames
* restrict function visibility
* expand restricted vis
* shutup clippy
---------
Co-authored-by: Floris Bruynooghe <flub@n0.computer>
Co-authored-by: Friedel Ziegelmayer <me@dignifiedquire.com>
When adding items to this we generally don't want to worry about
checking if the value is already pending. Currently we keep adding to
the Vec with the same value in some situations, resulting in sending
the frame many times for the same path.
This avoids duplicate entries.
With this we no longer get compile errors in vscode because the fuzzing
code uses fns that are themselves gated with cfg(fuzzing).
We have to tell rust about the feature in Cargo.toml to avoid warnings.
When dropping oversized datagrams after an MTU change, notify
callers of `send_datagram_wait` that are waiting for
space in the send buffer.
Tracking issue: https://github.com/quinn-rs/quinn/issues/2456
Ruediger Klaehn