Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: remove MAC RX activity watchdog. #1622

Merged
merged 1 commit into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 2 additions & 29 deletions task/net/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ mod idl {
use core::sync::atomic::{AtomicU32, Ordering};
use enum_map::Enum;
use multitimer::{Multitimer, Repeat};
use task_jefe_api::Jefe;
use task_net_api::MacAddressBlock;
use zerocopy::{AsBytes, U16};

Expand Down Expand Up @@ -148,10 +147,6 @@ const TX_RING_SZ: usize = 4;

const RX_RING_SZ: usize = 4;

/// How long to wait with no received packets before we decide the driver is
/// b0rked and restart it.
const RX_WATCHDOG_INTERVAL: u64 = 60_000;

/////////////////////////////////////////////////////////////////////////////
// Main driver loop.

Expand All @@ -163,7 +158,6 @@ static ITER_COUNT: AtomicU32 = AtomicU32::new(0);
fn main() -> ! {
let sys = SYS.get_task_id();
let sys = Sys::from(sys);
let jefe = Jefe::from(JEFE.get_task_id());

// Do any preinit tasks specific to this board. For hardware which requires
// explicit clock configuration, this is where the `net` tasks waits for
Expand Down Expand Up @@ -221,11 +215,11 @@ fn main() -> ! {
// Turn on our IRQ.
userlib::sys_irq_control(notifications::ETH_IRQ_MASK, true);

// We use two timers:
// We use only one timer, but we're using a multitimer in case we need to
// add a second one again (we previously had a second one for the watchdog):
#[derive(Copy, Clone, Enum)]
enum Timers {
Wake,
Watchdog,
}
let mut multitimer =
Multitimer::<Timers>::new(notifications::WAKE_TIMER_BIT);
Expand All @@ -242,9 +236,6 @@ fn main() -> ! {
);
}

// Start the watchdog timer running.
multitimer.set_timer(Timers::Watchdog, now + RX_WATCHDOG_INTERVAL, None);

// Go!
loop {
ITER_COUNT.fetch_add(1, Ordering::Relaxed);
Expand All @@ -253,18 +244,6 @@ fn main() -> ! {
let now = sys_get_timer().now;
let activity = server.poll(now);

if activity.mac_rx {
// Whenever we observe activity we bump the timer forward. Because
// we're going to poll the timer below (after doing this) and we
// always poll and immediately consume iter_fired, this will prevent
// the timer from firing this iteration.
multitimer.set_timer(
Timers::Watchdog,
now + RX_WATCHDOG_INTERVAL,
None,
);
}

if activity.ip {
// Ask the server to iterate over sockets looking for work
server.wake_sockets();
Expand All @@ -276,9 +255,6 @@ fn main() -> ! {
server.wake();
// timer is set to auto-repeat
}
Timers::Watchdog => {
jefe.restart_me();
}
}
}
let mut msgbuf = [0u8; idl::INCOMING_SIZE];
Expand All @@ -291,9 +267,6 @@ fn main() -> ! {
pub(crate) struct Activity {
/// Did the IP stack do anything? (i.e. do we need to process socket events)
ip: bool,
/// Did the MAC have anything available to receive? (i.e. is it still
/// working)
mac_rx: bool,
}

/// We can map an Ethernet MAC address into the IPv6 space as follows.
Expand Down
6 changes: 1 addition & 5 deletions task/net/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,6 @@ where
}

pub trait DeviceExt: smoltcp::phy::Device {
fn read_and_clear_activity_flag(&self) -> bool;

fn make_meta(
&self,
port: u16,
Expand Down Expand Up @@ -444,19 +442,17 @@ where
// Do not be tempted to use `Iterator::any` here, it short circuits and
// we really do want to poll all of them.
let mut ip = false;
let mut mac_rx = false;
for vlan in &mut self.vlan_state {
ip |= vlan.iface.poll(
instant,
&mut vlan.device,
&mut vlan.socket_set,
);
// Test and clear our receive activity flag.
mac_rx |= vlan.device.read_and_clear_activity_flag();
ip |= vlan.check_socket_watchdog();
}

crate::Activity { ip, mac_rx }
crate::Activity { ip }
}

/// Iterate over sockets, waking any that can do work.
Expand Down
17 changes: 1 addition & 16 deletions task/net/src/server_basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use crate::{
server::{DeviceExt, GenServerImpl, Storage},
MacAddressBlock,
};
use core::cell::Cell;
use mutable_statics::mutable_statics;
use task_net_api::UdpMetadata;

Expand Down Expand Up @@ -55,15 +54,11 @@ where

pub struct Smol<'d> {
eth: &'d eth::Ethernet,
mac_rx: Cell<bool>,
}

impl<'d> From<&'d eth::Ethernet> for Smol<'d> {
fn from(eth: &'d eth::Ethernet) -> Self {
Self {
eth,
mac_rx: Cell::new(false),
}
Self { eth }
}
}

Expand Down Expand Up @@ -105,12 +100,6 @@ impl<'a> smoltcp::phy::Device for Smol<'a> {
// Note that the can_recv and can_send checks remain valid because
// the token mutably borrows the phy.
if self.eth.can_recv() && self.eth.can_send() {
// We record this as "data available from the MAC" because it's
// sufficient to catch the bug we're defending against with the
// watchdog, even if the IP stack decides not to consume the token
// for some reason (that'd be a software bug instead).
self.mac_rx.set(true);

Some((OurRxToken(self.eth), OurTxToken(self.eth)))
} else {
None
Expand All @@ -134,10 +123,6 @@ impl<'a> smoltcp::phy::Device for Smol<'a> {
}

impl DeviceExt for Smol<'_> {
fn read_and_clear_activity_flag(&self) -> bool {
self.mac_rx.take()
}

fn make_meta(
&self,
port: u16,
Expand Down
8 changes: 0 additions & 8 deletions task/net/src/server_vlan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

use drv_stm32h7_eth as eth;

use core::cell::Cell;
use mutable_statics::mutable_statics;
use task_net_api::UdpMetadata;

Expand All @@ -32,7 +31,6 @@ fn claim_server_storage_statics() -> &'static mut [Storage; VLAN_COUNT] {
pub struct VLanEthernet<'a> {
pub eth: &'a eth::Ethernet,
pub vid: u16,
mac_rx: Cell<bool>,
}

impl<'a> smoltcp::phy::Device for VLanEthernet<'a> {
Expand All @@ -44,7 +42,6 @@ impl<'a> smoltcp::phy::Device for VLanEthernet<'a> {
_timestamp: smoltcp::time::Instant,
) -> Option<(Self::RxToken<'a>, Self::TxToken<'a>)> {
if self.eth.vlan_can_recv(self.vid, VLAN_RANGE) && self.eth.can_send() {
self.mac_rx.set(true);
Some((
VLanRxToken(self.eth, self.vid),
VLanTxToken(self.eth, self.vid),
Expand All @@ -69,10 +66,6 @@ impl<'a> smoltcp::phy::Device for VLanEthernet<'a> {
}

impl DeviceExt for VLanEthernet<'_> {
fn read_and_clear_activity_flag(&self) -> bool {
self.mac_rx.take()
}

fn make_meta(
&self,
port: u16,
Expand Down Expand Up @@ -133,7 +126,6 @@ where
|i| VLanEthernet {
eth,
vid: generated::VLAN_RANGE.start + i as u16,
mac_rx: Cell::new(false),
},
)
}
Loading