Add support for the `hiffy` network backend by mkeeter · Pull Request #616 · oxidecomputer/humility

mkeeter · 2026-04-14T14:32:00Z

This is the Humility side of oxidecomputer/hubris#2466

It adds a third backend to execute hiffy programs over the network; the three backends are now

Debugger
- Reads and writes to RAM directly
- Bytecode is executed by task-hiffy on the SP
Network-attached udprpc
- Bytecode is executed on the host
- Function calls in bytecode are handled by udprpc over the network
Network-attached hiffy (new!)
- Writes to hiffy arrays are done over the network using a new socket
- Reads are done using existing dump infrastructure to read from RAM
- Bytecode is executed by task-hiffy on the SP

We abstract over the debugger and network-attached hiffy with a new enum HiffyWrite to write to hiffy arrays, instead of calling HubrisCore functions directly. Network-attached udprpc remains a special case, because it can only handle a subset of HIF programs (ones that only use Send-flavored functions).

lzrd

Having a more helpful error message for archives where hiffy net feature is not configured is my only suggestion.

lzrd · 2026-04-15T20:43:58Z

-        if !core.is_net() {
+        let hiffy = if core.is_net() {
+            if let Some(hiffy_task) = hubris.lookup_task("hiffy")
+                && hubris.does_task_have_feature(hiffy_task, "net").unwrap()


Does/Can this error case give useful guidance to the user? e.g. "hiffy_task is not configured with net feature".
(Looking for other unwraps() that cold be more helpful in common failure cases...)

That unwrap can't fail once we've found hiffy_task. I've rewritten this to be more clear.

lzrd · 2026-04-15T20:59:05Z

+                let buf_size = self
+                    .hubris
+                    .manifest
+                    .get_socket_by_task("hiffy")


nit: (symbolic constants vs. literals) HIFFY_TASK_NAME? I know we're not going to change this and the function is get_socket_by_task(), but if there are enough "hiffy" names and symbols in different contexts it helps to know what kind of hiffy we are talking about.

I'm ambivalent – HIFFY_TASK_NAME is marginally more clear, but replacing "hiffy" with HIFFY_TASK_NAME everywhere in this file just makes a bunch of lines slightly longer (forcing one large block to reflow to the next indent level).

labbott

I think this looks okay, I'm going to give it another pass in a bit

jamesmunns

Mostly pedantic notes below, I didn't spot anything disagreeable!

One "bigger picture" note here: A lot of the code is somewhat verbose because we're matching on different HiffyImpls, or similar cases. If this was more abstract, I'd say it'd be worth trait-ify-ing, but since we're deciding at runtime, that doesn't make sense unless we want to go down the dyn Trait route, which again doesn't make sense because all the impls live in this crate anyway.

HOWEVER, it might be worth informally defining an impl API, so we can have top level dispatch functions like:

pub fn text_size(&self) -> usize {
  match &self.hiffy {
    HiffyImpl::Debugger(dbg) => dbg.text_size(),
    HiffyImpl::NetHiffy(net) => net.text_size(),
    HiffyImpl::NetUdpRpc(..) => HIFFY_NET_TEXT_SIZE,
  }
}

then have:

impl HiffyDebuggerImpl {
  fn text_size(&self) -> usize {
    self.vars.text.size
  }
}

This way, we can more easily extract the "backend specific" behavior, and make the high level flow more clear, as well as make some of the helper "backend" functions a little easier to refactor.

If NetUdpRpc is the Last Ever™️ impl, it might not be worth it to refactor, but I think making the "backend contract" even informally defined probably has some documentation value.

jamesmunns · 2026-04-21T14:18:15Z

@@ -126,6 +126,7 @@ pub trait Core {
 pub enum NetAgent {


Should we make this non_exhaustive to make additions like this not a breaking change (after this one)? Do we care about breaking changes?

I don't think so – this is only ever used as an input, so it would be weird for outside the net core to match on it.

In general, we (Hubris / Humility) don't care much about breaking changes to quasi-internal Rust APIs, where no one is consuming them besides us and blast radius is a single repository (no one is using NetAgent outside of the humility repo). This could change as Humility gets library-ified, but even then I suspect we'll err on the side of breaking things freely.

jamesmunns · 2026-04-21T14:20:11Z

    pub nbytes: U16<LittleEndian>,
 }

+/// Double of the RPC types from `hiffy` (with the `net` feature enabled)


Is it worth it to extract this into a shared crate somehow to avoid the risk of unsync'd changes?

In theory, that would be the most robust option – but in practice, we've got a few of these internal message types and they're primitive enough to never change. There's also the fact that if they did change, we'd want to support both versions in Humility, because Humility wants to talk to every Hubris image.

jamesmunns · 2026-04-21T14:22:01Z

+// Constants when running with the `NetUdpRpc` impl, which runs the program on
+// the host computer.  These values are much larger than anything we'd see on
+// the embedded system, so we can run any program.
+const HIFFY_NET_TEXT_SIZE: usize = 65536;


maybe note the "HOST" part in the name here somewhere to make it clear we're doing so/don't misuse these consts?

jamesmunns · 2026-04-21T14:25:17Z

+            HiffyImpl::Debugger(vars) | HiffyImpl::NetHiffy { vars, .. } => {
+                vars.data.size
+            }
+            HiffyImpl::NetUdpRpc { .. } => 0, // not supported


Maybe add a function doc comment to note that 0 is a sentinel for "not supported" (or return an Option if more invasive changes are palatable?)

I'll add a doc comment; 0 is both a sentinel for "not support" and technically a valid response (you can access 0 bytes of HIFFY_DATA), so I don't want to return an Option here.

I'm okay with that either way, but wouldn't that be BETTER to use a Option then? To disambiguate between "does not support" and "does support and you can access zero bytes"?

Not too fussed about it though :)

I was going to argue that there's not a user-visible difference between "data not supported" and "supports data of size 0", but then I managed to panic humility auxflash write:

humility/cmd/auxflash/src/lib.rs

Line 330 in 1e40779

for (i, chunk) in data.chunks(self.context.data_size()).enumerate() {

I've now switched data_size to return a Result<usize, DataNotSupported> (slowly sneaking thiserror-based types into Humility as I updated things).

jamesmunns · 2026-04-21T14:29:14Z

        timeout: u32,
    ) -> Result<HiffyContext<'a>> {
-        if !core.is_net() {
+        let hiffy = if core.is_net() {


nit: Maybe worth breaking out into a helper function to simplify the control flow/reduce rightward drift (mostly to allow early returns)?

jamesmunns · 2026-04-21T14:43:50Z

 }
+
+/// Abstraction for writing hiffy values over multiple transports
+enum HiffyWrite<'a, 'b> {


Nit: It might be worth making each of the variants a specific struct, e.g. Debugger(DebuggerWrite), to be able to implement methods on each subtype. That would make the top-level dispatch functions a little smaller.

mkeeter · 2026-04-22T18:08:13Z

@jamesmunns NetHiffy is going to be the last backend for the forseeable future, so I'm going to punt on more substantial refactorings (like making enum variants into their own types). We can always revisit if someone comes up with a new backend, but I'm not sure what that would be!

mkeeter requested review from cbiffle, Copilot, hawkw and labbott April 14, 2026 14:32

mkeeter mentioned this pull request Apr 14, 2026

Make hiffy callable over the network oxidecomputer/hubris#2466

Merged

Copilot started reviewing on behalf of mkeeter April 14, 2026 14:32 View session

This comment was marked as resolved.

Sign in to view

mkeeter force-pushed the mkeeter/hiffy-net branch 2 times, most recently from ef6dfe1 to f69ae67 Compare April 15, 2026 20:15

lzrd reviewed Apr 16, 2026

View reviewed changes

labbott reviewed Apr 20, 2026

View reviewed changes

mkeeter force-pushed the mkeeter/hiffy-net branch from b524b06 to 51eae64 Compare April 20, 2026 19:26

jamesmunns approved these changes Apr 21, 2026

View reviewed changes

mkeeter force-pushed the mkeeter/hiffy-net branch 4 times, most recently from a3023ab to 1262796 Compare April 21, 2026 20:36

mkeeter force-pushed the mkeeter/hiffy-net branch 3 times, most recently from 24b66e4 to a6f62df Compare April 23, 2026 17:08

mkeeter added 6 commits April 24, 2026 10:08

hiffy net plumbing

068eaf6

Remove suspicious unwrap

bf77dec

Review notes

998f3e1

Correctly apply timeout to net core

4eb6436

Make data_size fallible

8ec8d0e

Fix variant name

372ab5e

mkeeter force-pushed the mkeeter/hiffy-net branch from a6f62df to 372ab5e Compare April 24, 2026 14:10

Get scratch size without reading it

0d0069d

mkeeter force-pushed the mkeeter/hiffy-net branch from f5c0550 to 0d0069d Compare April 24, 2026 14:38

mkeeter mentioned this pull request Apr 24, 2026

humility hiffy should get HIFFY_SCRATCH size without reading it #624

Open

mkeeter merged commit cd161f6 into master Apr 24, 2026
18 of 19 checks passed

mkeeter deleted the mkeeter/hiffy-net branch April 24, 2026 17:37

Conversation

mkeeter commented Apr 14, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

lzrd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

labbott left a comment

Choose a reason for hiding this comment

Uh oh!

jamesmunns left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkeeter commented Apr 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants