From a0487380da42a71ab7532e2bc1befd1039c18a78 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 22 Sep 2022 16:42:49 +0200 Subject: Replace futures::future::poll_fn -> core::future::poll_fn. --- embassy-net/src/stack.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'embassy-net/src/stack.rs') diff --git a/embassy-net/src/stack.rs b/embassy-net/src/stack.rs index 8d2dd4bca..3a7610758 100644 --- a/embassy-net/src/stack.rs +++ b/embassy-net/src/stack.rs @@ -1,10 +1,9 @@ use core::cell::UnsafeCell; -use core::future::Future; +use core::future::{poll_fn, Future}; use core::task::{Context, Poll}; use embassy_sync::waitqueue::WakerRegistration; use embassy_time::{Instant, Timer}; -use futures::future::poll_fn; use futures::pin_mut; use heapless::Vec; #[cfg(feature = "dhcpv4")] -- cgit From 02abe00439ba873945bd6b60546a200b3da751f1 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Sat, 3 Dec 2022 00:56:16 +0100 Subject: net: don't use UnsafeCell. The "must not be called reentrantly" invariant is too "global" to maintain comfortably, and the cost of the RefCell is negligible, so this was a case of premature optimization. --- embassy-net/src/stack.rs | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) (limited to 'embassy-net/src/stack.rs') diff --git a/embassy-net/src/stack.rs b/embassy-net/src/stack.rs index 3a7610758..631087405 100644 --- a/embassy-net/src/stack.rs +++ b/embassy-net/src/stack.rs @@ -1,4 +1,4 @@ -use core::cell::UnsafeCell; +use core::cell::RefCell; use core::future::{poll_fn, Future}; use core::task::{Context, Poll}; @@ -62,8 +62,8 @@ pub enum ConfigStrategy { } pub struct Stack { - pub(crate) socket: UnsafeCell, - inner: UnsafeCell>, + pub(crate) socket: RefCell, + inner: RefCell>, } struct Inner { @@ -81,8 +81,6 @@ pub(crate) struct SocketStack { next_local_port: u16, } -unsafe impl Send for Stack {} - impl Stack { pub fn new( device: D, @@ -143,40 +141,38 @@ impl Stack { } Self { - socket: UnsafeCell::new(socket), - inner: UnsafeCell::new(inner), + socket: RefCell::new(socket), + inner: RefCell::new(inner), } } - /// SAFETY: must not call reentrantly. - unsafe fn with(&self, f: impl FnOnce(&SocketStack, &Inner) -> R) -> R { - f(&*self.socket.get(), &*self.inner.get()) + fn with(&self, f: impl FnOnce(&SocketStack, &Inner) -> R) -> R { + f(&*self.socket.borrow(), &*self.inner.borrow()) } - /// SAFETY: must not call reentrantly. - unsafe fn with_mut(&self, f: impl FnOnce(&mut SocketStack, &mut Inner) -> R) -> R { - f(&mut *self.socket.get(), &mut *self.inner.get()) + fn with_mut(&self, f: impl FnOnce(&mut SocketStack, &mut Inner) -> R) -> R { + f(&mut *self.socket.borrow_mut(), &mut *self.inner.borrow_mut()) } pub fn ethernet_address(&self) -> [u8; 6] { - unsafe { self.with(|_s, i| i.device.device.ethernet_address()) } + self.with(|_s, i| i.device.device.ethernet_address()) } pub fn is_link_up(&self) -> bool { - unsafe { self.with(|_s, i| i.link_up) } + self.with(|_s, i| i.link_up) } pub fn is_config_up(&self) -> bool { - unsafe { self.with(|_s, i| i.config.is_some()) } + self.with(|_s, i| i.config.is_some()) } pub fn config(&self) -> Option { - unsafe { self.with(|_s, i| i.config.clone()) } + self.with(|_s, i| i.config.clone()) } pub async fn run(&self) -> ! { poll_fn(|cx| { - unsafe { self.with_mut(|s, i| i.poll(cx, s)) } + self.with_mut(|s, i| i.poll(cx, s)); Poll::<()>::Pending }) .await; -- cgit From f7fe0c1441843b04fa17ba0fe94f8c8d4f851882 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 7 Dec 2022 00:28:38 +0100 Subject: net: update smoltcp --- embassy-net/src/stack.rs | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) (limited to 'embassy-net/src/stack.rs') diff --git a/embassy-net/src/stack.rs b/embassy-net/src/stack.rs index 631087405..5c4fb0442 100644 --- a/embassy-net/src/stack.rs +++ b/embassy-net/src/stack.rs @@ -266,21 +266,12 @@ impl Inner { None => {} Some(dhcpv4::Event::Deconfigured) => self.unapply_config(s), Some(dhcpv4::Event::Configured(config)) => { - let mut dns_servers = Vec::new(); - for s in &config.dns_servers { - if let Some(addr) = s { - dns_servers.push(addr.clone()).unwrap(); - } - } - - self.apply_config( - s, - Config { - address: config.address, - gateway: config.router, - dns_servers, - }, - ) + let config = Config { + address: config.address, + gateway: config.router, + dns_servers: config.dns_servers, + }; + self.apply_config(s, config) } } } else if old_link_up { -- cgit From ac74613b5a7be72acd8d5259055963f8b4aba7fd Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 7 Dec 2022 15:55:46 +0100 Subject: net: remove packet pool. The pool was prone to deadlocks, especially due to having a single pool for rx+tx. If the pool got full with rx'd packets it would deadlock because processing a rx packet requires doing another allocation on the pool, for the possibly tx'd response, before deallocating the rx'd packet. This also allows Device impls to allocate the packet memory in a particular RAM kind, if needed for example to do DMA. The `Device` trait is now token-based, like smoltcp's. In the end, this is better because it allows callers to manage memory however they want (including with a pool if they want to). --- embassy-net/src/stack.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) (limited to 'embassy-net/src/stack.rs') diff --git a/embassy-net/src/stack.rs b/embassy-net/src/stack.rs index 5c4fb0442..21316e485 100644 --- a/embassy-net/src/stack.rs +++ b/embassy-net/src/stack.rs @@ -12,7 +12,7 @@ use smoltcp::iface::{Interface, InterfaceBuilder, SocketSet, SocketStorage}; #[cfg(feature = "medium-ethernet")] use smoltcp::iface::{Neighbor, NeighborCache, Route, Routes}; #[cfg(feature = "medium-ethernet")] -use smoltcp::phy::{Device as _, Medium}; +use smoltcp::phy::Medium; #[cfg(feature = "dhcpv4")] use smoltcp::socket::dhcpv4; use smoltcp::time::Instant as SmolInstant; @@ -67,7 +67,7 @@ pub struct Stack { } struct Inner { - device: DeviceAdapter, + device: D, link_up: bool, config: Option, #[cfg(feature = "dhcpv4")] @@ -83,7 +83,7 @@ pub(crate) struct SocketStack { impl Stack { pub fn new( - device: D, + mut device: D, config: ConfigStrategy, resources: &'static mut StackResources, random_seed: u64, @@ -98,8 +98,6 @@ impl Stack { [0, 0, 0, 0, 0, 0] }; - let mut device = DeviceAdapter::new(device); - let mut b = InterfaceBuilder::new(); b = b.ip_addrs(&mut resources.addresses[..]); b = b.random_seed(random_seed); @@ -111,7 +109,10 @@ impl Stack { b = b.routes(Routes::new(&mut resources.routes[..])); } - let iface = b.finalize(&mut device); + let iface = b.finalize(&mut DeviceAdapter { + inner: &mut device, + cx: None, + }); let sockets = SocketSet::new(&mut resources.sockets[..]); @@ -155,7 +156,7 @@ impl Stack { } pub fn ethernet_address(&self) -> [u8; 6] { - self.with(|_s, i| i.device.device.ethernet_address()) + self.with(|_s, i| i.device.ethernet_address()) } pub fn is_link_up(&self) -> bool { @@ -238,11 +239,14 @@ impl Inner { } fn poll(&mut self, cx: &mut Context<'_>, s: &mut SocketStack) { - self.device.device.register_waker(cx.waker()); s.waker.register(cx.waker()); let timestamp = instant_to_smoltcp(Instant::now()); - if s.iface.poll(timestamp, &mut self.device, &mut s.sockets).is_err() { + let mut smoldev = DeviceAdapter { + cx: Some(cx), + inner: &mut self.device, + }; + if s.iface.poll(timestamp, &mut smoldev, &mut s.sockets).is_err() { // If poll() returns error, it may not be done yet, so poll again later. cx.waker().wake_by_ref(); return; @@ -250,7 +254,7 @@ impl Inner { // Update link up let old_link_up = self.link_up; - self.link_up = self.device.device.link_state() == LinkState::Up; + self.link_up = self.device.link_state(cx) == LinkState::Up; // Print when changed if old_link_up != self.link_up { -- cgit From aaaf5f23a8a3a0df1ad2186802e2afc061a74b72 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 7 Dec 2022 16:02:28 +0100 Subject: net: move stack into lib.rs --- embassy-net/src/stack.rs | 306 ----------------------------------------------- 1 file changed, 306 deletions(-) delete mode 100644 embassy-net/src/stack.rs (limited to 'embassy-net/src/stack.rs') diff --git a/embassy-net/src/stack.rs b/embassy-net/src/stack.rs deleted file mode 100644 index 21316e485..000000000 --- a/embassy-net/src/stack.rs +++ /dev/null @@ -1,306 +0,0 @@ -use core::cell::RefCell; -use core::future::{poll_fn, Future}; -use core::task::{Context, Poll}; - -use embassy_sync::waitqueue::WakerRegistration; -use embassy_time::{Instant, Timer}; -use futures::pin_mut; -use heapless::Vec; -#[cfg(feature = "dhcpv4")] -use smoltcp::iface::SocketHandle; -use smoltcp::iface::{Interface, InterfaceBuilder, SocketSet, SocketStorage}; -#[cfg(feature = "medium-ethernet")] -use smoltcp::iface::{Neighbor, NeighborCache, Route, Routes}; -#[cfg(feature = "medium-ethernet")] -use smoltcp::phy::Medium; -#[cfg(feature = "dhcpv4")] -use smoltcp::socket::dhcpv4; -use smoltcp::time::Instant as SmolInstant; -#[cfg(feature = "medium-ethernet")] -use smoltcp::wire::{EthernetAddress, HardwareAddress, IpAddress}; -use smoltcp::wire::{IpCidr, Ipv4Address, Ipv4Cidr}; - -use crate::device::{Device, DeviceAdapter, LinkState}; - -const LOCAL_PORT_MIN: u16 = 1025; -const LOCAL_PORT_MAX: u16 = 65535; - -pub struct StackResources { - addresses: [IpCidr; ADDR], - sockets: [SocketStorage<'static>; SOCK], - - #[cfg(feature = "medium-ethernet")] - routes: [Option<(IpCidr, Route)>; 1], - #[cfg(feature = "medium-ethernet")] - neighbor_cache: [Option<(IpAddress, Neighbor)>; NEIGHBOR], -} - -impl StackResources { - pub fn new() -> Self { - Self { - addresses: [IpCidr::new(Ipv4Address::UNSPECIFIED.into(), 32); ADDR], - sockets: [SocketStorage::EMPTY; SOCK], - #[cfg(feature = "medium-ethernet")] - routes: [None; 1], - #[cfg(feature = "medium-ethernet")] - neighbor_cache: [None; NEIGHBOR], - } - } -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Config { - pub address: Ipv4Cidr, - pub gateway: Option, - pub dns_servers: Vec, -} - -pub enum ConfigStrategy { - Static(Config), - #[cfg(feature = "dhcpv4")] - Dhcp, -} - -pub struct Stack { - pub(crate) socket: RefCell, - inner: RefCell>, -} - -struct Inner { - device: D, - link_up: bool, - config: Option, - #[cfg(feature = "dhcpv4")] - dhcp_socket: Option, -} - -pub(crate) struct SocketStack { - pub(crate) sockets: SocketSet<'static>, - pub(crate) iface: Interface<'static>, - pub(crate) waker: WakerRegistration, - next_local_port: u16, -} - -impl Stack { - pub fn new( - mut device: D, - config: ConfigStrategy, - resources: &'static mut StackResources, - random_seed: u64, - ) -> Self { - #[cfg(feature = "medium-ethernet")] - let medium = device.capabilities().medium; - - #[cfg(feature = "medium-ethernet")] - let ethernet_addr = if medium == Medium::Ethernet { - device.ethernet_address() - } else { - [0, 0, 0, 0, 0, 0] - }; - - let mut b = InterfaceBuilder::new(); - b = b.ip_addrs(&mut resources.addresses[..]); - b = b.random_seed(random_seed); - - #[cfg(feature = "medium-ethernet")] - if medium == Medium::Ethernet { - b = b.hardware_addr(HardwareAddress::Ethernet(EthernetAddress(ethernet_addr))); - b = b.neighbor_cache(NeighborCache::new(&mut resources.neighbor_cache[..])); - b = b.routes(Routes::new(&mut resources.routes[..])); - } - - let iface = b.finalize(&mut DeviceAdapter { - inner: &mut device, - cx: None, - }); - - let sockets = SocketSet::new(&mut resources.sockets[..]); - - let next_local_port = (random_seed % (LOCAL_PORT_MAX - LOCAL_PORT_MIN) as u64) as u16 + LOCAL_PORT_MIN; - - let mut inner = Inner { - device, - link_up: false, - config: None, - #[cfg(feature = "dhcpv4")] - dhcp_socket: None, - }; - let mut socket = SocketStack { - sockets, - iface, - waker: WakerRegistration::new(), - next_local_port, - }; - - match config { - ConfigStrategy::Static(config) => inner.apply_config(&mut socket, config), - #[cfg(feature = "dhcpv4")] - ConfigStrategy::Dhcp => { - let handle = socket.sockets.add(smoltcp::socket::dhcpv4::Socket::new()); - inner.dhcp_socket = Some(handle); - } - } - - Self { - socket: RefCell::new(socket), - inner: RefCell::new(inner), - } - } - - fn with(&self, f: impl FnOnce(&SocketStack, &Inner) -> R) -> R { - f(&*self.socket.borrow(), &*self.inner.borrow()) - } - - fn with_mut(&self, f: impl FnOnce(&mut SocketStack, &mut Inner) -> R) -> R { - f(&mut *self.socket.borrow_mut(), &mut *self.inner.borrow_mut()) - } - - pub fn ethernet_address(&self) -> [u8; 6] { - self.with(|_s, i| i.device.ethernet_address()) - } - - pub fn is_link_up(&self) -> bool { - self.with(|_s, i| i.link_up) - } - - pub fn is_config_up(&self) -> bool { - self.with(|_s, i| i.config.is_some()) - } - - pub fn config(&self) -> Option { - self.with(|_s, i| i.config.clone()) - } - - pub async fn run(&self) -> ! { - poll_fn(|cx| { - self.with_mut(|s, i| i.poll(cx, s)); - Poll::<()>::Pending - }) - .await; - unreachable!() - } -} - -impl SocketStack { - #[allow(clippy::absurd_extreme_comparisons)] - pub fn get_local_port(&mut self) -> u16 { - let res = self.next_local_port; - self.next_local_port = if res >= LOCAL_PORT_MAX { LOCAL_PORT_MIN } else { res + 1 }; - res - } -} - -impl Inner { - fn apply_config(&mut self, s: &mut SocketStack, config: Config) { - #[cfg(feature = "medium-ethernet")] - let medium = self.device.capabilities().medium; - - debug!("Acquired IP configuration:"); - - debug!(" IP address: {}", config.address); - self.set_ipv4_addr(s, config.address); - - #[cfg(feature = "medium-ethernet")] - if medium == Medium::Ethernet { - if let Some(gateway) = config.gateway { - debug!(" Default gateway: {}", gateway); - s.iface.routes_mut().add_default_ipv4_route(gateway).unwrap(); - } else { - debug!(" Default gateway: None"); - s.iface.routes_mut().remove_default_ipv4_route(); - } - } - for (i, s) in config.dns_servers.iter().enumerate() { - debug!(" DNS server {}: {}", i, s); - } - - self.config = Some(config) - } - - #[allow(unused)] // used only with dhcp - fn unapply_config(&mut self, s: &mut SocketStack) { - #[cfg(feature = "medium-ethernet")] - let medium = self.device.capabilities().medium; - - debug!("Lost IP configuration"); - self.set_ipv4_addr(s, Ipv4Cidr::new(Ipv4Address::UNSPECIFIED, 0)); - #[cfg(feature = "medium-ethernet")] - if medium == Medium::Ethernet { - s.iface.routes_mut().remove_default_ipv4_route(); - } - self.config = None - } - - fn set_ipv4_addr(&mut self, s: &mut SocketStack, cidr: Ipv4Cidr) { - s.iface.update_ip_addrs(|addrs| { - let dest = addrs.iter_mut().next().unwrap(); - *dest = IpCidr::Ipv4(cidr); - }); - } - - fn poll(&mut self, cx: &mut Context<'_>, s: &mut SocketStack) { - s.waker.register(cx.waker()); - - let timestamp = instant_to_smoltcp(Instant::now()); - let mut smoldev = DeviceAdapter { - cx: Some(cx), - inner: &mut self.device, - }; - if s.iface.poll(timestamp, &mut smoldev, &mut s.sockets).is_err() { - // If poll() returns error, it may not be done yet, so poll again later. - cx.waker().wake_by_ref(); - return; - } - - // Update link up - let old_link_up = self.link_up; - self.link_up = self.device.link_state(cx) == LinkState::Up; - - // Print when changed - if old_link_up != self.link_up { - info!("link_up = {:?}", self.link_up); - } - - #[cfg(feature = "dhcpv4")] - if let Some(dhcp_handle) = self.dhcp_socket { - let socket = s.sockets.get_mut::(dhcp_handle); - - if self.link_up { - match socket.poll() { - None => {} - Some(dhcpv4::Event::Deconfigured) => self.unapply_config(s), - Some(dhcpv4::Event::Configured(config)) => { - let config = Config { - address: config.address, - gateway: config.router, - dns_servers: config.dns_servers, - }; - self.apply_config(s, config) - } - } - } else if old_link_up { - socket.reset(); - self.unapply_config(s); - } - } - //if old_link_up || self.link_up { - // self.poll_configurator(timestamp) - //} - - if let Some(poll_at) = s.iface.poll_at(timestamp, &mut s.sockets) { - let t = Timer::at(instant_from_smoltcp(poll_at)); - pin_mut!(t); - if t.poll(cx).is_ready() { - cx.waker().wake_by_ref(); - } - } - } -} - -fn instant_to_smoltcp(instant: Instant) -> SmolInstant { - SmolInstant::from_millis(instant.as_millis() as i64) -} - -fn instant_from_smoltcp(instant: SmolInstant) -> Instant { - Instant::from_millis(instant.total_millis() as u64) -} -- cgit