Merge pull request #45882 from corhere/libn/ditch-idm

libnetwork/d/o/ovmanager: switch to using bitmap package directly

libnetwork/bitmap/sequence.go

@@ -47,7 +47,7 @@ type Bitmap struct {
 	noCopy noCopy
 }
 
-// NewHandle returns a new Bitmap n bits long.
+// NewHandle returns a new Bitmap of ordinals in the interval [0, n).
 func New(n uint64) *Bitmap {
 	return &Bitmap{
 		bits:       n,
@@ -176,7 +176,7 @@ func (s *sequence) fromByteArray(data []byte) error {
 	return nil
 }
 
-// SetAnyInRange sets the first unset bit in the range [start, end) and returns
+// SetAnyInRange sets the first unset bit in the range [start, end] and returns
 // the ordinal of the set bit.
 //
 // When serial=true, the bitmap is scanned starting from the ordinal following

libnetwork/drivers/overlay/ovmanager/ovmanager.go

@@ -9,26 +9,29 @@ import (
 	"sync"
 
 	"github.com/containerd/containerd/log"
+	"github.com/docker/docker/libnetwork/bitmap"
 	"github.com/docker/docker/libnetwork/datastore"
 	"github.com/docker/docker/libnetwork/discoverapi"
 	"github.com/docker/docker/libnetwork/driverapi"
-	"github.com/docker/docker/libnetwork/idm"
 	"github.com/docker/docker/libnetwork/netlabel"
 	"github.com/docker/docker/libnetwork/types"
 )
 
 const (
-	networkType  = "overlay"
+	networkType = "overlay"
+	// The lowest VNI value to auto-assign. Windows does not support VXLAN IDs
+	// which overlap the range of 802.1Q VLAN IDs [0, 4095].
 	vxlanIDStart = 4096
-	vxlanIDEnd   = (1 << 24) - 1
+	// The largest VNI value permitted by RFC 7348.
+	vxlanIDEnd = (1 << 24) - 1
 )
 
 type networkTable map[string]*network
 
 type driver struct {
+	mu       sync.Mutex
 	networks networkTable
-	vxlanIdm *idm.Idm
-	sync.Mutex
+	vxlanIdm *bitmap.Bitmap
 }
 
 type subnet struct {
@@ -41,7 +44,6 @@ type network struct {
 	id      string
 	driver  *driver
 	subnets []*subnet
-	sync.Mutex
 }
 
 // Init registers a new instance of the overlay driver.
@@ -53,22 +55,19 @@ func Init(dc driverapi.DriverCallback, _ map[string]interface{}) error {
 
 // Register registers a new instance of the overlay driver.
 func Register(r driverapi.Registerer, _ map[string]interface{}) error {
-	var err error
-	d := &driver{
-		networks: networkTable{},
-	}
-
-	d.vxlanIdm, err = idm.New(nil, "vxlan-id", 0, vxlanIDEnd)
-	if err != nil {
-		return fmt.Errorf("failed to initialize vxlan id manager: %v", err)
-	}
-
-	return r.RegisterDriver(networkType, d, driverapi.Capability{
+	return r.RegisterDriver(networkType, newDriver(), driverapi.Capability{
 		DataScope:         datastore.GlobalScope,
 		ConnectivityScope: datastore.GlobalScope,
 	})
 }
 
+func newDriver() *driver {
+	return &driver{
+		networks: networkTable{},
+		vxlanIdm: bitmap.New(vxlanIDEnd + 1), // The full range of valid vxlan IDs: [0, 2^24).
+	}
+}
+
 func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data, ipV6Data []driverapi.IPAMData) (map[string]string, error) {
 	if id == "" {
 		return nil, fmt.Errorf("invalid network id for overlay network")
@@ -103,19 +102,30 @@ func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data,
 		}
 	}
 
+	d.mu.Lock()
+	defer d.mu.Unlock()
 	for i, ipd := range ipV4Data {
 		s := &subnet{
 			subnetIP: ipd.Pool,
 			gwIP:     ipd.Gateway,
 		}
 
-		if len(vxlanIDList) > i {
+		if len(vxlanIDList) > i { // The VNI for this subnet was specified in the network options.
 			s.vni = vxlanIDList[i]
-		}
-
-		if err := n.obtainVxlanID(s); err != nil {
-			n.releaseVxlanID()
-			return nil, fmt.Errorf("could not obtain vxlan id for pool %s: %v", s.subnetIP, err)
+			err := d.vxlanIdm.Set(uint64(s.vni)) // Mark VNI as in-use.
+			if err != nil {
+				// The VNI is already in use by another subnet/network.
+				n.releaseVxlanID()
+				return nil, fmt.Errorf("could not assign vxlan id %v to pool %s: %v", s.vni, s.subnetIP, err)
+			}
+		} else {
+			// Allocate an available VNI for the subnet, outside the range of 802.1Q VLAN IDs.
+			vni, err := d.vxlanIdm.SetAnyInRange(vxlanIDStart, vxlanIDEnd, true)
+			if err != nil {
+				n.releaseVxlanID()
+				return nil, fmt.Errorf("could not obtain vxlan id for pool %s: %v", s.subnetIP, err)
+			}
+			s.vni = uint32(vni)
 		}
 
 		n.subnets = append(n.subnets, s)
@@ -127,8 +137,6 @@ func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data,
 	}
 	opts[netlabel.OverlayVxlanIDList] = val
 
-	d.Lock()
-	defer d.Unlock()
 	if _, ok := d.networks[id]; ok {
 		n.releaseVxlanID()
 		return nil, fmt.Errorf("network %s already exists", id)
@@ -143,8 +151,8 @@ func (d *driver) NetworkFree(id string) error {
 		return fmt.Errorf("invalid network id passed while freeing overlay network")
 	}
 
-	d.Lock()
-	defer d.Unlock()
+	d.mu.Lock()
+	defer d.mu.Unlock()
 	n, ok := d.networks[id]
 
 	if !ok {
@@ -159,43 +167,11 @@ func (d *driver) NetworkFree(id string) error {
 	return nil
 }
 
-func (n *network) obtainVxlanID(s *subnet) error {
-	var (
-		err error
-		vni uint64
-	)
-
-	n.Lock()
-	vni = uint64(s.vni)
-	n.Unlock()
-
-	if vni == 0 {
-		vni, err = n.driver.vxlanIdm.GetIDInRange(vxlanIDStart, vxlanIDEnd, true)
-		if err != nil {
-			return err
-		}
-
-		n.Lock()
-		s.vni = uint32(vni)
-		n.Unlock()
-		return nil
-	}
-
-	return n.driver.vxlanIdm.GetSpecificID(vni)
-}
-
 func (n *network) releaseVxlanID() {
-	n.Lock()
-	vnis := make([]uint32, 0, len(n.subnets))
 	for _, s := range n.subnets {
-		vnis = append(vnis, s.vni)
+		n.driver.vxlanIdm.Unset(uint64(s.vni))
 		s.vni = 0
 	}
-	n.Unlock()
-
-	for _, vni := range vnis {
-		n.driver.vxlanIdm.Release(uint64(vni))
-	}
 }
 
 func (d *driver) CreateNetwork(id string, option map[string]interface{}, nInfo driverapi.NetworkInfo, ipV4Data, ipV6Data []driverapi.IPAMData) error {

libnetwork/drivers/overlay/ovmanager/ovmanager_test.go

@@ -7,25 +7,12 @@ import (
 	"testing"
 
 	"github.com/docker/docker/libnetwork/driverapi"
-	"github.com/docker/docker/libnetwork/idm"
 	"github.com/docker/docker/libnetwork/netlabel"
 	"github.com/docker/docker/libnetwork/types"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 )
 
-func newDriver(t *testing.T) *driver {
-	d := &driver{
-		networks: networkTable{},
-	}
-
-	vxlanIdm, err := idm.New(nil, "vxlan-id", vxlanIDStart, vxlanIDEnd)
-	assert.NilError(t, err)
-
-	d.vxlanIdm = vxlanIdm
-	return d
-}
-
 func parseCIDR(t *testing.T, ipnet string) *net.IPNet {
 	subnet, err := types.ParseCIDR(ipnet)
 	assert.NilError(t, err)
@@ -33,7 +20,7 @@ func parseCIDR(t *testing.T, ipnet string) *net.IPNet {
 }
 
 func TestNetworkAllocateFree(t *testing.T) {
-	d := newDriver(t)
+	d := newDriver()
 
 	ipamData := []driverapi.IPAMData{
 		{
@@ -56,7 +43,7 @@ func TestNetworkAllocateFree(t *testing.T) {
 }
 
 func TestNetworkAllocateUserDefinedVNIs(t *testing.T) {
-	d := newDriver(t)
+	d := newDriver()
 
 	ipamData := []driverapi.IPAMData{
 		{