Skip to content

Make MsgBlk allocation fallible #962

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
FelixMcFelix wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Make MsgBlk allocation fallible #962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e50d4d0
It's a start!
FelixMcFelix Apr 1, 2026
643069f
Test fixups...
FelixMcFelix Apr 7, 2026
6951016
Fixup xde, `cargo test`
FelixMcFelix Apr 8, 2026
cbb75d3
Clippy, culler of dreams
FelixMcFelix Apr 8, 2026
07b25ab
Better per-layer KStats for errors
FelixMcFelix Apr 8, 2026
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
6 changes: 4 additions & 2 deletions lib/opte-test-utils/src/dhcp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ pub fn packet_from_client_dhcpv4_message(
let headers = (eth, ip, udp);
let total_len = msg.buffer_len() + headers.packet_length();

let mut pkt = MsgBlk::new_ethernet(total_len);
let mut pkt =
MsgBlk::new_ethernet(total_len).expect("infallible in std context");
pkt.emit_back(&headers).unwrap();
let dhcp_off = pkt.len();
pkt.resize(total_len).unwrap();
Expand Down Expand Up @@ -101,7 +102,8 @@ pub fn write_dhcpv6_packet(
let headers = (eth, ip, udp);
let total_len = msg.buffer_len() + headers.packet_length();

let mut pkt = MsgBlk::new_ethernet(total_len);
let mut pkt =
MsgBlk::new_ethernet(total_len).expect("infallible in std context");
pkt.emit_back(&headers).unwrap();
let dhcp_off = pkt.len();
pkt.resize(total_len).unwrap();
Expand Down
12 changes: 7 additions & 5 deletions lib/opte-test-utils/src/geneve_verify.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,15 @@ pub fn parse_geneve_packet(bytes: &[u8]) -> Result<GeneveInfo> {
///
/// # Example
/// ```no_run
/// let snoop_output = snoop_underlay.assert_packet("on underlay");
/// let stdout = String::from_utf8_lossy(&snoop_output.stdout);
/// use opte_test_utils::geneve_verify;
/// use opte::api::MulticastUnderlay;
///
/// let mcast_underlay = MulticastUnderlay::new("ff04::1".parse().unwrap()).unwrap();
/// geneve_verify::assert_geneve_packet(
/// &stdout,
/// vni,
/// "<...SnoopGuard output...>",
/// 77.try_into().unwrap(),
/// mcast_underlay,
/// Replication::External,
/// oxide_vpc::api::Replication::External,
Copy link
Copy Markdown
Collaborator Author

@FelixMcFelix FelixMcFelix Apr 8, 2026

Choose a reason for hiding this comment

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

cargo t was tripping up on illumos because of this doctest: no_run will still compile it!

/// );
/// ```
pub fn assert_geneve_packet(
Expand Down
45 changes: 24 additions & 21 deletions lib/opte-test-utils/src/icmp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,27 +147,28 @@ pub fn gen_icmp_echo(

let mut segments = vec![];

// Note: all unwraps below are expected to be infallible in test/std contexts.
match n_segments {
1 => {
return MsgBlk::new_ethernet_pkt((&eth, &ip, &icmp_bytes));
return MsgBlk::new_ethernet_pkt((&eth, &ip, &icmp_bytes)).unwrap();
}
2 => {
segments.push(MsgBlk::new_ethernet_pkt(eth));
segments.push(MsgBlk::new_pkt((&ip, &icmp_bytes)));
segments.push(MsgBlk::new_ethernet_pkt(eth).unwrap());
segments.push(MsgBlk::new_pkt((&ip, &icmp_bytes)).unwrap());
}
3 => {
segments.push(MsgBlk::new_ethernet_pkt(eth));
segments.push(MsgBlk::new_pkt(ip));
segments.push(MsgBlk::new_pkt(&icmp_bytes));
segments.push(MsgBlk::new_ethernet_pkt(eth).unwrap());
segments.push(MsgBlk::new_pkt(ip).unwrap());
segments.push(MsgBlk::new_pkt(&icmp_bytes).unwrap());
}
4 => {
// Used to test pullup behaviour around longer mblks
// which still have pkt bodies in guest memory.
assert!(icmp_bytes.len() > 8);
segments.push(MsgBlk::new_ethernet_pkt(eth));
segments.push(MsgBlk::new_pkt(ip));
segments.push(MsgBlk::new_pkt(&icmp_bytes[..8]));
segments.push(MsgBlk::new_pkt(&icmp_bytes[8..]));
segments.push(MsgBlk::new_ethernet_pkt(eth).unwrap());
segments.push(MsgBlk::new_pkt(ip).unwrap());
segments.push(MsgBlk::new_pkt(&icmp_bytes[..8]).unwrap());
segments.push(MsgBlk::new_pkt(&icmp_bytes[8..]).unwrap());
}
_ => {
panic!("only 1 2 or 3 segments allowed")
Expand Down Expand Up @@ -261,27 +262,28 @@ pub fn gen_icmpv6_echo(

let mut segments = vec![];

// Note: all unwraps below are expected to be infallible in test/std contexts.
match n_segments {
1 => {
return MsgBlk::new_ethernet_pkt((&eth, &ip, &body_bytes));
return MsgBlk::new_ethernet_pkt((&eth, &ip, &body_bytes)).unwrap();
}
2 => {
segments.push(MsgBlk::new_ethernet_pkt(eth));
segments.push(MsgBlk::new_pkt((&ip, &body_bytes)));
segments.push(MsgBlk::new_ethernet_pkt(eth).unwrap());
segments.push(MsgBlk::new_pkt((&ip, &body_bytes)).unwrap());
}
3 => {
segments.push(MsgBlk::new_ethernet_pkt(eth));
segments.push(MsgBlk::new_pkt(ip));
segments.push(MsgBlk::new_pkt(&body_bytes));
segments.push(MsgBlk::new_ethernet_pkt(eth).unwrap());
segments.push(MsgBlk::new_pkt(ip).unwrap());
segments.push(MsgBlk::new_pkt(&body_bytes).unwrap());
}
4 => {
// Used to test pullup behaviour around longer mblks
// which still have pkt bodies in guest memory.
assert!(body_bytes.len() > 8);
segments.push(MsgBlk::new_ethernet_pkt(eth));
segments.push(MsgBlk::new_pkt(ip));
segments.push(MsgBlk::new_pkt(&body_bytes[..8]));
segments.push(MsgBlk::new_pkt(&body_bytes[8..]));
segments.push(MsgBlk::new_ethernet_pkt(eth).unwrap());
segments.push(MsgBlk::new_pkt(ip).unwrap());
segments.push(MsgBlk::new_pkt(&body_bytes[..8]).unwrap());
segments.push(MsgBlk::new_pkt(&body_bytes[8..]).unwrap());
}
_ => {
panic!("only 1 2 or 3 segments allowed")
Expand Down Expand Up @@ -325,7 +327,8 @@ pub fn generate_ndisc(

let headers = (eth, ip);
let total_len = req.buffer_len() + headers.packet_length();
let mut pkt = MsgBlk::new_ethernet(total_len);
let mut pkt =
MsgBlk::new_ethernet(total_len).expect("infallible in std context");
pkt.emit_back(&headers).unwrap();
let ndisc_off = pkt.len();
pkt.resize(total_len).unwrap();
Expand Down
8 changes: 5 additions & 3 deletions lib/opte-test-utils/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ macro_rules! expect_modified {
);
#[allow(unused_assignments)]
if let Ok(Modified(spec)) = $res {
$pkt = spec.apply($pkt);
$pkt = spec.apply($pkt).unwrap();
}
};
}
Expand Down Expand Up @@ -503,7 +503,8 @@ pub fn ulp_pkt<
ulp: U,
body: &[u8],
) -> MsgBlk {
let mut pkt = MsgBlk::new_ethernet_pkt((eth, ip, ulp, body));
let mut pkt = MsgBlk::new_ethernet_pkt((eth, ip, ulp, body))
.expect("infallible in std context");

let view = Packet::parse_outbound(pkt.iter_mut(), GenericUlp {}).unwrap();
let mut view = view.to_full_meta();
Expand Down Expand Up @@ -1070,7 +1071,8 @@ fn _encap(
outer_ip,
outer_udp,
outer_geneve,
));
))
.expect("infallible in std context");
encap_pkt.append(inner_pkt);

encap_pkt
Expand Down
84 changes: 52 additions & 32 deletions lib/opte/src/ddi/mblk.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

// Copyright 2025 Oxide Computer Company
// Copyright 2026 Oxide Computer Company

use crate::engine::packet::BufferState;
use crate::engine::packet::Pullup;
Expand Down Expand Up @@ -286,20 +286,17 @@ impl MsgBlk {
/// `allocb(9F)` and `freeb(9F)`, which contains enough scaffolding
/// to satisfy OPTE's use of the underlying `mblk_t` and `dblk_t`
/// structures.
pub fn new(len: usize) -> Self {
let inner = NonNull::new(allocb(len))
.expect("somehow failed to get an mblk...");

Self(inner)
pub fn new(len: usize) -> Result<Self, PktAllocError> {
NonNull::new(allocb(len)).map(Self).ok_or(PktAllocError)
}

/// Allocates a new [`MsgBlk`] of size `buf.len()`, copying its
/// contents.
pub fn copy(buf: impl AsRef<[u8]>) -> Self {
let mut out = Self::new(buf.as_ref().len());
pub fn copy(buf: impl AsRef<[u8]>) -> Result<Self, PktAllocError> {
let mut out = Self::new(buf.as_ref().len())?;
// Unwrap safety -- just allocated length of input buffer.
out.write_bytes_back(buf).unwrap();
out
Ok(out)
}

/// Copy the first `n` bytes of this packet into a new `mblk_t`,
Expand Down Expand Up @@ -339,7 +336,7 @@ impl MsgBlk {
// in our userland mblk abstraction.
// Do the segmentation right, but otherwise it's fully cloned.
let to_ensure = n.map(|v| v.get()).unwrap_or(totlen);
let mut top_mblk = MsgBlk::new(to_ensure);
let mut top_mblk = MsgBlk::new(to_ensure)?;
let mut still_to_write = to_ensure;

for chunk in self.iter() {
Expand All @@ -355,7 +352,7 @@ impl MsgBlk {
left_in_chunk -= to_take;

if left_in_chunk != 0 {
top_mblk.append(MsgBlk::copy(&chunk[to_take..]));
top_mblk.append(MsgBlk::copy(&chunk[to_take..])?);
}
}

Expand All @@ -365,10 +362,12 @@ impl MsgBlk {
}

/// Creates a new [`MsgBlk`] using a given set of packet headers.
pub fn new_pkt(emit: impl Emit + EmitDoesNotRelyOnBufContents) -> Self {
let mut pkt = Self::new(emit.packet_length());
pub fn new_pkt(
emit: impl Emit + EmitDoesNotRelyOnBufContents,
) -> Result<Self, PktAllocError> {
let mut pkt = Self::new(emit.packet_length())?;
pkt.emit_back(emit).unwrap();
pkt
Ok(pkt)
}

/// Returns the number of bytes available for writing ahead of the
Expand Down Expand Up @@ -408,7 +407,7 @@ impl MsgBlk {
/// bytes with 2B of headroom/alignment.
///
/// This sets up 4B alignment on all post-ethernet headers.
pub fn new_ethernet(len: usize) -> Self {
pub fn new_ethernet(len: usize) -> Result<Self, PktAllocError> {
Self::new_with_headroom(2, len)
}

Expand All @@ -418,10 +417,10 @@ impl MsgBlk {
/// This sets up 4B alignment on all post-ethernet headers.
pub fn new_ethernet_pkt(
emit: impl Emit + EmitDoesNotRelyOnBufContents,
) -> Self {
let mut pkt = Self::new_ethernet(emit.packet_length());
) -> Result<Self, PktAllocError> {
let mut pkt = Self::new_ethernet(emit.packet_length())?;
pkt.emit_back(emit).unwrap();
pkt
Ok(pkt)
}

/// Return the number of initialised bytes in this `MsgBlk` over
Expand Down Expand Up @@ -482,8 +481,11 @@ impl MsgBlk {
///
/// The read/write pointer is set to have `head_len` bytes of
/// headroom and `body_len` bytes of capacity at the back.
pub fn new_with_headroom(head_len: usize, body_len: usize) -> Self {
let out = Self::new(head_len + body_len);
pub fn new_with_headroom(
head_len: usize,
body_len: usize,
) -> Result<Self, PktAllocError> {
let out = Self::new(head_len + body_len)?;

// SAFETY: alloc is contiguous and always larger than head_len.
let mut_out = out.0.as_ptr();
Expand All @@ -492,7 +494,7 @@ impl MsgBlk {
(*mut_out).b_wptr = (*mut_out).b_rptr;
}

out
Ok(out)
}

/// Provides a slice of length `n_bytes` at the back of an [`MsgBlk`]
Expand Down Expand Up @@ -1023,11 +1025,11 @@ impl MsgBlkIterMut<'_> {
}

impl Pullup for MsgBlkIterMut<'_> {
fn pullup(&self, prepend: Option<&[u8]>) -> MsgBlk {
fn pullup(&self, prepend: Option<&[u8]>) -> Result<MsgBlk, PktPullupError> {
let prepend = prepend.unwrap_or_default();
let bytes_in_self = BufferState::len(self);
let needed_alloc = prepend.len() + bytes_in_self;
let mut new_seg = MsgBlk::new(needed_alloc);
let mut new_seg = MsgBlk::new(needed_alloc)?;

new_seg
.write_bytes_back(prepend)
Expand Down Expand Up @@ -1077,7 +1079,19 @@ impl Pullup for MsgBlkIterMut<'_> {
}
}

new_seg
Ok(new_seg)
}
}

/// A new [`MsgBlk`] could not be allocated.
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Hash)]
pub struct PktAllocError;

impl core::error::Error for PktAllocError {}

impl core::fmt::Display for PktAllocError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.write_str("failed to allocate an mblk_t")
}
}

Expand Down Expand Up @@ -1118,6 +1132,12 @@ impl core::fmt::Display for PktPullupError {
}
}

impl From<PktAllocError> for PktPullupError {
fn from(_val: PktAllocError) -> Self {
Self::AllocFailed
}
}

/// Counts the number of segments in an `mblk_t` from `head`, linked
/// via `b_cont`.
unsafe fn count_mblk_chain(mut head: Option<NonNull<mblk_t>>) -> usize {
Expand Down Expand Up @@ -1397,7 +1417,7 @@ mod test {

#[test]
fn zero_byte_packet() {
let mut pkt = MsgBlk::new(0);
let mut pkt = MsgBlk::new(0).unwrap();
assert_eq!(pkt.len(), 0);
assert_eq!(pkt.seg_len(), 1);
assert_eq!(pkt.tail_capacity(), 16);
Expand All @@ -1413,7 +1433,7 @@ mod test {
_ => panic!("expected failure, accidentally succeeded at parsing"),
}

let pkt2 = MsgBlk::copy([]);
let pkt2 = MsgBlk::copy([]).unwrap();
assert_eq!(pkt2.len(), 0);
assert_eq!(pkt2.seg_len(), 1);
assert_eq!(pkt2.tail_capacity(), 16);
Expand Down Expand Up @@ -1470,9 +1490,9 @@ mod test {

#[test]
fn truncate() {
let mut p1 = MsgBlk::copy([0, 1, 2, 3]);
p1.append(MsgBlk::copy([4, 5, 6, 7]));
p1.append(MsgBlk::copy([8, 9, 10, 11]));
let mut p1 = MsgBlk::copy([0, 1, 2, 3]).unwrap();
p1.append(MsgBlk::copy([4, 5, 6, 7]).unwrap());
p1.append(MsgBlk::copy([8, 9, 10, 11]).unwrap());

assert_eq!(p1.seg_len(), 3);
assert_eq!(p1.byte_len(), 12);
Expand All @@ -1491,15 +1511,15 @@ mod test {
// Verify uninitialized packet.
#[test]
fn uninitialized_packet() {
let pkt = MsgBlk::new(200);
let pkt = MsgBlk::new(200).unwrap();
assert_eq!(pkt.len(), 0);
assert_eq!(pkt.seg_len(), 1);
assert_eq!(pkt.tail_capacity(), 200);
}

#[test]
fn expand_and_shrink() {
let mut seg = MsgBlk::new(18);
let mut seg = MsgBlk::new(18).unwrap();
assert_eq!(seg.len(), 0);
seg.resize(18).unwrap();
assert_eq!(seg.len(), 18);
Expand All @@ -1515,7 +1535,7 @@ mod test {

#[test]
fn prefix_len() {
let mut seg = MsgBlk::new(18);
let mut seg = MsgBlk::new(18).unwrap();
assert_eq!(seg.head_capacity(), 0);
seg.resize(18).unwrap();
assert_eq!(seg.head_capacity(), 0);
Expand Down
Loading