diff options
author | 2023-05-10 18:55:16 +0000 | |
---|---|---|
committer | 2023-05-10 18:56:45 +0000 | |
commit | df312c3e82cda24cb37358c2cbb1aecdb5aae93a (patch) | |
tree | 895fb170e507393a3bb5bc5a7bb4b9401c002dc4 | |
parent | 9066159c6aff46c222f996794686b5bec09d457f (diff) |
[GATT Server] Simplify API of IsolationManager
Test: unit
Bug: 274945531
Change-Id: I956e035b9646dac149357f74e3d4a3d0236a9b0e
-rw-r--r-- | system/rust/src/gatt/arbiter.rs | 6 | ||||
-rw-r--r-- | system/rust/src/gatt/ffi.rs | 2 | ||||
-rw-r--r-- | system/rust/src/gatt/server.rs | 9 | ||||
-rw-r--r-- | system/rust/src/gatt/server/isolation_manager.rs | 61 | ||||
-rw-r--r-- | system/rust/tests/gatt_server_test.rs | 2 |
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); }); } |