Fix: Parse MTU to Firecracker network configuration#836
Open
HARSHVARANDANI wants to merge 3 commits into
Open
Conversation
Ensure the MTU is retrieved from the container interface and populated in the Interface struct, preventing it from defaulting to 0. Signed-off-by: HARSHVARANDANI <hpvarandani@gmail.com>
Add MTU field to FirecrackerNet config struct and serialize it to the Firecracker config JSON to enable VIRTIO_NET_F_MTU negotiation with the guest. Signed-off-by: HARSHVARANDANI <hpvarandani@gmail.com>
… building Add test cases covering MTU parsing, JSON serialization, and baseline configuration generation for Firecracker. Signed-off-by: HARSHVARANDANI <hpvarandani@gmail.com>
✅ Deploy Preview for urunc canceled.
|
Contributor
|
Hello @HARSHVARANDANI , thank you for opening the PR. Regarding the test method, we will need to verify that this works end-to-end, meaning that the Firecracker guest getc configured with the MTU we specify. A unit test is not enough for the testing. You can take a look at #564 on how to test this end-to-end. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes an issue where the host veth's MTU was not propagated to Firecracker virtual machines.
The dynamic network implementation correctly reads the host veth's MTU, but the static network mode was leaving it unset (defaulting to
0). Furthermore, the Firecracker VMM driver completely lacked support for configuring the MTU in the generatedfc.jsonconfig file. Consequently, the guest Linux kernel defaulted to an MTU of1500regardless of the MTU configured on the host.Specifically, this change:
pkg/network/network_static.goto populate theMTUfield in the returnedInterfacestruct fromredirectLink.Attrs().MTU.mtufield to theFirecrackerNetJSON structure inpkg/unikontainers/hypervisors/firecracker.goand sets it usingargs.Net.MTU. This enablesVIRTIO_NET_F_MTUfeature negotiation with the guest.firecracker_test.go) covering the command construction, JSON config building, and JSON serialization for Firecracker.Related issues
How was this tested?
The unit tests written in
firecracker_test.goverify that the Firecracker command-line constructor (BuildExecCmd) correctly compiles and generates JSON configuration files. The test suite uses a table-driven approach to validate baseline parameters (like boot settings, seccomp filters, custom memory sizes, and virtual CPUs), verify the serialization of both default (1500) and custom (9000) MTU values, assert that zero-value MTUs are omitted from the serialized JSON configuration using the omitempty struct tag, and test the integration of auxiliary configurations such as vsock devices and custom initramfs images.LLM usage
Used Google Antigravity for writing tests and scoping the issue. All changes have been manually reviewed.
Checklist
make lint).make test_ctr,make test_nerdctl,make test_docker,make test_crictl).