Skip to content

Commit

Permalink
Allow --ip-range ending on a 64-bit boundary
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
robmry committed Aug 12, 2024
1 parent 7650efe commit 496b457
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 1 deletion.
103 changes: 103 additions & 0 deletions integration/network/bridge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
})
}
}
16 changes: 15 additions & 1 deletion libnetwork/ipams/defaultipam/allocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package defaultipam
import (
"context"
"fmt"
"math"
"net"
"net/netip"

Expand Down Expand Up @@ -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--
}

Expand Down Expand Up @@ -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)
}

Expand Down

0 comments on commit 496b457

Please sign in to comment.