Version the sprockets protocol#86
Conversation
|
This is a little hard to get in a unit test but manual testing shows that server and client will give errors if the version isn't what is expected. I'm guessing the expected path if we get a Very Old client/server is the mupdate recovery path which does not rely on sprockets? |
We will need to make changes the the protocol as we add more features (e.g. SPDM from the host). Introduce a basic versioning mechanism to allow this.
andrewjstone
left a comment
andrewjstone
left a comment
There was a problem hiding this comment.
Thanks @labbott. This looks good for now. I'm sure we'll refine it over time, but it's great to have the version negotation now before we get into live update on production systems.
|
|
||
| // Response from the server, either the same version back, version - 1 | ||
| // or an error if there is no way the server can support this request | ||
| type ProtocolResult = Result<u32, ()>; |
There was a problem hiding this comment.
I think it would be nice to return a proper error here, or at least the incompatible version. We expect success with the current and previous versions, and we'd probably like to know what version the system is actually on when debugging a failure.
| // get version from the client | ||
| let version_bytes = recv_msg(&mut stream).await?; | ||
| let version = | ||
| u32::from_le_bytes(version_bytes[..4].try_into().unwrap()); |
There was a problem hiding this comment.
Just to confirm: We don't have to worry about backwards compatibility here because we only use sprockets in RSS and we are currently only doing mupdates, right?
If we needed backwards compatibility we might have to add something like a magic number header to avoid ambiguity. That would suck though.
There was a problem hiding this comment.
We don't currently have this deployed anywhere so I think the risk is pretty low. I don't think adding a magic number would be terrible if we're concerned about compatibility.
2 participants
We will need to make changes the the protocol as we add more features (e.g. SPDM from the host). Introduce a basic versioning mechanism to allow this.