packrat: Return an error if an ereport was lost (#2225)

This commit changes `task-packrat-api`'s `deliver_ereport` IPC to return
an error if the ereport did not fit in the buffer, rather than being
infallible. In the future, this may allow some tasks to attempt
re-delivery periodically until there's space.

Author: hawkw

drv/cosmo-seq-server/src/vcore.rs

@@ -72,7 +72,8 @@ enum Trace {
     StatusCml(Rail, Result<u8, ResponseCode>),
     StatusMfrSpecific(Rail, Result<u8, ResponseCode>),
     I2cError(Rail, PmbusCmd, raa229620a::Error),
-    EreportSentOff(Rail, usize),
+    EreportSent(Rail, usize),
+    EreportLost(Rail, usize, packrat::EreportWriteError),
     EreportTooBig(Rail),
 }
 
@@ -448,7 +449,14 @@ fn deliver_ereport(
     match data.serialize(&mut s) {
         Ok(_) => {
             let len = s.into_encoder().into_writer().position();
-            packrat.deliver_ereport(&ereport_buf[..len]);
+            match packrat.deliver_ereport(&ereport_buf[..len]) {
+                Ok(_) => {
+                    ringbuf_entry!(Trace::EreportSent(rail, len));
+                }
+                Err(e) => {
+                    ringbuf_entry!(Trace::EreportLost(rail, len, e));
+                }
+            }
             ringbuf_entry!(Trace::EreportSentOff(rail, len));
         }
         Err(_) => {

drv/gimlet-seq-server/src/vcore.rs

@@ -61,7 +61,8 @@ enum Trace {
     StatusMfrSpecific(Result<u8, ResponseCode>),
     Reading { timestamp: u64, volts: units::Volts },
     Error(ResponseCode),
-    EreportSentOff(usize),
+    EreportSent(usize),
+    EreportLost(usize, packrat_api::EreportWriteError),
     EreportTooBig,
 }
 
@@ -343,8 +344,14 @@ fn deliver_ereport(packrat: &packrat_api::Packrat, data: &impl Serialize) {
     match data.serialize(&mut s) {
         Ok(_) => {
             let len = s.into_encoder().into_writer().position();
-            packrat.deliver_ereport(&ereport_buf[..len]);
-            ringbuf_entry!(Trace::EreportSentOff(len));
+            match packrat.deliver_ereport(&ereport_buf[..len]) {
+                Ok(_) => {
+                    ringbuf_entry!(Trace::EreportSent(len));
+                }
+                Err(e) => {
+                    ringbuf_entry!(Trace::EreportLost(len, e));
+                }
+            }
         }
         Err(_) => {
             // XXX(eliza): ereport didn't fit in buffer...what do

idl/packrat.idol

@@ -120,7 +120,10 @@ Interface(
             leases: {
                 "data": (type: "[u8]", read: true, max_len: Some(1024)),
             },
-            reply: Simple("()"),
+            reply: Result(
+                ok: "()",
+                err: CLike("EreportWriteError"),
+            ),
             idempotent: true,
         ),
         "read_ereports": (

task/ereportulator/src/main.rs

@@ -62,6 +62,10 @@ enum Trace {
     EreportDelivered {
         encoded_len: usize,
     },
+    EreportLost {
+        encoded_len: usize,
+        err: task_packrat_api::EreportWriteError,
+    },
 }
 
 counted_ringbuf!(Trace, 16, Trace::None);
@@ -121,8 +125,10 @@ impl idl::InOrderEreportulatorImpl for ServerImpl {
             encoder.into_writer().position()
         };
 
-        self.packrat.deliver_ereport(&self.buf[..encoded_len]);
-        ringbuf_entry!(Trace::EreportDelivered { encoded_len });
+        match self.packrat.deliver_ereport(&self.buf[..encoded_len]) {
+            Ok(_) => ringbuf_entry!(Trace::EreportDelivered { encoded_len }),
+            Err(err) => ringbuf_entry!(Trace::EreportLost { encoded_len, err }),
+        }
 
         Ok(())
     }

task/packrat-api/src/lib.rs

@@ -60,4 +60,13 @@ pub enum EreportReadError {
     RestartIdNotSet = 1,
 }
 
+#[derive(
+    Copy, Clone, Debug, FromPrimitive, Eq, PartialEq, IdolError, counters::Count,
+)]
+pub enum EreportWriteError {
+    /// Indicates that an ereport was lost because it would not have fit in
+    /// Packrat's ereport buffer.
+    Lost = 1,
+}
+
 include!(concat!(env!("OUT_DIR"), "/client_stub.rs"));

task/packrat/src/ereport.rs

@@ -14,12 +14,11 @@
 
 use super::ereport_messages;
 
-use core::convert::Infallible;
 use idol_runtime::{ClientError, Leased, LenLimit, RequestError};
 use minicbor::CborLen;
 use minicbor_lease::LeasedWriter;
 use ringbuf::{counted_ringbuf, ringbuf_entry};
-use task_packrat_api::{EreportReadError, VpdIdentity};
+use task_packrat_api::{EreportReadError, EreportWriteError, VpdIdentity};
 use userlib::{kipc, sys_get_timer, RecvMessage, TaskId};
 use zerocopy::IntoBytes;
 
@@ -118,7 +117,7 @@ impl EreportStore {
         &mut self,
         msg: &RecvMessage,
         data: LenLimit<Leased<idol_runtime::R, [u8]>, RECV_BUF_SIZE>,
-    ) -> Result<(), RequestError<Infallible>> {
+    ) -> Result<(), RequestError<EreportWriteError>> {
         data.read_range(0..data.len(), self.recv)
             .map_err(|_| ClientError::WentAway.fail())?;
         let timestamp = sys_get_timer().now;
@@ -132,7 +131,12 @@ impl EreportStore {
             len: data.len() as u32,
             result,
         });
-        Ok(())
+        match result {
+            snitch_core::InsertResult::Inserted => Ok(()),
+            snitch_core::InsertResult::Lost => {
+                Err(RequestError::from(EreportWriteError::Lost))
+            }
+        }
     }
 
     pub(crate) fn read_ereports(

task/packrat/src/main.rs

@@ -69,8 +69,8 @@ use idol_runtime::{Leased, LenLimit, NotificationHandler, RequestError};
 use ringbuf::{ringbuf, ringbuf_entry};
 use static_cell::ClaimOnceCell;
 use task_packrat_api::{
-    CacheGetError, CacheSetError, EreportReadError, HostStartupOptions,
-    MacAddressBlock, VpdIdentity,
+    CacheGetError, CacheSetError, EreportReadError, EreportWriteError,
+    HostStartupOptions, MacAddressBlock, VpdIdentity,
 };
 use userlib::RecvMessage;
 
@@ -519,7 +519,7 @@ impl idl::InOrderPackratImpl for ServerImpl {
         &mut self,
         _: &RecvMessage,
         _: LenLimit<Leased<idol_runtime::R, [u8]>, 1024usize>,
-    ) -> Result<(), RequestError<Infallible>> {
+    ) -> Result<(), RequestError<EreportWriteError>> {
         // go away, we don't know how to do that
         Err(idol_runtime::ClientError::UnknownOperation.fail())
     }
@@ -529,7 +529,7 @@ impl idl::InOrderPackratImpl for ServerImpl {
         &mut self,
         msg: &RecvMessage,
         data: LenLimit<Leased<idol_runtime::R, [u8]>, 1024usize>,
-    ) -> Result<(), RequestError<Infallible>> {
+    ) -> Result<(), RequestError<EreportWriteError>> {
         self.ereport_store.deliver_ereport(msg, data)
     }
 
@@ -585,7 +585,7 @@ impl NotificationHandler for ServerImpl {
 mod idl {
     use super::{
         ereport_messages, CacheGetError, CacheSetError, EreportReadError,
-        HostStartupOptions, MacAddressBlock, VpdIdentity,
+        EreportWriteError, HostStartupOptions, MacAddressBlock, VpdIdentity,
     };
 
     include!(concat!(env!("OUT_DIR"), "/server_stub.rs"));