allocators/bitmap: Lock the bitmap on use

This allocator was not safe for concurrent access. Add a simple lock
around operations on the bitmap itself to protect from that

Signed-off-by: Anatole Denis <anatole@unverle.fr>

plugins/allocators/bitmap/bitmap.go

@@ -15,6 +15,7 @@ import (
 	"fmt"
 	"net"
 	"strconv"
+	"sync"
 
 	"github.com/willf/bitset"
 
@@ -31,6 +32,7 @@ type Allocator struct {
 	containing net.IPNet
 	page       int
 	bitmap     *bitset.BitSet
+	l          sync.Mutex
 }
 
 // prefix must verify: containing.Mask.Size < prefix.Mask.Size < page
@@ -59,6 +61,8 @@ func (a *Allocator) Allocate(hint net.IPNet) (ret net.IPNet, err error) {
 	ret.Mask = net.CIDRMask(reqSize, 128)
 
 	// Try to allocate the requested prefix
+	a.l.Lock()
+	defer a.l.Unlock()
 	if hint.IP.To16() != nil && a.containing.Contains(hint.IP) {
 		idx, hintErr := a.toIndex(hint.IP)
 		if hintErr == nil && !a.bitmap.Test(idx) {
@@ -91,6 +95,9 @@ func (a *Allocator) Free(prefix net.IPNet) error {
 		return fmt.Errorf("Could not find prefix in pool: %w", err)
 	}
 
+	a.l.Lock()
+	defer a.l.Unlock()
+
 	if !a.bitmap.Test(idx) {
 		return &allocators.ErrDoubleFree{Loc: prefix}
 	}

plugins/allocators/bitmap/bitmap_test.go

@@ -5,16 +5,20 @@
 package bitmap
 
 import (
+	"math"
+	"math/rand"
 	"net"
 	"testing"
+
+	"github.com/willf/bitset"
 )
 
-func getAllocator() *Allocator {
+func getAllocator(bits int) *Allocator {
 	_, prefix, err := net.ParseCIDR("2001:db8::/56")
 	if err != nil {
 		panic(err)
 	}
-	alloc, err := NewBitmapAllocator(*prefix, 64)
+	alloc, err := NewBitmapAllocator(*prefix, 56+bits)
 	if err != nil {
 		panic(err)
 	}
@@ -22,7 +26,7 @@ func getAllocator() *Allocator {
 	return alloc
 }
 func TestAlloc(t *testing.T) {
-	alloc := getAllocator()
+	alloc := getAllocator(8)
 
 	net, err := alloc.Allocate(net.IPNet{})
 	if err != nil {
@@ -73,7 +77,7 @@ func TestExhaust(t *testing.T) {
 }
 
 func TestOutOfPool(t *testing.T) {
-	alloc := getAllocator()
+	alloc := getAllocator(8)
 	_, prefix, _ := net.ParseCIDR("fe80:abcd::/48")
 
 	res, err := alloc.Allocate(*prefix)
@@ -87,3 +91,47 @@ func TestOutOfPool(t *testing.T) {
 		t.Fatalf("Prefixes have wrong size %d/%d", prefLen, totalLen)
 	}
 }
+
+func prefixSizeForAllocs(allocs int) int {
+	return int(math.Ceil(math.Log2(float64(allocs))))
+}
+
+// Benchmark parallel Allocate, when the bitmap is mostly empty and we're allocating few values
+// compared to the available allocations
+func BenchmarkParallelAllocInitiallyEmpty(b *testing.B) {
+	// Run with -race to debug concurrency issues
+
+	alloc := getAllocator(prefixSizeForAllocs(b.N) + 2) // Use max 25% of the bitmap (initially empty)
+
+	b.RunParallel(func(pb *testing.PB) {
+		for pb.Next() {
+			if net, err := alloc.Allocate(net.IPNet{}); err != nil {
+				b.Logf("Could not allocate (got %v and an error): %v", net, err)
+				b.Fail()
+			}
+		}
+	})
+}
+
+func BenchmarkParallelAllocPartiallyFilled(b *testing.B) {
+	// We'll make a bitmap with 2x the number of allocs we want to make.
+	// Then randomly fill it to about 50% utilization
+	alloc := getAllocator(prefixSizeForAllocs(b.N) + 1)
+
+	// Build a replacement bitmap that we'll put in the allocator, with approx. 50% of values filled
+	newbmap := make([]uint64, alloc.bitmap.Len())
+	for i := uint(0); i < alloc.bitmap.Len(); i++ {
+		newbmap[i] = rand.Uint64()
+	}
+	alloc.bitmap = bitset.From(newbmap)
+
+	b.ResetTimer()
+	b.RunParallel(func(pb *testing.PB) {
+		for pb.Next() {
+			if net, err := alloc.Allocate(net.IPNet{}); err != nil {
+				b.Logf("Could not allocate (got %v and an error): %v", net, err)
+				b.Fail()
+			}
+		}
+	})
+}