[RFC] Zerocopy for serializing/deserializing ODP service messages#678
[RFC] Zerocopy for serializing/deserializing ODP service messages#678kurtjd wants to merge 1 commit intoOpenDevicePartnership:v0.2.0from
Conversation
9476651 to
f24cc62
Compare
f24cc62 to
1f42a1c
Compare
|
Though I'm not a huge fan of this approach because since we are copying bytes, we shouldn't need to worry about the underlying memory structure, so all this fiddling with alignment, padding, and endianness is just noise (which would be important if we were going for zero-cost serialization, but the But for the time-being though, ideally there would be some simple crate that is a proc macro or something that does automatically what we were doing manually (copying each field of the struct in series, in LE format, into a packed &mut [u8]). It looks like there are a couple but haven't played around with them much. Based on the EC tech forum discussion Friday, something like |
1f42a1c to
e5e4815
Compare
Some serialization/deserialization was omitted because it wouldn't be used by the EC, however the service-message crates can be used outside the EC, so add them in. I experimented with zerocopy for this in #678 but it's a bit clunky and getting it to work for Battery messages will be challenging, so just rolled it by hand for now since that's what we are already doing. Can revisit later once we've decided on a serialization strategy. Tested these by using the service message crates to serialize/deserialize messages over UART in the ratatui and works fine for the ones I could test (though many of the battery commands are not yet supported by the ratatui app). Was going to wait until OpenDevicePartnership/embedded-batteries#39 is merged, but it seems relying on an updated `embedded-batteries` causes a ton of breakage and headache, so just have functions for converting in here and can update a later time.
This PR demonstrates using zerocopy for serializing instead of doing it manually for thermal service messages (and can be used for all services if this approach seems okay).
Notes:
ThermalRequest/ThermalResponseenums themselves, so I derived the appropriate traits for structs which the enums wrapped, then just pattern match over the enum.zerocopy::byteorder::U<N>types to ensure LE representation on the wire (in the very rare case this is running on BE-native hardware) and to force alignment of fields to 1 (since zerocopy does not allow padding and a#repr(packed)could cause issues when accessing the fields from Rust code).U<N>types unfortunately don't impldefmt::Formatso had to manually do it (might try to upstream defmt support)mctp_rstrait methods require serializing into a buffer, instead of just returning a&[u8], so because of that, we lose the zero-copy benefit from zerocopy (and are really just using it for its serialization abilities)The goal is also eventually to have these service message crates be useable from other areas than just the EC (such as host-side secure services).