Skip to content

Use new NullStr based SpComponent#2545

Merged
jamesmunns merged 2 commits into
masterfrom
james/embrace-nullstr
Jun 3, 2026
Merged

Use new NullStr based SpComponent#2545
jamesmunns merged 2 commits into
masterfrom
james/embrace-nullstr

Conversation

@jamesmunns
Copy link
Copy Markdown
Contributor

@jamesmunns jamesmunns commented Jun 1, 2026

Codegen impl changes necessary to support oxidecomputer/management-gateway-service#496. Should wait for that to merge first. This has now merged. This PR bumps the git version used.

@jamesmunns jamesmunns requested a review from hawkw June 1, 2026 16:10
@jamesmunns jamesmunns force-pushed the james/embrace-nullstr branch 2 times, most recently from 568ce08 to f69dacd Compare June 1, 2026 17:29
@jamesmunns jamesmunns force-pushed the james/embrace-nullstr branch from f69dacd to 4c7b835 Compare June 2, 2026 14:12
@jamesmunns
Copy link
Copy Markdown
Contributor Author

(no more force pushes, promise)

@jamesmunns jamesmunns marked this pull request as ready for review June 2, 2026 14:12
@jamesmunns jamesmunns changed the title WIP: Use new NullStr based SpComponent Use new NullStr based SpComponent Jun 2, 2026
@jamesmunns
Copy link
Copy Markdown
Contributor Author

Note to self, to test this, I should probably:

@jamesmunns
Copy link
Copy Markdown
Contributor Author

This has now been tested against a local grapefruit board.

read-component-caboose was used to exercise mgs-to-sp, inventory was used to exercise sp-to-mgs, output of both tests was ~identical:

james@fool:~/oxide/management-gateway-service/faux-mgs git:(main) cargo run --release -- --interface en7 read-component-caboose --component sp --slot=active GITC
    Finished `release` profile [optimized] target(s) in 0.11s
     Running `/Users/james/oxide/management-gateway-service/target/release/faux-mgs --interface en7 read-component-caboose --component sp --slot=active GITC`
Jun 02 19:40:01.974 INFO creating SP handle on interface en7, component: faux-mgs
Jun 02 19:40:01.976 INFO initial discovery complete, addr: [fe80::c1d:98ff:fe5f:c881%22]:11111, interface: en7, socket: control-plane-agent, component: faux-mgs
4c7b83529b034b2e99e754945b5bc4f47e2b749d

james@fool:~/oxide/management-gateway-service/faux-mgs git:(main) cargo run --release -- --interface en7 inventory
    Finished `release` profile [optimized] target(s) in 0.32s
     Running `/Users/james/oxide/management-gateway-service/target/release/faux-mgs --interface en7 inventory`
Jun 02 19:39:56.193 INFO creating SP handle on interface en7, component: faux-mgs
Jun 02 19:39:56.196 INFO initial discovery complete, addr: [fe80::c1d:98ff:fe5f:c881%22]:11111, interface: en7, socket: control-plane-agent, component: faux-mgs
COMPONENT        STATUS       DEVICE           DESCRIPTION (CAPABILITIES)
sp               Present      sp               Service Processor (DeviceCapabilities(1))
sp-aux-flash     Present      sp-aux-flash     Service Processor auxiliary flash (DeviceCapabilities(0))
sp5-host-cpu     Present      sp5-host-cpu     Cosmo SP5 host cpu (DeviceCapabilities(4))
sp5-post-codes   Present      sp5-post-codes   Cosmo SP5 POST code buffer (DeviceCapabilities(0))
host-boot-flash  Present      host-boot-flash  Cosmo host boot flash (DeviceCapabilities(1))
host-boot-apob   Present      host-boot-apob   Cosmo host boot APOB region (DeviceCapabilities(0))
system-led       Present      system-led       System attention LED (DeviceCapabilities(8))

Copy link
Copy Markdown
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Well, this turns out to be a surprisingly small change, looks good to me!

As an aside, it would be interesting to see how much smaller this makes the control-plane-agent task, now that the strings in the generated DeviceDescription array are no longer being null-padded...

Comment thread task/control-plane-agent/src/inventory.rs
Comment thread task/control-plane-agent/src/inventory.rs Outdated
@jamesmunns
Copy link
Copy Markdown
Contributor Author

it would be interesting to see how much smaller this makes the control-plane-agent task, now that the strings in the generated DeviceDescription array are no longer being null-padded

I did realize that it's less of a clear win, because we're actually now building an array of slices instead of an array of arrays, which means the strings themselves are no longer padded, but we now have an extra array of 8 bytes per string, which means we win some when the components are < 8 bytes of text, and lose some when the components are > 8 bytes of text.

We'll spend a bit less time searching/comparing (we only compare used bytes, I would bet the search will also match lens first too), but this is probably in the category of "too small to be meaningful".

We could be more clever, but imo it's probably a tossup.

I do think it'll be a bigger win for pmbus rail names, since those are 32-byte max instead of 16-byte max.

@jamesmunns jamesmunns enabled auto-merge (squash) June 3, 2026 10:21
@jamesmunns jamesmunns merged commit 8f5e0f7 into master Jun 3, 2026
190 checks passed
@jamesmunns jamesmunns deleted the james/embrace-nullstr branch June 3, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants