Skip to content

Commit

Permalink
net: fix several unwraps of complex errors
Browse files Browse the repository at this point in the history
This knocks a lot of flash off most netstack task sizes, by removing
much of its dependence on the formatting machinery invoked by unwrap.
(It's not all gone, because smoltcp still uses it.) Concretely,

- Gimlet E net: -16,388 bytes
- Monorail server: -8,820 bytes
- Sidecar D net: -8 bytes, because something is still pulling in the
  goo.
  • Loading branch information
cbiffle committed Feb 27, 2024
1 parent 7b26e58 commit 6b6faea
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 15 deletions.
4 changes: 2 additions & 2 deletions drv/ksz8463/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub use registers::{MIBCounter, Register};

////////////////////////////////////////////////////////////////////////////////

#[derive(Copy, Clone, Debug, Eq, PartialEq)]

This comment has been minimized.

Copy link
@hawkw

hawkw Feb 28, 2024

Member

BTW, this change appears to have broken the build for gimletlet, since it has an error type which includes a KszError that implements Debug:

$ cargo xtask dist  app/gimletlet/app-mgmt.toml
    Finished dev [optimized + debuginfo] target(s) in 0.32s
     Running `target/debug/xtask dist app/gimletlet/app-mgmt.toml`
building crate task-jefe
    Finished release [optimized + debuginfo] target(s) in 0.22s
target/thumbv7em-none-eabihf/release/task-jefe -> target/gimletlet-mgmt/dist/jefe.elf
building crate drv-stm32xx-sys
    Finished release [optimized + debuginfo] target(s) in 0.14s
target/thumbv7em-none-eabihf/release/drv-stm32xx-sys -> target/gimletlet-mgmt/dist/sys.elf
building crate drv-user-leds
    Finished release [optimized + debuginfo] target(s) in 0.13s
target/thumbv7em-none-eabihf/release/drv-user-leds -> target/gimletlet-mgmt/dist/user_leds.elf
building crate task-hiffy
    Finished release [optimized + debuginfo] target(s) in 0.15s
target/thumbv7em-none-eabihf/release/task-hiffy -> target/gimletlet-mgmt/dist/hiffy.elf
building crate task-net
   Compiling task-net v0.1.0 (/home/eliza/Code/oxide/hubris/task/net)
   Compiling task-net v0.1.0 (/home/eliza/Code/oxide/hubris/task/net)
error[E0277]: `ksz8463::Error` doesn't implement `Debug`
  --> task/net/src/bsp/gimletlet_mgmt.rs:39:9
   |
31 | #[derive(Copy, Clone, Debug, Eq, PartialEq, counters::Count)]
   |                       ----- in this derive macro expansion
...
39 |         err: KszError,
   |         ^^^^^^^^^^^^^ `ksz8463::Error` cannot be formatted using `{:?}` because it doesn't implement `Debug`
   |
   = help: the trait `Debug` is not implemented for `ksz8463::Error`
   = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `vsc85xx::VscError` doesn't implement `Debug`
  --> task/net/src/bsp/gimletlet_mgmt.rs:71:9
   |
31 | #[derive(Copy, Clone, Debug, Eq, PartialEq, counters::Count)]
   |                       ----- in this derive macro expansion
...
71 |         err: VscError,
   |         ^^^^^^^^^^^^^ `vsc85xx::VscError` cannot be formatted using `{:?}` because it doesn't implement `Debug`
   |
   = help: the trait `Debug` is not implemented for `vsc85xx::VscError`
   = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.
error: could not compile `task-net` due to 2 previous errors
Error: failed to build net

Stumbled across this while working on a change to add event counters.

#[derive(Copy, Clone, Eq, PartialEq)]
pub enum Error {
SpiError(SpiError),
WrongChipId(u16),
Expand Down Expand Up @@ -50,7 +50,7 @@ pub enum Mode {

////////////////////////////////////////////////////////////////////////////////

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
#[derive(Copy, Clone, Eq, PartialEq)]
enum Trace {
None,
Read(Register, u16),
Expand Down
2 changes: 1 addition & 1 deletion drv/vsc-err/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
use drv_spi_api::SpiError;
use idol_runtime::ServerDeath;

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
#[derive(Copy, Clone, Eq, PartialEq)]
pub enum VscError {
SpiError(SpiError),
ServerDied,
Expand Down
4 changes: 2 additions & 2 deletions task/monorail-server/src/bsp/sidecar_bcd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use drv_sidecar_front_io::phy_smi::PhySmi;
use drv_sidecar_seq_api::Sequencer;
use ringbuf::*;
use userlib::{hl::sleep_for, task_slot};
use userlib::{hl::sleep_for, task_slot, UnwrapLite};
use vsc7448::{
config::Speed, miim_phy::Vsc7448MiimPhy, Vsc7448, Vsc7448Rw, VscError,
};
Expand Down Expand Up @@ -367,7 +367,7 @@ impl<'a, R: Vsc7448Rw> Bsp<'a, R> {

// The VSC8504 on the sidecar has its SIGDET GPIOs pulled down,
// for some reason.
self.vsc8504.set_sigdet_polarity(rw, true).unwrap();
self.vsc8504.set_sigdet_polarity(rw, true).unwrap_lite();

// Switch the GPIO to an output. Since the output register is low
// by default, this pulls COMA_MODE low, bringing the VSC8504 into
Expand Down
2 changes: 1 addition & 1 deletion task/monorail-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ fn main() -> ! {
// will ensure that any errors are available for inspection (which
// `Jefe::restart_me` would not) while making the restart cheaper.
ringbuf_entry!(Trace::BspInitFailed(e));
panic!("Could not initialize BSP: {:?}", e);
panic!();
}
};

Expand Down
2 changes: 1 addition & 1 deletion task/net/src/bsp/gimletlet_nic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use task_net_api::PhyError;
use userlib::hl::sleep_for;
use vsc7448_pac::types::PhyRegisterAddress;

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
#[derive(Copy, Clone, Eq, PartialEq)]
enum Trace {
None,
BspConfigured,
Expand Down
5 changes: 3 additions & 2 deletions task/net/src/bsp/nucleo_h7.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::pins;
use drv_stm32h7_eth as eth;
use drv_stm32xx_sys_api::{Alternate, Port, Sys};
use task_net_api::PhyError;
use userlib::UnwrapLite;
use vsc7448_pac::{phy, types::PhyRegisterAddress};
use vsc85xx::PhyRw;

Expand Down Expand Up @@ -50,12 +51,12 @@ impl crate::bsp_support::Bsp for BspImpl {
let phy = MiimBridge::new(eth);
let mut r = phy::standard::MODE_CONTROL::from(
phy.read_raw(PHYADDR, phy::STANDARD::MODE_CONTROL().addr)
.unwrap(),
.unwrap_lite(),
);
r.set_auto_neg_ena(1);
r.set_restart_auto_neg(1);
phy.write_raw(PHYADDR, phy::STANDARD::MODE_CONTROL().addr, r.into())
.unwrap();
.unwrap_lite();

Self {}
}
Expand Down
4 changes: 2 additions & 2 deletions task/net/src/bsp/sidecar_bcd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use drv_stm32xx_sys_api::{Alternate, Port, Sys};
use task_net_api::{
ManagementCounters, ManagementLinkStatus, MgmtError, PhyError,
};
use userlib::{hl::sleep_for, task_slot};
use userlib::{hl::sleep_for, task_slot, UnwrapLite};
use vsc7448_pac::types::PhyRegisterAddress;

task_slot!(SEQ, seq);
Expand Down Expand Up @@ -98,7 +98,7 @@ impl bsp_support::Bsp for BspImpl {
// The VSC8552 on the sidecar has its SIGDET GPIOs pulled down,
// for some reason.
let rw = &mut MiimBridge::new(eth);
bsp.vsc85x2.set_sigdet_polarity(rw, true).unwrap();
bsp.vsc85x2.set_sigdet_polarity(rw, true).unwrap_lite();

Self(bsp)
}
Expand Down
8 changes: 4 additions & 4 deletions task/net/src/mgmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use ringbuf::*;
use task_net_api::{
ManagementCounters, ManagementLinkStatus, MgmtError, PhyError,
};
use userlib::hl::sleep_for;
use userlib::{hl::sleep_for, UnwrapLite};
use vsc7448_pac::{phy, types::PhyRegisterAddress};
use vsc85xx::{vsc85x2::Vsc85x2, Counter, VscError};

Expand All @@ -26,7 +26,7 @@ pub enum Ksz8463ResetSpeed {
Normal,
}

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
#[derive(Copy, Clone, Eq, PartialEq)]
enum Trace {
None,
Ksz8463Err { port: u8, err: KszError },
Expand Down Expand Up @@ -94,7 +94,7 @@ impl Config {
// VSC8552 over 100-BASE FX
self.ksz8463
.configure(ksz8463::Mode::Fiber, self.ksz8463_vlan_mode)
.unwrap();
.unwrap_lite();
self.ksz8463
}

Expand Down Expand Up @@ -153,7 +153,7 @@ impl Config {
sys.gpio_reset(coma_mode);
}

vsc85x2.unwrap() // TODO
vsc85x2.unwrap_lite() // TODO
}
}

Expand Down

0 comments on commit 6b6faea

Please sign in to comment.