Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions lib/propolis/src/hw/virtio/p9fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -845,8 +845,7 @@ impl P9Handler for HostFSHandler {

let mut entries: Vec<proto::Dirent> = Vec::new();

let mut offset = 1;
for de in &dir[msg.offset as usize..] {
for (offset, de) in (1..).zip(dir[msg.offset as usize..].iter()) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thsi is addressing a recent clippy complaint.

let metadata = match de.metadata() {
Ok(m) => m,
Err(e) => {
Expand Down Expand Up @@ -885,7 +884,6 @@ impl P9Handler for HostFSHandler {

space_left -= dirent.wire_size();
entries.push(dirent);
offset += 1;
}

let response = Rreaddir::new(entries);
Expand Down
82 changes: 81 additions & 1 deletion lib/propolis/src/hw/virtio/softnpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use std::{
collections::BTreeMap,
fs::{self, File, OpenOptions},
io::{Result, Write},
io::{Error, Result, Write},
sync::{Arc, Mutex},
thread::{sleep, spawn},
time::Duration,
Expand Down Expand Up @@ -45,6 +45,9 @@ use slog::{error, info, warn, Logger};
const MTU: usize = 9216;
const SOFTNPU_CPU_AUX_PORT: u16 = 1000;

// Enough RX buffer space to hold ~100 frames
const RX_BUFFER_SIZE: u32 = 0x120000;

pub const MANAGEMENT_MESSAGE_PREAMBLE: u8 = 0b11100101;
pub const SOFTNPU_TTY: &str = "/dev/tty03";

Expand Down Expand Up @@ -209,6 +212,7 @@ impl SoftNpu {
let mut handles = Vec::new();
for x in data_links {
let h = dlpi::open(x, dlpi::sys::DLPI_RAW)?;
set_rx_buffer_size(h, RX_BUFFER_SIZE)?;

// Although we bind to the IPv6 SAP (Ethertype), the DL_PROMISC_SAP
// allows us to pick up everything. Binding to *something* to start
Expand Down Expand Up @@ -258,6 +262,82 @@ impl SoftNpu {
}
}

#[repr(C)]
struct StrIoctl {
ic_cmd: i32,
ic_timout: i32,
ic_len: i32,
ic_dp: *mut libc::c_char,
}

fn strioc<T>(fd: i32, cmd: i32, arg: &mut T) -> Result<()> {
#[cfg(target_os = "illumos")]
let rq = libc::I_STR;
#[cfg(not(target_os = "illumos"))]
let rq = 0xdeadbeef;

let mut si = StrIoctl {
ic_cmd: cmd,
ic_timout: -1,
ic_len: std::mem::size_of::<T>() as i32,
ic_dp: arg as *mut T as *mut libc::c_char,
};
if unsafe { libc::ioctl(fd, rq, &mut si as *mut StrIoctl) } < 0 {
return Err(Error::last_os_error());
}
Ok(())
}

// STREAMS uses a high water mark as a backpressure mechanism. When we hit the
// high water mark, messages are dropped until we drain to the low water mark.
// This essentially makes the high water mark a receive buffer size.
//
// For TCP on the STREAMS path (i.e. TPI consumers), it appears that the high
// water mark is set to SO_RCVBUF which is 128000 bytes. However, for DLPI
// the high water mark is not set and it defaults to 5120. This meaans we hit
// the mark at the first jumbo frame and thrash from there.
//
// It seems the only way to influence this outside the kernel is pushing a
// passthrough bufmod STREAMS module. So that's what we do here for the time
// being.
fn set_rx_buffer_size(h: dlpi::DlpiHandle, size: u32) -> Result<()> {
// <sys/bufmod.h>
const SBIOCSTIME: i32 = 0x4201; // ('B'<<8)|1
const SBIOCSCHUNK: i32 = 0x4204; // ('B'<<8)|4
const SBIOCSFLAGS: i32 = 0x4208; // ('B'<<8)|8
const SB_NO_HEADER: u32 = 0x02;
const SB_NO_PROTO_CVT: u32 = 0x04;
const SB_NO_DROPS: u32 = 0x10;

let fd = dlpi::fd(h)?;

// bufmod calculates the high water mark as 4*chunk + 512, do the
// inverse to get our target buffer size.
let mut chunk: u32 = size.saturating_sub(512) / 4;
// Put the bufmod in passthrough mode
let mut flags: u32 = SB_NO_HEADER | SB_NO_PROTO_CVT | SB_NO_DROPS;
// Disable chunking, still keeps the water mark that was applied for the
// supplied chunk. Yes this is relying on implicit behavior. Good motivation
// to get off DLPI entirely.
Comment on lines +319 to +321

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured I dunno how this works and it'd be interesting to fish through, my best guess is that doing all this to push q_hiwat to be large enough to at least contain this chunk since we've set it SB_NO_DROPS preventing the kernel from just dropping it? in which case, presumably we can't somehow strqset(.., QHIWAT, idk?, 0x120000)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushing the bufmod streams module is purely for the side-effect of impacting the high water mark. I set SB_NO_DROPS based on the comment in uts/common/io/bufmod.c that says

SB_NO_DROPS - bufmod behaves transparently in flow control and propagates the blocked flow
conditions downstream.

With the idea being having the bufmod be a pure passthrough. Afaict strqset is only available in the kernel so this is the best I could come up with. The snoop program does the same trick to effectively get a decent size receive buffer.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, as i was looking through kernel source and strqset looked vaguely like how i've seen ioctls get plumbed through, but i see now that it's under Kernel Functions for Drivers so that explains the interface and why it wouldn't be usable here. welp. thanks for the pointers 🫡

let mut zero = libc::timeval { tv_sec: 0, tv_usec: 0 };

// Push the STREAMS module
#[cfg(target_os = "illumos")]
let rq = libc::I_PUSH;
#[cfg(not(target_os = "illumos"))]
let rq = 0xdeadbeef;
if unsafe { libc::ioctl(fd, rq, c"bufmod".as_ptr()) } < 0 {
return Err(Error::last_os_error());
}

// Apply the chunk/flags/zero values described above.
strioc(fd, SBIOCSFLAGS, &mut flags)?;
strioc(fd, SBIOCSCHUNK, &mut chunk)?;
strioc(fd, SBIOCSTIME, &mut zero)?;

Ok(())
}

impl Lifecycle for SoftNpu {
fn type_name(&self) -> &'static str {
"softnpu"
Expand Down
Loading