From 2af95eb3daf41bf9e0981ad744afe2d499d67090 Mon Sep 17 00:00:00 2001 From: Marvin Gudel Date: Sun, 18 Jan 2026 16:49:48 +0100 Subject: [PATCH 1/6] I2C transport: Unit tests, return complete header when decoding I2C packet Signed-off-by: Marvin Gudel --- mctp-estack/src/i2c.rs | 64 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 58 insertions(+), 6 deletions(-) diff --git a/mctp-estack/src/i2c.rs b/mctp-estack/src/i2c.rs index 2680842..30a6180 100644 --- a/mctp-estack/src/i2c.rs +++ b/mctp-estack/src/i2c.rs @@ -43,9 +43,13 @@ impl MctpI2cHeader { ]) } - fn decode(pkt: &[u8]) -> Result { + /// Decode a 4-byte I2C header + /// + /// Checks and decodes destination and source address, + /// command code, and byte count. + fn decode(header: &[u8]) -> Result { let [dest, cmd, byte_count, source] = - pkt.try_into().map_err(|_| Error::BadArgument)?; + header.try_into().map_err(|_| Error::BadArgument)?; if dest & 1 != 0 { trace!("Bad i2c dest write bit"); return Err(Error::InvalidInput); @@ -85,7 +89,7 @@ impl MctpI2cEncap { &self, mut packet: &'f [u8], pec: bool, - ) -> Result<(&'f [u8], u8)> { + ) -> Result<(&'f [u8], MctpI2cHeader)> { if pec { // Remove the pec byte, check it. if packet.is_empty() { @@ -106,7 +110,7 @@ impl MctpI2cEncap { return Err(Error::InvalidInput); } - Ok((&packet[MCTP_I2C_HEADER..], header.source)) + Ok((&packet[MCTP_I2C_HEADER..], header)) } /// Handles a MCTP fragment with the PEC already validated @@ -117,7 +121,7 @@ impl MctpI2cEncap { &self, packet: &[u8], mctp: &'f mut Stack, - ) -> Result, u8)>> { + ) -> Result, MctpI2cHeader)>> { let (mctp_packet, i2c_src) = self.decode(packet, false)?; // Pass to MCTP stack @@ -252,7 +256,7 @@ impl MctpI2cHandler { &mut self, packet: &[u8], mctp: &'f mut Stack, - ) -> Result, u8)>> { + ) -> Result, MctpI2cHeader)>> { self.encap.receive_done_pec(packet, mctp) } @@ -369,3 +373,51 @@ enum HandlerSendState { i2c_dest: u8, }, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn i2c_codec_roundtrip() { + let codec = MctpI2cEncap::new(0x0A); + const PACKET: &[u8] = + &[0x01, 0x00, 0x09, 0xc8, 0x00, 0x8a, 0x01, 0x00, 0x0a]; + + let mut buf = [0; 128]; + let i2c_packet = codec.encode(0x0B, PACKET, &mut buf, false).unwrap(); + + assert_eq!(&i2c_packet[..4], [0x16, 0x0f, 0x0a, 0x15]); + + let codec = MctpI2cEncap::new(0x0B); + let (decoded_packet, header) = codec.decode(i2c_packet, false).unwrap(); + assert_eq!(decoded_packet, PACKET); + assert_eq!(header.source, 0x0a); + assert_eq!(header.dest, 0x0b); + } + + #[test] + fn test_partial_packet_decode() { + let codec = MctpI2cEncap::new(0x0A); + + // Test that empty packets are handled correctly + let res = codec.decode(&[], false); + assert!(res.is_err()); + let res = codec.decode(&[], true); + assert!(res.is_err()); + // Test that packets with only partial header are handled correctly + let res = codec.decode(&[0x16, 0x0f], false); + assert!(res.is_err()); + let res = codec.decode(&[0x16, 0x0f], true); + assert!(res.is_err()); + } + + #[test] + fn test_decode_byte_count_mismatch() { + let codec = MctpI2cEncap::new(0x0A); + + // Try to decode a packet with a `byte count` of 0x0a followed by only 3 bytes + let res = codec.decode(&[0x16, 0x0f, 0x0a, 0x15, 0x01, 0x02], false); + assert!(res.is_err_and(|e| matches!(e, Error::InvalidInput))); + } +} From 9c5e137413f5e0dfdb1f36fdc57b45d67010c8f3 Mon Sep 17 00:00:00 2001 From: Marvin Gudel Date: Sun, 18 Jan 2026 16:56:58 +0100 Subject: [PATCH 2/6] I2C transport: Remove the protocol header bit from source and slave address when decoding Signed-off-by: Marvin Gudel --- mctp-estack/src/i2c.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mctp-estack/src/i2c.rs b/mctp-estack/src/i2c.rs index 30a6180..917a301 100644 --- a/mctp-estack/src/i2c.rs +++ b/mctp-estack/src/i2c.rs @@ -63,8 +63,8 @@ impl MctpI2cHeader { return Err(Error::InvalidInput); } Ok(Self { - dest, - source, + dest: dest >> 1, + source: source >> 1, byte_count: byte_count as usize, }) } From 6363bc4d6c03ab47c854a9b69de65ff29fad63a7 Mon Sep 17 00:00:00 2001 From: Marvin Gudel Date: Sun, 18 Jan 2026 16:58:40 +0100 Subject: [PATCH 3/6] I2C transport: Always return early when trying to decode an empty packet Signed-off-by: Marvin Gudel --- mctp-estack/src/i2c.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mctp-estack/src/i2c.rs b/mctp-estack/src/i2c.rs index 917a301..cea5960 100644 --- a/mctp-estack/src/i2c.rs +++ b/mctp-estack/src/i2c.rs @@ -90,11 +90,11 @@ impl MctpI2cEncap { mut packet: &'f [u8], pec: bool, ) -> Result<(&'f [u8], MctpI2cHeader)> { + if packet.is_empty() { + return Err(Error::InvalidInput); + } if pec { // Remove the pec byte, check it. - if packet.is_empty() { - return Err(Error::InvalidInput); - } let packet_pec; (packet_pec, packet) = packet.split_last().unwrap(); let calc_pec = smbus_pec::pec(packet); From af172bbeae683d98a76fec4360a988594e4060ab Mon Sep 17 00:00:00 2001 From: Marvin Gudel Date: Sun, 18 Jan 2026 17:14:32 +0100 Subject: [PATCH 4/6] I2C transport: Use only first 4 bytes of packet to decode header The `MctpI2cHeader::decode(...)` method errors on slices that are not 4-byte. Signed-off-by: Marvin Gudel --- mctp-estack/src/i2c.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mctp-estack/src/i2c.rs b/mctp-estack/src/i2c.rs index cea5960..acbf7dd 100644 --- a/mctp-estack/src/i2c.rs +++ b/mctp-estack/src/i2c.rs @@ -104,7 +104,8 @@ impl MctpI2cEncap { } } - let header = MctpI2cHeader::decode(packet)?; + let header = + MctpI2cHeader::decode(packet.get(..4).ok_or(Error::InvalidInput)?)?; // +1 for i2c source address field if header.byte_count != packet.len() + 1 { return Err(Error::InvalidInput); From 8a8881432685f152c0c9a09c2d7b14504f29dacc Mon Sep 17 00:00:00 2001 From: Marvin Gudel Date: Sun, 18 Jan 2026 17:17:37 +0100 Subject: [PATCH 5/6] I2C transport: Fix check for Byte Count field and packet length The complete packet includes destination address, command code, byte count and source address. The check previously accounted only for the source adddress, while the checked packet includes the whole header at that point. Signed-off-by: Marvin Gudel --- mctp-estack/src/i2c.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mctp-estack/src/i2c.rs b/mctp-estack/src/i2c.rs index acbf7dd..da21c21 100644 --- a/mctp-estack/src/i2c.rs +++ b/mctp-estack/src/i2c.rs @@ -106,8 +106,10 @@ impl MctpI2cEncap { let header = MctpI2cHeader::decode(packet.get(..4).ok_or(Error::InvalidInput)?)?; - // +1 for i2c source address field - if header.byte_count != packet.len() + 1 { + // total packet len == byte_count + 3 (destination, command code, byte count) + // pec is not included + if header.byte_count + 3 != packet.len() { + trace!("Packet byte count mismatch"); return Err(Error::InvalidInput); } From 9e83a4f21e05dc7e741880a5489004ee673035d4 Mon Sep 17 00:00:00 2001 From: Marvin Gudel Date: Sun, 18 Jan 2026 17:21:32 +0100 Subject: [PATCH 6/6] I2C transport: Docs and logging Signed-off-by: Marvin Gudel --- mctp-estack/src/i2c.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/mctp-estack/src/i2c.rs b/mctp-estack/src/i2c.rs index da21c21..2aff2ce 100644 --- a/mctp-estack/src/i2c.rs +++ b/mctp-estack/src/i2c.rs @@ -21,13 +21,31 @@ const MCTP_I2C_HEADER: usize = 4; // bytecount is limited to u8, includes MCTP payload + 1 byte i2c source pub const MCTP_I2C_MAXMTU: usize = u8::MAX as usize - 1; +/// MCTP I2C encapsulation header pub struct MctpI2cHeader { + /// Destination address + /// + /// The 7-bit destination address of the encapsulated packet. + /// (Not including the SMBus R/W# bit) pub dest: u8, + /// Source address + /// + /// The 7-bit source address of the encapsulated packet. + /// (Not including fixed bit `[0]`) pub source: u8, + /// Byte count + /// + /// The count of bytes that follow the _Byte Count_ field up to, + /// but not including, the PEC byte. pub byte_count: usize, } impl MctpI2cHeader { + /// Encode this header + /// + /// Creates a 4-byte header with destination, command code, byte count, and source. + /// Returns a [BadArgument](Error::BadArgument) error when the addresses are not 7-bit, + /// or when the byte count is larger than [u8::MAX]. fn encode(&self) -> Result<[u8; 4]> { if self.dest > 0x7f || self.source > 0x7f { return Err(Error::BadArgument); @@ -85,6 +103,17 @@ impl MctpI2cEncap { self.own_addr } + /// Decode a I2C encapsulated packet + /// + /// Decodes and verifies the I2C transport header. + /// Optionally an included _PEC_ will be verified. + /// + /// The inner MCTP packet and the decoded header are returned on success. + /// + /// ### Errors when + /// - The _PEC_ is incorrect + /// - Header decoding fails + /// - The decoded byte count field does not match the packet size pub fn decode<'f>( &self, mut packet: &'f [u8], @@ -112,6 +141,9 @@ impl MctpI2cEncap { trace!("Packet byte count mismatch"); return Err(Error::InvalidInput); } + if header.dest != self.own_addr { + trace!("I2C destination address mismatch"); + } Ok((&packet[MCTP_I2C_HEADER..], header)) }