summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Rahul Arya <aryarahul@google.com> 2023-05-10 18:55:16 +0000
committer Rahul Arya <aryarahul@google.com> 2023-05-10 18:56:45 +0000
commitdf312c3e82cda24cb37358c2cbb1aecdb5aae93a (patch)
tree895fb170e507393a3bb5bc5a7bb4b9401c002dc4
parent9066159c6aff46c222f996794686b5bec09d457f (diff)
[GATT Server] Simplify API of IsolationManager
Test: unit Bug: 274945531 Change-Id: I956e035b9646dac149357f74e3d4a3d0236a9b0e
-rw-r--r--system/rust/src/gatt/arbiter.rs6
-rw-r--r--system/rust/src/gatt/ffi.rs2
-rw-r--r--system/rust/src/gatt/server.rs9
-rw-r--r--system/rust/src/gatt/server/isolation_manager.rs61
-rw-r--r--system/rust/tests/gatt_server_test.rs2
5 files changed, 36 insertions, 44 deletions
diff --git a/system/rust/src/gatt/arbiter.rs b/system/rust/src/gatt/arbiter.rs
index b6b25a10c6..156d0c9085 100644
--- a/system/rust/src/gatt/arbiter.rs
+++ b/system/rust/src/gatt/arbiter.rs
@@ -51,7 +51,7 @@ fn try_parse_att_server_packet(
tcb_idx: TransportIndex,
packet: Box<[u8]>,
) -> Option<OwnedAttView> {
- isolation_manager.get_conn_id(tcb_idx)?;
+ isolation_manager.get_server_id(tcb_idx)?;
let att = OwnedAttView::try_parse(packet).ok()?;
@@ -82,7 +82,7 @@ fn on_le_connect(tcb_idx: u8, advertiser: u8) {
fn on_le_disconnect(tcb_idx: u8) {
let tcb_idx = TransportIndex(tcb_idx);
- let was_isolated = with_arbiter(|arbiter| arbiter.get_conn_id(tcb_idx).is_some());
+ let was_isolated = with_arbiter(|arbiter| arbiter.is_connection_isolated(tcb_idx));
if was_isolated {
do_in_rust_thread(move |modules| {
if let Err(err) = modules.gatt_module.on_le_disconnect(tcb_idx) {
@@ -112,7 +112,7 @@ fn intercept_packet(tcb_idx: u8, packet: Vec<u8>) -> InterceptAction {
}
fn on_mtu_event(tcb_idx: TransportIndex, event: MtuEvent) {
- if with_arbiter(|arbiter| arbiter.get_conn_id(tcb_idx)).is_some() {
+ if with_arbiter(|arbiter| arbiter.is_connection_isolated(tcb_idx)) {
do_in_rust_thread(move |modules| {
let Some(bearer) = modules.gatt_module.get_bearer(tcb_idx) else {
error!("Bearer for {tcb_idx:?} not found");
diff --git a/system/rust/src/gatt/ffi.rs b/system/rust/src/gatt/ffi.rs
index 57543bb4fc..4f06f0b60b 100644
--- a/system/rust/src/gatt/ffi.rs
+++ b/system/rust/src/gatt/ffi.rs
@@ -431,7 +431,7 @@ fn is_connection_isolated(conn_id: u16) -> bool {
return false;
}
- with_arbiter(|arbiter| arbiter.is_connection_isolated(ConnectionId(conn_id)))
+ with_arbiter(|arbiter| arbiter.is_connection_isolated(ConnectionId(conn_id).get_tcb_idx()))
}
fn send_response(_server_id: u8, conn_id: u16, trans_id: u32, status: u8, value: &[u8]) {
diff --git a/system/rust/src/gatt/server.rs b/system/rust/src/gatt/server.rs
index d6869349c4..ee35a7ce30 100644
--- a/system/rust/src/gatt/server.rs
+++ b/system/rust/src/gatt/server.rs
@@ -83,15 +83,12 @@ impl GattModule {
info!("connected on tcb_idx {tcb_idx:?}");
self.isolation_manager.lock().unwrap().on_le_connect(tcb_idx, advertiser_id);
- let Some(conn_id) = self.isolation_manager.lock().unwrap().get_conn_id(tcb_idx) else {
+ let Some(server_id) = self.isolation_manager.lock().unwrap().get_server_id(tcb_idx) else {
bail!("non-isolated servers are not yet supported (b/274945531)")
};
- let database = self.databases.get(&conn_id.get_server_id());
+ let database = self.databases.get(&server_id);
let Some(database) = database else {
- bail!(
- "got connection to conn_id {conn_id:?} (server_id {:?}) but this server does not exist!",
- conn_id.get_server_id(),
- );
+ bail!("got connection to {server_id:?} but this server does not exist!");
};
let transport = self.transport.clone();
diff --git a/system/rust/src/gatt/server/isolation_manager.rs b/system/rust/src/gatt/server/isolation_manager.rs
index 2d0cfa6dde..b7f32a7b79 100644
--- a/system/rust/src/gatt/server/isolation_manager.rs
+++ b/system/rust/src/gatt/server/isolation_manager.rs
@@ -4,7 +4,7 @@ use std::collections::HashMap;
use log::{error, info};
-use crate::gatt::ids::{AdvertiserId, ConnectionId, ServerId, TransportIndex};
+use crate::gatt::ids::{AdvertiserId, ServerId, TransportIndex};
/// This class is responsible for tracking which connections and advertising we
/// own, and using this information to decide what servers should be exposed to
@@ -12,7 +12,7 @@ use crate::gatt::ids::{AdvertiserId, ConnectionId, ServerId, TransportIndex};
#[derive(Default)]
pub struct IsolationManager {
advertiser_to_server: HashMap<AdvertiserId, ServerId>,
- transport_to_owned_connection: HashMap<TransportIndex, ConnectionId>,
+ transport_to_server: HashMap<TransportIndex, ServerId>,
}
impl IsolationManager {
@@ -20,7 +20,7 @@ impl IsolationManager {
pub fn new() -> Self {
IsolationManager {
advertiser_to_server: HashMap::new(),
- transport_to_owned_connection: HashMap::new(),
+ transport_to_server: HashMap::new(),
}
}
@@ -45,9 +45,9 @@ impl IsolationManager {
self.advertiser_to_server.remove(&advertiser_id);
}
- /// Check if this conn_id is currently owned by the Rust stack
- pub fn is_connection_isolated(&self, conn_id: ConnectionId) -> bool {
- self.transport_to_owned_connection.values().any(|owned_conn_id| *owned_conn_id == conn_id)
+ /// Check if this transport is currently owned by the Rust stack
+ pub fn is_connection_isolated(&self, tcb_idx: TransportIndex) -> bool {
+ self.transport_to_server.contains_key(&tcb_idx)
}
/// Check if this advertiser is tied to a private server
@@ -55,9 +55,9 @@ impl IsolationManager {
self.advertiser_to_server.contains_key(&advertiser_id)
}
- /// Look up the conn_id for a given tcb_idx, if present
- pub fn get_conn_id(&self, tcb_idx: TransportIndex) -> Option<ConnectionId> {
- self.transport_to_owned_connection.get(&tcb_idx).copied()
+ /// Look up the server_id tied to a given tcb_idx, if one exists
+ pub fn get_server_id(&self, tcb_idx: TransportIndex) -> Option<ServerId> {
+ self.transport_to_server.get(&tcb_idx).copied()
}
/// Remove all linked advertising sets from the provided server
@@ -84,10 +84,9 @@ impl IsolationManager {
return;
};
info!("connection is isolated to server {server_id:?}");
- let conn_id = ConnectionId::new(tcb_idx, server_id);
- let old = self.transport_to_owned_connection.insert(tcb_idx, conn_id);
+ let old = self.transport_to_server.insert(tcb_idx, server_id);
if old.is_some() {
- error!("new server {server_id:?} on transport {tcb_idx:?} displacing existing registered connection {conn_id:?}")
+ error!("new server {server_id:?} on transport {tcb_idx:?} displacing existing server {server_id:?}")
}
}
@@ -96,7 +95,7 @@ impl IsolationManager {
/// This event should be supplied from the enclosing module, not directly from the upper layer.
pub fn on_le_disconnect(&mut self, tcb_idx: TransportIndex) {
info!("processing disconnection on transport {tcb_idx:?}");
- self.transport_to_owned_connection.remove(&tcb_idx);
+ self.transport_to_server.remove(&tcb_idx);
}
}
@@ -108,8 +107,6 @@ mod test {
const ADVERTISER_ID: AdvertiserId = AdvertiserId(3);
const SERVER_ID: ServerId = ServerId(4);
- const CONN_ID: ConnectionId = ConnectionId::new(TCB_IDX, SERVER_ID);
-
const ANOTHER_ADVERTISER_ID: AdvertiserId = AdvertiserId(5);
#[test]
@@ -117,9 +114,9 @@ mod test {
let mut isolation_manager = IsolationManager::new();
isolation_manager.on_le_connect(TCB_IDX, Some(ADVERTISER_ID));
- let conn_id = isolation_manager.get_conn_id(TCB_IDX);
+ let server_id = isolation_manager.get_server_id(TCB_IDX);
- assert!(conn_id.is_none())
+ assert!(server_id.is_none())
}
#[test]
@@ -128,9 +125,9 @@ mod test {
isolation_manager.associate_server_with_advertiser(SERVER_ID, ADVERTISER_ID);
isolation_manager.on_le_connect(TCB_IDX, Some(ADVERTISER_ID));
- let conn_id = isolation_manager.get_conn_id(TCB_IDX);
+ let server_id = isolation_manager.get_server_id(TCB_IDX);
- assert_eq!(conn_id, Some(CONN_ID));
+ assert_eq!(server_id, Some(SERVER_ID));
}
#[test]
@@ -139,9 +136,9 @@ mod test {
isolation_manager.associate_server_with_advertiser(SERVER_ID, ADVERTISER_ID);
isolation_manager.on_le_connect(TCB_IDX, Some(ANOTHER_ADVERTISER_ID));
- let conn_id = isolation_manager.get_conn_id(TCB_IDX);
+ let server_id = isolation_manager.get_server_id(TCB_IDX);
- assert!(conn_id.is_none())
+ assert!(server_id.is_none())
}
#[test]
@@ -153,11 +150,11 @@ mod test {
// a new advertiser appeared with the same ID and got a connection
isolation_manager.on_le_connect(TCB_IDX, Some(ADVERTISER_ID));
- let conn_id = isolation_manager.get_conn_id(TCB_IDX);
+ let server_id = isolation_manager.get_server_id(TCB_IDX);
// but we should not be isolated since this is a new advertiser reusing the old
// ID
- assert!(conn_id.is_none())
+ assert!(server_id.is_none())
}
#[test]
@@ -169,10 +166,10 @@ mod test {
// then afterwards we get a connection to this advertiser
isolation_manager.on_le_connect(TCB_IDX, Some(ADVERTISER_ID));
- let conn_id = isolation_manager.get_conn_id(TCB_IDX);
+ let server_id = isolation_manager.get_server_id(TCB_IDX);
// since the server is gone, we should not capture the connection
- assert!(conn_id.is_none())
+ assert!(server_id.is_none())
}
#[test]
@@ -180,10 +177,9 @@ mod test {
let mut isolation_manager = IsolationManager::new();
isolation_manager.associate_server_with_advertiser(SERVER_ID, ADVERTISER_ID);
isolation_manager.on_le_connect(TCB_IDX, Some(ADVERTISER_ID));
- let conn_id = isolation_manager.get_conn_id(TCB_IDX).unwrap();
isolation_manager.clear_advertiser(ADVERTISER_ID);
- let is_isolated = isolation_manager.is_connection_isolated(conn_id);
+ let is_isolated = isolation_manager.is_connection_isolated(TCB_IDX);
assert!(is_isolated)
}
@@ -193,10 +189,9 @@ mod test {
let mut isolation_manager = IsolationManager::new();
isolation_manager.associate_server_with_advertiser(SERVER_ID, ADVERTISER_ID);
isolation_manager.on_le_connect(TCB_IDX, Some(ADVERTISER_ID));
- let conn_id = isolation_manager.get_conn_id(TCB_IDX).unwrap();
isolation_manager.clear_server(SERVER_ID);
- let is_isolated = isolation_manager.is_connection_isolated(conn_id);
+ let is_isolated = isolation_manager.is_connection_isolated(TCB_IDX);
assert!(is_isolated)
}
@@ -208,7 +203,7 @@ mod test {
isolation_manager.on_le_connect(TCB_IDX, Some(ADVERTISER_ID));
isolation_manager.on_le_disconnect(TCB_IDX);
- let is_isolated = isolation_manager.is_connection_isolated(CONN_ID);
+ let is_isolated = isolation_manager.is_connection_isolated(TCB_IDX);
assert!(!is_isolated);
}
@@ -222,9 +217,9 @@ mod test {
isolation_manager.on_le_disconnect(TCB_IDX);
isolation_manager.on_le_connect(TCB_IDX, Some(ADVERTISER_ID));
- let conn_id = isolation_manager.get_conn_id(TCB_IDX);
+ let server_id = isolation_manager.get_server_id(TCB_IDX);
- assert!(conn_id.is_none());
- assert!(!isolation_manager.is_connection_isolated(CONN_ID));
+ assert!(server_id.is_none());
+ assert!(!isolation_manager.is_connection_isolated(TCB_IDX));
}
}
diff --git a/system/rust/tests/gatt_server_test.rs b/system/rust/tests/gatt_server_test.rs
index 7ee6fd7402..20532ab835 100644
--- a/system/rust/tests/gatt_server_test.rs
+++ b/system/rust/tests/gatt_server_test.rs
@@ -651,7 +651,7 @@ fn test_disconnection_unisolates_connection() {
gatt.on_le_disconnect(TCB_IDX).unwrap();
// assert
- let is_connection_isolated = gatt.get_isolation_manager().get_conn_id(TCB_IDX).is_some();
+ let is_connection_isolated = gatt.get_isolation_manager().is_connection_isolated(TCB_IDX);
assert!(!is_connection_isolated);
});
}