From 3cbc6874247d7b814cab8ec8762bfe2f6f385828 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 16 Oct 2023 23:41:58 +0200 Subject: [PATCH] net/driver: remove Medium, make HardwareAddress non_exhaustive. --- cyw43/src/control.rs | 4 +- embassy-net-driver-channel/CHANGELOG.md | 10 ++--- embassy-net-driver-channel/README.md | 18 ++++----- embassy-net-driver-channel/src/lib.rs | 20 ++------- embassy-net-driver/CHANGELOG.md | 10 ++--- embassy-net-driver/src/lib.rs | 54 +++++++++---------------- embassy-net-enc28j60/src/lib.rs | 3 +- embassy-net-esp-hosted/src/control.rs | 4 +- embassy-net/CHANGELOG.md | 4 +- embassy-net/src/device.rs | 19 ++------- embassy-net/src/lib.rs | 36 ++++++++++++----- embassy-stm32-wpan/src/mac/driver.rs | 11 +---- 12 files changed, 76 insertions(+), 117 deletions(-) diff --git a/cyw43/src/control.rs b/cyw43/src/control.rs index 2585b31dc..d2709304c 100644 --- a/cyw43/src/control.rs +++ b/cyw43/src/control.rs @@ -1,7 +1,7 @@ use core::cmp::{max, min}; -use ch::driver::LinkState; use embassy_net_driver_channel as ch; +use embassy_net_driver_channel::driver::{HardwareAddress, LinkState}; use embassy_time::Timer; pub use crate::bus::SpiBusCyw43; @@ -133,7 +133,7 @@ impl<'a> Control<'a> { Timer::after_millis(100).await; - self.state_ch.set_ethernet_address(mac_addr); + self.state_ch.set_hardware_address(HardwareAddress::Ethernet(mac_addr)); debug!("INIT DONE"); } diff --git a/embassy-net-driver-channel/CHANGELOG.md b/embassy-net-driver-channel/CHANGELOG.md index 589996cfd..b04d0a86b 100644 --- a/embassy-net-driver-channel/CHANGELOG.md +++ b/embassy-net-driver-channel/CHANGELOG.md @@ -5,14 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## 0.2.0 - 2023-10-15 +## 0.2.0 - 2023-10-18 -- Update embassy-net-driver -- `Runner::new` now takes an `embassy_net_driver::HardwareAddress` parameter -- Added `Runner::set_ieee802154_address`, `Runner::ieee802154_address` +- Update `embassy-net-driver` to v0.2 +- `Runner::new` now takes an `embassy_net_driver::HardwareAddress` parameter. +- `Runner::set_ethernet_address` is now `set_hardware_address`. ## 0.1.0 - 2023-06-29 - First release - - diff --git a/embassy-net-driver-channel/README.md b/embassy-net-driver-channel/README.md index 8f904ce95..90a216388 100644 --- a/embassy-net-driver-channel/README.md +++ b/embassy-net-driver-channel/README.md @@ -7,7 +7,9 @@ The `embassy-net-driver` trait is polling-based. To implement it, you must write hand, and hook up the `Waker`s provided by `embassy-net` to the right interrupt handlers so that `embassy-net` knows when to poll your driver again to make more progress. -With `embassy-net-driver-channel` +With `embassy-net-driver-channel` you get a "channel-like" interface instead, where you can send/receive packets +to/from embassy-net. The intended usage is to spawn a "driver task" in the background that does this, passing +packets between the hardware and the channel. ## A note about deadlocks @@ -18,19 +20,19 @@ loop { // Wait for either.. match select( // ... the chip signaling an interrupt, indicating a packet is available to receive, or - irq_pin.wait_for_low(), + irq_pin.wait_for_low(), // ... a TX buffer becoming available, i.e. embassy-net wants to send a packet tx_chan.tx_buf(), ).await { Either::First(_) => { // a packet is ready to be received! let buf = rx_chan.rx_buf().await; // allocate a rx buf from the packet queue - let n = receive_packet_over_spi(buf).await; + let n = receive_packet_over_spi(buf).await; rx_chan.rx_done(n); } Either::Second(buf) => { // a packet is ready to be sent! - send_packet_over_spi(buf).await; + send_packet_over_spi(buf).await; tx_chan.tx_done(); } } @@ -41,7 +43,7 @@ However, this code has a latent deadlock bug. The symptom is it can hang at `rx_ The reason is that, under load, both the TX and RX queues can get full at the same time. When this happens, the `embassy-net` task stalls trying to send because the TX queue is full, therefore it stops processing packets in the RX queue. Your driver task also stalls because the RX queue is full, therefore it stops processing packets in the TX queue. -The fix is to make sure to always service the TX queue while you're waiting for space to become available in the TX queue. For example, select on either "tx_chan.tx_buf() available" or "INT is low AND rx_chan.rx_buf() available": +The fix is to make sure to always service the TX queue while you're waiting for space to become available in the RX queue. For example, select on either "tx_chan.tx_buf() available" or "INT is low AND rx_chan.rx_buf() available": ```rust,ignore loop { @@ -58,12 +60,12 @@ loop { ).await { Either::First(buf) => { // a packet is ready to be received! - let n = receive_packet_over_spi(buf).await; + let n = receive_packet_over_spi(buf).await; rx_chan.rx_done(n); } Either::Second(buf) => { // a packet is ready to be sent! - send_packet_over_spi(buf).await; + send_packet_over_spi(buf).await; tx_chan.tx_done(); } } @@ -79,12 +81,10 @@ These `embassy-net` drivers are implemented using this crate. You can look at th - [`embassy-net-wiznet`](https://github.com/embassy-rs/embassy/tree/main/embassy-net-wiznet) for Wiznet SPI Ethernet MAC+PHY chips. - [`embassy-net-esp-hosted`](https://github.com/embassy-rs/embassy/tree/main/embassy-net-esp-hosted) for using ESP32 chips with the [`esp-hosted`](https://github.com/espressif/esp-hosted) firmware as WiFi adapters for another non-ESP32 MCU. - ## Interoperability This crate can run on any executor. - ## License This work is licensed under either of diff --git a/embassy-net-driver-channel/src/lib.rs b/embassy-net-driver-channel/src/lib.rs index bf7ae5217..bfb2c9c03 100644 --- a/embassy-net-driver-channel/src/lib.rs +++ b/embassy-net-driver-channel/src/lib.rs @@ -8,9 +8,8 @@ use core::cell::RefCell; use core::mem::MaybeUninit; use core::task::{Context, Poll}; -use driver::HardwareAddress; pub use embassy_net_driver as driver; -use embassy_net_driver::{Capabilities, LinkState, Medium}; +use embassy_net_driver::{Capabilities, LinkState}; use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embassy_sync::blocking_mutex::Mutex; use embassy_sync::waitqueue::WakerRegistration; @@ -161,18 +160,10 @@ impl<'d> StateRunner<'d> { }); } - pub fn set_ethernet_address(&self, address: [u8; 6]) { + pub fn set_hardware_address(&self, address: driver::HardwareAddress) { self.shared.lock(|s| { let s = &mut *s.borrow_mut(); - s.hardware_address = driver::HardwareAddress::Ethernet(address); - s.waker.wake(); - }); - } - - pub fn set_ieee802154_address(&self, address: [u8; 8]) { - self.shared.lock(|s| { - let s = &mut *s.borrow_mut(); - s.hardware_address = driver::HardwareAddress::Ieee802154(address); + s.hardware_address = address; s.waker.wake(); }); } @@ -232,11 +223,6 @@ pub fn new<'d, const MTU: usize, const N_RX: usize, const N_TX: usize>( ) -> (Runner<'d, MTU>, Device<'d, MTU>) { let mut caps = Capabilities::default(); caps.max_transmission_unit = MTU; - caps.medium = match &hardware_address { - HardwareAddress::Ethernet(_) => Medium::Ethernet, - HardwareAddress::Ieee802154(_) => Medium::Ieee802154, - HardwareAddress::Ip => Medium::Ip, - }; // safety: this is a self-referential struct, however: // - it can't move while the `'d` borrow is active. diff --git a/embassy-net-driver/CHANGELOG.md b/embassy-net-driver/CHANGELOG.md index 7be622820..165461eff 100644 --- a/embassy-net-driver/CHANGELOG.md +++ b/embassy-net-driver/CHANGELOG.md @@ -5,13 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## 0.2.0 - 2023-10-15 +## 0.2.0 - 2023-10-18 -- Added `Driver::ieee802154_address` -- Added `Medium::Ieee802154` +- Added support for IEEE 802.15.4 mediums. +- Added `Driver::hardware_address()`, `HardwareAddress`. +- Removed `Medium` enum. The medium is deduced out of the hardware address. +- Removed `Driver::ethernet_address()`. Replacement is `hardware_address()`. ## 0.1.0 - 2023-06-29 - First release - - diff --git a/embassy-net-driver/src/lib.rs b/embassy-net-driver/src/lib.rs index b64c10000..87f9f6ed1 100644 --- a/embassy-net-driver/src/lib.rs +++ b/embassy-net-driver/src/lib.rs @@ -7,12 +7,23 @@ use core::task::Context; /// Representation of an hardware address, such as an Ethernet address or an IEEE802.15.4 address. #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] +#[non_exhaustive] pub enum HardwareAddress { - /// A six-octet Ethernet address + /// Ethernet medium, with a A six-octet Ethernet address. + /// + /// Devices of this type send and receive Ethernet frames, + /// and interfaces using it must do neighbor discovery via ARP or NDISC. + /// + /// Examples of devices of this type are Ethernet, WiFi (802.11), Linux `tap`, and VPNs in tap (layer 2) mode. Ethernet([u8; 6]), - /// An eight-octet IEEE802.15.4 address + /// 6LoWPAN over IEEE802.15.4, with an eight-octet address. Ieee802154([u8; 8]), - /// Indicates that a Driver is IP-native, and has no hardware address + /// Indicates that a Driver is IP-native, and has no hardware address. + /// + /// Devices of this type send and receive IP frames, without an + /// Ethernet header. MAC addresses are not used, and no neighbor discovery (ARP, NDISC) is done. + /// + /// Examples of devices of this type are the Linux `tun`, PPP interfaces, VPNs in tun (layer 3) mode. Ip, } @@ -64,6 +75,10 @@ pub trait Driver { fn capabilities(&self) -> Capabilities; /// Get the device's hardware address. + /// + /// The returned hardware address also determines the "medium" of this driver. This indicates + /// what kind of packet the sent/received bytes are, and determines some behaviors of + /// the interface. For example, ARP/NDISC address resolution is only done for Ethernet mediums. fn hardware_address(&self) -> HardwareAddress; } @@ -124,13 +139,6 @@ pub trait TxToken { #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[non_exhaustive] pub struct Capabilities { - /// Medium of the device. - /// - /// This indicates what kind of packet the sent/received bytes are, and determines - /// some behaviors of Interface. For example, ARP/NDISC address resolution is only done - /// for Ethernet mediums. - pub medium: Medium, - /// Maximum transmission unit. /// /// The network device is unable to send or receive frames larger than the value returned @@ -161,32 +169,6 @@ pub struct Capabilities { pub checksum: ChecksumCapabilities, } -/// Type of medium of a device. -#[derive(Debug, Eq, PartialEq, Copy, Clone)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub enum Medium { - /// Ethernet medium. Devices of this type send and receive Ethernet frames, - /// and interfaces using it must do neighbor discovery via ARP or NDISC. - /// - /// Examples of devices of this type are Ethernet, WiFi (802.11), Linux `tap`, and VPNs in tap (layer 2) mode. - Ethernet, - - /// IP medium. Devices of this type send and receive IP frames, without an - /// Ethernet header. MAC addresses are not used, and no neighbor discovery (ARP, NDISC) is done. - /// - /// Examples of devices of this type are the Linux `tun`, PPP interfaces, VPNs in tun (layer 3) mode. - Ip, - - /// IEEE 802_15_4 medium - Ieee802154, -} - -impl Default for Medium { - fn default() -> Medium { - Medium::Ethernet - } -} - /// A description of checksum behavior for every supported protocol. #[derive(Debug, Clone, Default)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] diff --git a/embassy-net-enc28j60/src/lib.rs b/embassy-net-enc28j60/src/lib.rs index f96a6ff14..f18134927 100644 --- a/embassy-net-enc28j60/src/lib.rs +++ b/embassy-net-enc28j60/src/lib.rs @@ -19,7 +19,7 @@ mod traits; use core::cmp; use core::convert::TryInto; -use embassy_net_driver::{Capabilities, HardwareAddress, LinkState, Medium}; +use embassy_net_driver::{Capabilities, HardwareAddress, LinkState}; use embassy_time::Duration; use embedded_hal::digital::OutputPin; use embedded_hal::spi::{Operation, SpiDevice}; @@ -671,7 +671,6 @@ where fn capabilities(&self) -> Capabilities { let mut caps = Capabilities::default(); caps.max_transmission_unit = MTU; - caps.medium = Medium::Ethernet; caps } diff --git a/embassy-net-esp-hosted/src/control.rs b/embassy-net-esp-hosted/src/control.rs index a4996b584..50030f431 100644 --- a/embassy-net-esp-hosted/src/control.rs +++ b/embassy-net-esp-hosted/src/control.rs @@ -1,5 +1,5 @@ -use ch::driver::LinkState; use embassy_net_driver_channel as ch; +use embassy_net_driver_channel::driver::{HardwareAddress, LinkState}; use heapless::String; use crate::ioctl::Shared; @@ -77,7 +77,7 @@ impl<'a> Control<'a> { let mac_addr = self.get_mac_addr().await?; debug!("mac addr: {:02x}", mac_addr); - self.state_ch.set_ethernet_address(mac_addr); + self.state_ch.set_hardware_address(HardwareAddress::Ethernet(mac_addr)); Ok(()) } diff --git a/embassy-net/CHANGELOG.md b/embassy-net/CHANGELOG.md index 3e7c28772..7b91b844b 100644 --- a/embassy-net/CHANGELOG.md +++ b/embassy-net/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## 0.2.0 - 2023-10-15 +## 0.2.0 - 2023-10-18 - Re-export `smoltcp::wire::IpEndpoint` - Add poll functions on UdpSocket @@ -27,5 +27,3 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## 0.1.0 - 2023-06-29 - First release - - diff --git a/embassy-net/src/device.rs b/embassy-net/src/device.rs index 8c2b7d31a..54a0c47e8 100644 --- a/embassy-net/src/device.rs +++ b/embassy-net/src/device.rs @@ -1,7 +1,7 @@ use core::task::Context; -use embassy_net_driver::{Capabilities, Checksum, Driver, Medium, RxToken, TxToken}; -use smoltcp::phy; +use embassy_net_driver::{Capabilities, Checksum, Driver, RxToken, TxToken}; +use smoltcp::phy::{self, Medium}; use smoltcp::time::Instant; pub(crate) struct DriverAdapter<'d, 'c, T> @@ -11,6 +11,7 @@ where // must be Some when actually using this to rx/tx pub cx: Option<&'d mut Context<'c>>, pub inner: &'d mut T, + pub medium: Medium, } impl<'d, 'c, T> phy::Device for DriverAdapter<'d, 'c, T> @@ -46,19 +47,7 @@ where smolcaps.max_transmission_unit = caps.max_transmission_unit; smolcaps.max_burst_size = caps.max_burst_size; - smolcaps.medium = match caps.medium { - #[cfg(feature = "medium-ethernet")] - Medium::Ethernet => phy::Medium::Ethernet, - #[cfg(feature = "medium-ip")] - Medium::Ip => phy::Medium::Ip, - #[cfg(feature = "medium-ieee802154")] - Medium::Ieee802154 => phy::Medium::Ieee802154, - #[allow(unreachable_patterns)] - _ => panic!( - "Unsupported medium {:?}. Make sure to enable it in embassy-net's Cargo features.", - caps.medium - ), - }; + smolcaps.medium = self.medium; smolcaps.checksum.ipv4 = convert(caps.checksum.ipv4); smolcaps.checksum.tcp = convert(caps.checksum.tcp); smolcaps.checksum.udp = convert(caps.checksum.udp); diff --git a/embassy-net/src/lib.rs b/embassy-net/src/lib.rs index a0ad33c6b..c41faee2f 100644 --- a/embassy-net/src/lib.rs +++ b/embassy-net/src/lib.rs @@ -33,6 +33,7 @@ use heapless::Vec; pub use smoltcp::iface::MulticastError; #[allow(unused_imports)] use smoltcp::iface::{Interface, SocketHandle, SocketSet, SocketStorage}; +use smoltcp::phy::Medium; #[cfg(feature = "dhcpv4")] use smoltcp::socket::dhcpv4::{self, RetryConfig}; #[cfg(feature = "medium-ethernet")] @@ -264,14 +265,17 @@ pub(crate) struct SocketStack { next_local_port: u16, } -fn to_smoltcp_hardware_address(addr: driver::HardwareAddress) -> HardwareAddress { +fn to_smoltcp_hardware_address(addr: driver::HardwareAddress) -> (HardwareAddress, Medium) { match addr { #[cfg(feature = "medium-ethernet")] - driver::HardwareAddress::Ethernet(eth) => HardwareAddress::Ethernet(EthernetAddress(eth)), + driver::HardwareAddress::Ethernet(eth) => (HardwareAddress::Ethernet(EthernetAddress(eth)), Medium::Ethernet), #[cfg(feature = "medium-ieee802154")] - driver::HardwareAddress::Ieee802154(ieee) => HardwareAddress::Ieee802154(Ieee802154Address::Extended(ieee)), + driver::HardwareAddress::Ieee802154(ieee) => ( + HardwareAddress::Ieee802154(Ieee802154Address::Extended(ieee)), + Medium::Ieee802154, + ), #[cfg(feature = "medium-ip")] - driver::HardwareAddress::Ip => HardwareAddress::Ip, + driver::HardwareAddress::Ip => (HardwareAddress::Ip, Medium::Ip), #[allow(unreachable_patterns)] _ => panic!( @@ -289,7 +293,8 @@ impl Stack { resources: &'static mut StackResources, random_seed: u64, ) -> Self { - let mut iface_cfg = smoltcp::iface::Config::new(to_smoltcp_hardware_address(device.hardware_address())); + let (hardware_addr, medium) = to_smoltcp_hardware_address(device.hardware_address()); + let mut iface_cfg = smoltcp::iface::Config::new(hardware_addr); iface_cfg.random_seed = random_seed; let iface = Interface::new( @@ -297,6 +302,7 @@ impl Stack { &mut DriverAdapter { inner: &mut device, cx: None, + medium, }, instant_to_smoltcp(Instant::now()), ); @@ -356,7 +362,7 @@ impl Stack { /// Get the hardware address of the network interface. pub fn hardware_address(&self) -> HardwareAddress { - self.with(|_s, i| to_smoltcp_hardware_address(i.device.hardware_address())) + self.with(|_s, i| to_smoltcp_hardware_address(i.device.hardware_address()).0) } /// Get whether the link is up. @@ -812,18 +818,28 @@ impl Inner { fn poll(&mut self, cx: &mut Context<'_>, s: &mut SocketStack) { s.waker.register(cx.waker()); + let (_hardware_addr, medium) = to_smoltcp_hardware_address(self.device.hardware_address()); + #[cfg(any(feature = "medium-ethernet", feature = "medium-ieee802154"))] - if self.device.capabilities().medium == embassy_net_driver::Medium::Ethernet - || self.device.capabilities().medium == embassy_net_driver::Medium::Ieee802154 { - s.iface - .set_hardware_addr(to_smoltcp_hardware_address(self.device.hardware_address())); + let do_set = match medium { + #[cfg(feature = "medium-ethernet")] + Medium::Ethernet => true, + #[cfg(feature = "medium-ieee802154")] + Medium::Ieee802154 => true, + #[allow(unreachable_patterns)] + _ => false, + }; + if do_set { + s.iface.set_hardware_addr(_hardware_addr); + } } let timestamp = instant_to_smoltcp(Instant::now()); let mut smoldev = DriverAdapter { cx: Some(cx), inner: &mut self.device, + medium, }; s.iface.poll(timestamp, &mut smoldev, &mut s.sockets); diff --git a/embassy-stm32-wpan/src/mac/driver.rs b/embassy-stm32-wpan/src/mac/driver.rs index bfc4f1ee8..ffba6e5e8 100644 --- a/embassy-stm32-wpan/src/mac/driver.rs +++ b/embassy-stm32-wpan/src/mac/driver.rs @@ -3,7 +3,7 @@ use core::task::Context; -use embassy_net_driver::{Capabilities, HardwareAddress, LinkState, Medium}; +use embassy_net_driver::{Capabilities, HardwareAddress, LinkState}; use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex; use embassy_sync::channel::Channel; @@ -60,24 +60,15 @@ impl<'d> embassy_net_driver::Driver for Driver<'d> { let mut caps = Capabilities::default(); caps.max_transmission_unit = MTU; // caps.max_burst_size = Some(self.tx.len()); - - caps.medium = Medium::Ieee802154; caps } fn link_state(&mut self, _cx: &mut Context) -> LinkState { - // if self.phy.poll_link(&mut self.station_management, cx) { - // LinkState::Up - // } else { - // LinkState::Down - // } - LinkState::Down } fn hardware_address(&self) -> HardwareAddress { // self.mac_addr - HardwareAddress::Ieee802154([0; 8]) } }