packrat ereport storage and snitch implementation (#2126)

Things fail.

Not finding out about it sucks.

This branch implements the Hubris side of the ereport ingestion system,
as described [RFD 545]. Work on this was started by @cbiffle in #2002,
which implemented the core ring-buffer data structure used to store
ereports. Meanwhile, oxidecomputer/management-gateway-service#370, 
oxidecomputer/omicron#7803, and oxidecomputer/omicron#8296 added
the MGS and Omicron components of this system.

This branch picks up where Cliff left off, and "draws the rest of the
owl" by implementing the aggregation of ereports in the `packrat` task
using this data structure, and adding a new `snitch` task, which acts as
a proxy to allow ereports stored by `packrat` to be read over the
management network.

## Architecture

Ereports are stored by `packrat` because we would like as many tasks as
possible to be able to report errors by making IPC call to the task
responsible for ereport storage. This means that the task aggregating
ereports must be a high-priority task, so that as many other tasks as
possible may be its clients. Additionally, we would like to include the
system's VPD identity as metadata for ereports, and this data is already
stored by packrat. Finally, we would like to minimize the likelihood of
the task that stores ereports crashing, as this would result in data
loss, and packrat already is expected not to crash.

On the other hand, the task that actually evacuates these ereports over
the management network must run at a priority lower than that of the
`net` task, of which it is a client. Thus the separation of
responsibilities between `packrat` and the `snitch`. The snitch task is
fairly simple. It receives packets sent to the ereport socket,
interprets the request message, and forwards the request to packrat. Any
ereports sent back by packrat are sent in response to the request. The
snitch ends up being a pretty dumb, stateless proxy: as the response
packet is encoded by packrat; all we end up doing is taking the bytes
received from packrat and stuffing them into the socket's send queue.
The real purpose of this thing is just to serve as a trampoline between
the high priority level of packrat and a priority level lower than that
of the net task.

## `snitch-core` Fixes

While testing behavior when the ereport buffer is full, I found a
potential panic in the existing `snitch-core` code. Previously, every
time ereports are read from the buffer while it is in the `Losing` state
(i.e., ereports have been discarded because the buffer was full),
`snitch-core` attempts to insert a new loss record at the end of the
buffer (calling `recover_if_needed()`). This ensures that the data loss
is reported to the reader ASAP. The problem is that this code assumed
that there would always be space for an additional loss record, and
panicked if it didn't fit. I added a test reproducing this panic in
ff93754, and fixed it in
22044d1 by changing the calculation of
whether recovery is possible.

When `recover_if_needed` is called while in the `Losing` state, we call
the `free_space()` method to determine whether we can recover. In the
`Losing` state, [this method would calculate the free space by
subtracting the space required for the loss record][1] that must be
encoded to transition out of the `Losing` state. However, in the case
where `recover_if_required()` is called with `required_space: None`
(which indicates that we're not trying to recover because we want to
insert a new record, but just because we want to report ongoing data
loss to the caller), [we check that the free space is greater than or
equal to 0][2]. This means that we would still try to insert a loss
record even if the free space was 0, resulting in a panic. I've fixed
this by moving the check that there's space for a loss record out of the
calculation of `free_space()` and into the _required_ space, in addition
to the requested value (which is 0 in the "we are inserting the loss
record to report loss" case). This way, we only insert the loss record
if it fits, which is the correct behavior.

I've also changed the assignment of ENAs in `snitch-core` to start at 1,
rather than 0, since ENA 0 is reserved in the wire protocol to indicate
"no ENA". In the "committed ENA" request field this means "don't flush
any ereports", and in the "start ENA" response field, ENA 0 means "no
ereports in this packet". Thus, the ereport store must start assigning
ENAs at ENA 1 for the initial loss record.

## Testing

Currently, no tasks actually produce ereports. To test that everything
works correctly, it was necessary to add a source of ereports, so I've
added [a little task][3] that just generates test ereports when asked
via `hiffy`. I've included some of that in [this comment][4]. This was
also used for testing the data-loss behavior discussed above.

[RFD 545]: https://rfd.shared.oxide.computer/rfd/0545
[1]:
    https://github.com/oxidecomputer/hubris/blob/e846b9d2481b13cf2b18a2a073bb49eef5f654de/lib/snitch-core/src/lib.rs#L110-L121
[2]:
    https://github.com/oxidecomputer/hubris/blob/e846b9d2481b13cf2b18a2a073bb49eef5f654de/lib/snitch-core/src/lib.rs#L297-L300
[3]: https://github.com/oxidecomputer/hubris/blob/864fa57a7c34a6225deddcffa0c7d54c3063eab6/task/ereportulator/src/main.rs

Author: hawkw

Cargo.lock

@@ -92,6 +92,7 @@ leb128 = { version = "0.2.5", default-features = false }
 lpc55-pac = { version = "0.4", default-features = false }
 memchr = { version = "2.4", default-features = false }
 memoffset = { version = "0.6.5", default-features = false }
+minicbor = { version = "0.26.4", default-features = false }
 multimap = { version = "0.8.3", default-features = false }
 nb = { version = "1", default-features = false }
 num = { version = "0.4", default-features = false }
@@ -144,6 +145,7 @@ zip = { version = "0.6", default-features = false, features = ["bzip2", "deflate
 attest-data = { git = "https://github.com/oxidecomputer/dice-util", default-features = false, version = "0.4.0" }
 dice-mfg-msgs = { git = "https://github.com/oxidecomputer/dice-util", default-features = false, version = "0.2.1" }
 gateway-messages = { git = "https://github.com/oxidecomputer/management-gateway-service", default-features = false, features = ["smoltcp"] }
+gateway-ereport-messages = { git = "https://github.com/oxidecomputer/management-gateway-service", default-features = false }
 gimlet-inspector-protocol = { git = "https://github.com/oxidecomputer/gimlet-inspector-protocol", version = "0.1.0" }
 hif = { git = "https://github.com/oxidecomputer/hif", default-features = false }
 humpty = { git = "https://github.com/oxidecomputer/humpty", default-features = false, version = "0.1.3" }

Cargo.toml

@@ -122,10 +122,20 @@ notifications = ["i2c1-irq", "i2c2-irq", "i2c3-irq", "i2c4-irq"]
 [tasks.packrat]
 name = "task-packrat"
 priority = 1
+stacksize = 1040
 start = true
 # task-slots is explicitly empty: packrat should not send IPCs!
 task-slots = []
-features = ["cosmo"]
+features = ["cosmo", "ereport"]
+
+[tasks.rng_driver]
+features = ["h753", "ereport"]
+name = "drv-stm32h7-rng"
+priority = 6
+uses = ["rng"]
+start = true
+stacksize = 512
+task-slots = ["sys", "packrat"]
 
 [tasks.thermal]
 name = "task-thermal"
@@ -347,6 +357,17 @@ extern-regions = ["sram1", "sram2", "sram3", "sram4"]
 notifications = ["socket"]
 features = ["net", "vlan"]
 
+[tasks.snitch]
+name = "task-snitch"
+# The snitch should have a priority immediately below that of the net task,
+# to minimize the number of components that can starve it from resources.
+priority = 6
+stacksize = 1200
+start = true
+task-slots = ["net", "packrat"]
+features = ["vlan"]
+notifications = ["socket"]
+
 [tasks.spd]
 name = "task-cosmo-spd"
 priority = 7
@@ -1456,6 +1477,15 @@ port = 11113
 tx = { packets = 3, bytes = 1024 }
 rx = { packets = 3, bytes = 1024 }
 
+[config.net.sockets.ereport]
+kind = "udp"
+owner = {name = "snitch", notification = "socket"}
+port = 57005
+tx = { packets = 3, bytes = 1024 }
+# v0 ereport requests are always 35B, so just make the buffer exactly
+# that size...
+rx = { packets = 3, bytes = 35 }
+
 [config.auxflash]
 memory-size = 33_554_432 # 256 Mib / 32 MiB
 slot-count = 16 # 2 MiB slots

app/cosmo/base.toml

@@ -71,7 +71,6 @@ task-slots = ["sys"]
 [tasks.packrat]
 name = "task-packrat"
 priority = 2
-max-sizes = {flash = 8192, ram = 2048}
 start = true
 # task-slots is explicitly empty: packrat should not send IPCs!
 task-slots = []

app/demo-stm32h7-nucleo/app-h753.toml

@@ -123,10 +123,11 @@ notifications = ["i2c1-irq", "jefe-state-change"]
 [tasks.packrat]
 name = "task-packrat"
 priority = 1
+stacksize = 1040
 start = true
 # task-slots is explicitly empty: packrat should not send IPCs!
 task-slots = []
-features = ["gimlet"]
+features = ["gimlet", "ereport"]
 
 [tasks.thermal]
 name = "task-thermal"
@@ -194,6 +195,15 @@ interrupts = {"hash.irq" = "hash-irq"}
 task-slots = ["sys"]
 notifications = ["hash-irq"]
 
+[tasks.rng_driver]
+features = ["h753", "ereport"]
+name = "drv-stm32h7-rng"
+priority = 6
+uses = ["rng"]
+start = true
+stacksize = 512
+task-slots = ["sys", "packrat"]
+
 [tasks.hf]
 name = "drv-gimlet-hf-server"
 features = ["h753"]
@@ -336,6 +346,17 @@ extern-regions = ["sram1", "sram2", "sram3", "sram4"]
 notifications = ["socket"]
 features = ["net", "vlan"]
 
+[tasks.snitch]
+name = "task-snitch"
+# The snitch should have a priority immediately below that of the net task,
+# to minimize the number of components that can starve it from resources.
+priority = 6
+stacksize = 1200
+start = true
+task-slots = ["net", "packrat"]
+features = ["vlan"]
+notifications = ["socket"]
+
 [tasks.sbrmi]
 name = "drv-sbrmi"
 priority = 4
@@ -1315,3 +1336,11 @@ port = 23547
 tx = { packets = 3, bytes = 1024 }
 rx = { packets = 3, bytes = 512 }
 
+[config.net.sockets.ereport]
+kind = "udp"
+owner = {name = "snitch", notification = "socket"}
+port = 57005
+tx = { packets = 3, bytes = 1024 }
+# v0 ereport requests are always 35B, so just make the buffer exactly
+# that size...
+rx = { packets = 3, bytes = 35 }

app/gimlet/base.toml

@@ -0,0 +1,31 @@
+# Gimletlet Ereport test application
+#
+# This image includes the `ereportulator` task, which may be used to generate
+# fake error reports to test the ereport aggregation and evacuation subsystem.
+#
+# Ereports may be generated using `humility hiffy` to call the
+# `Ereportulator.fake_ereport` IPC operation. This takes one argument, `n`,
+# which is an arbitrary `u32` value included in the ereport data payload. This
+# is intended to be used to differentiate between multiple ereports during
+# testing.
+#
+# For example:
+#
+#   $ humility hiffy -t gimletlet hiffy -c Ereportulator.fake_ereport -a n=42
+#
+name = "gimletlet-ereportlet"
+inherit = "app.toml"
+
+[tasks.jefe.config.allowed-callers]
+request_reset = ["hiffy"]
+
+[tasks.hiffy]
+features = ["h753", "stm32h7", "i2c", "gpio", "spi"]
+task-slots = ["sys", "i2c_driver", "user_leds", "ereportulator"]
+
+[tasks.ereportulator]
+name = "task-ereportulator"
+priority = 5
+start = true
+task-slots = ["packrat"]
+notifications = []

app/gimletlet/app-ereportlet.toml

@@ -42,11 +42,12 @@ owner = {name = "sprot", notification = "rot_irq"}
 
 [tasks.packrat]
 name = "task-packrat"
-priority = 3
-max-sizes = {flash = 8192, ram = 2048}
+priority = 1
 start = true
 # task-slots is explicitly empty: packrat should not send IPCs!
 task-slots = []
+stacksize = 1040
+features = ["ereport"]
 
 [tasks.control_plane_agent]
 name = "task-control-plane-agent"
@@ -185,6 +186,21 @@ task-slots = ["net", "packrat"]
 features = ["vlan"]
 notifications = ["socket"]
 
+[tasks.snitch]
+name = "task-snitch"
+# The snitch should have a priority immediately below that of the net task,
+# to minimize the number of components that can starve it from resources.
+priority = 4
+stacksize = 1200
+start = true
+task-slots = ["net", "packrat"]
+features = ["vlan"]
+notifications = ["socket"]
+
+[tasks.rng_driver]
+features = ["h753", "ereport"]
+task-slots = ["sys", "user_leds", "packrat"]
+
 # VLAN configuration
 [config.net.vlans.sidecar1]
 vid = 0x301
@@ -233,6 +249,15 @@ port = 11113
 tx = { packets = 3, bytes = 1024 }
 rx = { packets = 3, bytes = 1024 }
 
+[config.net.sockets.ereport]
+kind = "udp"
+owner = {name = "snitch", notification = "socket"}
+port = 57005
+tx = { packets = 3, bytes = 1024 }
+# v0 ereport requests are always 35B, so just make the buffer exactly
+# that size...
+rx = { packets = 3, bytes = 35 }
+
 [tasks.sprot]
 name = "drv-stm32h7-sprot-server"
 priority = 5

app/gimletlet/app.toml

@@ -6,3 +6,43 @@ inherit = "base.toml"
 features = ["uart8"]
 uses = ["uart8"]
 interrupts = {"uart8.irq" = "usart-irq"}
+
+# Ereport stuff
+[tasks.packrat]
+stacksize = 1040
+features = ["ereport"]
+
+[tasks.snitch]
+name = "task-snitch"
+priority = 4
+stacksize = 1200
+start = true
+task-slots = ["net", "packrat"]
+features = ["vlan"]
+notifications = ["socket"]
+
+[tasks.rng_driver]
+features = ["h753", "ereport"]
+name = "drv-stm32h7-rng"
+priority = 6
+uses = ["rng"]
+start = true
+stacksize = 512
+task-slots = ["sys", "packrat"]
+
+# Demo/test task for ereports
+[tasks.ereportulator]
+name = "task-ereportulator"
+priority = 6
+start = true
+task-slots = ["packrat"]
+notifications = []
+
+[config.net.sockets.ereport]
+kind = "udp"
+owner = {name = "snitch", notification = "socket"}
+port = 57005
+tx = { packets = 3, bytes = 1024 }
+# v0 ereport requests are always 35B, so just make the buffer exactly
+# that size...
+rx = { packets = 3, bytes = 35 }

app/grapefruit/app-dev.toml

@@ -108,8 +108,7 @@ notifications = ["timer"]
 
 [tasks.packrat]
 name = "task-packrat"
-priority = 3
-max-sizes = {flash = 8192, ram = 2048}
+priority = 1
 start = true
 # task-slots is explicitly empty: packrat should not send IPCs!
 task-slots = []

app/grapefruit/base.toml

@@ -90,6 +90,14 @@ owner = {name = "sequencer", notification = "psu_pwr_ok_6"}
 
 
 
+[tasks.rng_driver]
+features = ["h753", "ereport"]
+name = "drv-stm32h7-rng"
+priority = 6
+uses = ["rng"]
+start = true
+stacksize = 512
+task-slots = ["sys", "packrat"]
 
 [tasks.i2c_driver]
 name = "drv-stm32xx-i2c-server"
@@ -109,11 +117,12 @@ notifications = ["i2c2-irq", "i2c3-irq"]
 
 [tasks.packrat]
 name = "task-packrat"
-priority = 3
-max-sizes = {flash = 8192, ram = 2048}
+priority = 1
+stacksize = 1040
 start = true
 # task-slots is explicitly empty: packrat should not send IPCs!
 task-slots = []
+features = ["ereport"]
 
 [tasks.sequencer]
 name = "drv-psc-seq-server"
@@ -313,6 +322,17 @@ extern-regions = [ "sram1", "sram2", "sram3", "sram4" ]
 notifications = ["socket"]
 features = ["net", "vlan"]
 
+[tasks.snitch]
+name = "task-snitch"
+# The snitch should have a priority immediately below that of the net task,
+# to minimize the number of components that can starve it from resources.
+priority = 5
+stacksize = 1200
+start = true
+task-slots = ["net", "packrat"]
+features = ["vlan"]
+notifications = ["socket"]
+
 [tasks.idle]
 name = "task-idle"
 priority = 7
@@ -535,3 +555,12 @@ owner = {name = "dump_agent", notification = "socket"}
 port = 11113
 tx = { packets = 3, bytes = 1024 }
 rx = { packets = 3, bytes = 1024 }
+
+[config.net.sockets.ereport]
+kind = "udp"
+owner = {name = "snitch", notification = "socket"}
+port = 57005
+tx = { packets = 3, bytes = 1024 }
+# v0 ereport requests are always 35B, so just make the buffer exactly
+# that size...
+rx = { packets = 3, bytes = 35 }