From b8cc37945db8cd016e4f9bc16761f1717b29e435 Mon Sep 17 00:00:00 2001 From: Brad Campbell Date: Mon, 20 May 2024 14:56:26 -0400 Subject: [PATCH] Revert "Merge pull request #3851 from tyler-potyondy/15.4-syscall" This reverts commit 81602d4894bb5bb9e5157b5ea2876a902996ba1d, reversing changes made to 20246e6b891fd3fe069b6686c167aa337f04864e. --- capsules/extra/src/ieee802154/device.rs | 19 -- capsules/extra/src/ieee802154/driver.rs | 176 ++++++------------- capsules/extra/src/ieee802154/framer.rs | 135 +++----------- capsules/extra/src/ieee802154/virtual_mac.rs | 8 - 4 files changed, 77 insertions(+), 261 deletions(-) diff --git a/capsules/extra/src/ieee802154/device.rs b/capsules/extra/src/ieee802154/device.rs index 30fb6e79ff..a3f28c2ae1 100644 --- a/capsules/extra/src/ieee802154/device.rs +++ b/capsules/extra/src/ieee802154/device.rs @@ -73,25 +73,6 @@ pub trait MacDevice<'a> { security_needed: Option<(SecurityLevel, KeyId)>, ) -> Result; - /// Creates an IEEE 802.15.4 Frame object that is compatible with the - /// MAC transmit and append payload methods. This serves to provide - /// functionality for sending packets fully formed by the userprocess - /// and that the 15.4 capsule does not modify. The len field may be less - /// than the length of the buffer as the len field is the length of - /// the current frame while the buffer is the maximum 15.4 frame size. - /// - /// - `buf`: The buffer to be used for the frame - /// - `len`: The length of the frame - /// - /// Returns a Result: - /// - on success a Frame object. - /// - on failure an error returning the buffer. - fn buf_to_frame( - &self, - buf: &'static mut [u8], - len: usize, - ) -> Result; - /// Transmits a frame that has been prepared by the above process. If the /// transmission process fails, the buffer inside the frame is returned so /// that it can be re-used. diff --git a/capsules/extra/src/ieee802154/driver.rs b/capsules/extra/src/ieee802154/driver.rs index 13294601df..7ef11aafd3 100644 --- a/capsules/extra/src/ieee802154/driver.rs +++ b/capsules/extra/src/ieee802154/driver.rs @@ -225,41 +225,9 @@ impl KeyDescriptor { } } -/// Denotes the type of pending transmission. A `Parse(..)` PendingTX -/// indicates that the 15.4 framer will need to form the packet header -/// from the provided address and security level. A `Raw` PendingTX -/// is formed by the userprocess and passes through the Framer unchanged. -enum PendingTX { - Parse(u16, Option<(SecurityLevel, KeyId)>), - Raw, - Empty, -} - -impl Default for PendingTX { - /// The default PendingTX is `Empty` - fn default() -> Self { - PendingTX::Empty - } -} - -impl PendingTX { - /// Returns true if the PendingTX state is `Empty` - fn is_empty(&self) -> bool { - match self { - PendingTX::Empty => true, - _ => false, - } - } - - /// Take the pending transmission, replacing it with `Empty` and return the current PendingTx - fn take(&mut self) -> PendingTX { - core::mem::replace(self, PendingTX::Empty) - } -} - #[derive(Default)] pub struct App { - pending_tx: PendingTX, + pending_tx: Option<(u16, Option<(SecurityLevel, KeyId)>)>, } pub struct RadioDriver<'a, M: device::MacDevice<'a>> { @@ -463,7 +431,7 @@ impl<'a, M: device::MacDevice<'a>> RadioDriver<'a, M> { for app in self.apps.iter() { let processid = app.processid(); app.enter(|app, _| { - if !app.pending_tx.is_empty() { + if app.pending_tx.is_some() { pending_app = Some(processid); } }); @@ -493,77 +461,56 @@ impl<'a, M: device::MacDevice<'a>> RadioDriver<'a, M> { /// idle and the app has a pending transmission. #[inline] fn perform_tx_sync(&self, processid: ProcessId) -> Result<(), ErrorCode> { - self.apps.enter(processid, |app, kernel_data| { - // The use of this take method is somewhat overkill, but serves to ensure that - // this, or future, implementations do not forget to "remove" the pending_tx - // from the app after processing. - let curr_tx = app.pending_tx.take(); - - // Before beginning the transmission process, confirm that the PendingTX - // is not Empty. In the Empty case, there is nothing to transmit and we - // can return Ok(()) immediately as there is nothing to transmit. - if let PendingTX::Empty = curr_tx { return Ok(()) } - - // At a high level, we must form a Frame from the provided userproceess data, - // place this frame into a static buffer, and then transmit the frame. This is - // somewhat complicated by the need to error handle each of these steps and also - // provide Raw and Parse sending modes (i.e. Raw mode userprocess fully forms - // 15.4 packet and Parse mode the 15.4 framer forms the packet from the userprocess - // parameters and payload). Because we first take this kernel buffer, we must - // replace the `kernel_tx` buffer upon handling any error. - self.kernel_tx.take().map_or(Err(ErrorCode::NOMEM), |kbuf| { - match curr_tx { - PendingTX::Empty => { - unreachable!("PendingTX::Empty should have been handled earlier with guard statement.") - } - PendingTX::Raw => { - // Here we form an empty frame from the buffer to later be filled by the specified - // userprocess frame data. Note, we must allocate the needed buffer for the frame, - // but set the `len` field to 0 as the frame is empty. - self.mac.buf_to_frame(kbuf, 0).map_err(|(err, buf)| { - self.kernel_tx.replace(buf); - err - })}, - PendingTX::Parse(dst_addr, security_needed) => { - // Prepare the frame headers - let pan = self.mac.get_pan(); - let dst_addr = MacAddress::Short(dst_addr); - let src_addr = MacAddress::Short(self.mac.get_address()); - self.mac.prepare_data_frame( - kbuf, - pan, - dst_addr, - pan, - src_addr, - security_needed, - ).map_or_else(|err_buf| { - self.kernel_tx.replace(err_buf); - Err(ErrorCode::FAIL) - }, | frame| - Ok(frame) - ) + self.apps.enter(processid, |app, kerel_data| { + let (dst_addr, security_needed) = match app.pending_tx.take() { + Some(pending_tx) => pending_tx, + None => { + return Ok(()); + } + }; + let result = self.kernel_tx.take().map_or(Err(ErrorCode::NOMEM), |kbuf| { + // Prepare the frame headers + let pan = self.mac.get_pan(); + let dst_addr = MacAddress::Short(dst_addr); + let src_addr = MacAddress::Short(self.mac.get_address()); + let mut frame = match self.mac.prepare_data_frame( + kbuf, + pan, + dst_addr, + pan, + src_addr, + security_needed, + ) { + Ok(frame) => frame, + Err(kbuf) => { + self.kernel_tx.replace(kbuf); + return Err(ErrorCode::FAIL); } + }; + + // Append the payload: there must be one + let result = kerel_data + .get_readonly_processbuffer(ro_allow::WRITE) + .and_then(|write| write.enter(|payload| frame.append_payload_process(payload))) + .unwrap_or(Err(ErrorCode::INVAL)); + if result != Ok(()) { + return result; } - }).map(|mut frame| { - - // Obtain the payload from the userprocess, append the "payload" to the previously formed frame - // and pass the frame to be transmitted. Note, the term "payload" is somewhat misleading in the - // case of Raw transmission as the payload is the entire 15.4 frame. - kernel_data - .get_readonly_processbuffer(ro_allow::WRITE) - .and_then(|write| write.enter(|payload| - frame.append_payload_process(payload) - ))?.map( |()| - { - self.mac.transmit(frame).map_or_else(|(errorcode, error_buf)| { - self.kernel_tx.replace(error_buf); - Err(errorcode) - }, |()| {self.current_app.set(processid); Ok(()) } - ) + + // Finally, transmit the frame + match self.mac.transmit(frame) { + Ok(()) => Ok(()), + Err((ecode, buf)) => { + self.kernel_tx.put(Some(buf)); + Err(ecode) + } } - )? + }); + if result == Ok(()) { + self.current_app.set(processid); + } + result })? - })? } /// Schedule the next transmission if there is one pending. Performs the @@ -726,8 +673,6 @@ impl<'a, M: device::MacDevice<'a>> SyscallDriver for RadioDriver<'a, M> { /// - `25`: Remove the key at an index. /// - `26`: Transmit a frame (parse required). Take the provided payload and /// parameters to encrypt, form headers, and transmit the frame. - /// - `27`: Transmit a frame (raw). Transmit preformed 15.4 frame (i.e. - /// headers and security etc completed by userprocess). /// - `28`: Set long address. /// - `29`: Get the long MAC address. fn command( @@ -969,7 +914,7 @@ impl<'a, M: device::MacDevice<'a>> SyscallDriver for RadioDriver<'a, M> { 26 => { self.apps .enter(processid, |app, kernel_data| { - if !app.pending_tx.is_empty() { + if app.pending_tx.is_some() { // Cannot support more than one pending tx per process. return Err(ErrorCode::BUSY); } @@ -1007,13 +952,7 @@ impl<'a, M: device::MacDevice<'a>> SyscallDriver for RadioDriver<'a, M> { if next_tx.is_none() { return Err(ErrorCode::INVAL); } - - match next_tx { - Some((dst_addr, sec)) => { - app.pending_tx = PendingTX::Parse(dst_addr, sec) - } - None => app.pending_tx = PendingTX::Empty, - } + app.pending_tx = next_tx; Ok(()) }) .map_or_else( @@ -1024,21 +963,6 @@ impl<'a, M: device::MacDevice<'a>> SyscallDriver for RadioDriver<'a, M> { }, ) } - 27 => { - self.apps - .enter(processid, |app, _| { - if !app.pending_tx.is_empty() { - // Cannot support more than one pending tx per process. - return Err(ErrorCode::BUSY); - } - app.pending_tx = PendingTX::Raw; - Ok(()) - }) - .map_or_else( - |err| CommandReturn::failure(err.into()), - |_| self.do_next_tx_sync(processid).into(), - ) - } 28 => { let addr_upper: u64 = arg2 as u64; let addr_lower: u64 = arg1 as u64; diff --git a/capsules/extra/src/ieee802154/framer.rs b/capsules/extra/src/ieee802154/framer.rs index 8028d4e9d9..50ba2b81be 100644 --- a/capsules/extra/src/ieee802154/framer.rs +++ b/capsules/extra/src/ieee802154/framer.rs @@ -99,52 +99,7 @@ use kernel::ErrorCode; #[derive(Eq, PartialEq, Debug)] pub struct Frame { buf: &'static mut [u8], - info: FrameInfoWrap, -} - -/// This enum wraps the `FrameInfo` struct and allows each sending type -/// (Parse or Raw) to only store the relevant information. In the -/// case of a Raw send, the `FrameInfo` struct is irrelevant as the -/// packet has been fully formed by the userprocess. For a Raw send, -/// we only require knowledge on the frame length. In the case of a -/// Parse send, the wrapper provides all required frame header information. -#[derive(Eq, PartialEq, Debug)] -enum FrameInfoWrap { - Raw(usize), - Parse(FrameInfo), -} - -impl FrameInfoWrap { - /// Obtain secured_length of the Frame - pub fn secured_length(&self) -> usize { - match self { - FrameInfoWrap::Parse(info) => info.secured_length(), - FrameInfoWrap::Raw(len) => *len, - } - } - - /// Obtain unsecured_length of the Frame - pub fn unsecured_length(&self) -> usize { - match self { - FrameInfoWrap::Parse(info) => info.unsecured_length(), - FrameInfoWrap::Raw(len) => *len, - } - } - - /// Fetcher of the FrameInfo struct for Parse sending. Panics if - /// called for Raw sending. - pub fn get_info(&self) -> FrameInfo { - match self { - FrameInfoWrap::Raw(_) => { - // This should never be called for a Raw send. The Framer should never - // require information other than the Frame length for a Raw send. This - // warrants a panic condition as fetching the `FrameInfo` struct for a - // Raw send is undefined behavior. - panic!("FrameInfoWrap::Raw called when expecting FrameInfoWrap::Parse") - } - FrameInfoWrap::Parse(info) => *info, - } - } + info: FrameInfo, } /// This contains just enough information about a frame to determine @@ -189,13 +144,8 @@ impl Frame { } let begin = self.info.unsecured_length(); self.buf[begin..begin + payload.len()].copy_from_slice(payload); - match self.info { - FrameInfoWrap::Raw(len) => self.info = FrameInfoWrap::Raw(len + payload.len()), - FrameInfoWrap::Parse(mut info) => { - info.data_len += payload.len(); - self.info = FrameInfoWrap::Parse(info); - } - } + self.info.data_len += payload.len(); + Ok(()) } @@ -210,13 +160,8 @@ impl Frame { } let begin = self.info.unsecured_length(); payload_buf.copy_to_slice(&mut self.buf[begin..begin + payload_buf.len()]); - match self.info { - FrameInfoWrap::Raw(len) => self.info = FrameInfoWrap::Raw(len + payload_buf.len()), - FrameInfoWrap::Parse(mut info) => { - info.data_len += payload_buf.len(); - self.info = FrameInfoWrap::Parse(info); - } - } + self.info.data_len += payload_buf.len(); + Ok(()) } } @@ -342,13 +287,13 @@ enum TxState { /// There is no frame to be transmitted. Idle, /// There is a valid frame that needs to be secured before transmission. - ReadyToEncrypt(FrameInfoWrap, &'static mut [u8]), + ReadyToEncrypt(FrameInfo, &'static mut [u8]), /// There is currently a frame being encrypted by the encryption facility. #[allow(dead_code)] - Encrypting(FrameInfoWrap), + Encrypting(FrameInfo), /// There is a frame that is completely secured or does not require /// security, and is waiting to be passed to the radio. - ReadyToTransmit(FrameInfoWrap, &'static mut [u8]), + ReadyToTransmit(FrameInfo, &'static mut [u8]), } #[derive(Eq, PartialEq, Debug)] @@ -356,16 +301,16 @@ enum RxState { /// There is no frame that has been received. Idle, /// There is a secured frame that needs to be decrypted. - /// ReadyToDecrypt(FrameInfoWrap, buf, lqi) - ReadyToDecrypt(FrameInfoWrap, &'static mut [u8], u8), + /// ReadyToDecrypt(FrameInfo, buf, lqi) + ReadyToDecrypt(FrameInfo, &'static mut [u8], u8), /// A secured frame is currently being decrypted by the decryption facility. - /// Decrypting(FrameInfoWrap, lqi) + /// Decrypting(FrameInfo, lqi) #[allow(dead_code)] - Decrypting(FrameInfoWrap, u8), + Decrypting(FrameInfo, u8), /// There is an unsecured frame that needs to be re-parsed and exposed to - /// the client. ReadyToYield(FrameInfoWrap, buf, lqi) + /// the client. ReadyToYield(FrameInfo, buf, lqi) #[allow(dead_code)] - ReadyToYield(FrameInfoWrap, &'static mut [u8], u8), + ReadyToYield(FrameInfo, &'static mut [u8], u8), } /// This struct wraps an IEEE 802.15.4 radio device `kernel::hil::radio::Radio` @@ -440,16 +385,7 @@ impl<'a, M: Mac<'a>, A: AES128CCM<'a>> Framer<'a, M, A> { /// Performs the first checks in the security procedure. The rest of the /// steps are performed as part of the transmission pipeline. /// Returns the next `TxState` to enter. - fn outgoing_frame_security( - &self, - buf: &'static mut [u8], - frame_info_wrap: FrameInfoWrap, - ) -> TxState { - let frame_info = match frame_info_wrap { - FrameInfoWrap::Parse(info) => info, - FrameInfoWrap::Raw(_) => return TxState::ReadyToTransmit(frame_info_wrap, buf), - }; - + fn outgoing_frame_security(&self, buf: &'static mut [u8], frame_info: FrameInfo) -> TxState { // IEEE 802.15.4-2015: 9.2.1, outgoing frame security // Steps a-e have already been performed in the frame preparation step, // so we only need to dispatch on the security parameters in the frame info @@ -458,12 +394,12 @@ impl<'a, M: Mac<'a>, A: AES128CCM<'a>> Framer<'a, M, A> { if level == SecurityLevel::None { // This case should never occur if the FrameInfo was // prepared by prepare_data_frame - TxState::ReadyToTransmit(frame_info_wrap, buf) + TxState::ReadyToTransmit(frame_info, buf) } else { - TxState::ReadyToEncrypt(frame_info_wrap, buf) + TxState::ReadyToEncrypt(frame_info, buf) } } - None => TxState::ReadyToTransmit(frame_info_wrap, buf), + None => TxState::ReadyToTransmit(frame_info, buf), } } @@ -580,7 +516,7 @@ impl<'a, M: Mac<'a>, A: AES128CCM<'a>> Framer<'a, M, A> { self.mac.set_receive_buffer(buf); RxState::Idle } - Some(frame_info) => RxState::ReadyToDecrypt(FrameInfoWrap::Parse(frame_info), buf, lqi), + Some(frame_info) => RxState::ReadyToDecrypt(frame_info, buf, lqi), } } @@ -594,14 +530,14 @@ impl<'a, M: Mac<'a>, A: AES128CCM<'a>> Framer<'a, M, A> { let (next_state, result) = match state { TxState::Idle => (TxState::Idle, Ok(())), TxState::ReadyToEncrypt(info, buf) => { - match info.get_info().security_params { + match info.security_params { None => { // `ReadyToEncrypt` should only be entered when // `security_params` is not `None`. (TxState::Idle, Err((ErrorCode::FAIL, buf))) } Some((level, key, nonce)) => { - let (m_off, m_len) = info.get_info().ccm_encrypt_ranges(); + let (m_off, m_len) = info.ccm_encrypt_ranges(); let (a_off, m_off) = (radio::PSDU_OFFSET, radio::PSDU_OFFSET + m_off); @@ -616,7 +552,7 @@ impl<'a, M: Mac<'a>, A: AES128CCM<'a>> Framer<'a, M, A> { a_off, m_off, m_len, - info.get_info().mic_len, + info.mic_len, level.encryption_needed(), true, ); @@ -662,14 +598,14 @@ impl<'a, M: Mac<'a>, A: AES128CCM<'a>> Framer<'a, M, A> { let next_state = match state { RxState::Idle => RxState::Idle, RxState::ReadyToDecrypt(info, buf, lqi) => { - match info.get_info().security_params { + match info.security_params { None => { // `ReadyToDecrypt` should only be entered when // `security_params` is not `None`. RxState::Idle } Some((level, key, nonce)) => { - let (m_off, m_len) = info.get_info().ccm_encrypt_ranges(); + let (m_off, m_len) = info.ccm_encrypt_ranges(); let (a_off, m_off) = (radio::PSDU_OFFSET, radio::PSDU_OFFSET + m_off); // Crypto setup failed; fail receiving packet and return to idle @@ -700,7 +636,7 @@ impl<'a, M: Mac<'a>, A: AES128CCM<'a>> Framer<'a, M, A> { a_off, m_off, m_len, - info.get_info().mic_len, + info.mic_len, level.encryption_needed(), true, ) @@ -905,36 +841,19 @@ impl<'a, M: Mac<'a>, A: AES128CCM<'a>> MacDevice<'a> for Framer<'a, M, A> { match header.encode(buf, true).done() { Some((data_offset, mac_payload_offset)) => Ok(Frame { buf: buf, - info: FrameInfoWrap::Parse(FrameInfo { + info: FrameInfo { frame_type: FrameType::Data, mac_payload_offset: mac_payload_offset, data_offset: data_offset, data_len: 0, mic_len: mic_len, security_params: security_desc.map(|(sec, key, nonce)| (sec.level, key, nonce)), - }), + }, }), None => Err(buf), } } - fn buf_to_frame( - &self, - buf: &'static mut [u8], - len: usize, - ) -> Result { - // Error check input for compliance with max 15.4 buffer size and - // that the provided len is compatibile with the provided buffer. - if buf.len() < radio::MAX_BUF_SIZE || len > buf.len() { - return Err((ErrorCode::INVAL, buf)); - } - - Ok(Frame { - buf: buf, - info: FrameInfoWrap::Raw(len), - }) - } - fn transmit(&self, frame: Frame) -> Result<(), (ErrorCode, &'static mut [u8])> { let Frame { buf, info } = frame; let state = match self.tx_state.take() { diff --git a/capsules/extra/src/ieee802154/virtual_mac.rs b/capsules/extra/src/ieee802154/virtual_mac.rs index 7f5895badb..a4284e8375 100644 --- a/capsules/extra/src/ieee802154/virtual_mac.rs +++ b/capsules/extra/src/ieee802154/virtual_mac.rs @@ -299,14 +299,6 @@ impl<'a, M: device::MacDevice<'a>> device::MacDevice<'a> for MacUser<'a, M> { .prepare_data_frame(buf, dst_pan, dst_addr, src_pan, src_addr, security_needed) } - fn buf_to_frame( - &self, - buf: &'static mut [u8], - len: usize, - ) -> Result { - self.mux.mac.buf_to_frame(buf, len) - } - fn transmit(&self, frame: framer::Frame) -> Result<(), (ErrorCode, &'static mut [u8])> { // If the muxer is idle, immediately transmit the frame, otherwise // attempt to queue the transmission request. However, each MAC user can