consomme: Enable port forwarding from openvmm command line by Brian-Perkins · Pull Request #3364 · microsoft/openvmm

Brian-Perkins · 2026-04-23T19:07:30Z

For consomme, forward host ports into the guest with hostfwd=:
--net consomme:hostfwd=tcp::3389-:3389
--net consomme:hostfwd=tcp:127.0.0.1:8080-:80
--net consomme:hostfwd=tcp:[::1]:8080-:80
--net consomme:10.0.0.0/24,hostfwd=tcp::22-:22,hostfwd=udp::5000-:5000

jstarks · 2026-04-23T19:14:39Z

+        ip_addr: Option<IpAddr>,
+        guest_port: u16,
+        host_port: u16,
+    ) -> Result<u16, DropReason> {


Kind of weird to use DropReason here.

Copilot

Pull request overview

This PR extends the Consomme networking backend to support host→guest port forwarding configured from the openvmm command line (--net consomme:hostfwd=...). It wires the new CLI-parsed forwarding rules through resource handles into the Consomme endpoint so ports can be bound when the NIC queue starts.

Changes:

Add hostfwd= parsing for --net consomme:... and plumb forwarding rules into net_backend_resources::consomme::ConsommeHandle.
Extend Consomme endpoint initialization to carry a list of port-forward bindings and attempt to bind them on queue startup.
Update Consomme TCP/UDP binding APIs to accept distinct host/guest ports and introduce a new DropReason::PortAlreadyBound.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
vm/devices/net/net_consomme/src/resolver.rs	Parses forwarded host bind address and converts resource port configs into endpoint port-forward configs.
vm/devices/net/net_consomme/src/lib.rs	Adds `PortForwardConfig`, endpoint constructor supporting port forwards, and binds configured forwards on queue startup.
vm/devices/net/net_consomme/consomme/src/udp.rs	Extends UDP forwarding/binding API to accept guest/host port (and attempts to support forwarded UDP ports).
vm/devices/net/net_consomme/consomme/src/tcp.rs	Extends TCP bind API for host/guest port mapping and returns actual bound host port.
vm/devices/net/net_consomme/consomme/src/lib.rs	Adds `DropReason::PortAlreadyBound` for duplicate port bind attempts.
vm/devices/net/net_backend_resources/src/lib.rs	Adds `HostPortConfig`/`HostPortProtocol` and a `ports` field to `ConsommeHandle`.
petri/src/vm/openvmm/modify.rs	Updates ConsommeHandle construction to include the new `ports` field.
openvmm/openvmm_entry/src/lib.rs	Plumbs CLI `host_fwd` rules into `ConsommeHandle.ports` when building NIC config.
openvmm/openvmm_entry/src/cli_args.rs	Adds CLI syntax/docs + parsing + tests for `consomme:hostfwd=...`.

Copilot · 2026-04-23T19:15:11Z

                let bind_addr: SocketAddr = match guest_addr {
-                    SocketAddr::V4(_) => {
-                        SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, 0))
-                    }
-                    SocketAddr::V6(_) => {
-                        SocketAddr::V6(SocketAddrV6::new(Ipv6Addr::UNSPECIFIED, 0, 0, 0))
-                    }
+                    SocketAddr::V4(_) => SocketAddr::V4(SocketAddrV4::new(
+                        Ipv4Addr::UNSPECIFIED,
+                        host_port.unwrap_or(0),
+                    )),
+                    SocketAddr::V6(_) => SocketAddr::V6(SocketAddrV6::new(
+                        Ipv6Addr::UNSPECIFIED,
+                        host_port.unwrap_or(0),
+                        0,
+                        0,
+                    )),
                };


get_or_insert() binds new UDP sockets to 0.0.0.0/:: unconditionally. For host port forwarding this ignores the requested host_address (e.g. 127.0.0.1 / ::1) and can inadvertently expose the forwarded UDP port on all interfaces. Binding should respect the configured host IP when creating the socket.

Copilot · 2026-04-23T19:15:12Z

+        let host_port = conn
+            .socket
+            .as_ref()
+            .map_or(0, |s| s.get().local_addr().map_or(0, |addr| addr.port()));


bind_udp_port() suppresses errors from local_addr() by returning 0 on failure. If the caller relies on the returned port (especially when binding to 0 for an ephemeral port), this can silently report the wrong value. Consider propagating the local_addr() error as DropReason::Io instead of defaulting to 0.

Suggested change

let host_port = conn

.socket

.as_ref()

.map_or(0, |s| s.get().local_addr().map_or(0, |addr| addr.port()));

let host_port = match conn.socket.as_ref() {

Some(socket) => socket

.get()

.local_addr()

.map_err(DropReason::Io)?

.port(),

None => 0,

};

Copilot · 2026-04-23T19:15:12Z

+                    .map(|a| {
+                        a.parse()
+                            .map_err(|_| ResolveConsommeError::InvalidAddress(a.to_owned()))
+                    })
+                    .transpose()?;


InvalidAddress is constructed by discarding the underlying parse error (map_err(|_| ...)). This loses useful detail (e.g. why parsing failed). Consider preserving the parse error in the message or as a #[source] so users can diagnose invalid hostfwd addresses more easily.

Copilot · 2026-04-23T19:15:13Z

+    ///   --net consomme:hostfwd=tcp::3389-:3389
+    ///   --net consomme:hostfwd=tcp:127.0.0.1:8080-:80
+    ///   --net consomme:hostfwd=tcp:[::1]:8080-:80
+    ///   --net consomme:10.0.0.0/24,hostfwd=tcp::22-:22,hostfwd=udp::5000-:5000


This PR adds a new CLI surface (--net consomme:hostfwd=...). The guide already documents Consomme (Guide/src/reference/backends/consomme.md) and mentions that inbound connections require port forwarding, but it doesn’t describe this CLI syntax. Please update the Guide to include the new hostfwd= option (or note a follow-up) to keep user-facing documentation in sync.

Suggested change

/// --net consomme:10.0.0.0/24,hostfwd=tcp::22-:22,hostfwd=udp::5000-:5000

/// --net consomme:10.0.0.0/24,hostfwd=tcp::22-:22,hostfwd=udp::5000-:5000

///

/// TODO(openvmm-guide): document `--net consomme:hostfwd=...` in

/// `Guide/src/reference/backends/consomme.md` to keep the published

/// Consomme backend documentation in sync with this CLI surface.

Copilot · 2026-04-23T19:15:13Z

+    pub address: Option<IpAddr>,
+    /// The port traffic is forwarded to on the guest.
+    pub guest_port: u16,
+    /// The host port to bind, or `None` for any available port.


PortForwardConfig::host_port is a u16, but the doc comment says it can be None for an ephemeral port. Either make the field an Option<u16> or document the actual sentinel value (e.g. 0 means “any available port”).

Suggested change

/// The host port to bind, or `None` for any available port.

/// The host port to bind, or `0` for any available port.

Copilot · 2026-04-23T19:15:13Z

+        let port_forwards =
+            std::mem::take(&mut queue.endpoint_state.as_mut().unwrap().port_forwards);
+        queue.with_consomme_no_pool(|c| {
+            c.refresh_driver();
+            for fwd in port_forwards {
+                let result = match fwd.protocol {
+                    IpProtocol::Tcp => c.bind_tcp_port(fwd.address, fwd.guest_port, fwd.host_port),
+                    IpProtocol::Udp => c.bind_udp_port(fwd.address, fwd.guest_port, fwd.host_port),
+                };
+                if let Err(err) = result {
+                    tracing::error!(
+                        protocol = ?fwd.protocol,
+                        guest_port = fwd.guest_port,
+                        host_port = fwd.host_port,
+                        error = &err as &dyn std::error::Error,
+                        "failed to bind port"
+                    );
+                }
+            }
+        });


Port-forward binding failures are only logged and VM startup continues. For a CLI-configured hostfwd=..., this makes misconfiguration (e.g. port already in use) easy to miss and leaves the guest without the requested forwarding. Consider propagating these errors (e.g. return an error from get_queues() / endpoint creation) so the command fails fast when forwarding can’t be established.

Copilot · 2026-04-23T19:15:14Z

    /// Binds to the specified host IP and port for forwarding inbound UDP
    /// packets to the guest.
-    pub fn bind_udp_port(&mut self, ip_addr: Option<IpAddr>, port: u16) -> Result<(), DropReason> {
+    pub fn bind_udp_port(
+        &mut self,
+        ip_addr: Option<IpAddr>,
+        guest_port: u16,
+        host_port: u16,
+    ) -> Result<u16, DropReason> {
        let guest_addr = match ip_addr {
-            Some(IpAddr::V4(ip)) => SocketAddr::V4(SocketAddrV4::new(ip, port)),
-            Some(IpAddr::V6(ip)) => SocketAddr::V6(SocketAddrV6::new(ip, port, 0, 0)),
-            None => SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, port)),
+            Some(IpAddr::V4(ip)) => SocketAddr::V4(SocketAddrV4::new(ip, guest_port)),
+            Some(IpAddr::V6(ip)) => SocketAddr::V6(SocketAddrV6::new(ip, guest_port, 0, 0)),
+            None => SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, guest_port)),
        };
-        let _ = self.get_or_insert(guest_addr, None)?;
-        Ok(())
+        let conn = self.get_or_insert(guest_addr, Some(host_port), None)?;


bind_udp_port() uses ip_addr to build guest_addr, but that SocketAddr is later used as the guest destination when forwarding inbound packets (UdpConnection::poll_conn uses the map key as the dst IP/port). Passing the host bind address here will cause inbound forwarded UDP packets to target the wrong guest IP (and None currently becomes 0.0.0.0). This likely needs a separate UDP listener map keyed by guest_port, and should always forward to the configured guest IP (e.g. params.client_ip / client_ip_ipv6) regardless of host bind address.

jstarks · 2026-04-23T19:16:11Z

+            std::mem::take(&mut queue.endpoint_state.as_mut().unwrap().port_forwards);
+        queue.with_consomme_no_pool(|c| {
+            c.refresh_driver();
+            for fwd in port_forwards {


Seems like we're rebinding on each get_queues call. Is this right? Shouldn't we do this once at construction time?

It will only happen once because we std::mem::take the port_forwards Vec. The issue is that get_queues is where we are given a Driver which is needed for a PolledSocket. This was on the easier side of solutions, but we can plumb in the Driver elsewhere, add some comments, split it out to a separate fn with comments, etc.

jstarks · 2026-04-23T19:19:59Z

+        pub host_address: Option<String>,
+        /// The host port to listen on.
+        pub host_port: u16,


The most sandboxing-friendly approach would be to just take a Socket here--require the bind to happen at VM construction time.

Failing that, we should at least parse this down to a better data type than String (although I imagine you're just following the precedent of cidr: Option<String>, which I regret.

github-actions · 2026-04-23T20:13:52Z

1 Petri tests failed (0 unstable)

consomme: Enable port forwarding from openvmm command line

204858e

Copilot AI review requested due to automatic review settings April 23, 2026 19:07

Brian-Perkins requested a review from a team as a code owner April 23, 2026 19:07

Copilot started reviewing on behalf of Brian-Perkins April 23, 2026 19:08 View session

jstarks reviewed Apr 23, 2026

View reviewed changes

Copilot AI reviewed Apr 23, 2026

View reviewed changes

jstarks reviewed Apr 23, 2026

View reviewed changes

-        let host_port = conn
-            .socket
-            .as_ref()
-            .map_or(0, |s| s.get().local_addr().map_or(0, |addr| addr.port()));
+        let host_port = match conn.socket.as_ref() {
+            Some(socket) => socket
+                .get()
+                .local_addr()
+                .map_err(DropReason::Io)?
+                .port(),
+            None => 0,
+        };

-    ///   --net consomme:10.0.0.0/24,hostfwd=tcp::22-:22,hostfwd=udp::5000-:5000
+    ///   --net consomme:10.0.0.0/24,hostfwd=tcp::22-:22,hostfwd=udp::5000-:5000
+    ///
+    /// TODO(openvmm-guide): document `--net consomme:hostfwd=...` in
+    /// `Guide/src/reference/backends/consomme.md` to keep the published
+    /// Consomme backend documentation in sync with this CLI surface.

	/// The host port to bind, or `None` for any available port.
	/// The host port to bind, or `0` for any available port.

Conversation

Brian-Perkins commented Apr 23, 2026

Uh oh!

jstarks Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

jstarks Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Brian-Perkins Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

jstarks Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants