PLDM file buffer sizes and pldm-file-host usability

pldm-file/Cargo.toml

@@ -18,6 +18,7 @@ num-traits = { workspace = true }
 pldm = { workspace = true }
 
 [dev-dependencies]
+argh = { workspace = true }
 anyhow = "1.0"
 chrono = { workspace = true, features = ["clock"] }
 mctp-linux = { workspace = true }

pldm-file/examples/client.rs

@@ -38,19 +38,21 @@ fn main() -> Result<()> {
 
         let mut buf = Vec::new();
         let req_len = 4096;
+        let mut part_buf = [0u8; 512 + 18];
 
         println!("Reading...");
-        let res = df_read_with(&mut req, fd, 0, req_len, |part| {
-            println!("  {} bytes", part.len());
-            if buf.len() + part.len() > req_len {
-                println!("  data overflow!");
-                Err(PldmError::NoSpace)
-            } else {
-                buf.extend_from_slice(part);
-                Ok(())
-            }
-        })
-        .await;
+        let res =
+            df_read_with(&mut req, fd, 0, req_len, &mut part_buf, |part| {
+                println!("  {} bytes", part.len());
+                if buf.len() + part.len() > req_len {
+                    println!("  data overflow!");
+                    Err(PldmError::NoSpace)
+                } else {
+                    buf.extend_from_slice(part);
+                    Ok(())
+                }
+            })
+            .await;
 
         println!("Read: {res:?}");
 

pldm-file/examples/host.rs

@@ -12,39 +12,163 @@ use std::io::{Read, Seek, SeekFrom};
 use std::os::unix::fs::MetadataExt;
 use std::time::{Duration, Instant};
 
+use argh;
+
+/// PLDM file host
+#[derive(argh::FromArgs)]
+struct Args {
+    /// file to serve
+    #[argh(
+        option,
+        short = 'f',
+        default = r#""pldm-file-host.bin".to_string()"#
+    )]
+    file: String,
+
+    /// serve zeroes
+    #[argh(switch)]
+    zeroes: bool,
+
+    /// serve a file containing the offset at intervals
+    #[argh(switch)]
+    offset: bool,
+}
+
 struct Host {
-    file: RefCell<File>,
-    stamp: RefCell<(Instant, usize)>,
+    file_size: u32,
+    speed: Speed,
+    mode: Mode,
+}
+
+enum Mode {
+    File {
+        file: RefCell<File>,
+        filename: String,
+    },
+    Offset,
+    Zeroes,
+}
+
+impl Mode {
+    fn filename(&self) -> String {
+        match self {
+            Self::File { filename, .. } => filename.clone(),
+            Self::Offset => "offset".to_string(),
+            Self::Zeroes => "zeroes".to_string(),
+        }
+    }
 }
 
-const FILENAME: &str = "pldm-file-host.bin";
 // Arbitrary, 0 is reserved.
 const PDR_HANDLE: u32 = 1;
 
 impl Host {
-    fn new() -> Result<Self> {
+    fn new(args: &Args) -> Result<Self> {
+        let speed = Speed::new();
+
+        let mut file_size = u32::MAX;
+        let mode = if args.zeroes {
+            info!("Serving zeroes");
+            Mode::Zeroes
+        } else if args.offset {
+            info!("Serving offset");
+            Mode::Offset
+        } else {
+            let file = File::open(&args.file).with_context(|| {
+                format!("cannot open input file {}", args.file)
+            })?;
+            file_size = file
+                .metadata()
+                .context("Metadata failed")?
+                .size()
+                .try_into()
+                .context("File size > u32")?;
+            info!("Serving {}, {} bytes", args.file, file_size);
+            Mode::File {
+                file: RefCell::new(file),
+                filename: args.file.clone(),
+            }
+        };
+
         Ok(Self {
-            file: RefCell::new(File::open(FILENAME).with_context(|| {
-                format!("cannot open input file {FILENAME}")
-            })?),
-            stamp: RefCell::new((Instant::now(), 0)),
+            mode,
+            file_size,
+            speed,
         })
     }
 }
 
 impl pldm_file::host::Host for Host {
     fn read(&self, buf: &mut [u8], offset: usize) -> std::io::Result<usize> {
+        self.speed.update(offset);
+
+        match &self.mode {
+            Mode::File { file, .. } => {
+                let mut file = file.borrow_mut();
+                file.seek(SeekFrom::Start(offset as u64))?;
+                read_whole(&mut file, buf)
+            }
+            Mode::Zeroes => {
+                buf.fill(0);
+                Ok(buf.len())
+            }
+            Mode::Offset => {
+                let mut o = offset;
+                for b in buf.chunks_mut(64) {
+                    if let Some(b) = b.get_mut(..4) {
+                        b.copy_from_slice(&o.to_be_bytes())
+                    }
+                    o += b.len();
+                }
+                Ok(buf.len())
+            }
+        }
+    }
+}
+
+/// Reads into buf until EOF or error.
+///
+/// A returned `length < buf.len()` means EOF. `buf` should not be empty.
+fn read_whole(f: &mut File, buf: &mut [u8]) -> std::io::Result<usize> {
+    let total = buf.len();
+    let mut rest = buf;
+    while !rest.is_empty() {
+        match f.read(rest) {
+            // EOF
+            Ok(0) => return Ok(total - rest.len()),
+            Ok(l) => rest = &mut rest[l..],
+            Err(e) if e.kind() == std::io::ErrorKind::Interrupted => continue,
+            Err(e) => return Err(e),
+        }
+    }
+    // Full output
+    return Ok(total);
+}
+
+struct Speed {
+    stamp: RefCell<(Instant, usize)>,
+}
+
+impl Speed {
+    fn new() -> Self {
+        Self {
+            stamp: RefCell::new((Instant::now(), 0)),
+        }
+    }
+
+    fn update(&self, offset: usize) {
         let mut stamp = self.stamp.borrow_mut();
         let now = Instant::now();
+        if offset == 0 {
+            stamp.0 = now;
+        }
+
         let del = now - stamp.0;
         if del > Duration::from_secs(2) {
             let rate = (offset - stamp.1) / del.as_millis() as usize;
             info!("{rate} kB/s, offset {offset}");
             *stamp = (now, offset);
         }
-        let mut file = self.file.borrow_mut();
-        file.seek(SeekFrom::Start(offset as u64))?;
-        file.read(buf)
     }
 }
 
@@ -57,7 +181,10 @@ fn main() -> Result<()> {
     let mut listener = MctpLinuxAsyncListener::new(mctp::MCTP_TYPE_PLDM, None)?;
     let mut pldm_ctrl = pldm::control::responder::Responder::<2>::new();
     let mut pldm_file = FileResponder::new();
-    let mut host = Host::new().context("unable to create file host")?;
+
+    let args: Args = argh::from_env();
+
+    let mut host = Host::new(&args).context("unable to create file host")?;
 
     FileResponder::register(&mut pldm_ctrl)?;
 
@@ -180,15 +307,6 @@ fn handle_get_pdr(
         return Ok(plat_codes::INVALID_RECORD_CHANGE_NUMBER);
     }
 
-    let file_max_size = host
-        .file
-        .borrow()
-        .metadata()
-        .context("Metadata failed")?
-        .size()
-        .try_into()
-        .context("File size > u32")?;
-
     let pdr_resp = GetPDRResp::new_single(
         PDR_HANDLE,
         PdrRecord::FileDescriptor(FileDescriptorPdr {
@@ -204,10 +322,15 @@ fn handle_get_pdr(
             oem_file_classification: 0,
             capabilities: file_capabilities::EX_READ_OPEN,
             file_version: 0xFFFFFFFF,
-            file_max_size,
+            file_max_size: host.file_size,
             // TODO
             file_max_desc_count: 1,
-            file_name: FILENAME.try_into().expect("Filename too long"),
+            file_name: host
+                .mode
+                .filename()
+                .as_str()
+                .try_into()
+                .expect("Filename too long"),
             oem_file_classification_name: Default::default(),
         }),
     )?;

pldm-file/src/client.rs

@@ -109,21 +109,25 @@ pub async fn df_close(
     Ok(())
 }
 
-const PART_SIZE: usize = 1024;
+pub const PART_SIZE: usize = 4096;
 
 /// Read a file from the host.
 ///
 /// The total file size read is returned.
+///
+/// `part_buf` must be sized as at least `multipart_size + 18`, where multipart_size
+/// is the size passed to [`pldm::control::requester::negotiate_transfer_parameters`].
 pub async fn df_read(
     comm: &mut impl mctp::AsyncReqChannel,
     file: FileDescriptor,
     offset: usize,
     buf: &mut [u8],
+    part_buf: &mut [u8],
 ) -> Result<usize> {
     let len = buf.len();
     let mut v = pldm::util::SliceWriter::new(buf);
 
-    df_read_with(comm, file, offset, len, |b| {
+    df_read_with(comm, file, offset, len, part_buf, |b| {
         let r = v.extend(b);
         debug_assert!(r.is_some(), "provided data should be <= len");
         Ok(())
@@ -135,11 +139,15 @@ pub async fn df_read(
 ///
 /// The `out` closure will be called repeatedly with sequential buffer chunks
 /// sent from the host. Any error returned by `out` will be returned from `df_read_with`.
+///
+/// `part_buf` must be sized as at least `multipart_size + 18`, where multipart_size
+/// is the size passed to [`pldm::control::requester::negotiate_transfer_parameters`].
 pub async fn df_read_with<F>(
     comm: &mut impl mctp::AsyncReqChannel,
     file: FileDescriptor,
     offset: usize,
     len: usize,
+    part_buf: &mut [u8],
     mut out: F,
 ) -> Result<usize>
 where
@@ -180,9 +188,8 @@ where
             tx_buf,
         );
 
-        // todo: negotiated length
-        let mut rx_buf = [0u8; 14 + PART_SIZE + 4];
-        let resp = pldm_xfer_buf_async(comm, pldm_req, &mut rx_buf).await?;
+        // todo: can part_buf.len() be checked against negotiated length?
+        let resp = pldm_xfer_buf_async(comm, pldm_req, part_buf).await?;
 
         let ((rest, _), read_resp) =
             MultipartReceiveResp::from_bytes((&resp.data, 0)).map_err(|e| {

pldm-file/src/host.rs

@@ -15,7 +15,8 @@ use crate::PLDM_TYPE_FILE_TRANSFER;
 
 const FILE_ID: FileIdentifier = FileIdentifier(0);
 
-const MAX_PART_SIZE: u16 = 1024;
+// Largest possible power of two
+const MAX_PART_SIZE: u16 = 8192;
 
 pub trait Host {
     /// Returns number of bytes read