Only set up an L3-ipvlan's default route when it's the gateway endpoint #49130
Conversation
robmry
force-pushed
the
48576_connected_default_routes
branch
2 times, most recently
from
905edd1 to
9d9c1f8
Compare
robmry
added
area/networking
robmry
force-pushed
the
48576_connected_default_routes
branch
from
9d9c1f8 to
19b6dad
Compare
| gw4, gw6 := ep.hasGatewayOrDefaultRoute() | ||
| if gw4 && gw6 { | ||
| return ep, ep |
There was a problem hiding this comment.
In this branch, we unconditionally return ep, but the branches below, we only return ep if ep4 or ep6 is nil; that's correct?
There was a problem hiding this comment.
Not changed in this PR, but yes - so that the first dual-stack network in the list is selected ... if there are only single-stack networks, it's searching for the first IPv4 and the first IPv6 endpoints.
Now gateway-priority is plumbed in, it's probably surprising that a dual-stack endpoints take priority over single stack endpoints with higher priorities. (But that's unrelated to this change.)
I'll add comments to the code saying that.
There was a problem hiding this comment.
Thanks! Yes, it was mostly an observation, and I wasn't sure if intentional or not ❤️
libnetwork/sandbox_linux.go
Outdated
| } | ||
| } else { | ||
| if err := osSbox.SetDefaultRouteIPv4(ep4.iface.srcName); err != nil { | ||
| return fmt.Errorf("failed to set IPv4 default route while updating gateway: %v", err) |
There was a problem hiding this comment.
Wondering if (part of) decorating the error should be in SetDefaultRoute and SetGateway, in that case, the caller of this Sandbox,updateGateway could just use a general decoration, instead of having to repeat the "while updating gateway" everywhere 🤔;
if err := sb.updateGateway(gwepAfter4, gwepAfter6); err != nil {
return fmt.Errorf("updating gateway: %w", err)
}| import ( | ||
| "fmt" | ||
| "net" | ||
| "slices" |
There was a problem hiding this comment.
can you add a //go:build in this file? Similar to
Lines 1 to 3 in ca0e6af
| // set up when the ipvlan is selected as the gateway endpoint. | ||
| // Regression test for https://github.com/moby/moby/issues/48576 | ||
| func TestMixL3IPVlanAndBridge(t *testing.T) { | ||
| skip.If(t, testEnv.DaemonInfo.OSType == "windows", "no ipvlan on Windows") |
There was a problem hiding this comment.
Perhaps (not for this PR) we should add a utility to check whether the daemon supports network-driver X; docker info should return this, so we could use that for detecting instead of based on platform;
docker info --format='{{ json .Plugins.Network }}'
["bridge","host","ipvlan","macvlan","null","overlay"]
Signed-off-by: Rob Murray <rob.murray@docker.com>
In L3 modes, the ipvlan driver can't set up a default gateway with a next hop address, because there's no L2 for it to resolve the gateway IP into a MAC address. Instead, it sets up a route to 0.0.0.0 or [::] that's connected to the network's interface. The end result is the same - the container has a default route. So, include those routes when searching for endpoints that can act as a container's default gateway. Signed-off-by: Rob Murray <rob.murray@docker.com>
In L3 modes, the ipvlan driver can't set up a default gateway with a next hop address, because there's no L2 for it to resolve the gateway IP into a MAC address. Instead, it sets up a route to 0.0.0.0 or [::] that's connected to the network's interface. The end result is the same - the container has a default route. So, don't set up routes to 0.0.0.0/:: when applying routes when an endpoint joins a sandbox, set them up when the endpoint is selected as the container's gateway. And, drop those routes when another endpoint becomes the gateway. Signed-off-by: Rob Murray <rob.murray@docker.com>
Check that when a container has endpoints in an l3-ipvlan and another network type (bridge), there's no longer any clash betwen the ipvlan's connected default route and the bridge's default gateway. Signed-off-by: Rob Murray <rob.murray@docker.com>
robmry
force-pushed
the
48576_connected_default_routes
branch
from
19b6dad to
35fcbc1
Compare
3 participants
- What I did
It wasn't possible to connect a container to a layer-3 ipvlan network as well as a bridge networks ... because they both had their own ideas about how to set up a default route.
See #48576 (comment) for the analysis.
- How I did it
getGatewayEndpointincludesroutesas well as gateways in its search.- How to verify it
New integration test.
- Description for the changelog
- fixed an issue that meant a container could not be attached to an L3 ipvlan at the same time as other network types