oxidecomputer / oxidecomputer/opte

`mblk_t` creation is more fallible than expected

Open
#840 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Rust
Stars
77
Forks
11
Avg merge
9d 20h
Merged PRs (30d)
8

Description

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

https://github.com/oxidecomputer/opte/blob/2331d7e98619b185ad2dca3df9c32eca7fa4178e/lib/opte/src/ddi/mblk.rs#L287-L292

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 during 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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/opte/src/ddi/mblk.rs at the linked MsgBlk::new implementation and trace how allocb failures affect packet processing. Check the hairpin and zone-originated outbound paths described in the issue; done means mblk_t creation is treated as fallible and affected packets are dropped when allocation fails.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.