[multicast] viona CTRL_RX/CTRL_MAC receive filtering + softnpu egress lock scoping + dep updates by zeeshanlakhani · Pull Request #1093 · oxidecomputer/propolis

zeeshanlakhani · 2026-03-30T01:41:28Z

viona: CTRL_RX and CTRL_MAC table receive filtering

This work extends the minimal VIRTIO_NET_F_CTRL_RX support from #1125 to cover CTRL_RX promiscuity/all-multicast controls and CTRL_MAC tableset handling. Propolis records the guest-supplied unicast and multicast MAC tables and installs the multicast table on the in-kernel device via the new VNA_IOC_SET_MAC_FILTERS ioctl. The unicast table is used to decide when the device must fall back to full promiscuity, since the kernel MAC client already carries the primary unicast address and does not install additional unicast filters.

Filter state is cleared on device reset and carried across live migration via VionaStateV1. The payload is exported unconditionally, matching the import behavior on master since #1125, which takes the payload without consulting negotiated features. Import still tolerates a missing payload from sources predating VionaStateV1. The device narrows to classified delivery once the driver's first MAC_TABLE_SET since reset is accepted (empty or not). Until then, the all-multicast lower bound holds, because some drivers, including illumos vioif, negotiate CTRL_RX but never actually manage a filter table. That marker is carried across migration as an optional multicast_table_managed field, where older sources omit it (the target infers it from non-empty tables) and older targets ignore it. Table installation is tracked independently of promiscuity, so guest promiscuity toggles leave installed classified flows in place rather than unplumbing and replumbing them accordingly.

On falcon links attached to softnpu, promiscuity is pinned at construction rather than derived from guest filter state. A host without ALL_VLAN support pins to full promiscuity, because classified unicast delivery would drop transit frames for the emulated fabric. Other falcon VMs are no longer forced promiscuous.

viona: guest MAC replacement via CTRL_MAC_ADDR

This adds VIRTIO_NET_F_CTRL_MAC_ADDR support, letting a driver replace the device's active unicast MAC address via VIRTIO_NET_CTRL_MAC_ADDR_SET. Propolis installs the replacement on the in-kernel device through the new VNA_IOC_SET_MAC_ADDR ioctl (viona interface version 7) and reflects it in the device configuration space the driver reads the MAC from. The kernel removes the current address and, with it, the installed multicast filters, before installing the replacement. Therefore, the device raises full promiscuity (or the falcon superset pin) to cover delivery across the swap, and then reconciles promiscuity and the filter table with the resulting state.

The override is carried across live migration in VionaStateV1 as an optional mac_addr field, and device reset swaps the nominal address back in.

Advertising the feature requires an explicit allow_guest_mac_change policy grant (default off), since viona performs the swap as directed and leaves validating the requested address against host policy to its consumer. propolis-server keeps the grant off until Nexus supplies an allowed-MAC policy, while propolis-standalone accepts it as a device option. A destination without the grant rejects migration imports that carry the negotiated bit or an installed override.

softnpu: release pipeline lock before egress I/O

Every port worker and the guest Tx path serialize on the pipeline mutex. Previously, the guard was held across both the dlpi and virtio egress, meaning that one worker blocked in egress I/O stalled the others. This is especially costly for multicast, where a single input frame fans out to several ports for replication.

This change scopes the lock to pipeline evaluation (only). The egress packets borrow from the input frame rather than the pipeline, so they remain valid after the guard is dropped. Lock acquisition moves from the callers into process_external_packet and process_guest_packet, which now take the mutex directly.

deps and callouts

softnpu is pinned to its zl/multicast branch pending upstream.
oxide-tokio-rt is bumped to match omicron.
Kernel-side filter installation and guest MAC replacement use viona interface version 7 support as per illumos-gate change 775 (stlouis#986, now merged). Without it, the device remains at its all-multicast fallback.

This wires up the softnpu ASIC backend to support multicast end-to-end by translating DPD's sidecar.p4 table operations into sidecar-lite.p4's simplified P4 pipeline. ## AsicMulticastOps We replace the stubbed AsicMulticastOps implementation (which returned "OperationUnsupported" for group creation and port addition with in-memory group tracking via McGroupData, following the tofino_stub pattern. Group membership is used by the table translation layer to build port bitmaps for sidecar-lite's Replicate extern. Ports >= 128 are rejected at add time to match sidecar-lite's 128-bit bitmap width. ## Table translation (asic/src/softnpu/table.rs) We map sidecar.p4 table names to sidecar-lite equivalents and translate action parameters where the designs differ for emulation. All multicast action arms are gated with #[cfg(feature = "multicast")]. ## References - [softnpu #183](oxidecomputer/softnpu#183) - [propolis #1093](oxidecomputer/propolis#1093) - [p4rs #240](oxidecomputer/p4#240) - [sidecar-lite #152](oxidecomputer/sidecar-lite#152) - tokio: 1.50 (due to softnpu) - oxide-tokio-rt: 0.1.3 (following-up from tokio's move to 1.50)

This wires up the softnpu ASIC backend to support multicast end-to-end by translating DPD's sidecar.p4 table operations into sidecar-lite.p4's simplified P4 pipeline. ## AsicMulticastOps We replace the stubbed AsicMulticastOps implementation (which returned "OperationUnsupported" for group creation and port addition with in-memory group tracking via McGroupData, following the tofino_stub pattern. Group membership is used by the table translation layer to build port bitmaps for sidecar-lite's Replicate extern. Ports >= 128 are rejected at add time to match sidecar-lite's 128-bit bitmap width. ## Table translation (asic/src/softnpu/table.rs) We map sidecar.p4 table names to sidecar-lite equivalents and translate action parameters where the designs differ for emulation. ## Fix update semantics We fix an issue with stale in-memory sources after update with absent source list(s). `modify_group_external` wrote back `new.sources.or(existing)` to the in-memory record while `update_external_tables` in the same call canonicalized `new.sources` directly to rewrite the P4 source-filter entries. A `None` update therefore cleared the P4 table but left a stale `Some([...])` in memory. All multicast action arms are gated with #[cfg(feature = "multicast")]. ## References and Deps - [softnpu #183](oxidecomputer/softnpu#183) - [propolis #1093](oxidecomputer/propolis#1093) - [p4rs #240](oxidecomputer/p4#240) - [sidecar-lite #152](oxidecomputer/sidecar-lite#152) - tokio: 1.50 (due to softnpu) - oxide-tokio-rt: 0.1.3 (following-up from tokio's move to 1.50) - bump oxide-tokio-rt to 0.1.4, tokio to 1.52.1 (matching Omicron)

Pin softnpu and p4rs to their zl/multicast branches for multicast support in the emulated ASIC, and bump oxide-tokio-rt to 0.1.4.

This work extends the minimal `VIRTIO_NET_F_CTRL_RX` support from [1125](#1125) to the full control-queue receive filtering surface, including promiscuity levels and guest-supplied unicast/multicast MAC tables being applied to the in-kernel device via the new [VNA_IOC_SET_MAC_FILTERS](https://code.oxide.computer/c/illumos-gate/+/775) ioctl. Filter state is cleared on device reset and carried across live migration via `VionaStateV1`. The payload is exported unconditionally, matching the import behavior on master since #1125, which takes the payload without consulting negotiated features. Import still tolerates a missing payload from sources predating VionaStateV1. The device narrows to classified delivery only once the guest installs a non-empty multicast table. An empty table holds the device at its all-multicast default, where some drivers (illumos vioif among them) negotiate `CTRL_RX` but never manage a filter table, and narrowing on an empty table would silently drop their multicast. The virtio spec permits the resulting unrequested traffic, but not the loss. On falcon links, a host without ALL_VLAN support now falls back to full promiscuity rather than all-multicast, since classified unicast delivery would drop transit frames for the emulated fabric. When viona interface version 7 (kernel-side MAC filter table support in st.louis) is available as per https://code.oxide.computer/c/illumos-gate/+/775 (in review), then this is fully lit up.

Every port worker and the guest Tx path serialize on the pipeline mutex. Previously, the guard was held across both the dlpi and virtio egress, meaning that one worker blocked in egress I/O stalled the others. This is especially costly for multicast, where a single input frame fans out to several ports for replication. This change makes the lock scoped to pipeline evaluation only. The egress packets borrow from the input frame rather than the pipeline, so they remain valid after the guard is dropped. Lock acquisition moves from the callers into `process_external_packet` and `process_guest_packet`, which now take the mutex directly. The latter function returns a bool so the guest read loop can stop early when no P4 program is loaded or the lock is poisoned, preserving the prior break semantics.

FelixMcFelix

Thanks Zeeshan; hopefully these comments makes sense, and thanks for the extra tests around the actual handling of control queue messages. I am partly looking at this from the angle of minimising the amount of things we need to do in viona/MAC, and from keeping as many (guest ⨉ test environment) pairs out of promiscuous mode as is doable.

Here are the follow-ups from review of the CTRL_RX/CTRL_MAC receive filtering work: - Withhold classified delivery until the first accepted MAC_TABLE_SET since reset. illumos vioif negotiates CTRL_RX but never programs the multicast table, so the all-multicast lower bound holds until the driver demonstrates that it can manage its own filtering. - We now carry that marker across migration as an optional payload field (multicast_table_managed). Older sources omit it and the target infers from non-empty tables, while older targets ignore it. - Track table installation independently of promiscuity (mac_filters_dirty), so guest promiscuity toggles no longer unplumb and replumb the MAC client's classified flows. Failed installs and clearings stay dirty and are reissued on a later application. - We scope the falcon promiscuity pin to softnpu-attached links rather than forcing every falcon VM promiscuous. - Return `Result<(), NoP4Program>` from softnpu's `process_guest_packet` instead of just a bool. - Write MAC filter entries directly into `vmf_mcast` rather than collecting them into an intermediate vector. - Bump HEADER_CHECK_REF to gerrit patchset 4.

We now advertise `VIRTIO_NET_F_CTRL_MAC_ADDR` when the kernel offers the V7 MAC filtering ioctls, and implement `VIRTIO_NET_CTRL_MAC_ADDR_SET` by swapping the active unicast address through `VNA_IOC_SET_MAC_ADDR`. Driver writes to the config-space `mac` field remain ignored because only the control-queue command is honored. The kernel removes the current address (and the installed multicast filters) before installing the replacement, so that the swap runs under full promiscuity (or the falcon pin), with `apply_rx_config` reconciling the level and the filter table afterward. Failure outcomes are decoded from the semantic codes copied out through `vioc_mac_addr`: - a partial table restore (`VMA_ERR_MCAST_RESTORE`) that keeps the new address and reissues the table. - a failed swap with the previous address still installed, which fails the command and reissues the table (the kernel may shed entries while restoring it). - a client left with no unicast address, which demands a device reset. DTrace probes report the copied-out error state of failed filter and swap ioctls. The override installed by the guest driver feeds config-space MAC reads and unicast filter classification, yields to the nominal address on device reset, and is carried in the migration payload. Targets install it before reapplying the Rx configuration and refuse the import without kernel support. This work also includes: - mirroring the [775, patchset-6](https://code.oxide.computer/c/illumos-gate/+/775) `vioc_mac_filters` layout (out-of-band table through `vmf_addrs`) in viona-api. - passing a null pointer for an empty table. - bumping the header-check ref to patchset 6. - tracing the new ioctls in scripts/viona.d. - testing replacement, reset, and migration of the override against a live V7 kernel, asserting the kernel-visible address and filter table through test-only GET ioctls.

FelixMcFelix

Thanks Zeeshan. I think I'm generally happy with the actual plumbing of the filter tables, as well as the logic to set/unset various promisc levels in response to the error conditions aside from maybe one or two points of confusion. It looks good on the functionality front, though I haven't yet tested it myself and made sure that everything looks right in mdb for various guests yet.

I'd need to defer to @iximeow or @papertigers on the VirtioDevice changes, as well as the accuracy of the new tests which are actually driving the rings via send_ctrl_cmd.

FelixMcFelix · 2026-08-31T20:05:11Z

+            // An empty table is never installed. Classified delivery with
+            // no table covers the device MAC and broadcast, which is
+            // exactly what an empty table requests.


Guests do often start with an empty table from what I recall, then add the pile of all-routers, etc., related MACs. These tables are assumed to refer to MACs in addition to the config space MAC and broadcast. I think it's just a wording thing for this comment, more than anything?

FelixMcFelix · 2026-08-31T21:10:58Z

+        // SET_PAIRS can fail while restoring the kernel's receive callbacks,
+        // and SET_USEPAIRS can fail independently. Attempt both restorations,
+        // and on failure surface NEEDS_RESET to the guest rather than
+        // panicking.
+        let use_pairs = self.set_use_pairs(1);
+        let pairs = self.hdl.set_pairs(1);
+        if use_pairs.is_err() || pairs.is_err() {
+            self.virtio_state.set_needs_reset(self);
+        }


If we're at this point we are resetting the NIC, so I'm unsure that signalling needs_reset will be of much use. What exactly is the failure condition for these ioctls, now?

Leaving aside the addition/split of VirtioDevice::device_reset for now, why are we resetting the queuepair counts in a separate spot from the rest of the device logic? #1093 (comment) makes it seem as though this logic should live there too, if the guest forces a reset.

Both points make sense.

On the failure conditions: VNA_IOC_SET_USEPAIRS(1) reaches viona_ioc_link_usepairs. The request is within the valid range, so the validation failure paths do not apply here. VNA_IOC_SET_PAIRS(1) reaches viona_ioc_link_setpairs, which can fail if the requested count is below l_usepairs, if a ring is not in VRS_RESET, or if viona_rx_set fails while restoring receive callbacks.

The first two conditions are not reachable here: we lower l_usepairs first, and virtio_reset has already reset every ring before invoking device_reset. The viona_rx_set failure is recoverable, however, as it drops the active mode to VIONA_PROMISC_NONE while retaining the request in l_promisc, so the promiscuity restore below retries callback installation.

This makes needs_reset both unnecessary and misleading honestly, so it is now gone. The promiscuity restore in device_reset is the retry for that callback-restoration failure.

On placement, yeah..., it is a bug rather than an inconsistency. A guest writing zero to the status register goes through set_status to virtio_reset and then device_reset, but it does not go through Lifecycle::reset. A driver that negotiated multiqueue and then unloaded therefore left the kernel holding its old pair count.

Resetting the pair count in device_reset fixes that path. The split itself was needed for resetting the VIRTIO_NET_F_CTRL_RX state. Restoring the pair count is an additional benefit of having the device-level reset callback.

Refs:

https://github.com/oxidecomputer/illumos-gate/blob/e73f2575430340bc2ebdfcb0b5d49ef8fa72d816/usr/src/uts/intel/io/viona/viona_main.c#L1403-L1423

https://github.com/oxidecomputer/illumos-gate/blob/e73f2575430340bc2ebdfcb0b5d49ef8fa72d816/usr/src/uts/intel/io/viona/viona_main.c#L1426-L1432

https://github.com/oxidecomputer/illumos-gate/blob/e73f2575430340bc2ebdfcb0b5d49ef8fa72d816/usr/src/uts/intel/io/viona/viona_main.c#L1034-L1066.

FelixMcFelix · 2026-08-31T21:15:27Z

+        /// Whether the source had accepted a `MAC_TABLE_SET` since its last
+        /// reset.
+        ///
+        /// This is optional for compatibility, as older sources omit it and
+        /// older destinations ignore it.
+        #[serde(default)]
+        pub multicast_table_managed: Option<bool>,
+        /// The unicast MAC address installed by the driver in place of the
+        /// nominal one the device was created with, if any
+        /// (`VIRTIO_NET_CTRL_MAC_ADDR_SET`).
+        ///
+        /// This is optional for compatibility, as older sources omit it.
+        ///
+        /// An older destination ignores it and reverts config space to the
+        /// nominal address, while the guest continues using the replaced
+        /// one, which that host will not classify.
+        #[serde(default)]
+        pub mac_addr: Option<MacAddr>,


My feeling is that this might have to be a V2 payload, because if we do migrate from a propolis with this capability (and which allows guest-driven MAC address config) to an older version, the guest will be left in a very strange state where it believes the host knows about its brand new MAC address but the kernel and PCI cfg-space are not thusly configured.

~~removed~~

No Viona migration payload has shipped yet, and this PR introduces the first schema.

Sorry, what do you mean by this? I think migrate::VionaStateV1 has been in the product since R21.

@FelixMcFelix derp, yeah, I'm a dummy, it's just the initial one. I'll introduce a second-versioned payload with the filters, etc, and update mac_addr there to not be the option.

The mcast prototype tree points to the mcast-smbios-test branch (https://github.com/oxidecomputer/propolis/tree/mcast-smbios-test) for the multicast-related pin.

FelixMcFelix · 2026-09-03T10:06:24Z

+        /// Whether the source had accepted a `MAC_TABLE_SET` since its last
+        /// reset.
+        ///
+        /// This is optional for compatibility, as older sources omit it and
+        /// older destinations ignore it.
+        #[serde(default)]
+        pub multicast_table_managed: Option<bool>,
+        /// The unicast MAC address installed by the driver in place of the
+        /// nominal one the device was created with, if any
+        /// (`VIRTIO_NET_CTRL_MAC_ADDR_SET`).
+        ///
+        /// This is optional for compatibility, as older sources omit it.
+        ///
+        /// An older destination ignores it and reverts config space to the
+        /// nominal address, while the guest continues using the replaced
+        /// one, which that host will not classify.
+        #[serde(default)]
+        pub mac_addr: Option<MacAddr>,


No Viona migration payload has shipped yet, and this PR introduces the first schema.

Sorry, what do you mean by this? I think migrate::VionaStateV1 has been in the product since R21.

FelixMcFelix · 2026-09-03T11:09:53Z

                0
            } else {
-                addrs.as_bytes().as_ptr() as u64
+                u64::try_from(addrs.as_bytes().as_ptr().addr())


I don't think you need the .as_bytes() here.

FelixMcFelix · 2026-09-03T11:10:17Z

        let mut vmf = viona_api::vioc_mac_filters {
            vmf_nmcast: addrs.len() as u32,
-            vmf_addrs: addrs.as_mut_bytes().as_mut_ptr() as u64,
+            vmf_addrs: u64::try_from(addrs.as_mut_bytes().as_mut_ptr().addr())


Ditto on not needing .as_mut_bytes().

FelixMcFelix · 2026-09-03T11:26:54Z

+        let _ = self.set_use_pairs(1);
+        let _ = self.hdl.set_pairs(1);


From looking at stlouis again, my read is that we can keep an unwrap or expect on the set_use_pairs call because we know that the input is well-founded. I think the call to set_pairs could do with at least a probe rather than discarding the value.

Gotcha, yeah. ok.

this should either expect or maybe set NEEDS_RESET, yeah. if we're failing to set the device back to one queue pair we're operating viona wrong and that's a Propolis bug.

iximeow

I've really only looked at most of the viona/virtio-net changes so far, but I have a few other questions about the change at large, please bear with me..

how does this relate to multicast? the MAC filtering is "just" more comprehensive support for virtio features which guests could use regardless of multicastness, and iiuc aren't necessary for a guest to work with multicast traffic?

the dependency updates are totally incidental, right? we don't need to bump oxide-tokio-rt here for any particular reason, nor p4 or softnpu? if yes, since that's trivial to piece out, lets just do that?

likewise, the softnpu egress stuff is .. a performance-related change for softnpu right? so that is both a really clear distinct change we could make and review in its own right, and not necessary for the MAC filtering bits, nor related to dependency updates?

my understanding is that this was a draft while you've been working on multiqueue generally, and so this is an accumulation of Propolis changes along the way. that's fine of course, but since the boundaries between the changes are really clear, lets not tie all it together?

part of why I'm asking is, after the conversation yesterday about VMM/viona interface version numbers, it has become clear to me that it is just outright wrong for a Propolis to have a different "current" interface version than the kernel it's running on. I'm actually thinking about putting a patch together that bumps the viona interface version to 7 to match stlouis, and masks CTRL_RX so that guests do not think the feature is available until we've landed the corresponding changes here. as-is, I actually do not understand why a TUF would be able to be installed and start a Propolis, because stlouis has already been updated and there is simply no corresponding Propolis that could safely run there; if racklettes do work on current TUFs, they should not. not to mention Propolis CI being broken, but that's less concerning to me than "the product works when it seems like it should not" :)

iximeow · 2026-09-01T18:03:54Z

+    /// Replace the device's active unicast MAC address on behalf of a
+    /// driver's `VIRTIO_NET_CTRL_MAC_ADDR_SET` command.
+    ///
+    /// The comments below distinguish the nominal address, the
+    /// [`PciVirtioViona::mac_addr`] the device was created with, from a
+    /// driver-installed override.
+    fn set_mac_override(&self, mac: MacAddr) -> Result<(), ()> {


fwiw unless we think there's a reason we might want to allow guests to change their NIC's MAC in the future, I don't think there's a reason we should even theoretically plumb the command to viona, even if viona supports it? that is, why not implement this as return Err(()) and be done with it?

if there's a way this is helpful then I don't mean to stand in the way, but if this is plumbed up only because the spec says the command has to be accepted with CTRL_RX being negotiated, this seems like all risk (because if you end up able to set your MAC to another guest's NIC...), plus implementation to support, for little gain?

This isn't something we need. But I think the feature negotiation wrt. this command specifically is described a bit strangely. We do have this feature flag defined:

VIRTIO_NET_F_CTRL_MAC_ADDR(23) Set MAC address through control channel.

My interpretation then is that:

VIRTIO_NET_CTRL_MAC_TABLE_SET requires VIRTIO_NET_F_CTRL_{VQ | RX}

VIRTIO_NET_CTRL_MAC_ADDR_SET requires VIRTIO_NET_F_CTRL_{VQ | RX | MAC_ADDR}

So then we can offer VIRTIO_NET_F_CTRL_RX without accepting MAC_ADDR_SET.

The Linux virtio_net.c imposes the same requirement, as far as I can see. [1, 2] As does FreeBSD, after converting to VTNET_FLAG_CTRL_MAC [3].

But the spec text (at least as of v1.4) only mentions the flag in a roundabout way, through the above description in §5.1.3 and then only in the 'Legacy Interface' subsections to say that older devices may assume they can just do a write to the config-space MAC rather than negotiating this feature. ...I wonder if they would take a PR/patch to clarify that?

iximeow · 2026-09-01T18:13:08Z

+    /// Whether the viona device supports the MAC filtering ioctls of
+    /// [`viona_api::ApiVersion::V7`]: `VNA_IOC_SET_MAC_FILTERS` and
+    /// `VNA_IOC_SET_MAC_ADDR`.
+    kernel_mac_filters: bool,


when VIONA_CURRENT_INTERFACE_VERSION is 7 this is true, and by making changing the API version expected in check_api_version() to V7 in this file you can cause Propolis to fail to start if the host bits are too old. lets do that here and not worry about feature-detecting with kernel_mac_filters and all that.

iximeow · 2026-09-01T18:35:59Z

+                probes::virtio_viona_promisc_err!(|| (
+                    usize::from(level) as u8,
+                    usize::from(state.promisc) as u8,
+                    e.raw_os_error().unwrap_or(0),


it occurs to me that we probably want an error variant out of ioctls that guarantees it can offer the OS error from the ioctl (if any). it would look something like this anyway but it at least is a way for us to know that 0 really means that the instance is gone rather than the ioctl returned 0. not a change for this PR, I'll do a separate issue about it.

iximeow · 2026-09-02T19:18:07Z

+    ///
+    /// This requires [viona_api::ApiVersion::V7] or greater.
+    fn set_mac_filters(&self, multicast: &[MacAddr]) -> io::Result<()> {
+        assert!(multicast.len() <= viona_api::VIONA_MAX_MCAST_FILTERS);


are we obligated to limit the number of multicast MACs before issuing the SET_MAC_FILTERS ioctl? this assert seems weird: presumably the OS can tolerate too too many multicast MACs by returning a normal error, which would be an error either of the viona user (Propolis, in the current implementation), or an error for the guest driver to handle ("setting the MAC filters failed", if that's an idea that guests can handle).

in either case it seems inappropriate to assert here unless the device operation would be incorrect if we proceeded?

iximeow · 2026-09-02T19:48:23Z

+            // A count of zero clears the table without the kernel reading
+            // the buffer. We pass no pointer rather than a dangling one.
+            vmf_addrs: if addrs.is_empty() {
+                0
+            } else {
+                u64::try_from(addrs.as_bytes().as_ptr().addr())
+                    .expect("usize fits in u64 on 64-bit targets")
+            },


nit: I'd mildly prefer if we could figure out addrs.as_bytes().as_ptr().addr() else std::ptr::null() above vmf, and just vmf_addrs: u64::try_from(that_ptr).expect("..etc...") here. just because that separates out "we're converting the data to a pointer" and "we're converting the pointer to the ABI-correct type", and the first part is not totally trivial like addrs.len() is above

iximeow · 2026-09-03T18:29:47Z

+            // Give each ring a page of its own by stepping a full page per
+            // ring.
+            //
+            // Aligning with `next_multiple_of` would not work here:
+            // `next_queue_gpa` is already page-aligned, so it would be a
+            // noop and leave the descriptor table, available ring, and used
+            // ring all at the same address.
+            let avail_gpa = descriptor_table_gpa + page_u64;


I suppose you can see that the tests i'd written did not usefully use the queues beyond laying them out, lol. I had used next_multiple_of because the queue sizes are probably not exactly one page, and I wanted to keep all of these addresses page-aligned to help catch obviously-wrong values when debugging. what was some_gpa.next_multiple_of(page_u64) should really be (some_gpa + len).next_multiple_of(page_u64).

if one of these items is larger than a page, this change will be wrong and make for very confusing test issues.

iximeow · 2026-09-03T18:35:33Z

+        let _ = self.set_use_pairs(1);
+        let _ = self.hdl.set_pairs(1);


this should either expect or maybe set NEEDS_RESET, yeah. if we're failing to set the device back to one queue pair we're operating viona wrong and that's a Propolis bug.

iximeow · 2026-09-03T19:15:09Z

+        // Release the state lock before calling into the device so that its
+        // reset handling may query generic virtio state.
+        drop(state);
+        dev.device_reset();


we should not release the mutex here, or device_reset implementations must be cautious that while they are resetting device state, the corresponding virtio, queues, and PCI state may not be the state at reset anymore. we'd probably want to pass the mutex along here, but this is the kind of complication that has me leaning towards pushing set_features maybe a bit too far (see below)

iximeow · 2026-09-03T19:23:21Z

-    fn reset(&self) {
-        self.virtio_state.reset(self);
+
+    fn device_reset(&self) {


generally, set_features is equivalently "called once per device initialization", and might be a better place to handle filters for now? mainly, FEATURES_OK must be set before enabling queues, so we know that if any has persisted through to set_features, it won't cause guest-visible misbehavior because there won't be queues configured to discover it.

it's a bit funky to overload set_features like that, but I think keeping ourselves to reasoning about the local "viona set_features() + virtio device reset" helps avoid the complication of who can reset what state with what guarantees we'd have with device_reset() (though in general it's not a bad idea, and if we had other kinds of reset it's probably worth thinking through)

concretely, I think we already have a bug that if you initialize a device, negotiate F_CTRL_RX, set a promiscuity, reset, and did not negotiate the feature a second time, you end up with viona still in a promiscuous mode that you'd never actually asked for. that's unfortunate!

it seems to me that we should always set_promisc(PromiscLevel::None), and clear filters, and ... etc to clear all configurable state, rather than doing it piecemeal based on what we think the negotiated features were at the last time we reset. here, if we kept MAC overrides, that simplifies things a lot: just set PromiscLevel::None and be done with it. likewise, self.hdl.set_mac_filters(&[]) always when we're going through here, maybe a guest negotiates the feature to change it again in the future, but that's the end of it and we never have to ask if there were filters before.

it probably does still make sense to parcel out the state resetting into a standalone function, then you "only" have the weirdness of describing why it would happen in set_features rather that something that's a first-class part of a virtio device lifecycle. and later if/when lifecycle bits for it are more thought through we'll just move that call to reset out from the start of set_features and to wherever someone poked that bit into the status register.

that's more work on the reset path that might not be strictly necessary, but if someone is bound on our ability to reset the virtio-net device, uhhhh.... what is happening :) otoh it's all really simple to think through and extend in the future.

zeeshanlakhani · 2026-09-04T00:31:35Z

@iximeow Admittedly, over the last few weeks, I've been changing this code a lot just to get multicast to work in voxel, so it has been super prototype-y, but I've never written legit propolis code before (minus stats). But, this came about in how were we using promisicuous mode and delivering multicast packets for link-local (previously), which was the inherent issue for the duplication.

To your points, maybe we don't need the guest-MAC override ability presented here to get the work through. And, as you mentioned I conflated the dependency and locking issues, the former to match Omicron, to make the end-to-end demo work.

As I've been doing in dendrite and other repos related to mcast changes, I'm going to clsoe this PR and split out the work in smaller chunks, given the questions and discussion pieces you've mentioned. Though, it was still good for a first look to know what to split out and do (and test for).

sion42x · 2026-09-04T00:39:06Z

@iximeow I know this PR is closed now, but this comment concerns me:

as-is, I actually do not understand why a TUF would be able to be installed and start a Propolis, because stlouis has already been updated and there is simply no corresponding Propolis that could safely run there; if racklettes do work on current TUFs, they should not. not to mention Propolis CI being broken, but that's less concerning to me than "the product works when it seems like it should not"

Can you elaborate?

iximeow · 2026-09-04T01:08:50Z

@sion42x I since figured out why things work, and it's not really.. great, but at least explains why things work.

check_api_version() in viona.rs checks that the current viona API version is at least whatever it is that we consider the minimum, and will silently be OK with the host's viona version being newer than what we support. that's a different interpretation of the version number than the kernel and c-byhve have, where they simply must just match. the kernel's interpretation is because the kernel does things like offer virtio-net features that we pass through to the guest, so a different viona version can tell a guest that it should try doing things that Propolis does not support.

the lifecycle of all this is very similar for VMM_CURRENT_INTERFACE_VERSION but checked in slightly different places for the different API.

we should at least warn and probably in an omicron-build build of Propolis fail to run if the kernel's versions are not exactly equal to the ones Propolis expects to be operating on.

zeeshanlakhani mentioned this pull request Mar 30, 2026

[multicast] wire up softnpu backend to multicast table ops oxidecomputer/dendrite#242

Merged

zeeshanlakhani self-assigned this Apr 17, 2026

zeeshanlakhani force-pushed the zl/multicast branch 3 times, most recently from bad9d65 to 784de5e Compare April 18, 2026 03:36

zeeshanlakhani mentioned this pull request Jul 3, 2026

[multicast] default the multicast feature and wire SoftNPU end-to-end oxidecomputer/dendrite#224

Draft

zeeshanlakhani force-pushed the zl/multicast branch from 75518ad to 0de0c5b Compare July 11, 2026 08:07

zeeshanlakhani changed the title ~~[multicast] dep updates~~ [multicast] viona CTRL_RX/CTRL_MAC receive filtering + softnpu egress lock scoping + dep updates Jul 11, 2026

zeeshanlakhani force-pushed the zl/multicast branch from 0de0c5b to a9779dc Compare July 12, 2026 15:51

zeeshanlakhani requested a review from FelixMcFelix July 12, 2026 15:51

zeeshanlakhani marked this pull request as ready for review July 12, 2026 15:51

zeeshanlakhani force-pushed the zl/multicast branch from a9779dc to 95c195a Compare July 12, 2026 16:26

zeeshanlakhani added 3 commits July 12, 2026 18:07

[multicast] dep updates

b0c16f1

Pin softnpu and p4rs to their zl/multicast branches for multicast support in the emulated ASIC, and bump oxide-tokio-rt to 0.1.4.

zeeshanlakhani force-pushed the zl/multicast branch from 95c195a to cd0e261 Compare July 12, 2026 18:23

zeeshanlakhani mentioned this pull request Jul 16, 2026

Multicast e2e oxidecomputer/omicron#9912

Closed

FelixMcFelix reviewed Jul 20, 2026

View reviewed changes

zeeshanlakhani requested a review from FelixMcFelix July 22, 2026 02:21

zeeshanlakhani added 3 commits July 29, 2026 15:51

[update] adapt to 775 patchset 7

cc0b57f

merge master

df5fd0a

zeeshanlakhani force-pushed the zl/multicast branch 2 times, most recently from 1fb9a90 to 6885fe0 Compare August 6, 2026 06:21

merge master

e691d68

zeeshanlakhani force-pushed the zl/multicast branch from 6885fe0 to 2fb6852 Compare August 19, 2026 04:14

zeeshanlakhani force-pushed the zl/multicast branch from 2fb6852 to 603194b Compare August 19, 2026 22:12

[viona] adapt to stlouis#986 (775, merged)

3c07d60

zeeshanlakhani force-pushed the zl/multicast branch from 603194b to 3c07d60 Compare August 19, 2026 22:33

zeeshanlakhani mentioned this pull request Aug 25, 2026

[multicast] Host-side plumbing for externally sourced multicast oxidecomputer/voxel#28

Open

zeeshanlakhani requested review from rcgoodfellow and sion42x August 27, 2026 18:40

rcgoodfellow mentioned this pull request Aug 28, 2026

finagle a STREAMS rx buffer size for DLPI handles #1204

Merged

zeeshanlakhani requested a review from papertigers August 31, 2026 14:03

iximeow mentioned this pull request Aug 31, 2026

don't header-check the bhyve or viona API version #1208

Closed

FelixMcFelix reviewed Aug 31, 2026

View reviewed changes

zeeshanlakhani force-pushed the zl/multicast branch from eb8271c to 3e32624 Compare September 2, 2026 04:04

zeeshanlakhani added 2 commits September 2, 2026 04:17

[review] address Kyle's comments/suggestions

e10c1f9

[deps] point softnpu back at main

4fb9bd5

The mcast prototype tree points to the mcast-smbios-test branch (https://github.com/oxidecomputer/propolis/tree/mcast-smbios-test) for the multicast-related pin.

zeeshanlakhani force-pushed the zl/multicast branch from 3e32624 to e10c1f9 Compare September 2, 2026 04:29

zeeshanlakhani requested a review from FelixMcFelix September 3, 2026 09:22

FelixMcFelix reviewed Sep 3, 2026

View reviewed changes

iximeow requested changes Sep 3, 2026

View reviewed changes

zeeshanlakhani closed this Sep 4, 2026

		let _ = self.set_use_pairs(1);
		let _ = self.hdl.set_pairs(1);

Uh oh!

Conversation

zeeshanlakhani commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

viona: CTRL_RX and CTRL_MAC table receive filtering

viona: guest MAC replacement via CTRL_MAC_ADDR

softnpu: release pipeline lock before egress I/O

deps and callouts

Uh oh!

FelixMcFelix left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FelixMcFelix left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeeshanlakhani Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeeshanlakhani Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

iximeow 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

zeeshanlakhani commented Mar 30, 2026 •

edited

Loading

zeeshanlakhani Sep 1, 2026 •

edited

Loading

zeeshanlakhani Sep 1, 2026 •

edited

Loading