init/aws-nitro: support partial read/write ethernet packet forwarding

[jakecorrenti]

Ensure the requested number of bytes is forwarded to/from the host vsock providing the network access and the guest TAP device routing the application network traffic.

To write this code I used Cursor and the claude-4.6-opus-high model.

Reproducer

To reproduce the issue, have the examples/nitro.c example execute the following:

TOKEN=$(curl -sf -X PUT 'http://169.254.169.254/latest/api/token' \
          -H 'X-aws-ec2-metadata-token-ttl-seconds: 21600')
echo "IMDSv2 token acquired"
ROLE=$(curl -sf 'http://169.254.169.254/latest/meta-data/iam/security-credentials/' \
              -H "X-aws-ec2-metadata-token: $TOKEN")
echo "--- IAM role: $ROLE ---"
curl -sf "http://169.254.169.254/latest/meta-data/iam/security-credentials/$ROLE" \
                -H "X-aws-ec2-metadata-token: $TOKEN" || { echo "error: failed to fetch credentials for role: $ROLE" >&2; exit 1; }

Without the above, about 90% of the time the final curl command will take about 15 seconds before failing because the client connection is closed. With the new diff, it should work 100% of the time.

[jakecorrenti]

cc @tylerfanelli

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces read_exact and write_all utility functions to ensure robust I/O operations by handling partial reads/writes and EINTR signals. These utilities are integrated into the packet forwarding logic between the vsock and TAP device. Feedback highlights a critical buffer overflow risk where incoming packet lengths are not validated against the MTU. Additionally, improvements are suggested for read_exact to set errno on partial reads and for write_all to handle zero-length writes to avoid infinite loops.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request enhances the reliability of the TAP-to-vsock forwarding mechanism by implementing read_exact and write_all functions to handle partial reads/writes and system call interruptions. It also updates the buffer allocation to include the Ethernet header length and adds validation for incoming frame sizes. Review feedback suggests using standard exit codes instead of negative values, replacing hardcoded sizes with sizeof(uint32_t), and using ntohl for network-to-host conversion. Additionally, a concern was raised regarding the use of write_all on packet-oriented TAP devices, which could lead to fragmented packets.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves the reliability of the AWS Nitro TAP-to-vsock forwarder by introducing read_exact and write_all helper functions to handle partial I/O and signal interruptions. It also updates buffer allocation to include Ethernet headers and adds validation for incoming frame sizes. Review feedback recommends standardizing exit codes to EXIT_FAILURE instead of errno and refining error checks to explicitly handle partial writes and unexpected EOF during frame reads.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request enhances the AWS Nitro TAP-to-vsock forwarder by implementing robust I/O helpers for partial transfers and EINTR handling, adjusting buffer sizes for Ethernet headers, and adding frame validation. Review feedback suggests using fixed-width uint32_t types for protocol headers to ensure portability and recommends handling EINTR for TAP device reads to maintain consistency.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves the robustness of network packet forwarding in net_tap_afvsock.c by implementing helper functions for complete I/O operations and handling signal interruptions. Key changes include correcting buffer allocations for Ethernet headers, fixing byte-order handling for frame lengths, and enhancing error management. Feedback suggests further improving portability and type safety by using uint32_t for variables used in network size exchanges to avoid platform-dependent issues.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves the reliability of network packet forwarding between the TAP device and vsock by introducing read_exact and write_all helpers to handle partial I/O and EINTR signals. It also expands the internal buffer to accommodate the Ethernet header. Feedback indicates that the MTU value sent to the host may be insufficient as it excludes the Ethernet header, potentially causing buffer overflows or stream corruption on the host side. Furthermore, a lack of error handling for TAP device reads could result in an infinite busy-loop if a persistent error occurs.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves the robustness of the Nitro enclave network proxy by implementing helper functions for exact reads and writes, enhancing error handling, and ensuring proper memory management. It also updates the communication protocol to account for the Ethernet header length. Review feedback highlights an inconsistency in endianness during the initial MTU exchange, recommending the use of network byte order to match the subsequent packet framing.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request enhances the network TAP-to-vsock forwarding mechanism by implementing robust I/O handling and cross-endian compatibility. Key improvements include the introduction of read_exact and write_all helper functions to manage partial reads/writes and system interrupts, the use of network byte order for frame size communication, and the addition of proper memory cleanup on error paths to prevent leaks. Feedback was provided regarding type consistency, specifically suggesting the use of uint32_t for the frame length variable to match the return type of network byte order conversion functions.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces per-packet framing and robust I/O handling for network traffic between the enclave and host, including endianness-aware length headers and improved error management. Feedback indicates a critical protocol mismatch because the Rust proxy's rcv and send methods were not updated to handle the new framing. Additionally, the poll loop in the C implementation should be modified to properly report errors upon termination instead of exiting with a success status.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request enhances the network proxy between the AWS Nitro enclave and the host by improving I/O robustness and ensuring endianness consistency. Key changes include the introduction of read_exact and write_all helpers to handle partial transfers and interrupts, the use of big-endian encoding for frame length headers, and improved error handling and memory management. Feedback was provided regarding an inconsistent error path in the poll loop that uses exit instead of returning an error code, which would align better with the rest of the function's logic.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request enhances the network proxy between the enclave and the host by improving I/O robustness and ensuring endianness consistency. Key changes include the addition of read_exact and write_all helper functions to handle partial reads/writes and interrupts, the use of uint32_t with network byte order for frame size headers, and improved error handling in the C implementation. A bug in tap_afvsock_init was also corrected to properly check the TAP device file descriptor. Review feedback suggests refactoring the error handling in tap_vsock_forward to use a unified cleanup section with goto labels for better consistency and maintainability.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request enhances the network proxy by introducing read_exact and write_all helpers to handle partial I/O and interrupts, and standardizes communication using big-endian byte order for frame sizes. It also adjusts buffer allocations to include Ethernet headers and fixes a logic error in TAP device initialization. Review feedback identifies a critical bug where the child process fails to exit upon successful completion of the forwarding loop, and an error handling oversight where a failed read does not correctly update the return status before cleanup.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request enhances the network proxy between the AWS Nitro enclave and the host by implementing robust I/O handling and ensuring cross-platform compatibility. Key improvements include the addition of read_exact and write_all helpers to handle partial transfers and interrupts, the use of big-endian byte ordering for frame sizes, and refined error management. A review comment suggests closing the tun_fd in the parent process after forking to prevent a file descriptor leak.

[jakecorrenti]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves the reliability and cross-platform compatibility of the network proxy between the AWS Nitro enclave and the host. Key changes include the implementation of robust I/O helpers (read_exact and write_all) to handle partial transfers and interrupts, and the transition to big-endian byte order for frame headers to ensure consistency between the enclave and host. Additionally, the update enhances error handling, ensures proper resource cleanup, and fixes a bug in the TAP device initialization. I have no feedback to provide as the changes are sound and follow best practices for systems programming.