From 4b16d475da51855d504dd790d3d3271d75093839 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 | 72 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 62 insertions(+), 10 deletions(-) diff --git a/mctp-estack/src/i2c.rs b/mctp-estack/src/i2c.rs index 2680842..2acfd58 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,25 +110,25 @@ 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 /// /// `packet` should omit the PEC byte. - /// Returns the MCTP message and the i2c source address. + /// Returns the MCTP message and the i2c header. pub fn receive_done_pec<'f>( &self, packet: &[u8], mctp: &'f mut Stack, - ) -> Result, u8)>> { - let (mctp_packet, i2c_src) = self.decode(packet, false)?; + ) -> Result, MctpI2cHeader)>> { + let (mctp_packet, i2c_header) = self.decode(packet, false)?; // Pass to MCTP stack let m = mctp.receive(mctp_packet)?; - // Return a (message, i2c_source) tuple on completion - Ok(m.map(|msg| (msg, i2c_src))) + // Return a (message, i2c_header) tuple on completion + Ok(m.map(|msg| (msg, i2c_header))) } /// `out` must be sized to hold 8+mctp_mtu, to allow for MCTP and I2C headers @@ -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 02e19f51061d961e8b619ed4645b1407551e3a1c 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 2acfd58..f15d289 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 e6b4734cb884746f92622d983a795792ca14bb3c 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 f15d289..9ffe703 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 38c8f5f7d594625c01d547c7b9646d06ef669857 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 9ffe703..3373250 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 efbca0375b911f728629b7b7c899908c30c9cdd7 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 3373250..8a5c792 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 beedd95e812d6e8f02289ba41d61c6e49f1f695a 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 8a5c792..da709e6 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)) }