Skip to content

Conversation

huang-jl
Copy link
Contributor

@huang-jl huang-jl commented Jul 21, 2025

Existing setup in network-for-clones.md about setup egress connectivity cannot work.

Changes

Update the docs/snapshotting/network-for-clones.md to be consistent with tests/framework/microvm_helpers.py.

Reason

There lacks one iptable configuration about setup egress connectivity. As shown in

f"iptables -t nat -A POSTROUTING -s {veth_net} -o {upstream_dev} -j MASQUERADE"

It should add a MASQUERADE rule inside the netns. After that, the host iptable rules of source ip 10.0.0.0/30 can work. Without that, the host can only see packet of source ip (e.g., 192.168.241.1/29) from guest VM.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • I have read and understand CONTRIBUTING.md.
  • I have run tools/devtool checkbuild --all to verify that the PR passes
    build checks on all supported architectures.
  • I have run tools/devtool checkstyle to verify that the PR passes the
    automated style checks.
  • I have described what is done in these changes, why they are needed, and
    how they are solving the problem in a clear and encompassing way.
  • I have updated any relevant documentation (both in code and in the docs)
    in the PR.
  • I have mentioned all user-facing changes in CHANGELOG.md.
  • If a specific issue led to this PR, this PR closes the issue.
  • When making API changes, I have followed the
    Runbook for Firecracker API changes.
  • I have tested all new and changed functionalities in unit tests and/or
    integration tests.
  • I have linked an issue to every new TODO.

  • This functionality cannot be added in rust-vmm.

@zulinx86 zulinx86 self-requested a review July 25, 2025 10:27
pb8o
pb8o previously approved these changes Jul 25, 2025
zulinx86
zulinx86 previously approved these changes Jul 25, 2025
@zulinx86 zulinx86 enabled auto-merge (rebase) July 25, 2025 13:04
@zulinx86
Copy link
Contributor

@huang-jl it looks good to me, but the style check failed.
https://buildkite.com/firecracker/firecracker-pr/builds/14209#019841ae-cd69-4617-ae6c-4638f5fc8d7f/58-280

If you'd like to have a link in commit message, please do as follows:

blah blah blah [1], blah blah

[1]: https://github.com/firecracker-microvm/firecracker/blob/7dfe2765849c2444a22defa11be8641508c5ce5c/tests/framework/microvm_helpers.py#L190
Signed-off-by: blah blah blah

As the PR check says, you can check it using tools/devtool checkstyle.

@zulinx86 zulinx86 added the Status: Awaiting author Indicates that an issue or pull request requires author action label Jul 25, 2025
There lacks one iptables configuration about setup egress connectivity.
As shown in [1], it should add a MASQUERADE rules inside the netns.
After that, the host iptable rules of source ip 10.0.0.0/30 should work.
Without that, the host can only see packet of ip 192.168.241.1/29 from
guest VM.

[1]: https://github.com/firecracker-microvm/firecracker/blame/7dfe2765849c2444a22defa11be8641508c5ce5c/tests/framework/microvm_helpers.py#L194

Signed-off-by: huang-jl <[email protected]>
auto-merge was automatically disabled July 25, 2025 15:57

Head branch was pushed to by a user without write access

@huang-jl huang-jl dismissed stale reviews from zulinx86 and pb8o via 1a652d7 July 25, 2025 15:57
@huang-jl
Copy link
Contributor Author

@zulinx86 Thanks for your instruction. I have update the commit message and update as @pb8o pointed out. I have test the integration_tests/style locally, and pass the previously failed gitlint test.

@zulinx86 zulinx86 enabled auto-merge (rebase) July 25, 2025 16:09
@zulinx86 zulinx86 merged commit ba8ad7a into firecracker-microvm:main Jul 25, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting author Indicates that an issue or pull request requires author action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants