[bt][bt-gap] RestoreBonds in chunks of 16
When invoking fuchsia.bluetooth.host.Host/RestoreBonds, bt-gap will use
at most MAX_BONDS_PER_RESTORE_BONDS bonds in each successive call until
all bonds are restored from secure stash. This prevents any call from
exceeding message size limits.
Bug: fxbug.dev/80564
Test: fx test -o bt-gap-unittests (new host_device_restore_bonds test)
Manually reboot astro using MAX_BONDS_PER_RESTORE_BONDS = 2 and
observe that all 5 device bonds are restored over 3 calls
Change-Id: Iecf4cd825d9966c5abc139dd0b5ad4e421f5367b
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/559781
Commit-Queue: Xo Wang <xow@google.com>
Reviewed-by: Jeff Belgum <belgum@google.com>
diff --git a/src/connectivity/bluetooth/core/bt-gap/BUILD.gn b/src/connectivity/bluetooth/core/bt-gap/BUILD.gn
index afaa1dd..6ffe4a2 100644
--- a/src/connectivity/bluetooth/core/bt-gap/BUILD.gn
+++ b/src/connectivity/bluetooth/core/bt-gap/BUILD.gn
@@ -79,6 +79,8 @@
"src/watch_peers.rs",
]
+ test_deps = [ "//src/lib/test_util" ]
+
configs -= [ "//build/config/rust:allow_unused_results" ]
}
diff --git a/src/connectivity/bluetooth/core/bt-gap/src/host_device.rs b/src/connectivity/bluetooth/core/bt-gap/src/host_device.rs
index acb3330..2c8d657 100644
--- a/src/connectivity/bluetooth/core/bt-gap/src/host_device.rs
+++ b/src/connectivity/bluetooth/core/bt-gap/src/host_device.rs
@@ -11,7 +11,7 @@
inspect::Inspectable,
types::{Address, BondingData, HostData, HostId, HostInfo, Peer, PeerId},
},
- futures::{Future, FutureExt, StreamExt, TryFutureExt},
+ futures::{future::try_join_all, Future, FutureExt, StreamExt, TryFutureExt},
log::{error, info, trace, warn},
parking_lot::RwLock,
pin_utils::pin_mut,
@@ -177,10 +177,25 @@
&self,
bonds: Vec<BondingData>,
) -> impl Future<Output = types::Result<Vec<sys::BondingData>>> {
- self.0
- .proxy
- .restore_bonds(&mut bonds.into_iter().map(sys::BondingData::from))
- .map_err(|e| e.into())
+ // TODO(fxbug.dev/80564): Due to the maximum message size, the RestoreBonds call has an
+ // upper limit on the number of bonds that may be restored at once. However, this limit is
+ // based on the complexity of fields packed into sys::BondingData, which can be measured
+ // dynamically with measure-tape as the vector is built. Maximizing the number of bonds per
+ // call may improve performance. Instead, a conservative fixed maximum is chosen here (at
+ // this time, the maximum message size is 64 KiB and a fully-populated BondingData without
+ // peer service records is close to 700 B).
+ const MAX_BONDS_PER_RESTORE_BONDS: usize = 16;
+ let bonds_chunks = bonds.chunks(MAX_BONDS_PER_RESTORE_BONDS);
+
+ // Bonds that can't be restored are sent back in Ok(Vec<_>), which would not cause
+ // try_join_all to bail early
+ try_join_all(bonds_chunks.map(|c| {
+ self.0
+ .proxy
+ .restore_bonds(&mut c.into_iter().cloned().map(sys::BondingData::from))
+ .map_err(|e| e.into())
+ }))
+ .map_ok(|v| v.into_iter().flatten().collect())
}
pub fn set_connectable(&self, value: bool) -> impl Future<Output = types::Result<()>> {
diff --git a/src/connectivity/bluetooth/core/bt-gap/src/test/host_device.rs b/src/connectivity/bluetooth/core/bt-gap/src/test/host_device.rs
index 031dd24..9fd3ab0 100644
--- a/src/connectivity/bluetooth/core/bt-gap/src/test/host_device.rs
+++ b/src/connectivity/bluetooth/core/bt-gap/src/test/host_device.rs
@@ -7,10 +7,13 @@
fidl::endpoints::RequestStream,
fidl_fuchsia_bluetooth_host::{HostControlHandle, HostMarker, HostRequest, HostRequestStream},
fidl_fuchsia_bluetooth_sys::HostInfo as FidlHostInfo,
- fuchsia_bluetooth::types::{Address, BondingData, HostId, HostInfo, Peer, PeerId},
+ fuchsia_bluetooth::types::{
+ bonding_data::example, Address, BondingData, HostId, HostInfo, Peer, PeerId,
+ },
futures::{future, join, stream::StreamExt},
parking_lot::RwLock,
std::{path::Path, sync::Arc},
+ test_util::assert_gt,
};
use crate::host_device::{HostDevice, HostListener};
@@ -118,6 +121,42 @@
Ok(())
}
+// Create a HostDevice with a fake channel, restore bonds, check that the FIDL method is called
+// over multiple paginated iterations
+#[fuchsia_async::run_until_stalled(test)]
+async fn host_device_restore_bonds() -> Result<(), Error> {
+ let (client, server) = fidl::endpoints::create_proxy_and_stream::<HostMarker>()?;
+ let address = Address::Public([0, 0, 0, 0, 0, 0]);
+ let host = HostDevice::mock(HostId(1), address, Path::new("/dev/class/bt-host/test"), client);
+
+ // TODO(fxbug.dev/80564): Assume that 256 bonds is enough to cause HostDevice to use multiple
+ // FIDL calls to transmit all of the bonds (at this time, the maximum message size is 64 KiB
+ // and a fully-populated BondingData without peer service records is close to 700 B).
+ let bonds = (0..=255).map(|i| {
+ example::bond(Address::Public([i, i, i, i, i, i]), Address::Public([i, i, i, i, i, i]))
+ });
+ let restore_bonds = host.restore_bonds(bonds.collect());
+
+ // Create testing Host server that responds to each request with a completed future until
+ // restore_bonds yields a result
+ let mut num_restore_bonds_calls: i32 = 0;
+ let run_host = server.take_until(restore_bonds).for_each(|request| {
+ match request {
+ Ok(HostRequest::RestoreBonds { responder, .. }) => {
+ // Succeed always by not sending back any rejected bonds
+ let _ = responder.send(&mut std::iter::empty());
+ num_restore_bonds_calls += 1;
+ }
+ x => panic!("Expected RestoreBonds Request but got: {:?}", x),
+ }
+ future::ready(())
+ });
+
+ let _ = run_host.await;
+ assert_gt!(num_restore_bonds_calls, 1);
+ Ok(())
+}
+
// TODO(fxbug.dev/39373): Add host.fidl emulation to bt-fidl-mocks and use that instead.
async fn expect_call<F>(stream: Arc<RwLock<HostRequestStream>>, f: F) -> Result<(), Error>
where
