Skip to content

ACKs are delivered in order panic when packets are reordered #1889

@tthebst

Description

@tthebst

I was trying to upgrade to quinn 0.11.2 and I have some tests where I implement AsyncUdpSocket with turmoil and my tests panic with ACKs are delivered in order (line 392 in mtud.rs) which originates from the BlackHoleDetector . After some experimentation I found that this happens because in our test with turmoil we have reordering of packets.

The issue can be reproduced if you locally add some reordering sudo tc qdisc add dev lo root netem delay 50ms 100ms and send some messages between a client and a server.

thread 'tokio-runtime-worker' panicked at quinn-proto/src/connection/mtud.rs:392:9:
ACKs are delivered in order
stack backtrace:
hello
   0: rust_begin_unwind
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/panicking.rs:72:14
   2: quinn_proto::connection::mtud::BlackHoleDetector::on_non_probe_acked
             at ./quinn-proto/src/connection/mtud.rs:392:9
   3: quinn_proto::connection::mtud::MtuDiscovery::on_acked
             at ./quinn-proto/src/connection/mtud.rs:105:13
   4: quinn_proto::connection::Connection::on_ack_received
             at ./quinn-proto/src/connection/mod.rs:1387:35
   5: quinn_proto::connection::Connection::process_payload
             at ./quinn-proto/src/connection/mod.rs:2670:21
   6: quinn_proto::connection::Connection::process_decrypted_packet
             at ./quinn-proto/src/connection/mod.rs:2299:38
   7: quinn_proto::connection::Connection::handle_packet
             at ./quinn-proto/src/connection/mod.rs:2237:21
   8: quinn_proto::connection::Connection::handle_decode
             at ./quinn-proto/src/connection/mod.rs:2133:13
   9: quinn_proto::connection::Connection::handle_event
             at ./quinn-proto/src/connection/mod.rs:1069:17
  10: quinn::connection::State::process_conn_events
             at ./quinn/src/connection.rs:1026:21
  11: <quinn::connection::ConnectionDriver as core::future::future::Future>::poll
             at ./quinn/src/connection.rs:230:25
  12: quinn::connection::Connecting::new::{{closure}}
             at ./quinn/src/connection.rs:66:40
  13: <tracing::instrument::Instrumented<T> as core::future::future::Future>::poll

This PR should fix the issue. It removes the ACK ordering requirement for non mtu probe acks.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions