Skip to content

Port nvx-ingress to Rust - #533

Merged
Pedro Henrique Penna (ppenna) merged 2 commits into
devfrom
agents/fix-issue-358-ingress
Oct 16, 2026
Merged

Pedro Henrique Penna (ppenna) merged 2 commits into
devfrom
agents/fix-issue-358-ingress

Conversation

@ppenna

@ppenna Pedro Henrique Penna (ppenna) commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • port nvx-ingress to a static Rust guest binary that keeps the C program's
    command line, exit statuses, output, timing, and NVXL wire format, split by
    concern: socket-table parsing, address classification, socket-diagnostics
    policy, UDP settlement, announcement encoding, the live sender, and snapshot
    test mode
  • follow the guest conventions that Port the small guest tools to Rust #528 and Port the managed agent to Rust #531 established: system calls
    only through nix behind safe nvx::guest::sys wrappers, nix's errno
    text, the shared CommandError and ExitStatus, owned descriptors, and no
    direct libc
  • parse the kernel's socket tables, bindv6only, and snapshot times strictly
    with the standard library, in the form in which the kernel writes them,
    rather than emulate C's strtoul, sscanf, and fscanf
  • name every protocol, kernel, and buffer constant, and document why it has
    its value
  • port all eight C-driven ingress cases and build and test the binary through
    the manifest-driven nvx.py guest-binary workflow

Part of #358 (PR 3 of 5). Rebased onto dev after #528 and #531 merged, so
the PR now holds only its own two commits.

Conventions and system boundary

The ingress module denies unsafe code and calls no libc function, and it adds
none to guest::sys. Its new sys operations make their calls through nix:

  • UdpSocket: an owned close-on-exec IPv4 UDP socket that binds a port on
    every local address and sends datagrams; the sender retries only
    EADDRINUSE from port 1023 down through 512
  • sock_diag: the NETLINK_SOCK_DIAG messages, which nix does not model.
    dump_ipv6_tcp_sockets sends a byte-built inet_diag_req_v2 request from
    an owned close-on-exec socket and receives the dump in parts of at most
    32 KiB, each within a 100 ms timeout, and decode_inet_diag_dump decodes
    its netlink framing, inet_diag_msg, and INET_DIAG_SKV6ONLY attribute
    from explicit byte layouts into typed InetDiagMessages, so that the
    program keeps only its policy for wildcard listeners
  • ignore_interrupt_and_hangup: the ignored SIGINT and SIGHUP dispositions
  • monotonic_ms: the existing monotonic clock, now built for every
    architecture, in whole milliseconds
  • std::io::Read for FileDescriptor: one read call per call, as for
    std::fs::File

Every descriptor stays with the library that opened it. The program opens the
socket tables, bindv6only, and snapshot files through sys::open with
O_CLOEXEC, so every descriptor that it opens comes from nix and has one
owner, and BufReader and Read::take buffer and bound them through Read
without a second owner. Because sys::Stdout writes standard output directly,
the guest module now denies print! and eprint! through
clippy::print_stdout and clippy::print_stderr, and its documentation
states the rule.

The standard library sleeps, resuming an interrupted sleep as the C
nanosleep loop did, parses the gateway, where Ipv4Addr accepts exactly
what inet_pton(AF_INET, ...) accepts, and classifies loopback, link-local,
and IPv4-mapped addresses with Ipv4Addr and Ipv6Addr. Raw descriptors and
native socket addresses do not leave sys, and netlink messages are built and
parsed as explicit byte layouts without pointer casts. Guests continue to ship
guest/common/nvx-ingress.c until PR 5 changes the image build.

The integration commit adds nvx-ingress to GuestRustBuildConstants.BINARIES
and ADDED_BINARIES, so a build upgrades an earlier build's output in place,
and tests the upgrade of the immediate predecessor, which lacks only
nvx-ingress.

Behavioral parity

No library parses the kernel's socket tables, but once a row is split into
columns, the standard library's strict parsers read its numbers. The program
reads each table in lines of at most 511 bytes, as the C program's fgets
did, splits each row at ASCII white space, and parses the decimal and
hexadecimal columns, bindv6only, and snapshot times with parse_decimal and
a new parse_hexadecimal in nvx::guest::parse. It reads everything that the
kernel writes, %4d: row numbers, %08X address words, %04X ports, %02X
states, and %lu inodes, as the C program did.

Unlike the C program, it rejects the lenient forms that C's strtoul and
scanf also read, which only synthetic snapshots contain: signs and 0x
prefixes, including a signed address word such as -0000001, which an
earlier review made wrap to 0xffffffff; bytes after a number; vertical tabs;
NUL bytes, which ended C's line; tokens longer than 63 bytes, which C split;
and numbers too large for their type, which C saturated or truncated. It
skips such a row, keeps the dual-stack default for such a bindv6only, and
ends test mode with status 1 for such a time. A bindv6only from 2^32 to
2^64 − 1, which C truncated to an int, counts as nonzero. The nvx::guest
documentation records this exception to the compatibility contract.

The port also preserves strict IPv4 syntax, timing, socket behavior,
native-endian netlink framing, malformed-message termination, per-port
IPv6-only diagnostics, per-(port, inode) UDP settlement and retention, the
reachability rules, the 1,024-entry tracking limit, and the sorted 64-entry
NVXL payload with its omitted flag. The golden vector remains
4e56584c0100000206011f90060220fb. Each protocol, kernel, and buffer constant
documents why it has its value: OpenVMM's announcement port, privileged
source ports, and 64-listener limit, the 100 ms scans, 2 s refreshes, and 1 s
UDP settling that doc/run.md documents, Linux's socket states, and the C
program's buffer sizes.

Diagnostics describe errno values with nix's text, as the other guest
programs' do; a few differ from musl's strerror text, such as EMFILE's.

Validation

This head, on WSL (Ubuntu)

  • every check-nvx-rs cargo command: format, both Clippy modes, all-feature
    and default tests (238 library, 6 fake-OpenVMM, 23 guest-tool, and doc
    tests), rustdoc, the Rust 1.89 MSRV, and the aarch64-apple-darwin check
  • every check-aci-edge-sandboxes command with CI's environment, including
    test-agent-bundling
  • nvx.py check-guest-binaries: musl Clippy and musl tests (167 library, 23
    guest-tool, and doc tests)
  • nvx.py build-guest-binaries into a fresh directory: nvx-ingress is a
    422,368-byte stripped static PIE (dfdb9887...c9ff)
  • nvx.py test-guest-binaries: all 23 prebuilt integration tests pass
  • test_guest_binaries.py and test_guest_ingress.py: 23 tests pass without
    skips

Previous head (f78eaab47)

These cover files that this head leaves unchanged:

  • on Windows, Ruff check and format, and Linux and Windows Pyright with 0
    errors
  • an output built at the first commit, with six binaries, upgrades in place to
    seven, and a fresh build writes an identical manifest

Differential and live

  • At the previous head, the C program, built with musl 1.2.5 exactly as the
    image builds it (musl-gcc -static -Os -s -Wall -Wextra -Werror), and the
    Rust program matched status, stdout, and stderr byte for byte for 14,000
    generated and mutated snapshot sequences and 75 command-line cases.
  • This head matches the previous head byte for byte on 25,000 generated
    kernel-format snapshot sequences, 5,000 of them with tab, form-feed, and
    carriage-return separators, and on 12 command-line cases. The corpus covers
    loopback, link-local, IPv4-mapped, and unspecified addresses, more than 64
    listeners, UDP settling and identity across snapshots, wrapping times,
    bindv6only 0, 1, or absent, and well-formed and malformed diagnostic
    dumps. On the mutated corpus, which writes the lenient forms above into most
    sequences, 8,891 of 10,000 sequences differ.
  • In a private network namespace (unshare -rn), this head and the previous
    head send the same announcements from port 1023 at the same times: a TCP
    IPv4 listener, an IPv6-only and a dual-stack wildcard listener, which the
    netlink diagnostics distinguish, no loopback listener, the UDP socket once
    it settles after 1 s, and a refresh after 2 s. Both ignore SIGINT and SIGHUP
    and stop on SIGTERM.

CI

CI run 38911772485
is in progress at this head; the previous head's run passed.

Copilot AI balanced review requested due to automatic review settings October 13, 2026 21:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The central live networking loop lacks automated coverage.

1 open finding
What changed in this PR

Ports nvx-ingress to the Rust guest tooling while preserving the existing C protocol and behavior.

Changes:

  • Adds bounded socket, address, netlink, and UDP-settling logic.
  • Implements snapshot mode and the live announcement sender.
  • Adds musl build validation and extensive parser/collector tests.
File Description
.github/​actions/​check-nvx-rs/​action.yml Builds and validates nvx-ingress.
nvx-rs/​Cargo.toml Registers the guest binary.
nvx-rs/​README.md Documents the binary and build.
nvx-rs/​src/​bin/​nvx-ingress.rs Adds the executable entry point.
nvx-rs/​src/​guest/​ingress/​address.rs Classifies listener reachability.
nvx-rs/​src/​guest/​ingress/​collect.rs Collects and settles listeners.
nvx-rs/​src/​guest/​ingress/​diag.rs Parses socket diagnostics.
nvx-rs/​src/​guest/​ingress/​encode.rs Encodes NVXL announcements.
nvx-rs/​src/​guest/​ingress/​mod.rs Defines ingress behavior and constants.
nvx-rs/​src/​guest/​ingress/​sender.rs Implements live announcement sending.
nvx-rs/​src/​guest/​ingress/​table.rs Parses procfs socket tables.
nvx-rs/​src/​guest/​ingress/​test_mode.rs Implements snapshot test mode.
nvx-rs/​src/​guest/​mod.rs Exposes the ingress module.
nvx-rs/​tests/​guest_tools.rs Exercises the compiled binary.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread nvx-rs/src/guest/ingress/sender.rs Outdated
Copilot AI balanced review requested due to automatic review settings October 13, 2026 21:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The broad privileged, protocol, networking, and unsafe freestanding changes warrant final human review despite extensive parity coverage.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The address parser rejects signed hexadecimal snapshot words accepted by C, violating the required parsing compatibility.

1 open finding

🧠 Review effort: Balanced

Comment thread nvx-rs/src/guest/ingress/address.rs Outdated
Copilot AI balanced review requested due to automatic review settings October 14, 2026 21:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The privileged guest interfaces, unsafe runtime boundaries, and strict C-compatibility requirements warrant final human review.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 15, 2026 01:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The privileged guest migration and unsafe runtime boundaries warrant final human review despite no confirmed blocking findings.

0 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 15, 2026 05:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The stacked port’s privileged guest operations and low-level runtime boundaries warrant final human review despite reported successful validation.

0 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 16, 2026 12:12
@ppenna
Pedro Henrique Penna (ppenna) marked this pull request as ready for review October 16, 2026 12:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Exact C compatibility and privileged networking behavior warrant final human review despite no concrete blocking defects identified.

0 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 16, 2026 13:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Strict parsing breaks the promised C-compatible snapshot behavior, including the previously fixed signed-address case.

4 open findings

🧠 Review effort: Balanced

Comment thread nvx-rs/src/guest/ingress/address.rs
Comment thread nvx-rs/src/guest/ingress/mod.rs
Comment thread nvx-rs/src/guest/ingress/table.rs
Comment thread nvx-rs/src/guest/ingress/table.rs
Port the listener announcer as a static Rust guest binary that keeps the C
program's command line, exit statuses, output, timing, and wire format.
Split socket-table parsing, address classification, socket diagnostics,
UDP settling, announcement encoding, the live sender, and snapshot test
mode by concern. Name every protocol, kernel, and buffer constant, and
document why it has its value: OpenVMM's announcement port, privileged
source ports, and 64-listener limit, the 100 ms scans, 2 s refreshes, and
1 s UDP settling that doc/run.md documents, Linux's socket states, and the
C program's buffer sizes.

Follow the guest programs' conventions. The program fails with the shared
CommandError and exits with the shared statuses 0, 1, and 2, and its
diagnostics describe errno values with nix's text. The safe nvx::guest::sys
wrappers make its system calls through nix: an owned UDP socket that binds
and sends, and the ignored SIGINT and SIGHUP dispositions. A new
sys::sock_diag module owns the NETLINK_SOCK_DIAG messages, which nix does
not model: it queries the kernel's IPv6 TCP listeners with a receive
timeout and decodes the dump's netlink framing, inet_diag_msg, and
INET_DIAG_SKV6ONLY attribute from explicit byte layouts into typed
messages, so that the program keeps only its policy for wildcard
listeners. The monotonic clock now serves every architecture, in
milliseconds as well; the standard library sleeps, parses the gateway as
inet_pton did, and classifies loopback, link-local, and IPv4-mapped
addresses; and no resource is dropped by hand.

Keep every descriptor with the library that opened it. The program opens
the socket tables, the sysctl, and snapshot files through sys with
O_CLOEXEC, so that every descriptor that it opens comes from nix, and
FileDescriptor implements std::io::Read, so that BufReader and Read::take
buffer and bound a descriptor without becoming a second owner. The guest
module documents the rule and denies the standard library's print macros,
whose buffered standard output would reorder with sys::Stdout's direct
writes.

No library parses the kernel's socket tables, but the standard library's
strict parsers read their numbers once the program splits the columns.
The program reads each table in lines of at most 511 bytes, as the C
program's fgets did, splits each row at ASCII white space, and parses the
kernel's decimal and hexadecimal columns, bindv6only, and snapshot times
with parse_decimal and a new parse_hexadecimal. It thus reads all that the
kernel writes as the C program did, but rejects the signed or 0x-prefixed
numbers and address words, overflowing values, numbers followed by other
bytes, and vertical tabs that C's strtoul and scanf also accepted, which
only malformed snapshots contain. It also rejects a row with a NUL byte,
which ended C's string, or with an address column longer than the 63
bytes that C's %63s read, which C split. The guest module's documentation
records this exception to the compatibility contract.

Port the eight C-driven ingress cases, pin the golden wire vector, and
cover the kernel's row format and the C forms that the program rejects,
the strict parsers, malformed diagnostics, the netlink layouts and a live
dump, bounded encoding, sender failures, refreshes, the UDP socket,
descriptor reads through std::io, and the real binary through
NVX_GUEST_BINARIES.

Part of #358.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add nvx-ingress to the Rust guest-binary inventory. The existing nvx.py
build-guest-binaries and test-guest-binaries commands now build its static
musl executable in the same Cargo build as the other guest programs, check
its ELF form, include its digest in the manifest, and run its prebuilt
integration tests.

A build adds nvx-ingress, as it adds nvx-managed-agent, to an earlier output
whose manifest does not record it. Test that the immediate predecessor's
output, which lacks only nvx-ingress, upgrades in place. Document the command
line, and count seven binaries in the command documentation. The guest-image
build remains unchanged until the final switch.

Part of #358.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 16, 2026 14:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The substantial privileged-networking port and shared system-layer changes warrant final human review.

0 open findings

4 resolved since last review

🧠 Review effort: Balanced

@ppenna
Pedro Henrique Penna (ppenna) merged commit 36fc8ae into dev Oct 16, 2026
34 checks passed
@ppenna
Pedro Henrique Penna (ppenna) deleted the agents/fix-issue-358-ingress branch October 16, 2026 15:12
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