Skip to content

Commit

Permalink
Merge pull request #2076 from embassy-rs/net-driver-simplify
Browse files Browse the repository at this point in the history
net/driver: remove Medium, make HardwareAddress non_exhaustive.
  • Loading branch information
Dirbaio authored Oct 18, 2023
2 parents 4f7b831 + 3cbc687 commit 35ffdf2
Show file tree
Hide file tree
Showing 12 changed files with 76 additions and 117 deletions.
4 changes: 2 additions & 2 deletions cyw43/src/control.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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");
}
Expand Down
10 changes: 4 additions & 6 deletions embassy-net-driver-channel/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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


18 changes: 9 additions & 9 deletions embassy-net-driver-channel/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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();
}
}
Expand All @@ -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 {
Expand All @@ -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();
}
}
Expand All @@ -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
Expand Down
20 changes: 3 additions & 17 deletions embassy-net-driver-channel/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
});
}
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions embassy-net-driver/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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


54 changes: 18 additions & 36 deletions embassy-net-driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))]
Expand Down
3 changes: 1 addition & 2 deletions embassy-net-enc28j60/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -671,7 +671,6 @@ where
fn capabilities(&self) -> Capabilities {
let mut caps = Capabilities::default();
caps.max_transmission_unit = MTU;
caps.medium = Medium::Ethernet;
caps
}

Expand Down
4 changes: 2 additions & 2 deletions embassy-net-esp-hosted/src/control.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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(())
}
Expand Down
4 changes: 1 addition & 3 deletions embassy-net/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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


19 changes: 4 additions & 15 deletions embassy-net/src/device.rs
Original file line number Diff line number Diff line change
@@ -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>
Expand All @@ -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>
Expand Down Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 35ffdf2

Please sign in to comment.