From 496b457ad86ffde9a9e28d2e9a07cb7de79df913 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Sun, 11 Aug 2024 11:49:55 +0100 Subject: [PATCH] Allow --ip-range ending on a 64-bit boundary When defaultipam.newPoolData is asked for a pool of 64-bits or more, it ends up with an overflowed u64 - so, it just subtracts one to get a nearly-big-enough range (for a 64-bit subnet). When defaultipam.getAddress is called with an ipr (sub-pool range), the range it calls bitmask.SetAnyInRange with is exclusive of end. So, its end param can't be MaxUint64, because that's the max value for the top end of the range and, when checking the range, SetAnyInRange fails. When fixed-cidr-v6 behaves more like fixed-cidr, it will ask for a 64-bit range if that's what fixed-cidr-v6 needs. So, it hits the bug when allocating an address for, for example: docker network create --ipv6 --subnet fddd::/64 --ip-range fddd::/64 b46 The additional check for "ipr == base" avoids the issue in this case, by ignoring the ipr/sub-pool range if ipr is the same as the pool itself (not really a sub-pool). But, it still fails when ipr!=base. For example: docker network create --ipv6 --subnet fddd::/56 --ip-range fddd::/64 b46 So, also subtract one from 'end' if it's going to hit the max value allowed by the Bitmap. Signed-off-by: Rob Murray --- integration/network/bridge_test.go | 103 ++++++++++++++++++++++ libnetwork/ipams/defaultipam/allocator.go | 16 +++- 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/integration/network/bridge_test.go b/integration/network/bridge_test.go index c71f6639d1ddc..e0c45ad67e625 100644 --- a/integration/network/bridge_test.go +++ b/integration/network/bridge_test.go @@ -7,10 +7,12 @@ import ( "testing" "time" + containertypes "github.com/docker/docker/api/types/container" networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/versions" ctr "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/integration/internal/network" + "github.com/docker/docker/testutil" "github.com/docker/docker/testutil/daemon" "gotest.tools/v3/assert" "gotest.tools/v3/skip" @@ -97,3 +99,104 @@ func TestCreateWithIPv6WithoutEnableIPv6Flag(t *testing.T) { t.Fatalf("Network %s has no ULA prefix, expected one.", nwName) } + +// Check that it's possible to create IPv6 networks with a 64-bit ip-range, +// in 64-bit and bigger subnets, with and without a gateway. +func Test64BitIPRange(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType == "windows", "no bridge or IPv6 on Windows") + ctx := setupTest(t) + c := testEnv.APIClient() + + type kv struct{ k, v string } + subnets := []kv{ + {"64-bit-subnet", "fd2e:b68c:ce26::/64"}, + {"56-bit-subnet", "fd2e:b68c:ce26::/56"}, + } + ipRanges := []kv{ + {"no-range", ""}, + {"64-bit-range", "fd2e:b68c:ce26::/64"}, + } + gateways := []kv{ + {"no-gateway", ""}, + {"with-gateway", "fd2e:b68c:ce26::1"}, + } + + for _, sn := range subnets { + for _, ipr := range ipRanges { + for _, gw := range gateways { + ipamSetter := network.WithIPAMRange(sn.v, ipr.v, gw.v) + t.Run(sn.k+"/"+ipr.k+"/"+gw.k, func(t *testing.T) { + ctx := testutil.StartSpan(ctx, t) + const netName = "test64br" + network.CreateNoError(ctx, t, c, netName, network.WithIPv6(), ipamSetter) + defer network.RemoveNoError(ctx, t, c, netName) + }) + } + } + } +} + +// Demonstrate a limitation of the IP address allocator, it can't +// allocate the last address in range that ends on a 64-bit boundary. +func TestIPRangeAt64BitLimit(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType == "windows", "no bridge or IPv6 on Windows") + ctx := setupTest(t) + c := testEnv.APIClient() + + testcases := []struct { + name string + subnet string + ipRange string + expCtrFail bool + }{ + { + name: "ipRange before end of 64-bit subnet", + subnet: "fda9:8d04:086e::/64", + ipRange: "fda9:8d04:086e::ffff:ffff:ffff:ff0e/127", + }, + { + name: "ipRange at end of 64-bit subnet", + subnet: "fda9:8d04:086e::/64", + ipRange: "fda9:8d04:086e::ffff:ffff:ffff:fffe/127", + // FIXME(robmry) - there should be two addresses available for + // allocation, just like the previous test. One for the gateway + // and one for the container. But, because the Bitmap in the + // allocator can't cope with a range that includes MaxUint64, + // only one address is currently available - so the container + // will not start. + expCtrFail: true, + }, + { + name: "ipRange at 64-bit boundary inside 56-bit subnet", + subnet: "fda9:8d04:086e::/56", + ipRange: "fda9:8d04:086e:aa:ffff:ffff:ffff:fffe/127", + // FIXME(robmry) - same issue as above, but this time the ip-range + // is in the middle of the subnet (on a 64-bit boundary) rather + // than at the top end. + expCtrFail: true, + }, + } + + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + ctx := testutil.StartSpan(ctx, t) + const netName = "test64bl" + network.CreateNoError(ctx, t, c, netName, + network.WithIPv6(), + network.WithIPAMRange(tc.subnet, tc.ipRange, ""), + ) + defer network.RemoveNoError(ctx, t, c, netName) + + id := ctr.Create(ctx, t, c, ctr.WithNetworkMode(netName)) + defer c.ContainerRemove(ctx, id, containertypes.RemoveOptions{Force: true}) + err := c.ContainerStart(ctx, id, containertypes.StartOptions{}) + if tc.expCtrFail { + assert.Assert(t, err != nil) + t.Skipf("XFAIL: Container startup failed with error: %v", err) + } else { + assert.NilError(t, err) + } + }) + } +} diff --git a/libnetwork/ipams/defaultipam/allocator.go b/libnetwork/ipams/defaultipam/allocator.go index 25d82ca587278..6725ac4c174a0 100644 --- a/libnetwork/ipams/defaultipam/allocator.go +++ b/libnetwork/ipams/defaultipam/allocator.go @@ -3,6 +3,7 @@ package defaultipam import ( "context" "fmt" + "math" "net" "net/netip" @@ -215,6 +216,7 @@ func newPoolData(pool netip.Prefix) *PoolData { // Allow /64 subnet if pool.Addr().Is6() && numAddresses == 0 { + // FIXME(robmry) - see the comment in getAddress numAddresses-- } @@ -298,13 +300,25 @@ func getAddress(base netip.Prefix, bitmask *bitmap.Bitmap, prefAddress netip.Add if bitmask.Unselected() == 0 { return netip.Addr{}, ipamapi.ErrNoAvailableIPs } - if ipr == (netip.Prefix{}) && prefAddress == (netip.Addr{}) { + if (ipr == (netip.Prefix{}) || ipr == base) && prefAddress == (netip.Addr{}) { ordinal, err = bitmask.SetAny(serial) } else if prefAddress != (netip.Addr{}) { ordinal = netiputil.HostID(prefAddress, uint(base.Bits())) err = bitmask.Set(ordinal) } else { start, end := netiputil.SubnetRange(base, ipr) + if end == math.MaxUint64 { + // FIXME(robmry) ... + // When newPoolData is asked for a range of 64-bits or more, it ends up with an + // overflowed u64 - so, it just subtracts one to get a nearly-big-enough range + // (for a 64-bit subnet). But, here, the range is exclusive of end. So, when + // checking the range, SetAnyInRange fails if it's asked for any bit in the whole + // of a 64-bit range. For now, just subtract one here. To see the problem, + // remove this workaround, then try: + // docker network create --ipv6 --subnet fddd::/56 --ip-range fddd::/64 b46 + // See Test64BitIPRange and TestIPRangeAt64BitLimit. + end -= 1 + } ordinal, err = bitmask.SetAnyInRange(start, end, serial) }