netstack: fix bubblewrap: add if up/down, add lo address on if up, remove ipv6 on if down#13532
Open
lyphyser wants to merge 8 commits into
Open
netstack: fix bubblewrap: add if up/down, add lo address on if up, remove ipv6 on if down#13532lyphyser wants to merge 8 commits into
lyphyser wants to merge 8 commits into
Conversation
Previously netstack reported all interfaces as IFF_UP unconditionally and
ignored RTM_NEWLINK requests to change IFF_UP ("Netstack interfaces are
always up"). This made it impossible to model an interface that is
administratively down.
Wire IFF_UP through to the existing NIC enable/disable machinery:
SetInterface now enables the NIC when IFF_UP is set and disables it when
cleared, and nicInfo reports IFF_UP based on whether the NIC is enabled
(IFF_RUNNING already did). Interface creation paths (veth, bridge) keep
their current behavior by not requesting a state change, so all existing
interfaces continue to be created enabled and report up.
Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AddInterfaceAddr adds a protocol address and, like the Linux kernel, adds the connected route for its subnet. Split the body into an unexported addInterfaceAddr() that works with tcpip types and returns *syserr.Error, so other in-stack callers can add an address (and its route) the same way RTM_NEWADDR does, rather than poking the route table themselves. No behavior change. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Previously the loopback interface of an application-created network namespace was created already configured with 127.0.0.1/8 and ::1/128. Linux instead creates the loopback interface administratively DOWN with no addresses, and the kernel assigns the default loopback addresses when the interface is first brought up. Match that: create the loopback NIC disabled and without addresses, and when setLinkLocked brings a loopback interface up (IFF_UP), assign the default loopback addresses, skipping any that are already present. Bringing the interface down does not remove them, so a subsequent up is a no-op, just like Linux. The addresses are added through addInterfaceAddr, the same path used by RTM_NEWADDR, so their connected routes are installed automatically rather than via loopback-specific route handling. The initial network namespace is still configured explicitly in Network.CreateLinksAndRoutes and is unaffected. While here, add the socket_netlink_route_util include and build dep that network_namespace_test was already missing for the existing loopback test. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Assisted-By: Codex <noreply@openai.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Assisted-By: Codex <noreply@openai.com>
lyphyser
force-pushed
the
if-down-and-lo-address
branch
from
ef807fd to
6b1bcba
Compare
Contributor
Author
Fixed, it was asking for a CLA from Claude due to mistakenly using Co-Authored-By instead of Assisted-By |
nybidari
reviewed
Jun 22, 2026
nybidari
reviewed
Jun 22, 2026
nybidari
reviewed
Jun 22, 2026
nybidari
requested changes
Jun 22, 2026
When bringing an interface down, removeIPv6Addrs no longer aborts if a removeInterfaceAddr call fails; instead it logs a warning and continues, still ignoring ErrBadLocalAddress. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Read /proc/sys/net/ipv6/conf/all/keep_addr_on_down during Stack.Configure to reflect the host setting, similarly to tcp_rmem/tcp_wmem. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Pushed 3 extra commits to address the review comments |
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
3 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.
AI usage: written by Claude with guidance: I think it's fine in general, but I suspect it might need adjustments since I'm not really familiar with the codebase and I reviewed it but not in a very in-depth way
This pull requests adds support for pulling interfaces up and down, and aligns the address addition semantics to Linux ones: create loopback with no addresses and add them on up if not already present, and remove IPv6 addresses on down unless keep_addr_on_down sysctl is set and they are not local/loopback.
The motivation is to make bwrap --unshare-net work (currently fails because gVisor starts loopback with 127.0.0.1 assigned and bwrap tries to add it without ignoring errors) and thus Claude Code and Codex with sandbox configurations that partially or fully deny network access.
I can squash the commits if approved, but it seems better to keep them separate for review.
Commit 1: adds support to set interfaces up/down. This is necessary so that we can add loopback addresses on up like Linux does and also aligns behavior in network namespaces to the Linux one of starting with loopback down, and generally provides support for ip link set up/down
Commit 2: extract helper to prepare for next commit
Commit 3: start loopback down with no addresses and add them on up if they are not present. This is the motivation of the PR, and makes bubblewrap --unshare-net work.
Commit 4: this removes IPv6 addresses on interface down, to align with Linux behavior (mysteriously, by default IPv6 addresses are removed but not IPv4 ones...)
Commit 5: adds support for the IPv6 "keep_addr_on_down" sysctl to not remove IPv6 addresses (except linklocal and loopback addresses). This is optional, but since the default Linux IPv6 stack behavior is kind of absurd (why would one automatically remove addresses on down, especially given IPv4 doesn't...), and previously gVisor didn't do this, this sysctl allows to restore saner behavior. It only supports the all/ version since gVisor currently has no per-netdev sysctls. This is probably the commit that is most likely to need refactoring to adjust to gVisor conventions.