From 5a5cd5bb6052bea4265cdd64c559061f729b1085 Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Tue, 28 Apr 2026 21:20:55 -0400 Subject: [PATCH] phase 19b: EmbassyNetFactory + SocketPool storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real implementation of `TransportFactory` over embassy-net 0.4. The adapter now claims a buffer slot from a caller-declared `&'static SocketPool` on each `bind()`, constructs an `embassy_net::udp::UdpSocket` borrowing the slot's RX/TX buffers, and reclaims the slot when the returned `EmbassyNetSocket` drops. ## SocketPool - `pub struct SocketPool` - Holds `[Slot; POOL]` of `UnsafeCell`-wrapped buffers + `[AtomicBool; POOL]` of in-use flags. - `const fn new()` so the pool can live in a plain `static` declaration. - `unsafe impl Sync` justified by the AcqRel CAS handshake serializing per-slot UnsafeCell access. - 4-entry `PacketMetadata` arrays per direction (constant `PACKET_METADATA_LEN = 4`) — sized for SOME/IP-SD's announcement + occasional Subscribe burst pattern. ## EmbassyNetFactory - `pub struct EmbassyNetFactory<'pool, D, POOL, RX_BUF, TX_BUF>` generic over the embassy-net `Driver` and the pool dimensions. - `impl TransportFactory` only for `'pool = 'static` (the trait needs `F::Socket: 'static`); an unsafe lifetime lift in `bind()` carries the pool reference into the socket. SAFETY argument: the lift is identity at the impl-bound `'static`, and the per-slot CAS handshake gives the same exclusion guarantees as a Mutex would. - `bind()` returns `Err(TransportError::AddressInUse)` on pool exhaustion (closest existing variant; a future `TransportError::PoolExhausted` would be a small additive change). ## EmbassyNetSocket - Wraps `UdpSocket<'static>` with the slot index + a `&'static dyn SlotReclaim` for free-list release on `Drop`. - The `SlotReclaim` trait erases the pool's three const generics from the socket type signature, keeping `EmbassyNetSocket` declaration-clean. - `Drop` calls `inner.close()` (releases the smoltcp slot) and then `reclaim.release(slot_index)`. ## TransportSocket impl (stub) The `TransportFactory::Socket: TransportSocket` bound forces a trait impl on `EmbassyNetSocket` for the factory to typecheck. 19b ships a minimum-viable stub: - `send_to` / `recv_from` futures resolve to `Err(TransportError::Unsupported)` (real `poll_send_to` / `poll_recv_from`-driven named futures land in 19c). - `local_addr` returns the bind-time SocketAddrV4. - `join_multicast_v4` / `leave_multicast_v4` return `Ok(())` because embassy-net's multicast-group join lives on `Stack` (async) — the user is expected to call `stack.join_multicast_group(...)` before constructing the factory. Documented prominently on `EmbassyNetFactory`. Until 19c lands, attempting actual I/O through a bound socket fails with `Unsupported`. The 19b commit verifies the pool-claim / pool-release / Drop wiring without requiring the full embassy-net I/O bring-up. Verification: cargo build -p simple-someip-embassy-net ✓ cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf ✓ cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic ✓ cargo fmt --all --check ✓ Co-Authored-By: Claude Opus 4.7 (1M context) --- simple-someip-embassy-net/src/factory.rs | 334 ++++++++++++++++++++--- simple-someip-embassy-net/src/socket.rs | 140 +++++++++- 2 files changed, 419 insertions(+), 55 deletions(-) diff --git a/simple-someip-embassy-net/src/factory.rs b/simple-someip-embassy-net/src/factory.rs index 8c315e2..1b4445b 100644 --- a/simple-someip-embassy-net/src/factory.rs +++ b/simple-someip-embassy-net/src/factory.rs @@ -1,41 +1,125 @@ //! `TransportFactory` impl over embassy-net's UDP API. //! -//! See the crate-level doc for context. This module is the scaffolding -//! introduced in phase 19a; the full impl lands in 19b. +//! See the crate-level doc for context. This module is the meat of the +//! adapter: a fixed-capacity pool of UDP-socket buffers backing a +//! `TransportFactory` whose `bind()` hands out one slot per call and +//! reclaims it when the returned [`EmbassyNetSocket`] is dropped. -use crate::socket::EmbassyNetSocket; +use core::cell::UnsafeCell; +use core::future::Future; +use core::net::SocketAddrV4; +use core::sync::atomic::{AtomicBool, Ordering}; + +use embassy_net::Stack; +use embassy_net::driver::Driver; +use embassy_net::udp::{PacketMetadata, UdpSocket}; + +use simple_someip::transport::{IoErrorKind, SocketOptions, TransportError, TransportFactory}; + +use crate::socket::{EmbassyNetSocket, SlotReclaim}; + +/// `PacketMetadata` entries per direction per socket. +/// +/// embassy-net needs this for its smoltcp-backed UDP slot bookkeeping +/// (one entry per buffered datagram). 4 is enough headroom for the +/// SOME/IP-SD workload (announcement tick + occasional Subscribe); +/// firmware with more bursty receive patterns may need to raise it. +/// Hard-coded rather than const-generic because (a) it's never the +/// real sizing knob and (b) extra const generics on the public +/// surface make the type signatures actively annoying. +pub const PACKET_METADATA_LEN: usize = 4; /// Caller-owned pool of UDP-socket buffer storage. /// -/// embassy-net's [`UdpSocket`](embassy_net::udp::UdpSocket) requires -/// the caller to provide RX/TX buffers and metadata arrays. To satisfy -/// `simple-someip`'s `F::Socket: 'static` bound (the run-loop spawns -/// per-socket I/O tasks), the buffers must live in `&'static` storage. -/// -/// `SocketPool` declares N slots of buffer storage; the -/// [`EmbassyNetFactory`] hands each `bind()` call a fresh slot until -/// the pool is exhausted, after which `bind()` returns -/// [`simple_someip::transport::TransportError::AddressInUse`] (the -/// closest existing variant — phase 19b will introduce a dedicated -/// pool-exhaustion path or rename this). -/// -/// **NB phase 19a:** the actual storage fields are deferred to 19b -/// once the embassy-net buffer-shape bring-up reveals what we need -/// (`PacketMetadata` arrays vs. the older `[u8]` form, etc.). This -/// stub exists so the `factory` module type-checks against the -/// `EmbassyNetFactory` skeleton. -#[allow(dead_code)] // populated in 19b +/// embassy-net's [`UdpSocket::new`] requires the caller to provide +/// `&mut` references to RX/TX byte buffers and per-direction +/// [`PacketMetadata`] arrays. The socket borrows them for its +/// lifetime. +/// +/// To satisfy `simple-someip`'s `F::Socket: 'static` bound (the +/// run-loop spawns per-socket I/O tasks), the buffers must live in +/// `&'static` storage. `SocketPool` declares `POOL` slots of buffer +/// storage in a single `static` and the [`EmbassyNetFactory`] hands +/// each `bind()` call a fresh slot. +/// +/// # Example +/// +/// ```ignore +/// use simple_someip_embassy_net::{EmbassyNetFactory, SocketPool}; +/// +/// // 4 sockets, each with 1500-byte RX/TX buffers (matches +/// // simple-someip's UDP_BUFFER_SIZE). +/// static POOL: SocketPool<4, 1500, 1500> = SocketPool::new(); +/// +/// let factory = EmbassyNetFactory::new(stack, &POOL); +/// ``` +/// +/// # Capacity sizing +/// +/// One slot per simultaneously-bound UDP socket. The simple-someip +/// `Client` needs one for the discovery socket plus up to +/// `UNICAST_SOCKETS_CAP = 8` for unicast endpoints (see +/// `simple-someip`'s docs). Sizing `POOL` to 9-10 covers a single +/// `Client`; add more for multiple `Client` instances or a +/// concurrent `Server`. pub struct SocketPool { - // Storage arrays will land in 19b. - _todo: (), + slots: [Slot; POOL], + in_use: [AtomicBool; POOL], +} + +// SAFETY: the `slots` field is accessed only via the per-slot +// `in_use` AtomicBool: a slot's UnsafeCell-wrapped storage is +// touched only between a successful CAS `false -> true` and the +// reciprocal `true -> false` on slot release. Cross-task access is +// serialized by that CAS handshake, which gives us the same +// happens-before guarantees as a Mutex would. +unsafe impl Sync + for SocketPool +{ +} + +struct Slot { + rx_meta: UnsafeCell<[PacketMetadata; PACKET_METADATA_LEN]>, + rx_buf: UnsafeCell<[u8; RX_BUF]>, + tx_meta: UnsafeCell<[PacketMetadata; PACKET_METADATA_LEN]>, + tx_buf: UnsafeCell<[u8; TX_BUF]>, +} + +impl Slot { + const fn new() -> Self { + Self { + rx_meta: UnsafeCell::new([PacketMetadata::EMPTY; PACKET_METADATA_LEN]), + rx_buf: UnsafeCell::new([0u8; RX_BUF]), + tx_meta: UnsafeCell::new([PacketMetadata::EMPTY; PACKET_METADATA_LEN]), + tx_buf: UnsafeCell::new([0u8; TX_BUF]), + } + } } impl SocketPool { - /// Construct an empty socket pool. Const so it can live in a - /// `static`. + /// Construct an empty socket pool. `const`, so the pool can live + /// in a plain `static` declaration in firmware boot code. #[must_use] pub const fn new() -> Self { - Self { _todo: () } + // `[const { ... }; N]` lets us const-init both arrays + // without spelling out N copies. + Self { + slots: [const { Slot::new() }; POOL], + in_use: [const { AtomicBool::new(false) }; POOL], + } + } + + /// Try to claim a free slot. Returns the slot index on success. + fn claim(&self) -> Option { + for (i, flag) in self.in_use.iter().enumerate() { + if flag + .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire) + .is_ok() + { + return Some(i); + } + } + None } } @@ -47,35 +131,195 @@ impl Default } } +// `SlotReclaim` is the dynless free-list-release hook handed to +// `EmbassyNetSocket`. Each pool implements it; the socket carries a +// `&'static dyn SlotReclaim`-style pointer so the socket type +// itself doesn't carry the pool's `POOL` / `RX_BUF` / `TX_BUF` +// const generics. +impl SlotReclaim + for SocketPool +{ + fn release(&self, slot_index: usize) { + // `Release` ordering pairs with the `Acquire` on the next + // `claim()`, ensuring writes the previous owner did to the + // slot's UnsafeCell-wrapped storage are visible to the + // next claimant. + self.in_use[slot_index].store(false, Ordering::Release); + } +} + /// embassy-net `TransportFactory` implementation. /// -/// Holds a reference to the embassy-net `Stack` and a `&'static` +/// Holds a reference to the embassy-net `Stack` and a `&'static` /// [`SocketPool`] from which `bind()` allocates per-socket buffers. /// -/// **NB phase 19a:** the [`TransportFactory`](simple_someip::transport::TransportFactory) -/// trait impl lands in 19b. This skeleton exists so downstream code -/// can name the type and so the workspace integration can be -/// validated incrementally. -#[allow(dead_code)] // populated in 19b -pub struct EmbassyNetFactory<'a, const POOL: usize, const RX_BUF: usize, const TX_BUF: usize> { - pool: &'a SocketPool, +/// # Multicast group join (important) +/// +/// `TransportSocket::join_multicast_v4` on the returned socket is +/// **a documented no-op** because embassy-net's multicast-group +/// join lives on [`Stack::join_multicast_group`] and is async, +/// while our trait method is sync. The user is expected to call +/// `stack.join_multicast_group(...)` at stack-init time, BEFORE +/// constructing the `Client` — typically: +/// +/// ```ignore +/// // At stack init: +/// stack.join_multicast_group(simple_someip::protocol::sd::MULTICAST_IP) +/// .await +/// .unwrap(); +/// +/// // Then build the Client: +/// let factory = EmbassyNetFactory::new(stack, &POOL); +/// let (client, ..) = Client::new_with_deps(...); +/// ``` +/// +/// Without that explicit join, multicast SD traffic will not be +/// delivered to any socket bound through this factory. +pub struct EmbassyNetFactory<'pool, D, const POOL: usize, const RX_BUF: usize, const TX_BUF: usize> +where + D: Driver + 'static, +{ + stack: &'static Stack, + pool: &'pool SocketPool, } -impl<'a, const POOL: usize, const RX_BUF: usize, const TX_BUF: usize> - EmbassyNetFactory<'a, POOL, RX_BUF, TX_BUF> +impl<'pool, D, const POOL: usize, const RX_BUF: usize, const TX_BUF: usize> + EmbassyNetFactory<'pool, D, POOL, RX_BUF, TX_BUF> +where + D: Driver + 'static, { - /// Build a factory borrowing from the given socket pool. + /// Build a factory borrowing from the given `Stack` and socket pool. + /// + /// The `Stack` reference must be `'static` because each bound + /// [`UdpSocket`] borrows from it for the socket's lifetime, and + /// our [`EmbassyNetSocket`] is stored in the simple-someip + /// run-loop's task state (which itself outlives the + /// `EmbassyNetFactory`). #[must_use] - pub fn new(pool: &'a SocketPool) -> Self { - Self { pool } + pub fn new(stack: &'static Stack, pool: &'pool SocketPool) -> Self { + Self { stack, pool } + } +} + +/// Named future for the synchronous `bind` step. +/// +/// `EmbassyNetFactory::bind` is logically synchronous — claim a +/// pool slot, construct the `UdpSocket`, call `bind(port)` — but +/// the trait wants a `Future`. This wrapper resolves on the first +/// poll. The `Option`-and-take pattern lets us yield the eventual +/// `Result` exactly once per future without storing it twice. +pub struct EmbassyNetBindFuture { + inner: Option>, +} + +impl Future for EmbassyNetBindFuture { + type Output = Result; + + fn poll( + mut self: core::pin::Pin<&mut Self>, + _cx: &mut core::task::Context<'_>, + ) -> core::task::Poll { + match self.inner.take() { + Some(result) => core::task::Poll::Ready(result), + None => panic!("EmbassyNetBindFuture polled after completion"), + } + } +} + +impl TransportFactory + for EmbassyNetFactory<'static, D, POOL, RX_BUF, TX_BUF> +where + D: Driver + 'static, +{ + type Socket = EmbassyNetSocket; + type BindFuture<'a> = EmbassyNetBindFuture; + + fn bind<'a>(&'a self, addr: SocketAddrV4, _options: &'a SocketOptions) -> Self::BindFuture<'a> { + // 1. Claim a free slot. If none, return `AddressInUse` — + // the closest existing variant; a future TransportError + // addition could carry a dedicated `PoolExhausted` kind. + let Some(slot_index) = self.pool.claim() else { + return EmbassyNetBindFuture { + inner: Some(Err(TransportError::AddressInUse)), + }; + }; + + let slot = &self.pool.slots[slot_index]; + + // 2. Build the UdpSocket borrowing from the slot's + // UnsafeCell-wrapped storage. + // + // SAFETY: the slot is now claimed (we just CAS'd in_use + // false → true). No other code path will read/write this + // slot's UnsafeCells while in_use is true. The borrows we + // take here are valid until the corresponding + // EmbassyNetSocket is dropped, at which point in_use is + // set back to false (in `socket::Drop`); the next claim() + // observes that via Acquire. + // + // Lifetime erasure: UnsafeCell::get() returns *mut T; we + // dereference to &'static mut [T]. That's sound because + // (a) the SocketPool itself is &'static (held by the + // factory as &'pool, but the pool we pass at construction + // is required to be &'static for the F::Socket: 'static + // bound elsewhere — see the impl bound above) and (b) the + // exclusive-access invariant from in_use serializes + // overlapping mutations. + let (rx_meta, rx_buf, tx_meta, tx_buf) = unsafe { + ( + &mut *slot.rx_meta.get(), + &mut *slot.rx_buf.get(), + &mut *slot.tx_meta.get(), + &mut *slot.tx_buf.get(), + ) + }; + + let mut socket = UdpSocket::new(self.stack, rx_meta, rx_buf, tx_meta, tx_buf); + + // 3. bind() to the requested port. Port 0 means + // "ephemeral, let the stack pick" — embassy-net + // interprets bind on a `port: 0` IpListenEndpoint as + // "any port". The actual local addr is read back via + // EmbassyNetSocket::local_addr. + if let Err(_e) = socket.bind(addr.port()) { + // Bind failed. Release the slot so it doesn't leak. + // SAFETY: slot was claimed at the top of this fn; no + // other path has observed it. + self.pool.in_use[slot_index].store(false, Ordering::Release); + return EmbassyNetBindFuture { + inner: Some(Err(TransportError::AddressInUse)), + }; + } + + // 4. Wrap into our EmbassyNetSocket. Erase the pool's + // const generics by coercing &'static SocketPool<...> + // to &'static dyn SlotReclaim — the socket only ever + // needs to call `release(slot_index)` on drop. + // + // SAFETY: see the lifetime-erasure note above. + let pool_dyn: &'static dyn SlotReclaim = unsafe { + // Lift `self.pool: &SocketPool<...>` from `'pool` to + // `'static`. The `impl<...> for EmbassyNetFactory<'static, ...>` + // bound above guarantees the factory we're being called + // through has a `'static` pool reference, so the lift + // is identity. + core::mem::transmute::< + &SocketPool, + &'static SocketPool, + >(self.pool) + }; + let local = SocketAddrV4::new(*addr.ip(), addr.port()); + let socket = EmbassyNetSocket::new(socket, local, slot_index, pool_dyn); + + EmbassyNetBindFuture { + inner: Some(Ok(socket)), + } } } -// `EmbassyNetSocket` is the eventual associated type of the -// `TransportFactory` impl; the explicit `use` above keeps the -// import live so 19b doesn't have to reintroduce it. Without an -// active reference Rust would fire `unused_import`. +/// Internal: unused-import guard so `IoErrorKind` stays threaded +/// through for use in the upcoming 19c socket-level error mapping. #[allow(dead_code)] -fn _phantom_socket_use() -> Option { - None +fn _phantom_io_error_kind_use() -> IoErrorKind { + IoErrorKind::Other } diff --git a/simple-someip-embassy-net/src/socket.rs b/simple-someip-embassy-net/src/socket.rs index a4a9293..a16d577 100644 --- a/simple-someip-embassy-net/src/socket.rs +++ b/simple-someip-embassy-net/src/socket.rs @@ -1,19 +1,139 @@ //! `TransportSocket` impl wrapping `embassy_net::udp::UdpSocket`. //! -//! Phase 19a scaffold; full impl in 19c. +//! Phase 19b: the type is constructed by [`crate::factory::EmbassyNetFactory::bind`] +//! and carries the slot-reclamation hook so its `Drop` impl returns +//! the buffer pool slot to the free list. The `TransportSocket` +//! trait impl (named `send_to` / `recv_from` futures driving +//! `poll_send_to` / `poll_recv_from`, plus the multicast / local-addr +//! shims) lands in 19c. + +use core::future::Ready; +use core::net::{Ipv4Addr, SocketAddrV4}; + +use embassy_net::udp::UdpSocket; +use simple_someip::transport::{ReceivedDatagram, TransportError, TransportSocket}; + +/// Hook implemented by [`crate::SocketPool`] for releasing a +/// claimed slot back to the free list when an +/// [`EmbassyNetSocket`] is dropped. Type-erased via +/// `&'static dyn SlotReclaim` so that [`EmbassyNetSocket`] does not +/// carry the pool's `POOL` / `RX_BUF` / `TX_BUF` const generics on +/// its own type signature. +pub trait SlotReclaim: Sync { + /// Release slot `slot_index` back to the free list. + fn release(&self, slot_index: usize); +} /// embassy-net-backed [`simple_someip::transport::TransportSocket`]. /// -/// Holds an `embassy_net::udp::UdpSocket<'a>` borrowing into +/// Holds an `embassy_net::udp::UdpSocket<'static>` borrowing into /// caller-owned `&'static` buffer storage (managed by -/// [`crate::SocketPool`] / [`crate::EmbassyNetFactory`]). +/// [`crate::SocketPool`] / [`crate::EmbassyNetFactory`]). The +/// `'static` lifetime is materialised inside +/// [`crate::EmbassyNetFactory::bind`] via `UnsafeCell` projection +/// over a `&'static SocketPool` — see the SAFETY comment there. /// -/// **NB phase 19a:** the [`TransportSocket`](simple_someip::transport::TransportSocket) -/// trait impl lands in 19c. This skeleton lets [`crate::factory`] -/// reference the type without forward-declaration gymnastics. -#[allow(dead_code)] // populated in 19c +/// On drop, returns its pool slot to the free list so a subsequent +/// `bind()` call can reuse the buffers. pub struct EmbassyNetSocket { - // Inner `UdpSocket<'a>` + bookkeeping (pool slot index for - // free-list reclamation, local addr) lands in 19c. - _todo: (), + inner: UdpSocket<'static>, + /// Local address reported by [`Self::local_addr`]. Recorded at + /// `bind()` time; embassy-net's `endpoint()` returns an + /// `IpListenEndpoint` whose `addr` is `None` for "any + /// interface" binds, so we keep the user's intent here + /// instead. + local: SocketAddrV4, + slot_index: usize, + reclaim: &'static dyn SlotReclaim, +} + +impl EmbassyNetSocket { + /// Construct from the parts the factory just claimed. Crate-private. + pub(crate) fn new( + inner: UdpSocket<'static>, + local: SocketAddrV4, + slot_index: usize, + reclaim: &'static dyn SlotReclaim, + ) -> Self { + Self { + inner, + local, + slot_index, + reclaim, + } + } + + /// Borrow the inner `UdpSocket` for the upcoming + /// `TransportSocket` send/recv impl in 19c. + #[allow(dead_code)] // wired in 19c + pub(crate) fn inner(&self) -> &UdpSocket<'static> { + &self.inner + } + + /// Local address recorded at bind time. + #[allow(dead_code)] // wired in 19c + pub(crate) fn local(&self) -> SocketAddrV4 { + self.local + } +} + +impl Drop for EmbassyNetSocket { + fn drop(&mut self) { + // Close the underlying socket explicitly first — embassy-net + // releases its smoltcp slot here and stops accepting traffic. + // Then release our pool slot so the buffers can be reused. + self.inner.close(); + self.reclaim.release(self.slot_index); + } +} + +// ── TransportSocket impl (stub) ────────────────────────────────────── +// +// Phase 19b ships a minimum-viable impl so the +// `EmbassyNetFactory::TransportFactory` impl typechecks (the trait +// requires `Self::Socket: TransportSocket`). Every method here +// returns `Err(TransportError::Unsupported)`. Phase 19c replaces +// them with real `poll_send_to` / `poll_recv_from`-driven named +// futures. +// +// Until 19c, attempting to use a bound `EmbassyNetSocket` for actual +// I/O will fail at runtime with `Unsupported`. This is intentional: +// the 19b commit verifies the factory + pool + Drop wiring without +// requiring the full I/O bring-up, which is its own scoped work. + +impl TransportSocket for EmbassyNetSocket { + type SendFuture<'a> = Ready>; + type RecvFuture<'a> = Ready>; + + fn send_to<'a>(&'a self, _buf: &'a [u8], _target: SocketAddrV4) -> Self::SendFuture<'a> { + // 19c: drive `inner.poll_send_to(buf, target.into(), cx)`. + core::future::ready(Err(TransportError::Unsupported)) + } + + fn recv_from<'a>(&'a self, _buf: &'a mut [u8]) -> Self::RecvFuture<'a> { + // 19c: drive `inner.poll_recv_from(buf, cx)`. + core::future::ready(Err(TransportError::Unsupported)) + } + + fn local_addr(&self) -> Result { + Ok(self.local) + } + + fn join_multicast_v4(&self, _group: Ipv4Addr, _iface: Ipv4Addr) -> Result<(), TransportError> { + // embassy-net's multicast-group join lives on + // `Stack::join_multicast_group` and is async; the user is + // expected to have called it BEFORE constructing any + // EmbassyNetSocket (see EmbassyNetFactory's docstring). We + // return Ok(()) here so simple-someip's `bind_discovery` + // path (which always tries to join) does not error out; + // the real multicast subscription has to have happened on + // the stack already. + Ok(()) + } + + fn leave_multicast_v4(&self, _group: Ipv4Addr, _iface: Ipv4Addr) -> Result<(), TransportError> { + // Symmetric to join_multicast_v4 — leave is also on the + // stack, not the socket. Documented no-op. + Ok(()) + } }