From 0fc6bf9a6e843bc5f3d91fc79f167e042802d81b Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Tue, 4 Jul 2023 19:22:57 -0400 Subject: [PATCH] libn/d/o/ovmanager: assign vxlans using bitmap pkg The idm package wraps bitseq.Handle to provide an offset and synchronization. bitseq.Handle wraps bitmap.Bitmap to provide persistence in a datastore. As no datastore is passed and the offset is zero, the idm.Idm instance is nothing more than a concurrency-safe wrapper around a bitmap.Bitmap with differently-named methods. Switch over to using bitmap.Bitmap directly, using the ovmanager driver's mutex for concurrency control. Hold the driver mutex for the entire duration that VXLANs are being assigned to the new network. This makes allocating VXLANs for a network an atomic operation. Signed-off-by: Cory Snider --- .../drivers/overlay/ovmanager/ovmanager.go | 33 +++++++++---------- .../overlay/ovmanager/ovmanager_test.go | 17 ++-------- 2 files changed, 17 insertions(+), 33 deletions(-) diff --git a/libnetwork/drivers/overlay/ovmanager/ovmanager.go b/libnetwork/drivers/overlay/ovmanager/ovmanager.go index 54902929a5..0b4ee45eee 100644 --- a/libnetwork/drivers/overlay/ovmanager/ovmanager.go +++ b/libnetwork/drivers/overlay/ovmanager/ovmanager.go @@ -9,10 +9,10 @@ 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" ) @@ -31,7 +31,7 @@ type networkTable map[string]*network type driver struct { mu sync.Mutex networks networkTable - vxlanIdm *idm.Idm + vxlanIdm *bitmap.Bitmap } type subnet struct { @@ -55,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") @@ -105,6 +102,8 @@ 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, @@ -113,7 +112,7 @@ func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data, if len(vxlanIDList) > i { // The VNI for this subnet was specified in the network options. s.vni = vxlanIDList[i] - err := d.vxlanIdm.GetSpecificID(uint64(s.vni)) // Mark VNI as in-use. + 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() @@ -121,7 +120,7 @@ func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data, } } else { // Allocate an available VNI for the subnet, outside the range of 802.1Q VLAN IDs. - vni, err := d.vxlanIdm.GetIDInRange(vxlanIDStart, vxlanIDEnd, true) + 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) @@ -138,8 +137,6 @@ func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data, } opts[netlabel.OverlayVxlanIDList] = val - d.mu.Lock() - defer d.mu.Unlock() if _, ok := d.networks[id]; ok { n.releaseVxlanID() return nil, fmt.Errorf("network %s already exists", id) @@ -172,7 +169,7 @@ func (d *driver) NetworkFree(id string) error { func (n *network) releaseVxlanID() { for _, s := range n.subnets { - n.driver.vxlanIdm.Release(uint64(s.vni)) + n.driver.vxlanIdm.Unset(uint64(s.vni)) s.vni = 0 } } diff --git a/libnetwork/drivers/overlay/ovmanager/ovmanager_test.go b/libnetwork/drivers/overlay/ovmanager/ovmanager_test.go index 319387345f..2066796259 100644 --- a/libnetwork/drivers/overlay/ovmanager/ovmanager_test.go +++ b/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{ {