Fix ACK_MP preventing bundling a ping when eliciting an ACK

.github/workflows/stable.yml

@@ -9,6 +9,11 @@ env:
 jobs:
   quiche:
     runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        tls-feature:
+          - "" # default, boringssl-vendored
+          - "boringssl-boring-crate"
     # Only run on "pull_request" event for external PRs. This is to avoid
     # duplicate builds for PRs created from internal branches.
     if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository
@@ -30,15 +35,15 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --verbose --all-targets --features=ffi,qlog
+          args: --verbose --all-targets --features=ffi,qlog,${{ matrix.tls-feature }}
 
       # Need to run doc tests separately.
       # (https://github.com/rust-lang/cargo/issues/6669)
       - name: Run cargo doc test
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --verbose --doc --features=ffi,qlog
+          args: --verbose --doc --features=ffi,qlog,${{ matrix.tls-feature }}
 
       - name: Run cargo package
         uses: actions-rs/cargo@v1
@@ -50,13 +55,13 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: clippy
-          args: --features=ffi,qlog -- -D warnings
+          args: --features=ffi,qlog,${{ matrix.tls-feature }} -- -D warnings
 
       - name: Run cargo clippy on examples
         uses: actions-rs/cargo@v1
         with:
           command: clippy
-          args: --examples --features=ffi,qlog -- -D warnings
+          args: --examples --features=ffi,qlog,${{ matrix.tls-feature }} -- -D warnings
 
       - name: Run cargo doc
         uses: actions-rs/cargo@v1

apps/Cargo.toml

@@ -22,15 +22,17 @@ default = ["qlog", "sfv"]
 
 [dependencies]
 docopt = "1"
-env_logger = "0.6"
+env_logger = "0.10"
 mio = { version = "0.8", features = ["net", "os-poll"] }
 url = "1"
 log = "0.4"
 octets = { version = "0.2", path = "../octets" }
-ring = "0.16"
+ring = "0.17"
 quiche = { path = "../quiche" }
 libc = "0.2"
 nix = { version = "0.27", features = ["net", "socket", "uio"] }
+slab = "0.4"
+itertools = "0.10"
 
 [lib]
 crate-type = ["lib"]

apps/src/args.rs

@@ -24,6 +24,9 @@
 // NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 // SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+use std::net::SocketAddr;
+use std::str::FromStr;
+
 use super::common::alpns;
 
 pub trait Args {
@@ -54,6 +57,7 @@ pub struct CommonArgs {
     pub qpack_max_table_capacity: Option<u64>,
     pub qpack_blocked_streams: Option<u64>,
     pub initial_cwnd_packets: u64,
+    pub multipath: bool,
 }
 
 /// Creates a new `CommonArgs` structure using the provided [`Docopt`].
@@ -80,6 +84,7 @@ pub struct CommonArgs {
 /// --qpack-max-table-capacity BYTES  Max capacity of dynamic QPACK decoding.
 /// --qpack-blocked-streams STREAMS  Limit of blocked streams while decoding.
 /// --initial-cwnd-packets      Size of initial congestion window, in packets.
+/// --multipath                 Enable multipath support.
 ///
 /// [`Docopt`]: https://docs.rs/docopt/1.1.0/docopt/
 impl Args for CommonArgs {
@@ -191,6 +196,8 @@ impl Args for CommonArgs {
             .parse::<u64>()
             .unwrap();
 
+        let multipath = args.get_bool("--multipath");
+
         CommonArgs {
             alpns,
             max_data,
@@ -214,6 +221,7 @@ impl Args for CommonArgs {
             qpack_max_table_capacity,
             qpack_blocked_streams,
             initial_cwnd_packets,
+            multipath,
         }
     }
 }
@@ -243,6 +251,7 @@ impl Default for CommonArgs {
             qpack_max_table_capacity: None,
             qpack_blocked_streams: None,
             initial_cwnd_packets: 10,
+            multipath: false,
         }
     }
 }
@@ -271,7 +280,7 @@ Options:
   --dump-responses PATH    Dump response payload as files in the given directory.
   --dump-json              Dump response headers and payload to stdout in JSON format.
   --max-json-payload BYTES  Per-response payload limit when dumping JSON [default: 10000].
-  --connect-to ADDRESS     Override ther server's address.
+  --connect-to ADDRESS     Override the server's address.
   --no-verify              Don't verify server's certificate.
   --trust-origin-ca-pem <file>  Path to the pem file of the origin's CA, if not publicly trusted.
   --no-grease              Don't send GREASE.
@@ -280,6 +289,10 @@ Options:
   --max-active-cids NUM    The maximum number of active Connection IDs we can support [default: 2].
   --enable-active-migration   Enable active connection migration.
   --perform-migration      Perform connection migration on another source port.
+  --multipath              Enable multipath support.
+  -A --address ADDR ...    Specify addresses to be used instead of the unspecified address. Non-routable addresses will lead to connectivity issues.
+  -R --rm-addr TIMEADDR ...   Specify addresses to stop using after the provided time (format time,addr).
+  -S --status TIMEADDRSTAT ...   Specify availability status to advertise to the peer after the provided time (format time,addr,available).
   -H --header HEADER ...   Add a request header.
   -n --requests REQUESTS   Send the given number of identical requests [default: 1].
   --send-priority-update   Send HTTP/3 priority updates if the query string params 'u' or 'i' are present in URLs
@@ -309,6 +322,9 @@ pub struct ClientArgs {
     pub source_port: u16,
     pub perform_migration: bool,
     pub send_priority_update: bool,
+    pub addrs: Vec<SocketAddr>,
+    pub rm_addrs: Vec<(std::time::Duration, SocketAddr)>,
+    pub status: Vec<(std::time::Duration, SocketAddr, bool)>,
 }
 
 impl Args for ClientArgs {
@@ -386,6 +402,57 @@ impl Args for ClientArgs {
 
         let send_priority_update = args.get_bool("--send-priority-update");
 
+        let addrs = args
+            .get_vec("--address")
+            .into_iter()
+            .filter_map(|a| a.parse().ok())
+            .collect();
+
+        let rm_addrs = args
+            .get_vec("--rm-addr")
+            .into_iter()
+            .filter_map(|ta| {
+                let s = ta.split(',').collect::<Vec<_>>();
+                if s.len() != 2 {
+                    return None;
+                }
+                let secs = match s[0].parse::<u64>() {
+                    Ok(s) => s,
+                    Err(_) => return None,
+                };
+                let addr = match SocketAddr::from_str(s[1]) {
+                    Ok(a) => a,
+                    Err(_) => return None,
+                };
+                Some((std::time::Duration::from_secs(secs), addr))
+            })
+            .collect();
+
+        let status = args
+            .get_vec("--status")
+            .into_iter()
+            .filter_map(|ta| {
+                let s = ta.split(',').collect::<Vec<_>>();
+                if s.len() != 3 {
+                    return None;
+                }
+                let secs = match s[0].parse::<u64>() {
+                    Ok(s) => s,
+                    Err(_) => return None,
+                };
+                let addr = match SocketAddr::from_str(s[1]) {
+                    Ok(a) => a,
+                    Err(_) => return None,
+                };
+                let status = match s[2].parse::<u64>() {
+                    Ok(0) => false,
+                    Ok(_) => true,
+                    Err(_) => return None,
+                };
+                Some((std::time::Duration::from_secs(secs), addr, status))
+            })
+            .collect();
+
         ClientArgs {
             version,
             dump_response_path,
@@ -402,6 +469,9 @@ impl Args for ClientArgs {
             source_port,
             perform_migration,
             send_priority_update,
+            addrs,
+            rm_addrs,
+            status,
         }
     }
 }
@@ -424,6 +494,9 @@ impl Default for ClientArgs {
             source_port: 0,
             perform_migration: false,
             send_priority_update: false,
+            addrs: vec![],
+            rm_addrs: vec![],
+            status: vec![],
         }
     }
 }
@@ -464,6 +537,7 @@ Options:
   --disable-gso               Disable GSO (linux only).
   --disable-pacing            Disable pacing (linux only).
   --initial-cwnd-packets PACKETS      The initial congestion window size in terms of packet count [default: 10].
+  --multipath                 Enable multipath support.
   -h --help                   Show this screen.
 ";
 

apps/src/bin/quiche-client.rs

@@ -31,9 +31,7 @@ use quiche_apps::common::*;
 use quiche_apps::client::*;
 
 fn main() {
-    env_logger::builder()
-        .default_format_timestamp_nanos(true)
-        .init();
+    env_logger::builder().format_timestamp_nanos().init();
 
     // Parse CLI parameters.
     let docopt = docopt::Docopt::new(CLIENT_USAGE).unwrap();

apps/src/bin/quiche-server.rs

@@ -58,9 +58,7 @@ fn main() {
     let mut out = [0; MAX_BUF_SIZE];
     let mut pacing = false;
 
-    env_logger::builder()
-        .default_format_timestamp_nanos(true)
-        .init();
+    env_logger::builder().format_timestamp_nanos().init();
 
     // Parse CLI parameters.
     let docopt = docopt::Docopt::new(SERVER_USAGE).unwrap();
@@ -124,6 +122,7 @@ fn main() {
     config.set_initial_congestion_window_packets(
         usize::try_from(conn_args.initial_cwnd_packets).unwrap(),
     );
+    config.set_multipath(conn_args.multipath);
 
     config.set_max_connection_window(conn_args.max_window);
     config.set_max_stream_window(conn_args.max_stream_window);
@@ -512,13 +511,9 @@ fn main() {
             }
 
             // Provides as many CIDs as possible.
-            while client.conn.source_cids_left() > 0 {
+            while client.conn.scids_left() > 0 {
                 let (scid, reset_token) = generate_cid_and_reset_token(&rng);
-                if client
-                    .conn
-                    .new_source_cid(&scid, reset_token, false)
-                    .is_err()
-                {
+                if client.conn.new_scid(&scid, reset_token, false).is_err() {
                     break;
                 }
 
@@ -547,16 +542,24 @@ fn main() {
                     client.max_datagram_size *
                     client.max_datagram_size;
             let mut total_write = 0;
-            let mut dst_info = None;
+            let mut dst_info: Option<quiche::SendInfo> = None;
 
             while total_write < max_send_burst {
-                let (write, send_info) = match client
-                    .conn
-                    .send(&mut out[total_write..max_send_burst])
-                {
+                let res = match dst_info {
+                    Some(info) => client.conn.send_on_path(
+                        &mut out[total_write..max_send_burst],
+                        Some(info.from),
+                        Some(info.to),
+                    ),
+                    None =>
+                        client.conn.send(&mut out[total_write..max_send_burst]),
+                };
+
+                let (write, send_info) = match res {
                     Ok(v) => v,
 
                     Err(quiche::Error::Done) => {
+                        continue_write = dst_info.is_some();
                         trace!("{} done writing", client.conn.trace_id());
                         break;
                     },
@@ -602,6 +605,14 @@ fn main() {
 
             trace!("{} written {} bytes", client.conn.trace_id(), total_write);
 
+            if continue_write {
+                trace!(
+                    "{} pause writing and consider another path",
+                    client.conn.trace_id()
+                );
+                break;
+            }
+
             if total_write >= max_send_burst {
                 trace!("{} pause writing", client.conn.trace_id(),);
                 continue_write = true;
@@ -703,7 +714,8 @@ fn handle_path_events(client: &mut Client) {
                 client
                     .conn
                     .probe_path(local_addr, peer_addr)
-                    .expect("cannot probe");
+                    .map_err(|e| error!("cannot probe: {}", e))
+                    .ok();
             },
 
             quiche::PathEvent::Validated(local_addr, peer_addr) => {
@@ -713,6 +725,13 @@ fn handle_path_events(client: &mut Client) {
                     local_addr,
                     peer_addr
                 );
+                if client.conn.is_multipath_enabled() {
+                    client
+                        .conn
+                        .set_active(local_addr, peer_addr, true)
+                        .map_err(|e| error!("cannot set path active: {}", e))
+                        .ok();
+                }
             },
 
             quiche::PathEvent::FailedValidation(local_addr, peer_addr) => {
@@ -724,12 +743,14 @@ fn handle_path_events(client: &mut Client) {
                 );
             },
 
-            quiche::PathEvent::Closed(local_addr, peer_addr) => {
+            quiche::PathEvent::Closed(local_addr, peer_addr, err, reason) => {
                 info!(
-                    "{} Path ({}, {}) is now closed and unusable",
+                    "{} Path ({}, {}) is now closed and unusable; err = {} reason = {:?}",
                     client.conn.trace_id(),
                     local_addr,
-                    peer_addr
+                    peer_addr,
+                    err,
+                    reason,
                 );
             },
 
@@ -751,6 +772,15 @@ fn handle_path_events(client: &mut Client) {
                     peer_addr
                 );
             },
+
+            quiche::PathEvent::PeerPathStatus(addr, path_status) => {
+                info!("Peer asks status {:?} for {:?}", path_status, addr,);
+                client
+                    .conn
+                    .set_path_status(addr.0, addr.1, path_status, false)
+                    .map_err(|e| error!("cannot follow status request: {}", e))
+                    .ok();
+            },
         }
     }
 }