diff --git a/pldm-file/src/client.rs b/pldm-file/src/client.rs index 2cef5e1..2902326 100644 --- a/pldm-file/src/client.rs +++ b/pldm-file/src/client.rs @@ -1,5 +1,5 @@ use deku::{DekuContainerRead, DekuContainerWrite}; -use log::trace; +use log::{trace, warn}; use pldm::control::{MultipartReceiveReq, MultipartReceiveResp}; use pldm::{ pldm_xfer_buf_async, proto_error, PldmError, PldmRequest, PldmResult, @@ -153,6 +153,9 @@ pub async fn df_read_with( where F: FnMut(&[u8]) -> PldmResult<()>, { + // Maximum retries for the same offset. Avoids looping forever. + const CRC_FAIL_RETRIES: usize = 20; + let req_offset = u32::try_from(offset).map_err(|_| { trace!("invalid offset"); PldmError::InvalidArgument @@ -177,6 +180,7 @@ where }; let crc32 = crc::Crc::::new(&crc::CRC_32_ISO_HDLC); let mut digest = crc32.digest(); + let mut crc_fail_count = 0; loop { let mut tx_buf = [0; 18]; let l = req.to_slice(&mut tx_buf).map_err(|_| PldmError::NoSpace)?; @@ -205,13 +209,25 @@ where let (resp_data, resp_cs) = rest.split_at(resp_data_len); - digest.update(resp_data); + let mut up_digest = digest.clone(); + up_digest.update(resp_data); // unwrap: we have asserted the lengths above let cs = u32::from_le_bytes(resp_cs.try_into().unwrap()); - if digest.clone().finalize() != cs { - return Err(proto_error!("data checksum mismatch")); + if up_digest.clone().finalize() != cs { + warn!( + "data checksum mismatch requesting offset {}", + req_offset as usize + part_offset + ); + crc_fail_count += 1; + if crc_fail_count > CRC_FAIL_RETRIES { + return Err(proto_error!("excess CRC failures")); + } + req.xfer_op = pldm::control::xfer_op::CURRENT_PART; + continue; } + digest = up_digest; + crc_fail_count = 0; // Ensure the host doesn't send more data than requested let total_len = part_offset diff --git a/pldm-file/src/host.rs b/pldm-file/src/host.rs index c3e7e70..c2ee9c4 100644 --- a/pldm-file/src/host.rs +++ b/pldm-file/src/host.rs @@ -36,6 +36,9 @@ struct FileTransferContext { // Current transfer 0..len offset: usize, digest: crc::Digest<'static, u32, crc::Table<16>>, + // Digest after adding the current part. Is copied to + // `digest` after the next part is requested (to allow for retries). + next_digest: crc::Digest<'static, u32, crc::Table<16>>, } impl FileTransferContext { @@ -45,6 +48,7 @@ impl FileTransferContext { len, offset: 0, digest: CRC32.digest(), + next_digest: CRC32.digest(), } } @@ -384,7 +388,11 @@ impl Responder { let offset = match cmd.xfer_op { pldm::control::xfer_op::FIRST_PART | pldm::control::xfer_op::CURRENT_PART => xfer_ctx.offset, - pldm::control::xfer_op::NEXT_PART => xfer_ctx.offset + part_size, + pldm::control::xfer_op::NEXT_PART => { + // have moved to the next part, can include the previous part's digest + xfer_ctx.digest = xfer_ctx.next_digest.clone(); + xfer_ctx.offset + part_size + } _ => Err(CCode::ERROR_INVALID_DATA)?, }; @@ -427,8 +435,9 @@ impl Responder { host.read(data, xfer_ctx.start + offset) .map_err(|_| CCode::ERROR)?; - xfer_ctx.digest.update(data); - let cs = xfer_ctx.digest.clone().finalize(); + xfer_ctx.next_digest = xfer_ctx.digest.clone(); + xfer_ctx.next_digest.update(data); + let cs = xfer_ctx.next_digest.clone().finalize(); resp_data.extend_from_slice(&cs.to_le_bytes()); xfer_ctx.offset = offset;