Skip to content

mblk_t creation is more fallible than expected #840

@FelixMcFelix

Description

@FelixMcFelix

The current implementation of MsgBlk::new assumes that allocb should never fail.

pub fn new(len: usize) -> Self {
let inner = NonNull::new(allocb(len))
.expect("somehow failed to get an mblk...");
Self(inner)
}

This was hit by an omicron-deploy test here. In context this would have been asking for an extra ~70B to push in front of the packet, but there are two parts in allocb which can trip us up:

  • This header fragment will be drawn from the streams_dblk_128 dblk cache, using kmem_cache_alloc(cache, KM_NOSLEEP).
  • If this cache is exhausted, it will attempt to create new elements. The KM_NOSLEEP flag allows any internal allocation to exit after a few reclamation attempts (noting that the kmem cache internals are a bit more complex than just calling out to kmem_alloc for a 128B packet buffer).

This differs from the infallible allocation model we're using everywhere else, which is to kmem_alloc(sz, KM_SLEEP) (which matches Rust's expectations around allocations).

We should treat mblk_t creation as fallible and most likely drop packets whch fail to allocate furing processing. In practice this will affect two classes of traffic: hairpin packets sent in response to packets from the guest, and outbound packets from zones (NTP, Nexus, DNS). Although this should be rare, given the length of time it's taken for this to occur once. Non-hairpin packets from guests should already have sufficient headroom and won't be vulnerable to this.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions