Browse code

bridge/macvlan endpoints always use a random MAC address

Now a gratuitous/unsolicted ARP is sent, there's no need to
use an IPv4-based MAC address to preserve arp-cache mappings
between an endpoint's IP addresses and its MAC addresses.

Because a random MAC address is used for the default bridge,
it no longer makes sense to derive container IPv6 addresses
from the MAC address. This "postIPv6" behaviour was needed
before IPv6 addresses could be configured, but not now. So,
IPv6 addresses will now be IPAM-allocated on the default
bridge network, just as they are for user-defined bridges.

Signed-off-by: Rob Murray <rob.murray@docker.com>

Rob Murray authored on 2024/07/29 20:17:06
Showing 10 changed files
... ...
@@ -988,30 +988,12 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig
988 988
 		return err
989 989
 	}
990 990
 
991
-	var deferIPv6Alloc bool
992 991
 	var ipamV6Conf []*libnetwork.IpamConf
993 992
 	if cfg.EnableIPv6 {
994 993
 		ipamV6Conf, err = getDefaultBridgeIPAMConf(bridgeName, userManagedBridge, defBrOptsV6{cfg})
995 994
 		if err != nil {
996 995
 			return err
997 996
 		}
998
-		// If the subnet has at least 48 host bits, preserve the legacy default bridge
999
-		// behaviour of constructing a MAC address from the IPv4 address, then
1000
-		// constructing an IPv6 addresses based on that MAC address. Tell libnetwork to
1001
-		// defer the IPv6 address allocation for endpoints on this network until after
1002
-		// the driver has created the endpoint and proposed an IPv4 address. Libnetwork
1003
-		// will then reserve this address with the ipam driver. If no preferred pool has
1004
-		// been set the built-in ULA prefix will be used, assume it has at-least 48-bits.
1005
-		if len(ipamV6Conf) == 0 || ipamV6Conf[0].PreferredPool == "" {
1006
-			deferIPv6Alloc = true
1007
-		} else {
1008
-			_, ppNet, err := net.ParseCIDR(ipamV6Conf[0].PreferredPool)
1009
-			if err != nil {
1010
-				return err
1011
-			}
1012
-			ones, _ := ppNet.Mask.Size()
1013
-			deferIPv6Alloc = ones <= 80
1014
-		}
1015 997
 	}
1016 998
 
1017 999
 	// Initialize default network on "bridge" with the same name
... ...
@@ -1020,7 +1002,7 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig
1020 1020
 		libnetwork.NetworkOptionEnableIPv6(cfg.EnableIPv6),
1021 1021
 		libnetwork.NetworkOptionDriverOpts(netOption),
1022 1022
 		libnetwork.NetworkOptionIpam("default", "", ipamV4Conf, ipamV6Conf, nil),
1023
-		libnetwork.NetworkOptionDeferIPv6Alloc(deferIPv6Alloc))
1023
+	)
1024 1024
 	if err != nil {
1025 1025
 		return fmt.Errorf(`error creating default %q network: %v`, network.NetworkBridge, err)
1026 1026
 	}
... ...
@@ -309,25 +309,6 @@ func (s *DockerDaemonSuite) TestDaemonIPv6FixedCIDR(c *testing.T) {
309 309
 	assert.Equal(c, strings.Trim(out, " \r\n'"), "2001:db8:2::100", "Container should have a global IPv6 gateway")
310 310
 }
311 311
 
312
-// TestDaemonIPv6FixedCIDRAndMac checks that when the daemon is started with ipv6 fixed CIDR
313
-// the running containers are given an IPv6 address derived from the MAC address and the ipv6 fixed CIDR
314
-func (s *DockerDaemonSuite) TestDaemonIPv6FixedCIDRAndMac(c *testing.T) {
315
-	// IPv6 setup is messing with local bridge address.
316
-	testRequires(c, testEnv.IsLocalDaemon)
317
-	// Delete the docker0 bridge if its left around from previous daemon. It has to be recreated with
318
-	// ipv6 enabled
319
-	deleteInterface(c, "docker0")
320
-
321
-	s.d.StartWithBusybox(testutil.GetContext(c), c, "--ipv6", "--fixed-cidr-v6=2001:db8:1::/64")
322
-
323
-	out, err := s.d.Cmd("run", "-d", "--name=ipv6test", "--mac-address", "AA:BB:CC:DD:EE:FF", "busybox", "top")
324
-	assert.NilError(c, err, out)
325
-
326
-	out, err = s.d.Cmd("inspect", "--format", "{{.NetworkSettings.Networks.bridge.GlobalIPv6Address}}", "ipv6test")
327
-	assert.NilError(c, err, out)
328
-	assert.Equal(c, strings.Trim(out, " \r\n'"), "2001:db8:1::aabb:ccdd:eeff")
329
-}
330
-
331 312
 // TestDaemonIPv6HostMode checks that when the running a container with
332 313
 // network=host the host ipv6 addresses are not removed
333 314
 func (s *DockerDaemonSuite) TestDaemonIPv6HostMode(c *testing.T) {
... ...
@@ -5,7 +5,6 @@ import (
5 5
 	"flag"
6 6
 	"fmt"
7 7
 	"net"
8
-	"net/netip"
9 8
 	"os/exec"
10 9
 	"regexp"
11 10
 	"strconv"
... ...
@@ -541,17 +540,6 @@ func TestDefaultBridgeIPv6(t *testing.T) {
541 541
 			inspect := container.Inspect(ctx, t, c, cID)
542 542
 			gIPv6 := inspect.NetworkSettings.Networks[networkName].GlobalIPv6Address
543 543
 
544
-			// The container's MAC and IPv6 addresses should be derived from the
545
-			// IPAM-allocated IPv4 address.
546
-			addr4, err := netip.ParseAddr(inspect.NetworkSettings.Networks[networkName].IPAddress)
547
-			assert.NilError(t, err)
548
-			mac, err := net.ParseMAC(inspect.NetworkSettings.Networks[networkName].MacAddress)
549
-			assert.NilError(t, err)
550
-			assert.Check(t, is.DeepEqual(addr4.AsSlice(), []byte(mac)[2:]))
551
-			addr6, err := netip.ParseAddr(gIPv6)
552
-			assert.NilError(t, err)
553
-			assert.Check(t, is.DeepEqual(addr4.AsSlice(), addr6.AsSlice()[12:]))
554
-
555 544
 			attachCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
556 545
 			defer cancel()
557 546
 			res := container.RunAttach(attachCtx, t, c,
... ...
@@ -1230,17 +1230,8 @@ func (d *driver) CreateEndpoint(ctx context.Context, nid, eid string, ifInfo dri
1230 1230
 	endpoint.addr = ifInfo.Address()
1231 1231
 	endpoint.addrv6 = ifInfo.AddressIPv6()
1232 1232
 
1233
-	// We assign a default MAC address derived from the IP address to make sure
1234
-	// that if a container is disconnected and reconnected in a short timeframe,
1235
-	// stale ARP entries will still point to the right container.
1236 1233
 	if endpoint.macAddress == nil {
1237
-		if endpoint.addr != nil {
1238
-			endpoint.macAddress = netutils.GenerateMACFromIP(endpoint.addr.IP)
1239
-		} else {
1240
-			// TODO(robmry) - generate unsolicited Neighbour Advertisement to
1241
-			//  associate this MAC address with the IPv6 address.
1242
-			endpoint.macAddress = netutils.GenerateRandomMAC()
1243
-		}
1234
+		endpoint.macAddress = netutils.GenerateRandomMAC()
1244 1235
 		if err := ifInfo.SetMacAddress(endpoint.macAddress); err != nil {
1245 1236
 			return err
1246 1237
 		}
... ...
@@ -1255,33 +1246,6 @@ func (d *driver) CreateEndpoint(ctx context.Context, nid, eid string, ifInfo dri
1255 1255
 		"ifi":        host.Attrs().Index,
1256 1256
 	}).Debug("bridge endpoint host link is up")
1257 1257
 
1258
-	// Generate a MAC-address-based IPv6 address, which may be used by the default
1259
-	// bridge network instead of an IPAM allocated address.
1260
-	if endpoint.addrv6 == nil && config.EnableIPv6 {
1261
-		var ip6 net.IP
1262
-		network := n.bridge.bridgeIPv6
1263
-		if config.AddressIPv6 != nil {
1264
-			network = config.AddressIPv6
1265
-		}
1266
-		// If there are fewer than 48 host-bits in the network, it's not possible to
1267
-		// generate an IPv6 address from the MAC address. The network size has already
1268
-		// been checked, so we can safely assume this endpoint must not need a MAC-based
1269
-		// IPv6 address.
1270
-		ones, _ := network.Mask.Size()
1271
-		if ones <= 80 {
1272
-			ip6 = make(net.IP, len(network.IP))
1273
-			copy(ip6, network.IP)
1274
-			for i, h := range endpoint.macAddress {
1275
-				ip6[i+10] = h
1276
-			}
1277
-
1278
-			endpoint.addrv6 = &net.IPNet{IP: ip6, Mask: network.Mask}
1279
-			if err = ifInfo.SetIPAddress(endpoint.addrv6); err != nil {
1280
-				return err
1281
-			}
1282
-		}
1283
-	}
1284
-
1285 1258
 	if err = d.storeUpdate(ctx, endpoint); err != nil {
1286 1259
 		return fmt.Errorf("failed to save bridge endpoint %.7s to store: %v", endpoint.id, err)
1287 1260
 	}
... ...
@@ -403,21 +403,19 @@ func TestCreateFullOptionsLabels(t *testing.T) {
403 403
 		t.Fatalf("Unexpected: %v", nw.config.DefaultGatewayIPv6)
404 404
 	}
405 405
 
406
-	// In short here we are testing --fixed-cidr-v6 daemon option
407
-	// plus --mac-address run option
408
-	te := newTestEndpoint(ipdList[0].Pool, 20)
409
-	te.iface.mac = netutils.MustParseMAC("aa:bb:cc:dd:ee:ff")
410
-	err = d.CreateEndpoint(context.Background(), "dummy", "ep1", te.Interface(), map[string]interface{}{})
411
-	if err != nil {
412
-		t.Fatal(err)
413
-	}
406
+	// Check that a MAC address is generated if not already configured.
407
+	te1 := newTestEndpoint(ipdList[0].Pool, 20)
408
+	err = d.CreateEndpoint(context.Background(), "dummy", "ep1", te1.Interface(), map[string]interface{}{})
409
+	assert.NilError(t, err)
410
+	assert.Check(t, is.Len(te1.iface.mac, 6))
414 411
 
415
-	if !nwV6.Contains(te.Interface().AddressIPv6().IP) {
416
-		t.Fatalf("endpoint got assigned address outside of container network(%s): %s", nwV6.String(), te.Interface().AddressIPv6())
417
-	}
418
-	if te.Interface().AddressIPv6().IP.String() != "2001:db8:2600:2700:2800:aabb:ccdd:eeff" {
419
-		t.Fatalf("Unexpected endpoint IPv6 address: %v", te.Interface().AddressIPv6().IP)
420
-	}
412
+	// Check that a configured --mac-address isn't overwritten by a generated address.
413
+	te2 := newTestEndpoint(ipdList[0].Pool, 20)
414
+	const macAddr = "aa:bb:cc:dd:ee:ff"
415
+	te2.iface.mac = netutils.MustParseMAC(macAddr)
416
+	err = d.CreateEndpoint(context.Background(), "dummy", "ep2", te2.Interface(), map[string]interface{}{})
417
+	assert.NilError(t, err)
418
+	assert.Check(t, is.Equal(te2.iface.mac.String(), macAddr))
421 419
 }
422 420
 
423 421
 func TestCreate(t *testing.T) {
... ...
@@ -580,6 +578,20 @@ func newTestEndpoint(nw *net.IPNet, ordinal byte) *testEndpoint {
580 580
 	return &testEndpoint{iface: &testInterface{addr: addr}}
581 581
 }
582 582
 
583
+// newTestEndpoint46 is like newTestEndpoint, but assigns an IPv6 address as well as IPv4.
584
+func newTestEndpoint46(nw4, nw6 *net.IPNet, ordinal byte) *testEndpoint {
585
+	addr4 := types.GetIPNetCopy(nw4)
586
+	addr4.IP[len(addr4.IP)-1] = ordinal
587
+	addr6 := types.GetIPNetCopy(nw6)
588
+	addr6.IP[len(addr6.IP)-1] = ordinal
589
+	return &testEndpoint{
590
+		iface: &testInterface{
591
+			addr:   addr4,
592
+			addrv6: addr6,
593
+		},
594
+	}
595
+}
596
+
583 597
 func (te *testEndpoint) Interface() *testInterface {
584 598
 	return te.iface
585 599
 }
... ...
@@ -30,12 +30,12 @@ func TestLinkCreate(t *testing.T) {
30 30
 	}
31 31
 
32 32
 	ipdList := getIPv4Data(t)
33
-	err := d.CreateNetwork("dummy", option, nil, ipdList, getIPv6Data(t))
33
+	ipd6List := getIPv6Data(t)
34
+	err := d.CreateNetwork("dummy", option, nil, ipdList, ipd6List)
34 35
 	if err != nil {
35 36
 		t.Fatalf("Failed to create bridge: %v", err)
36 37
 	}
37
-
38
-	te := newTestEndpoint(ipdList[0].Pool, 10)
38
+	te := newTestEndpoint46(ipdList[0].Pool, ipd6List[0].Pool, 10)
39 39
 	err = d.CreateEndpoint(context.Background(), "dummy", "", te.Interface(), nil)
40 40
 	if err != nil {
41 41
 		if _, ok := err.(InvalidEndpointIDError); !ok {
... ...
@@ -141,12 +141,13 @@ func TestPortMappingV6Config(t *testing.T) {
141 141
 	netOptions[netlabel.GenericData] = netConfig
142 142
 
143 143
 	ipdList4 := getIPv4Data(t)
144
-	err := d.CreateNetwork("dummy", netOptions, nil, ipdList4, getIPv6Data(t))
144
+	ipdList6 := getIPv6Data(t)
145
+	err := d.CreateNetwork("dummy", netOptions, nil, ipdList4, ipdList6)
145 146
 	if err != nil {
146 147
 		t.Fatalf("Failed to create bridge: %v", err)
147 148
 	}
148 149
 
149
-	te := newTestEndpoint(ipdList4[0].Pool, 11)
150
+	te := newTestEndpoint46(ipdList4[0].Pool, ipdList6[0].Pool, 11)
150 151
 	err = d.CreateEndpoint(context.Background(), "dummy", "ep1", te.Interface(), nil)
151 152
 	if err != nil {
152 153
 		t.Fatalf("Failed to create the endpoint: %s", err.Error())
... ...
@@ -32,13 +32,7 @@ func (d *driver) CreateEndpoint(ctx context.Context, nid, eid string, ifInfo dri
32 32
 		mac:    ifInfo.MacAddress(),
33 33
 	}
34 34
 	if ep.mac == nil {
35
-		if ep.addr != nil {
36
-			ep.mac = netutils.GenerateMACFromIP(ep.addr.IP)
37
-		} else {
38
-			// TODO(robmry) - generate an unsolicited Neighbor Advertisement to
39
-			//  associate this MAC address with the IP.
40
-			ep.mac = netutils.GenerateRandomMAC()
41
-		}
35
+		ep.mac = netutils.GenerateRandomMAC()
42 36
 		if err := ifInfo.SetMacAddress(ep.mac); err != nil {
43 37
 			return err
44 38
 		}
... ...
@@ -1,7 +1,6 @@
1 1
 package libnetwork_test
2 2
 
3 3
 import (
4
-	"bytes"
5 4
 	"context"
6 5
 	"encoding/json"
7 6
 	"fmt"
... ...
@@ -1418,59 +1417,6 @@ func TestHost(t *testing.T) {
1418 1418
 	}
1419 1419
 }
1420 1420
 
1421
-// Testing IPV6 from MAC address
1422
-func TestBridgeIpv6FromMac(t *testing.T) {
1423
-	defer netnsutils.SetupTestOSContext(t)()
1424
-	controller := newController(t)
1425
-
1426
-	netOption := options.Generic{
1427
-		netlabel.GenericData: map[string]string{
1428
-			bridge.BridgeName:         "testipv6mac",
1429
-			bridge.EnableICC:          "true",
1430
-			bridge.EnableIPMasquerade: "true",
1431
-		},
1432
-	}
1433
-	ipamV4ConfList := []*libnetwork.IpamConf{{PreferredPool: "192.168.100.0/24", Gateway: "192.168.100.1"}}
1434
-	ipamV6ConfList := []*libnetwork.IpamConf{{PreferredPool: "fe90::/64", Gateway: "fe90::22"}}
1435
-
1436
-	network, err := controller.NewNetwork(bridgeNetType, "testipv6mac", "",
1437
-		libnetwork.NetworkOptionGeneric(netOption),
1438
-		libnetwork.NetworkOptionEnableIPv4(true),
1439
-		libnetwork.NetworkOptionEnableIPv6(true),
1440
-		libnetwork.NetworkOptionIpam(defaultipam.DriverName, "", ipamV4ConfList, ipamV6ConfList, nil),
1441
-		libnetwork.NetworkOptionDeferIPv6Alloc(true))
1442
-	if err != nil {
1443
-		t.Fatal(err)
1444
-	}
1445
-
1446
-	mac := net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}
1447
-	epOption := options.Generic{netlabel.MacAddress: mac}
1448
-
1449
-	ep, err := network.CreateEndpoint(context.Background(), "testep", libnetwork.EndpointOptionGeneric(epOption))
1450
-	if err != nil {
1451
-		t.Fatal(err)
1452
-	}
1453
-
1454
-	iface := ep.Info().Iface()
1455
-	if !bytes.Equal(iface.MacAddress(), mac) {
1456
-		t.Fatalf("Unexpected mac address: %v", iface.MacAddress())
1457
-	}
1458
-
1459
-	ip, expIP, _ := net.ParseCIDR("fe90::aabb:ccdd:eeff/64")
1460
-	expIP.IP = ip
1461
-	if !types.CompareIPNet(expIP, iface.AddressIPv6()) {
1462
-		t.Fatalf("Expected %v. Got: %v", expIP, iface.AddressIPv6())
1463
-	}
1464
-
1465
-	if err := ep.Delete(context.Background(), false); err != nil {
1466
-		t.Fatal(err)
1467
-	}
1468
-
1469
-	if err := network.Delete(); err != nil {
1470
-		t.Fatal(err)
1471
-	}
1472
-}
1473
-
1474 1421
 func checkSandbox(t *testing.T, info libnetwork.EndpointInfo) {
1475 1422
 	key := info.Sandbox().Key()
1476 1423
 	sbNs, err := netns.GetFromPath(key)
... ...
@@ -1513,7 +1459,7 @@ func TestEndpointJoin(t *testing.T) {
1513 1513
 		libnetwork.NetworkOptionEnableIPv4(true),
1514 1514
 		libnetwork.NetworkOptionEnableIPv6(true),
1515 1515
 		libnetwork.NetworkOptionIpam(defaultipam.DriverName, "", nil, ipamV6ConfList, nil),
1516
-		libnetwork.NetworkOptionDeferIPv6Alloc(true))
1516
+	)
1517 1517
 	if err != nil {
1518 1518
 		t.Fatal(err)
1519 1519
 	}
... ...
@@ -189,7 +189,6 @@ type Network struct {
189 189
 	ipamV6Info       []*IpamInfo
190 190
 	enableIPv4       bool
191 191
 	enableIPv6       bool
192
-	postIPv6         bool
193 192
 	epCnt            *endpointCnt
194 193
 	generic          options.Generic
195 194
 	dbIndex          uint64
... ...
@@ -461,7 +460,6 @@ func (n *Network) CopyTo(o datastore.KVObject) error {
461 461
 	dstN.enableIPv4 = n.enableIPv4
462 462
 	dstN.enableIPv6 = n.enableIPv6
463 463
 	dstN.persist = n.persist
464
-	dstN.postIPv6 = n.postIPv6
465 464
 	dstN.dbIndex = n.dbIndex
466 465
 	dstN.dbExists = n.dbExists
467 466
 	dstN.drvOnce = n.drvOnce
... ...
@@ -585,7 +583,6 @@ func (n *Network) MarshalJSON() ([]byte, error) {
585 585
 		netMap["generic"] = n.generic
586 586
 	}
587 587
 	netMap["persist"] = n.persist
588
-	netMap["postIPv6"] = n.postIPv6
589 588
 	if len(n.ipamV4Config) > 0 {
590 589
 		ics, err := json.Marshal(n.ipamV4Config)
591 590
 		if err != nil {
... ...
@@ -688,9 +685,6 @@ func (n *Network) UnmarshalJSON(b []byte) (err error) {
688 688
 	if v, ok := netMap["persist"]; ok {
689 689
 		n.persist = v.(bool)
690 690
 	}
691
-	if v, ok := netMap["postIPv6"]; ok {
692
-		n.postIPv6 = v.(bool)
693
-	}
694 691
 	if v, ok := netMap["ipamType"]; ok {
695 692
 		n.ipamType = v.(string)
696 693
 	} else {
... ...
@@ -898,16 +892,6 @@ func NetworkOptionDynamic() NetworkOption {
898 898
 	}
899 899
 }
900 900
 
901
-// NetworkOptionDeferIPv6Alloc instructs the network to defer the IPV6 address allocation until after the endpoint has been created
902
-// It is being provided to support the specific docker daemon flags where user can deterministically assign an IPv6 address
903
-// to a container as combination of fixed-cidr-v6 + mac-address
904
-// TODO: Remove this option setter once we support endpoint ipam options
905
-func NetworkOptionDeferIPv6Alloc(enable bool) NetworkOption {
906
-	return func(n *Network) {
907
-		n.postIPv6 = enable
908
-	}
909
-}
910
-
911 901
 // NetworkOptionConfigOnly tells controller this network is
912 902
 // a configuration only network. It serves as a configuration
913 903
 // for other networks.
... ...
@@ -1256,7 +1240,7 @@ func (n *Network) createEndpoint(ctx context.Context, name string, options ...En
1256 1256
 
1257 1257
 	wantIPv6 := n.enableIPv6 && !ep.disableIPv6
1258 1258
 
1259
-	if err = ep.assignAddress(ipam, n.enableIPv4, wantIPv6 && !n.postIPv6); err != nil {
1259
+	if err = ep.assignAddress(ipam, n.enableIPv4, wantIPv6); err != nil {
1260 1260
 		return nil, err
1261 1261
 	}
1262 1262
 	defer func() {
... ...
@@ -1289,14 +1273,6 @@ func (n *Network) createEndpoint(ctx context.Context, name string, options ...En
1289 1289
 		}
1290 1290
 	}()
1291 1291
 
1292
-	if wantIPv6 {
1293
-		if err = ep.assignAddress(ipam, false, n.postIPv6); err != nil {
1294
-			return nil, err
1295
-		}
1296
-	} else {
1297
-		ep.iface.addrv6 = nil
1298
-	}
1299
-
1300 1292
 	if !n.getController().isSwarmNode() || n.Scope() != scope.Swarm || !n.driverIsMultihost() {
1301 1293
 		n.updateSvcRecord(context.WithoutCancel(ctx), ep, true)
1302 1294
 		defer func() {