Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

splitNetwork and addIntToIP fails for IPv6 addresses on /64+ size pools #42801

Closed
aaliddell opened this issue Aug 30, 2021 · 3 comments · Fixed by #47768
Closed

splitNetwork and addIntToIP fails for IPv6 addresses on /64+ size pools #42801

aaliddell opened this issue Aug 30, 2021 · 3 comments · Fixed by #47768
Labels
area/networking kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed.

Comments

@aaliddell
Copy link

aaliddell commented Aug 30, 2021

Description

When attempting to create the pools for IPv6 addresses ranges, the output of splitNetwork produces the same CIDR N times rather than the N different CIDRs if the output network size is >= 64 bits. This is due to the value passed to addIntToIP being a 64 bit int, which is insufficient to contain the required value to offset IPv6 ranges.

Steps to reproduce the issue:

  1. Add an IPv6 range to default-address-pools in config and enable IPv6.

For example:

   "default-address-pools": [
      < IPv4 pools >
      {
        "base": "fd00:0000:0000::/48",
        "size": 64
      }
    ]
  1. Attempt to create two IPv6 networks, the second will fail with could not find an available, non-overlapping IPv6 address pool among the defaults to assign to the network, suggesting there is only one pool available

Describe the results you received:

Only a single IPv6 pool is available per entry in default-address-pools

Describe the results you expected:

The output of splitting fd00:0000:0000::/48 into /64 networks should yield 2^16 pools, not one.

Additional information you deem important (e.g. issue happens only occasionally):

This is caused by splitNetwork and addIntToIP, which works OK for IPv4 but fails for IPv6 networks of /64 or larger due to the ordinal argument to addIntToIP being a 64 bit int, which ends up being zero when using a /64 or larger. Crucially, the statement uint(i<<s) yields zero here, since s > 64

For example, fmt.Println(splitNetworks([]*NetworkToSplit{{"fd00:0000:0000::/48", 50}})) yields the same CIDR repeated 4 times rather than 4 separate CIDRs:

[fd00::/50 fd00::/50 fd00::/50 fd00::/50]

To fix this, those two functions need to be amended to allow values up to 128 bits to be added to an IP, perhaps with an ordinalUpper and ordinalLower as two 64 bit uints.

I believe this issue may be the underlying cause of #41438

Output of docker version:

Client:
 Version:           18.09.1
 API version:       1.39
 Go version:        go1.11.6
 Git commit:        4c52b90
 Built:             Sun, 21 Feb 2021 18:18:35 +0100
 OS/Arch:           linux/amd64
 Experimental:      false

Server:
 Engine:
  Version:          18.09.1
  API version:      1.39 (minimum version 1.12)
  Go version:       go1.11.6
  Git commit:       4c52b90
  Built:            Sun Feb 21 17:18:35 2021
  OS/Arch:          linux/amd64
  Experimental:     false

Output of docker info:

Containers: 27
 Running: 27
 Paused: 0
 Stopped: 0
Images: 31
Server Version: 18.09.1
Storage Driver: overlay2
 Backing Filesystem: extfs
 Supports d_type: true
 Native Overlay Diff: true
Logging Driver: journald
Cgroup Driver: cgroupfs
Plugins:
 Volume: local
 Network: bridge host ipvlan macvlan null overlay
 Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
Swarm: inactive
Runtimes: runc
Default Runtime: runc
Init Binary: docker-init
containerd version: 9754871865f7fe2f4e74d43e2fc7ccd237edcbce
runc version: 1.0.0~rc6+dfsg1-3
init version: v0.18.0 (expected: fec3683b971d9c3ef73f284f176672c44b448662)
Security Options:
 apparmor
 seccomp
  Profile: default
Kernel Version: 4.19.0-14-amd64
Operating System: Debian GNU/Linux 10 (buster)
OSType: linux
Architecture: x86_64
CPUs: 4
Total Memory: 11.4GiB
Name: 
ID: GXW4:VDGE:BS4J:AY3S:5DHN:COS3:TCUC:JOBN:KRPG:V6MW:YO37:A2GO
Docker Root Dir: /var/lib/docker
Debug Mode (client): false
Debug Mode (server): true
 File Descriptors: 191
 Goroutines: 192
 System Time: 2021-08-30T22:44:47.757633368Z
 EventsListeners: 0
Registry: https://index.docker.io/v1/
Labels:
Experimental: false
Insecure Registries:
 127.0.0.0/8
Live Restore Enabled: false

Additional environment details (AWS, VirtualBox, physical, etc.):
None

@thaJeztah thaJeztah added area/networking kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed. labels Aug 31, 2021
@aaliddell
Copy link
Author

Here's some checks I ran to find this, which might be helpful for tests:

fd00:0000:0000::/63 split into /65: [fd00::/65 fd00::8000:0:0:0/65 fd00:0:0:1::/65 fd00:0:0:1:8000::/65
fd00:0000:0000::/48 split into /50: [fd00::/50 fd00:0:0:4000::/50 fd00:0:0:8000::/50 fd00:0:0:c000::/50]
fd00:0000:0000::/80 split into /82: [fd00::/82 fd00::4000:0:0/82 fd00::8000:0:0/82 fd00::c000:0:0/82]

This may also be an opportunity to address #40275, where using a large base pool and a large split size leads to generating huge amounts of output from splitNetwork and potentially consuming all RAM when preallocating the pools. Either limiting the number of pools directly or limiting the difference between base size and pool size. e.g. /48 split into /64s is allowed, but /48 split into /96s is not.

@Xyaren
Copy link

Xyaren commented Oct 30, 2021

This seems to be quite a big issue for me and is blocking me from using ipv6 auto created networks right now.

akerouanton added a commit to akerouanton/docker that referenced this issue Nov 15, 2021
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, a new type offset has been introduced and is used
by NetworkSplitter. It uses two uint64 to represents an unsigned int of
128 bits. Moreover, addIntToIP has been changed to take two uint64 (one
upper and one lower addend) and computes two ordinals applied to,
respectively, the lower 64 bits and the upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upper is not used).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Nov 15, 2021
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, a new type offset has been introduced and is used
by NetworkSplitter. It uses two uint64 to represents an unsigned int of
128 bits. Moreover, addIntToIP has been changed to take two uint64 (one
upper and one lower addend) and computes two ordinals applied to,
respectively, the lower 64 bits and the upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upper is not used).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Nov 19, 2021
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, addIntToIP has been changed to take an addend and
computes two ordinals applied to respectively the lower 64 bits and the
upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upperOrdinal is always 0).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Jun 3, 2022
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, addIntToIP has been changed to take an addend and
computes two ordinals applied to respectively the lower 64 bits and the
upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upperOrdinal is always 0).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Jun 3, 2022
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, addIntToIP has been changed to take an addend and
computes two ordinals applied to respectively the lower 64 bits and the
upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upperOrdinal is always 0).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Jun 11, 2022
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, addIntToIP has been changed to take an addend and
computes two ordinals applied to respectively the lower 64 bits and the
upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upperOrdinal is always 0).

Signed-off-by: Albin Kerouanton <[email protected]>
@gucki
Copy link

gucki commented Oct 11, 2022

This bug is more or less a complete show stopper when using docker-compose 👎

akerouanton added a commit to akerouanton/docker that referenced this issue Dec 23, 2022
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, addIntToIP has been changed to take an addend and
computes two ordinals applied to respectively the lower 64 bits and the
upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upperOrdinal is always 0).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Dec 23, 2022
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, addIntToIP has been changed to take an addend and
computes two ordinals applied to respectively the lower 64 bits and the
upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upperOrdinal is always 0).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Dec 27, 2022
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, addIntToIP has been changed to take an addend and
computes two ordinals applied to respectively the lower 64 bits and the
upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upperOrdinal is always 0).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Dec 27, 2022
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, addIntToIP has been changed to take an addend and
computes two ordinals applied to respectively the lower 64 bits and the
upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upperOrdinal is always 0).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Jan 9, 2023
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, addIntToIP has been changed to take an addend and
computes two ordinals applied to respectively the lower 64 bits and the
upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upperOrdinal is always 0).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Jan 9, 2023
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, addIntToIP has been changed to take an addend and
computes two ordinals applied to respectively the lower 64 bits and the
upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upperOrdinal is always 0).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Jan 9, 2023
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, addIntToIP has been changed to take an addend and
computes two ordinals applied to respectively the lower 64 bits and the
upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upperOrdinal is always 0).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Jan 12, 2023
Prior to this change, IPv6 subnets generation was broken because of a
bitwise shift overflowing uint64.

To solve this issue, addIntToIP has been changed to take an addend and
computes two ordinals applied to respectively the lower 64 bits and the
upper 64 bits of IPv6 addresses.

Nothing change for IPv4 (ie. upperOrdinal is always 0).

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Apr 11, 2023
A new Subnetter structure is added to lazily sub-divide an address pool
into subnets. This fixes moby#40275.

Prior to this change, the list of NetworkToSplit was eagerly split into
smaller subnets when ipamutils package was loaded, when
ConfigGlobalScopeDefaultNetworks was called or when the function
SetDefaultIPAddressPool from the default IPAM driver was called. In the
latter case, if the list of NetworkToSplit contained an IPv6 prefix,
eagerly enumerating all subnets could eat all the available memory. For
instance, fd00::/8 split into /96 would take ~5*10^27 bytes.

Although this change trades memory consumption for computation cost, the
Subnetter is used by libnetwork/ipam package in such a way that it
only have to compute the address of the next subnet. When
the Subnetter reach the end of NetworkToSplit, it's resetted by
libnetwork/ipam only if there were some subnets released beforehand. In
such case, ipam package might iterate over all the subnets before
finding one available.

Also, the Subnetter leverages the newly introduced ipbits package, which
handles IPv6 addresses correctly. Before this commit, a bitwise shift
was overflowing uint64 and thus only a single subnet could be enumerated
from an IPv6 prefix. This fixes moby#42801.

Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton added a commit to akerouanton/docker that referenced this issue Apr 11, 2023
A new Subnetter structure is added to lazily sub-divide an address pool
into subnets. This fixes moby#40275.

Prior to this change, the list of NetworkToSplit was eagerly split into
smaller subnets when ipamutils package was loaded, when
ConfigGlobalScopeDefaultNetworks was called or when the function
SetDefaultIPAddressPool from the default IPAM driver was called. In the
latter case, if the list of NetworkToSplit contained an IPv6 prefix,
eagerly enumerating all subnets could eat all the available memory. For
instance, fd00::/8 split into /96 would take ~5*10^27 bytes.

Although this change trades memory consumption for computation cost, the
Subnetter is used by libnetwork/ipam package in such a way that it
only have to compute the address of the next subnet. When
the Subnetter reach the end of NetworkToSplit, it's resetted by
libnetwork/ipam only if there were some subnets released beforehand. In
such case, ipam package might iterate over all the subnets before
finding one available.

Also, the Subnetter leverages the newly introduced ipbits package, which
handles IPv6 addresses correctly. Before this commit, a bitwise shift
was overflowing uint64 and thus only a single subnet could be enumerated
from an IPv6 prefix. This fixes moby#42801.

Signed-off-by: Albin Kerouanton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/networking kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed.
Projects
None yet
4 participants