Refine update delivery messages from MGS to SP

gateway-messages/src/lib.rs

@@ -31,7 +31,7 @@ pub mod version {
 
 /// Messages from a gateway to an SP.
 #[derive(
-    Debug, Clone, SerializedSize, Serialize, Deserialize, PartialEq, Eq,
+    Debug, Clone, Copy, SerializedSize, Serialize, Deserialize, PartialEq, Eq,
 )]
 pub struct Request {
     pub version: u32,
@@ -40,7 +40,7 @@ pub struct Request {
 }
 
 #[derive(
-    Debug, Clone, SerializedSize, Serialize, Deserialize, PartialEq, Eq,
+    Debug, Clone, Copy, SerializedSize, Serialize, Deserialize, PartialEq, Eq,
 )]
 pub enum RequestKind {
     Discover,
@@ -63,9 +63,11 @@ pub enum RequestKind {
         offset: u64,
     },
     SerialConsoleDetach,
-    UpdateStart(UpdateStart),
+    UpdatePrepare(UpdatePrepare),
+    UpdatePrepareStatus(UpdatePrepareStatusRequest),
     /// `UpdateChunk` always includes trailing raw data.
     UpdateChunk(UpdateChunk),
+    UpdateAbort(SpComponent),
     SysResetPrepare,
     SysResetTrigger,
 }
@@ -95,8 +97,10 @@ pub enum ResponseKind {
     BulkIgnitionState(BulkIgnitionState),
     IgnitionCommandAck,
     SpState(SpState),
-    UpdateStartAck,
+    UpdatePrepareAck,
+    UpdatePrepareStatus(UpdatePrepareStatusResponse),
     UpdateChunkAck,
+    UpdateAbortAck,
     SerialConsoleAttachAck,
     SerialConsoleWriteAck { furthest_ingested_offset: u64 },
     SerialConsoleDetachAck,
@@ -142,18 +146,29 @@ pub enum ResponseError {
     /// Cannot attach to the serial console because another MGS instance is
     /// already attached.
     SerialConsoleAlreadyAttached,
+    /// An update has not been prepared yet.
+    UpdateNotPrepared,
+    /// An update-related message arrived at the SP, but it is already
+    /// processing an update with the stream id `sp_stream_id`.
+    InvalidUpdateStreamId { sp_stream_id: u64 },
     /// An update is already in progress with the specified amount of data
     /// already provided. MGS should resume the update at that offset.
     UpdateInProgress { bytes_received: u32 },
     /// Received an invalid update chunk; the in-progress update must be
-    /// abandoned and restarted.
+    /// aborted and restarted.
     InvalidUpdateChunk,
     /// An update operation failed with the associated code.
     UpdateFailed(u32),
+    /// An update is not possible at this time (e.g., the target slot is locked
+    /// by another device).
+    UpdateSlotBusy,
     /// Received a `SysResetTrigger` request without first receiving a
     /// `SysResetPrepare` request. This can be used to detect a successful
     /// reset.
     SysResetTriggerWithoutPrepare,
+    /// Request mentioned a slot number for a component that does not have that
+    /// slot.
+    InvalidSlotForComponent,
 }
 
 impl fmt::Display for ResponseError {
@@ -177,18 +192,30 @@ impl fmt::Display for ResponseError {
             ResponseError::SerialConsoleAlreadyAttached => {
                 write!(f, "serial console already attached")
             }
+            ResponseError::UpdateNotPrepared => {
+                write!(f, "SP has not received update prepare request")
+            }
+            ResponseError::InvalidUpdateStreamId { sp_stream_id } => {
+                write!(f, "bad update stream ID (update already in progress, stream id {:#x})", sp_stream_id)
+            }
             ResponseError::UpdateInProgress { bytes_received } => {
                 write!(f, "update still in progress ({bytes_received} bytes received so far)")
             }
+            ResponseError::UpdateSlotBusy => {
+                write!(f, "update currently unavailable (slot busy)")
+            }
             ResponseError::InvalidUpdateChunk => {
                 write!(f, "invalid update chunk")
             }
             ResponseError::UpdateFailed(code) => {
                 write!(f, "update failed (code {})", code)
             }
-            &ResponseError::SysResetTriggerWithoutPrepare => {
+            ResponseError::SysResetTriggerWithoutPrepare => {
                 write!(f, "sys reset trigger requested without a preceding sys reset prepare")
             }
+            ResponseError::InvalidSlotForComponent => {
+                write!(f, "invalid slot number for component")
+            }
         }
     }
 }
@@ -227,26 +254,47 @@ pub enum SpMessageKind {
 }
 
 #[derive(
-    Debug,
-    Default,
-    Clone,
-    Copy,
-    PartialEq,
-    Eq,
-    SerializedSize,
-    Serialize,
-    Deserialize,
+    Debug, Clone, Copy, PartialEq, Eq, SerializedSize, Serialize, Deserialize,
 )]
-pub struct UpdateStart {
+pub struct UpdatePrepare {
+    pub component: SpComponent,
+    /// MGS sets `stream_id` to a random `u64`, and the SP uses it to correlate
+    /// any subsequent `UpdateChunk` messages with this update. This is a
+    /// safeguard against the SP receiving multiple `UpdateChunk` messages
+    /// concurrently and not being able to tell which are associated with the
+    /// stream it is currently updating.
+    pub stream_id: u64,
+    /// The number of available slots depends on `component`; passing an invalid
+    /// slot number will result in a [`ResponseError::InvalidSlotForComponent`].
+    pub slot: u16,
     pub total_size: u32,
     // TODO auth info? checksum/digest?
     // TODO should we inline the first chunk?
 }
 
 #[derive(
-    Debug, Clone, PartialEq, Eq, SerializedSize, Serialize, Deserialize,
+    Debug, Clone, Copy, PartialEq, Eq, SerializedSize, Serialize, Deserialize,
+)]
+pub struct UpdatePrepareStatusRequest {
+    pub component: SpComponent,
+    /// See [`UpdatePrepare::stream_id`].
+    pub stream_id: u64,
+}
+
+#[derive(
+    Debug, Clone, Copy, PartialEq, Eq, SerializedSize, Serialize, Deserialize,
+)]
+pub struct UpdatePrepareStatusResponse {
+    pub done: bool,
+}
+
+#[derive(
+    Debug, Clone, Copy, PartialEq, Eq, SerializedSize, Serialize, Deserialize,
 )]
 pub struct UpdateChunk {
+    pub component: SpComponent,
+    /// See [`UpdatePrepare::stream_id`].
+    pub stream_id: u64,
     /// Offset in bytes of this chunk from the beginning of the update data.
     pub offset: u32,
 }
@@ -335,9 +383,11 @@ impl SpComponent {
     /// Maximum number of bytes for a component ID.
     pub const MAX_ID_LENGTH: usize = 16;
 
+    /// The SP itself.
+    pub const SP_ITSELF: Self = Self { id: *b"sp\0\0\0\0\0\0\0\0\0\0\0\0\0\0" };
+
     /// The `sp3` host CPU.
-    pub const SP3_HOST_CPU: Self =
-        Self { id: *b"sp3\0\0\0\0\0\0\0\0\0\0\0\0\0" };
+    pub const SP3_HOST_CPU: Self = Self { id: *b"sp3-host-cpu\0\0\0\0" };
 
     /// Interpret the component name as a human-readable string.
     ///

gateway-messages/src/sp_impl.rs

@@ -19,7 +19,9 @@ use crate::SpMessageKind;
 use crate::SpPort;
 use crate::SpState;
 use crate::UpdateChunk;
-use crate::UpdateStart;
+use crate::UpdatePrepare;
+use crate::UpdatePrepareStatusRequest;
+use crate::UpdatePrepareStatusResponse;
 use core::convert::Infallible;
 use core::mem;
 
@@ -67,13 +69,20 @@ pub trait SpHandler {
         port: SpPort,
     ) -> Result<SpState, ResponseError>;
 
-    fn update_start(
+    fn update_prepare(
         &mut self,
         sender: SocketAddrV6,
         port: SpPort,
-        update: UpdateStart,
+        update: UpdatePrepare,
     ) -> Result<(), ResponseError>;
 
+    fn update_prepare_status(
+        &mut self,
+        sender: SocketAddrV6,
+        port: SpPort,
+        request: UpdatePrepareStatusRequest,
+    ) -> Result<UpdatePrepareStatusResponse, ResponseError>;
+
     fn update_chunk(
         &mut self,
         sender: SocketAddrV6,
@@ -82,6 +91,13 @@ pub trait SpHandler {
         data: &[u8],
     ) -> Result<(), ResponseError>;
 
+    fn update_abort(
+        &mut self,
+        sender: SocketAddrV6,
+        port: SpPort,
+        component: SpComponent,
+    ) -> Result<(), ResponseError>;
+
     fn serial_console_attach(
         &mut self,
         sender: SocketAddrV6,
@@ -211,12 +227,18 @@ pub fn handle_message<H: SpHandler>(
         RequestKind::SpState => {
             handler.sp_state(sender, port).map(ResponseKind::SpState)
         }
-        RequestKind::UpdateStart(update) => handler
-            .update_start(sender, port, update)
-            .map(|()| ResponseKind::UpdateStartAck),
+        RequestKind::UpdatePrepare(update) => handler
+            .update_prepare(sender, port, update)
+            .map(|()| ResponseKind::UpdatePrepareAck),
+        RequestKind::UpdatePrepareStatus(req) => handler
+            .update_prepare_status(sender, port, req)
+            .map(ResponseKind::UpdatePrepareStatus),
         RequestKind::UpdateChunk(chunk) => handler
             .update_chunk(sender, port, chunk, trailing_data)
             .map(|()| ResponseKind::UpdateChunkAck),
+        RequestKind::UpdateAbort(component) => handler
+            .update_abort(sender, port, component)
+            .map(|()| ResponseKind::UpdateAbortAck),
         RequestKind::SerialConsoleAttach(component) => handler
             .serial_console_attach(sender, port, component)
             .map(|()| ResponseKind::SerialConsoleAttachAck),

gateway-sp-comms/Cargo.toml

@@ -6,6 +6,7 @@ license = "MPL-2.0"
 
 [dependencies]
 futures = "0.3.24"
+rand = "0.8.5"
 serde = { version = "1.0", features = ["derive"] }
 serde_with = "2.0.0"
 thiserror = "1.0.34"

gateway-sp-comms/src/communicator.rs

@@ -24,6 +24,7 @@ use gateway_messages::IgnitionState;
 use gateway_messages::ResponseKind;
 use gateway_messages::SpComponent;
 use gateway_messages::SpState;
+use gateway_messages::UpdatePrepareStatusResponse;
 use slog::info;
 use slog::o;
 use slog::Logger;
@@ -213,11 +214,13 @@ impl Communicator {
     pub async fn update(
         &self,
         sp: SpIdentifier,
+        component: SpComponent,
+        slot: u16,
         image: Vec<u8>,
     ) -> Result<(), Error> {
         let port = self.id_to_port(sp)?;
         let sp = self.switch.sp(port).ok_or(Error::SpAddressUnknown(sp))?;
-        Ok(sp.update(image).await?)
+        Ok(sp.update(component, slot, image).await?)
     }
 
     /// Reset a given SP.
@@ -297,10 +300,16 @@ pub(crate) trait ResponseKindExt {
 
     fn expect_serial_console_detach_ack(self) -> Result<(), BadResponseType>;
 
-    fn expect_update_start_ack(self) -> Result<(), BadResponseType>;
+    fn expect_update_prepare_ack(self) -> Result<(), BadResponseType>;
+
+    fn expect_update_prepare_status(
+        self,
+    ) -> Result<UpdatePrepareStatusResponse, BadResponseType>;
 
     fn expect_update_chunk_ack(self) -> Result<(), BadResponseType>;
 
+    fn expect_update_abort_ack(self) -> Result<(), BadResponseType>;
+
     fn expect_sys_reset_prepare_ack(self) -> Result<(), BadResponseType>;
 }
 
@@ -327,8 +336,14 @@ impl ResponseKindExt for ResponseKind {
             ResponseKind::SerialConsoleDetachAck => {
                 response_kind_names::SERIAL_CONSOLE_DETACH_ACK
             }
-            ResponseKind::UpdateStartAck => {
-                response_kind_names::UPDATE_START_ACK
+            ResponseKind::UpdatePrepareAck => {
+                response_kind_names::UPDATE_PREPARE_ACK
+            }
+            ResponseKind::UpdatePrepareStatus(_) => {
+                response_kind_names::UPDATE_PREPARE_STATUS
+            }
+            ResponseKind::UpdateAbortAck => {
+                response_kind_names::UPDATE_ABORT_ACK
             }
             ResponseKind::UpdateChunkAck => {
                 response_kind_names::UPDATE_CHUNK_ACK
@@ -423,11 +438,23 @@ impl ResponseKindExt for ResponseKind {
         }
     }
 
-    fn expect_update_start_ack(self) -> Result<(), BadResponseType> {
+    fn expect_update_prepare_ack(self) -> Result<(), BadResponseType> {
         match self {
-            ResponseKind::UpdateStartAck => Ok(()),
+            ResponseKind::UpdatePrepareAck => Ok(()),
             other => Err(BadResponseType {
-                expected: response_kind_names::UPDATE_START_ACK,
+                expected: response_kind_names::UPDATE_PREPARE_ACK,
+                got: other.name(),
+            }),
+        }
+    }
+
+    fn expect_update_prepare_status(
+        self,
+    ) -> Result<UpdatePrepareStatusResponse, BadResponseType> {
+        match self {
+            ResponseKind::UpdatePrepareStatus(status) => Ok(status),
+            other => Err(BadResponseType {
+                expected: response_kind_names::UPDATE_PREPARE_ACK,
                 got: other.name(),
             }),
         }
@@ -443,6 +470,16 @@ impl ResponseKindExt for ResponseKind {
         }
     }
 
+    fn expect_update_abort_ack(self) -> Result<(), BadResponseType> {
+        match self {
+            ResponseKind::UpdateAbortAck => Ok(()),
+            other => Err(BadResponseType {
+                expected: response_kind_names::UPDATE_ABORT_ACK,
+                got: other.name(),
+            }),
+        }
+    }
+
     fn expect_sys_reset_prepare_ack(self) -> Result<(), BadResponseType> {
         match self {
             ResponseKind::SysResetPrepareAck => Ok(()),
@@ -466,7 +503,9 @@ mod response_kind_names {
         "serial_console_write_ack";
     pub(super) const SERIAL_CONSOLE_DETACH_ACK: &str =
         "serial_console_detach_ack";
-    pub(super) const UPDATE_START_ACK: &str = "update_start_ack";
+    pub(super) const UPDATE_PREPARE_ACK: &str = "update_prepare_ack";
+    pub(super) const UPDATE_PREPARE_STATUS: &str = "update_prepare_status";
+    pub(super) const UPDATE_ABORT_ACK: &str = "update_abort_ack";
     pub(super) const UPDATE_CHUNK_ACK: &str = "update_chunk_ack";
     pub(super) const SYS_RESET_PREPARE_ACK: &str = "sys_reset_prepare_ack";
 }