From 66e964335f2981f4f5e4c862fa38df51d5fa5c36 Mon Sep 17 00:00:00 2001 From: Lachezar Lechev <8925621+elpiel@users.noreply.github.com> Date: Mon, 1 May 2023 18:58:23 +0300 Subject: [PATCH 1/4] fix: Cargo.toml package version to 0.4.1 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 840572f..7b7c523 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "w5500" -version = "0.4.0" +version = "0.4.1" authors = ["Michael Watzko ", "Jonah Dahlquist ", "Ryan Summers Date: Sun, 23 Apr 2023 00:12:29 +0200 Subject: [PATCH 2/4] Allow truncated `RawDevice::read_frame()` - `RawDevice::read_frame()` succeeds with a receive buffer smaller than the frame. - Introduce `RxCursor` and `TxCursor` to track/update RX/TX buffer pointers making it easier to reason about functions such as `read_frame` / `write_frame`. --- src/cursor.rs | 126 ++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/raw_device.rs | 94 +++++++--------------------------- 3 files changed, 144 insertions(+), 77 deletions(-) create mode 100644 src/cursor.rs diff --git a/src/cursor.rs b/src/cursor.rs new file mode 100644 index 0000000..6bba7de --- /dev/null +++ b/src/cursor.rs @@ -0,0 +1,126 @@ +use crate::bus::Bus; +use crate::register::socketn::Command; +use crate::socket::Socket; + +pub(crate) struct RxCursor<'a, SpiBus> +where + SpiBus: Bus, +{ + sock: &'a mut Socket, + bus: &'a mut SpiBus, + ptr: u16, + size: u16, +} + +impl<'a, SpiBus> RxCursor<'a, SpiBus> +where + SpiBus: Bus, +{ + pub fn new(sock: &'a mut Socket, bus: &'a mut SpiBus) -> Result { + let size = sock.get_receive_size(bus)?; + let ptr = sock.get_rx_read_pointer(bus)?; + Ok(Self { + sock, + bus, + ptr, + size, + }) + } + + #[inline] + pub fn available(&self) -> u16 { + self.size + } + + /// Read up to `buf.len()` bytes. The actual number of bytes read is bounded by `available()`. + pub fn read(&mut self, buf: &mut [u8]) -> Result { + if buf.is_empty() { + return Ok(0); + } + + let count = self.available().min(buf.len() as u16); + self.bus + .read_frame(self.sock.rx_buffer(), self.ptr, &mut buf[..count as _])?; + Ok(self.skip(count)) + } + + /// Read up to `max` bytes. The actual number of bytes read is bounded by buf.len() and available(). + pub fn read_upto(&mut self, buf: &mut [u8], max: u16) -> Result { + let bounded_buf = if buf.len() > max as usize { + &mut buf[..max as _] + } else { + buf + }; + self.read(bounded_buf) + } + + /// Skip up to count bytes. The actual number of bytes skipped is bounded by available(). + pub fn skip(&mut self, count: u16) -> u16 { + let bounded_count = self.available().min(count); + self.ptr += bounded_count; + self.size -= bounded_count; + bounded_count + } + + /// Return ownership of the portion of the RX buffer that has already been read back to the + /// chip and issue the next receive command. + pub fn commit(mut self) -> Result<(), SpiBus::Error> { + self.sock.set_rx_read_pointer(self.bus, self.ptr)?; + self.sock.command(self.bus, Command::Receive)?; + Ok(()) + } +} + +pub(crate) struct TxCursor<'a, SpiBus> +where + SpiBus: Bus, +{ + sock: &'a mut Socket, + bus: &'a mut SpiBus, + ptr: u16, + size: u16, +} + +impl<'a, SpiBus> TxCursor<'a, SpiBus> +where + SpiBus: Bus, +{ + pub fn new(sock: &'a mut Socket, bus: &'a mut SpiBus) -> Result { + let size = sock.get_tx_free_size(bus)?; + let ptr = sock.get_tx_write_pointer(bus)?; + Ok(Self { + sock, + bus, + ptr, + size, + }) + } + + #[inline] + pub fn available(&self) -> u16 { + self.size + } + + /// Write all bytes in buf to the current TX buffer position and update the cursor position + /// and remaining size on success. + pub fn write(&mut self, buf: &[u8]) -> Result { + if buf.is_empty() || buf.len() > self.available() as _ { + return Ok(0); + } + + let count = buf.len() as u16; + self.bus + .write_frame(self.sock.tx_buffer(), self.ptr, &buf[..count as _])?; + self.ptr += count; + self.size -= count; + Ok(count) + } + + /// Pass ownership of the portion of the TX buffer that has already been written back to the + /// chip and issue the next send command. + pub fn commit(mut self) -> Result<(), SpiBus::Error> { + self.sock.set_tx_write_pointer(self.bus, self.ptr)?; + self.sock.command(self.bus, Command::Send)?; + Ok(()) + } +} diff --git a/src/lib.rs b/src/lib.rs index 717c1d8..475ba9e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,6 +4,7 @@ #![doc = include_str!("../README.md")] pub mod bus; +mod cursor; mod device; mod host; pub mod net; diff --git a/src/raw_device.rs b/src/raw_device.rs index 25413fc..a8f74fa 100644 --- a/src/raw_device.rs +++ b/src/raw_device.rs @@ -39,38 +39,6 @@ impl RawDevice { Ok(Self { bus, raw_socket }) } - // Read bytes from the RX buffer. - // - // # Args - // * `buffer` - The location to read data into. The length of this slice determines how much - // data is read. - // * `offset` - The offset into current RX data to start reading from in bytes. - // - // # Returns - // The number of bytes successfully read. - fn read_bytes(&mut self, buffer: &mut [u8], offset: u16) -> Result { - let rx_size = self.raw_socket.get_receive_size(&mut self.bus)? as usize; - - let read_buffer = if rx_size > buffer.len() + offset as usize { - buffer - } else { - &mut buffer[..rx_size - offset as usize] - }; - - let read_pointer = self - .raw_socket - .get_rx_read_pointer(&mut self.bus)? - .wrapping_add(offset); - self.bus - .read_frame(self.raw_socket.rx_buffer(), read_pointer, read_buffer)?; - self.raw_socket.set_rx_read_pointer( - &mut self.bus, - read_pointer.wrapping_add(read_buffer.len() as u16), - )?; - - Ok(read_buffer.len()) - } - /// Read an ethernet frame from the device. /// /// # Args @@ -79,40 +47,29 @@ impl RawDevice { /// # Returns /// The number of bytes read into the provided frame buffer. pub fn read_frame(&mut self, frame: &mut [u8]) -> Result { + let mut rx_cursor = crate::cursor::RxCursor::new(&mut self.raw_socket, &mut self.bus)?; + // Check if there is anything to receive. - let rx_size = self.raw_socket.get_receive_size(&mut self.bus)? as usize; - if rx_size == 0 { + if rx_cursor.available() == 0 { return Ok(0); } // The W5500 specifies the size of the received ethernet frame in the first two bytes. // Refer to https://forum.wiznet.io/t/topic/979/2 for more information. - let expected_frame_size: usize = { + let expected_frame_size = { let mut frame_bytes = [0u8; 2]; - assert!(self.read_bytes(&mut frame_bytes[..], 0)? == 2); + assert!(rx_cursor.read(&mut frame_bytes[..])? == 2); - u16::from_be_bytes(frame_bytes) as usize - 2 + u16::from_be_bytes(frame_bytes).saturating_sub(2) }; - // Read the ethernet frame - let read_buffer = if frame.len() > expected_frame_size { - &mut frame[..expected_frame_size] - } else { - frame - }; - - let received_frame_size = self.read_bytes(read_buffer, 2)?; - - // Register the reception as complete. - self.raw_socket - .command(&mut self.bus, register::socketn::Command::Receive)?; - - // If we couldn't read the whole frame, drop it instead. + let received_frame_size = rx_cursor.read_upto(frame, expected_frame_size)?; if received_frame_size < expected_frame_size { - Ok(0) - } else { - Ok(received_frame_size) + rx_cursor.skip(expected_frame_size - received_frame_size); } + + rx_cursor.commit()?; + Ok(received_frame_size as _) } /// Write an ethernet frame to the device. @@ -123,37 +80,20 @@ impl RawDevice { /// # Returns /// The number of bytes successfully transmitted from the provided buffer. pub fn write_frame(&mut self, frame: &[u8]) -> Result { - let max_size = self.raw_socket.get_tx_free_size(&mut self.bus)? as usize; - - let write_data = if frame.len() < max_size { - frame - } else { - &frame[..max_size] - }; - - // Append the data to the write buffer after the current write pointer. - let write_pointer = self.raw_socket.get_tx_write_pointer(&mut self.bus)?; - - // Write data into the buffer and update the writer pointer. - self.bus - .write_frame(self.raw_socket.tx_buffer(), write_pointer, write_data)?; - self.raw_socket.set_tx_write_pointer( - &mut self.bus, - write_pointer.wrapping_add(write_data.len() as u16), - )?; - - // Wait for the socket transmission to complete. + // Reset the transmission complete flag, we'll wait on it later. self.raw_socket .reset_interrupt(&mut self.bus, register::socketn::Interrupt::SendOk)?; - self.raw_socket - .command(&mut self.bus, register::socketn::Command::Send)?; + let mut tx_cursor = crate::cursor::TxCursor::new(&mut self.raw_socket, &mut self.bus)?; + let count = tx_cursor.write(frame)?; + tx_cursor.commit()?; + // Wait for the socket transmission to complete. while !self .raw_socket .has_interrupt(&mut self.bus, register::socketn::Interrupt::SendOk)? {} - Ok(write_data.len()) + Ok(count as _) } } From 9aeb331fb01e3441ca63e196ceb7d30f38956c85 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Thu, 22 Jun 2023 11:25:23 +0200 Subject: [PATCH 3/4] Update CHANGELOG.md Adding CHANGELOG for MACRAW read fix --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b34a45..e8becb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `defmt` features for enabling `defmt::Format` to most structs and errors by [@elpiel](https://github.com/elpiel) ([#39](https://github.com/kellerkindt/w5500/issues/39)) - Fixed an issue where internal function names were conflicting with trait names by [@ryan-summers](https://github.com/ryan-summers) ([#36](https://github.com/kellerkindt/w5500/issues/36)) +### Fixed + +- Fixed an issue that caused corruption when reading partial MACRAW frames by [@Felix-El](https://github.com/Felix-El) ([#47](https://github.com/kellerkindt/w5500/pull/47)) + ## [0.4.1] - January 2nd, 2023 ### Added From 1eb9b25c7ee2cd14e184bc50c4375f90ab0ea193 Mon Sep 17 00:00:00 2001 From: Felix Lelchuk Date: Fri, 23 Jun 2023 13:45:51 +0200 Subject: [PATCH 4/4] Fix cursor: wrapping_add for ptr, immutable sock --- src/cursor.rs | 12 ++++++------ src/raw_device.rs | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/cursor.rs b/src/cursor.rs index 6bba7de..0d1c84d 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -6,7 +6,7 @@ pub(crate) struct RxCursor<'a, SpiBus> where SpiBus: Bus, { - sock: &'a mut Socket, + sock: &'a Socket, bus: &'a mut SpiBus, ptr: u16, size: u16, @@ -16,7 +16,7 @@ impl<'a, SpiBus> RxCursor<'a, SpiBus> where SpiBus: Bus, { - pub fn new(sock: &'a mut Socket, bus: &'a mut SpiBus) -> Result { + pub fn new(sock: &'a Socket, bus: &'a mut SpiBus) -> Result { let size = sock.get_receive_size(bus)?; let ptr = sock.get_rx_read_pointer(bus)?; Ok(Self { @@ -57,7 +57,7 @@ where /// Skip up to count bytes. The actual number of bytes skipped is bounded by available(). pub fn skip(&mut self, count: u16) -> u16 { let bounded_count = self.available().min(count); - self.ptr += bounded_count; + self.ptr = self.ptr.wrapping_add(bounded_count); self.size -= bounded_count; bounded_count } @@ -75,7 +75,7 @@ pub(crate) struct TxCursor<'a, SpiBus> where SpiBus: Bus, { - sock: &'a mut Socket, + sock: &'a Socket, bus: &'a mut SpiBus, ptr: u16, size: u16, @@ -85,7 +85,7 @@ impl<'a, SpiBus> TxCursor<'a, SpiBus> where SpiBus: Bus, { - pub fn new(sock: &'a mut Socket, bus: &'a mut SpiBus) -> Result { + pub fn new(sock: &'a Socket, bus: &'a mut SpiBus) -> Result { let size = sock.get_tx_free_size(bus)?; let ptr = sock.get_tx_write_pointer(bus)?; Ok(Self { @@ -111,7 +111,7 @@ where let count = buf.len() as u16; self.bus .write_frame(self.sock.tx_buffer(), self.ptr, &buf[..count as _])?; - self.ptr += count; + self.ptr = self.ptr.wrapping_add(count); self.size -= count; Ok(count) } diff --git a/src/raw_device.rs b/src/raw_device.rs index a8f74fa..25b09a9 100644 --- a/src/raw_device.rs +++ b/src/raw_device.rs @@ -47,7 +47,7 @@ impl RawDevice { /// # Returns /// The number of bytes read into the provided frame buffer. pub fn read_frame(&mut self, frame: &mut [u8]) -> Result { - let mut rx_cursor = crate::cursor::RxCursor::new(&mut self.raw_socket, &mut self.bus)?; + let mut rx_cursor = crate::cursor::RxCursor::new(&self.raw_socket, &mut self.bus)?; // Check if there is anything to receive. if rx_cursor.available() == 0 { @@ -84,7 +84,7 @@ impl RawDevice { self.raw_socket .reset_interrupt(&mut self.bus, register::socketn::Interrupt::SendOk)?; - let mut tx_cursor = crate::cursor::TxCursor::new(&mut self.raw_socket, &mut self.bus)?; + let mut tx_cursor = crate::cursor::TxCursor::new(&self.raw_socket, &mut self.bus)?; let count = tx_cursor.write(frame)?; tx_cursor.commit()?;