From 3a938df4b570aad3bfb4d5342379582e872fc1a3 Mon Sep 17 00:00:00 2001
From: Mark Feit <mfeit@internet2.edu>
Date: Mon, 12 Jul 2021 21:37:36 +0000
Subject: [PATCH] Make the network allocator handle IPv4 blocks too small for
 network/broadcast addresses.

This was originally in docker/libnetwork#2624, which has been closed since the
code was moved here.

When creating a new network, IPAM's address allocator attempts to reserve the
network and broadcast addresses on IPv4 networks of all sizes. For RFC 3021
point-to-point networks (IPv4 /31s), this consumes both available addresses and
renders any attempt to allocate an address from the block unsuccessful.

This change prevents those reservations from taking place on IPv4 networks having
two or fewer addresses (i.e., /31s and /32s) while retaining the existing behavior
for larger IPv4 blocks and all IPv6 blocks.

In case you're wondering why anyone would allocate /31s:  I work for a network
service provider.  We use a lot of point-to-point networks.  This cuts our
address space utilization for those by 50%, which makes ARIN happy.

This patch modifies the network allocator to recognize when an network is too
small for network and broadcast addresses and skip those reservations.

There are additional unit tests to make sure the functions involved behave as expected.

Try these out:

 * `docker network create --driver bridge --subnet 10.200.1.0/31 --ip-range 10.200.1.0/31 test-31`
 * `docker network create --driver bridge --subnet 10.200.1.0/32 --ip-range 10.200.1.0/32 test-32`

My installation has been running this patch in production with /31s since March.

Signed-off-by: Mark Feit <mfeit@internet2.edu>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 libnetwork/ipam/allocator.go      | 13 +++---
 libnetwork/ipam/allocator_test.go | 66 +++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/libnetwork/ipam/allocator.go b/libnetwork/ipam/allocator.go
index d7ed917637..d2cb3dd485 100644
--- a/libnetwork/ipam/allocator.go
+++ b/libnetwork/ipam/allocator.go
@@ -348,12 +348,15 @@ func (a *Allocator) insertBitMask(key SubnetKey, pool *net.IPNet) error {
 		return err
 	}
 
-	// Do not let network identifier address be reserved
-	// Do the same for IPv6 so that bridge ip starts with XXXX...::1
-	h.Set(0)
+	// Pre-reserve the network address on IPv4 networks large
+	// enough to have one (i.e., anything bigger than a /31.
+	if !(ipVer == v4 && numAddresses <= 2) {
+		h.Set(0)
+	}
 
-	// Do not let broadcast address be reserved
-	if ipVer == v4 {
+	// Pre-reserve the broadcast address on IPv4 networks large
+	// enough to have one (i.e., anything bigger than a /31).
+	if ipVer == v4 && numAddresses > 2 {
 		h.Set(numAddresses - 1)
 	}
 
diff --git a/libnetwork/ipam/allocator_test.go b/libnetwork/ipam/allocator_test.go
index 8b05d32371..2c3150a636 100644
--- a/libnetwork/ipam/allocator_test.go
+++ b/libnetwork/ipam/allocator_test.go
@@ -1055,6 +1055,72 @@ func TestOverlappingRequests(t *testing.T) {
 	}
 }
 
+func TestUnusualSubnets(t *testing.T) {
+
+	subnet := "192.168.0.2/31"
+
+	outsideTheRangeAddresses := []struct {
+		address string
+	}{
+		{"192.168.0.1"},
+		{"192.168.0.4"},
+		{"192.168.0.100"},
+	}
+
+	expectedAddresses := []struct {
+		address string
+	}{
+		{"192.168.0.2"},
+		{"192.168.0.3"},
+	}
+
+	for _, store := range []bool{false, true} {
+
+		allocator, err := getAllocator(store)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		//
+		// IPv4 /31 blocks.  See RFC 3021.
+		//
+
+		pool, _, _, err := allocator.RequestPool(localAddressSpace, subnet, "", nil, false)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// Outside-the-range
+
+		for _, outside := range outsideTheRangeAddresses {
+			_, _, errx := allocator.RequestAddress(pool, net.ParseIP(outside.address), nil)
+			if errx != ipamapi.ErrIPOutOfRange {
+				t.Fatalf("Address %s failed to throw expected error: %s", outside.address, errx.Error())
+			}
+		}
+
+		// Should get just these two IPs followed by exhaustion on the next request
+
+		for _, expected := range expectedAddresses {
+			got, _, errx := allocator.RequestAddress(pool, nil, nil)
+			if errx != nil {
+				t.Fatalf("Failed to obtain the address: %s", errx.Error())
+			}
+			expectedIP := net.ParseIP(expected.address)
+			gotIP := got.IP
+			if !gotIP.Equal(expectedIP) {
+				t.Fatalf("Failed to obtain sequentialaddress. Expected: %s, Got: %s", expectedIP, gotIP)
+			}
+		}
+
+		_, _, err = allocator.RequestAddress(pool, nil, nil)
+		if err != ipamapi.ErrNoAvailableIPs {
+			t.Fatal("Did not get expected error when pool is exhausted.")
+		}
+
+	}
+}
+
 func TestRelease(t *testing.T) {
 	var (
 		subnet = "192.168.0.0/23"