[VOL-4211] Persist meters in the Core
Change-Id: I0f9a2914996a69be080bd8f77b3c7ae6cc902cb3
diff --git a/rw_core/core/device/flow/cache_test.go b/rw_core/core/device/flow/cache_test.go
index a418904..d0d8049 100644
--- a/rw_core/core/device/flow/cache_test.go
+++ b/rw_core/core/device/flow/cache_test.go
@@ -29,14 +29,14 @@
// TestLoadersIdentical ensures that the group, flow, and meter loaders always have an identical implementation.
func TestLoadersIdentical(t *testing.T) {
- types := []string{"flow", "group", "meter"}
+ types := []string{"flow", "group"}
identical := [][]string{
- {`ofp\.OfpFlowStats`, `ofp\.OfpGroupEntry`, `ofp\.OfpMeterEntry`},
- {`\.Id`, `\.Desc\.GroupId`, `\.Config\.MeterId`},
- {`uint64`, `uint32`, `uint32`},
- {`Flow`, `Group`, `Meter`},
- {`flow`, `group`, `meter`},
+ {`ofp\.OfpFlowStats`, `ofp\.OfpGroupEntry`},
+ {`\.Id`, `\.Desc\.GroupId`},
+ {`uint64`, `uint32`},
+ {`Flow`, `Group`},
+ {`flow`, `group`},
}
regexes := make([][]*regexp.Regexp, len(identical[0]))
diff --git a/rw_core/core/device/logical_agent.go b/rw_core/core/device/logical_agent.go
index a12bd2d..b9ad95a 100644
--- a/rw_core/core/device/logical_agent.go
+++ b/rw_core/core/device/logical_agent.go
@@ -58,10 +58,10 @@
startOnce sync.Once
stopOnce sync.Once
- flowCache *flow.Cache
- meterCache *meter.Cache
- groupCache *group.Cache
- portLoader *port.Loader
+ flowCache *flow.Cache
+ meterLoader *meter.Loader
+ groupCache *group.Cache
+ portLoader *port.Loader
}
func newLogicalAgent(ctx context.Context, id string, sn string, deviceID string, ldeviceMgr *LogicalManager,
@@ -78,10 +78,10 @@
defaultTimeout: defaultTimeout,
requestQueue: coreutils.NewRequestQueue(),
- flowCache: flow.NewCache(),
- groupCache: group.NewCache(),
- meterCache: meter.NewCache(),
- portLoader: port.NewLoader(dbProxy.SubPath("logical_ports").Proxy(id)),
+ flowCache: flow.NewCache(),
+ groupCache: group.NewCache(),
+ meterLoader: meter.NewLoader(dbProxy.SubPath("logical_meters").Proxy(id)),
+ portLoader: port.NewLoader(dbProxy.SubPath("logical_ports").Proxy(id)),
}
}
@@ -163,6 +163,9 @@
// now that the root device is known, create DeviceRoutes with it
agent.deviceRoutes = route.NewDeviceRoutes(agent.logicalDeviceID, agent.rootDeviceID, agent.deviceMgr.listDevicePorts)
+ // load the meters from KV to cache
+ agent.meterLoader.Load(ctx)
+
// load the logical ports from KV to cache
agent.portLoader.Load(ctx)
}
diff --git a/rw_core/core/device/logical_agent_meter.go b/rw_core/core/device/logical_agent_meter.go
index 5ee8269..7e824d2 100644
--- a/rw_core/core/device/logical_agent_meter.go
+++ b/rw_core/core/device/logical_agent_meter.go
@@ -28,15 +28,16 @@
)
// listLogicalDeviceMeters returns logical device meters
-func (agent *LogicalAgent) listLogicalDeviceMeters() map[uint32]*ofp.OfpMeterEntry {
- meterIDs := agent.meterCache.ListIDs()
+func (agent *LogicalAgent) listLogicalDeviceMeters(ctx context.Context) map[uint32]*ofp.OfpMeterEntry {
+ meterIDs := agent.meterLoader.ListIDs()
meters := make(map[uint32]*ofp.OfpMeterEntry, len(meterIDs))
for meterID := range meterIDs {
- if meterHandle, have := agent.meterCache.Lock(meterID); have {
+ if meterHandle, have := agent.meterLoader.Lock(meterID); have {
meters[meterID] = meterHandle.GetReadOnly()
meterHandle.Unlock()
}
}
+ logger.Debugw(ctx, "list-logical-device-meters", log.Fields{"logical-device-id": agent.logicalDeviceID, "num-meters": len(meters)})
return meters
}
@@ -67,7 +68,7 @@
meterEntry := fu.MeterEntryFromMeterMod(ctx, meterMod)
- meterHandle, created, err := agent.meterCache.LockOrCreate(ctx, meterEntry)
+ meterHandle, created, err := agent.meterLoader.LockOrCreate(ctx, meterEntry)
if err != nil {
return err
}
@@ -88,7 +89,7 @@
}
logger.Debug(ctx, "meterDelete", log.Fields{"meterMod": *meterMod, "logical-device-id": agent.logicalDeviceID})
- meterHandle, have := agent.meterCache.Lock(meterMod.MeterId)
+ meterHandle, have := agent.meterLoader.Lock(meterMod.MeterId)
if !have {
logger.Warnw(ctx, "meter-not-found", log.Fields{"meterID": meterMod.MeterId, "logical-device-id": agent.logicalDeviceID})
return nil
@@ -116,7 +117,7 @@
return nil
}
- meterHandle, have := agent.meterCache.Lock(meterMod.MeterId)
+ meterHandle, have := agent.meterLoader.Lock(meterMod.MeterId)
if !have {
return fmt.Errorf("no-meter-to-modify: %d, logical-device-id: %s", meterMod.MeterId, agent.logicalDeviceID)
}
diff --git a/rw_core/core/device/logical_agent_meter_helpers.go b/rw_core/core/device/logical_agent_meter_helpers.go
index bb9de0a..19f7aad 100644
--- a/rw_core/core/device/logical_agent_meter_helpers.go
+++ b/rw_core/core/device/logical_agent_meter_helpers.go
@@ -32,7 +32,7 @@
if flowMeterID := fu.GetMeterIdFromFlow(flow); flowMeterID != 0 {
if _, have := metersConfig[flowMeterID]; !have {
// Meter is present in the flow, Get from logical device
- meterHandle, have := agent.meterCache.Lock(flowMeterID)
+ meterHandle, have := agent.meterLoader.Lock(flowMeterID)
if !have {
logger.Errorw(ctx, "Meter-referred-by-flow-is-not-found-in-logicaldevice",
log.Fields{"meterID": flowMeterID, "Available-meters": metersConfig, "flow": *flow})
@@ -65,7 +65,7 @@
return true
}
- meterHandle, have := agent.meterCache.Lock(meterID)
+ meterHandle, have := agent.meterLoader.Lock(meterID)
if !have {
logger.Debugw(ctx, "Meter-is-not-present-in-logical-device", log.Fields{"meterID": meterID})
return true
diff --git a/rw_core/core/device/logical_agent_test.go b/rw_core/core/device/logical_agent_test.go
index ac7662e..96729a2 100644
--- a/rw_core/core/device/logical_agent_test.go
+++ b/rw_core/core/device/logical_agent_test.go
@@ -264,7 +264,7 @@
localWG.Wait()
meterEntry := fu.MeterEntryFromMeterMod(ctx, meterMod)
- meterHandle, have := ldAgent.meterCache.Lock(meterMod.MeterId)
+ meterHandle, have := ldAgent.meterLoader.Lock(meterMod.MeterId)
assert.Equal(t, have, true)
if have {
assert.True(t, proto.Equal(meterEntry, meterHandle.GetReadOnly()))
diff --git a/rw_core/core/device/logical_manager.go b/rw_core/core/device/logical_manager.go
index c43ecdb..26021ca 100644
--- a/rw_core/core/device/logical_manager.go
+++ b/rw_core/core/device/logical_manager.go
@@ -512,7 +512,7 @@
if agent == nil {
return nil, status.Errorf(codes.NotFound, "%s", id.Id)
}
- meters := agent.listLogicalDeviceMeters()
+ meters := agent.listLogicalDeviceMeters(ctx)
ctr, ret := 0, make([]*openflow_13.OfpMeterEntry, len(meters))
for _, meter := range meters {
ret[ctr] = meter
diff --git a/rw_core/core/device/meter/cache.go b/rw_core/core/device/meter/cache.go
deleted file mode 100644
index 8912bbe..0000000
--- a/rw_core/core/device/meter/cache.go
+++ /dev/null
@@ -1,148 +0,0 @@
-/*
- * Copyright 2018-present Open Networking Foundation
-
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
-
- * http://www.apache.org/licenses/LICENSE-2.0
-
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package meter
-
-import (
- "context"
- "sync"
-
- ofp "github.com/opencord/voltha-protos/v4/go/openflow_13"
-)
-
-// Cache hides all low-level locking & synchronization related to meter state updates
-type Cache struct {
- // this lock protects the meters map, it does not protect individual meters
- lock sync.RWMutex
- meters map[uint32]*chunk
-}
-
-// chunk keeps a meter and the lock for this meter
-type chunk struct {
- // this lock is used to synchronize all access to the meter, and also to the "deleted" variable
- lock sync.Mutex
- deleted bool
-
- meter *ofp.OfpMeterEntry
-}
-
-func NewCache() *Cache {
- return &Cache{
- meters: make(map[uint32]*chunk),
- }
-}
-
-// LockOrCreate locks this meter if it exists, or creates a new meter if it does not.
-// In the case of meter creation, the provided "meter" must not be modified afterwards.
-func (cache *Cache) LockOrCreate(ctx context.Context, meter *ofp.OfpMeterEntry) (*Handle, bool, error) {
- // try to use read lock instead of full lock if possible
- if handle, have := cache.Lock(meter.Config.MeterId); have {
- return handle, false, nil
- }
-
- cache.lock.Lock()
- entry, have := cache.meters[meter.Config.MeterId]
- if !have {
- entry := &chunk{meter: meter}
- cache.meters[meter.Config.MeterId] = entry
- entry.lock.Lock()
- cache.lock.Unlock()
-
- return &Handle{loader: cache, chunk: entry}, true, nil
- }
- cache.lock.Unlock()
-
- entry.lock.Lock()
- if entry.deleted {
- entry.lock.Unlock()
- return cache.LockOrCreate(ctx, meter)
- }
- return &Handle{loader: cache, chunk: entry}, false, nil
-}
-
-// Lock acquires the lock for this meter, and returns a handle which can be used to access the meter until it's unlocked.
-// This handle ensures that the meter cannot be accessed if the lock is not held.
-// Returns false if the meter is not present.
-// TODO: consider accepting a ctx and aborting the lock attempt on cancellation
-func (cache *Cache) Lock(id uint32) (*Handle, bool) {
- cache.lock.RLock()
- entry, have := cache.meters[id]
- cache.lock.RUnlock()
-
- if !have {
- return nil, false
- }
-
- entry.lock.Lock()
- if entry.deleted {
- entry.lock.Unlock()
- return cache.Lock(id)
- }
- return &Handle{loader: cache, chunk: entry}, true
-}
-
-// Handle is allocated for each Lock() call, all modifications are made using it, and it is invalidated by Unlock()
-// This enforces correct Lock()-Usage()-Unlock() ordering.
-type Handle struct {
- loader *Cache
- chunk *chunk
-}
-
-// GetReadOnly returns an *ofp.OfpMeterEntry which MUST NOT be modified externally, but which is safe to keep indefinitely
-func (h *Handle) GetReadOnly() *ofp.OfpMeterEntry {
- return h.chunk.meter
-}
-
-// Update updates an existing meter in cache.
-// The provided "meter" must not be modified afterwards.
-func (h *Handle) Update(ctx context.Context, meter *ofp.OfpMeterEntry) error {
- h.chunk.meter = meter
- return nil
-}
-
-// Delete removes the meter from the cache
-func (h *Handle) Delete(ctx context.Context) error {
- h.chunk.deleted = true
-
- h.loader.lock.Lock()
- delete(h.loader.meters, h.chunk.meter.Config.MeterId)
- h.loader.lock.Unlock()
-
- h.Unlock()
- return nil
-}
-
-// Unlock releases the lock on the meter
-func (h *Handle) Unlock() {
- if h.chunk != nil {
- h.chunk.lock.Unlock()
- h.chunk = nil // attempting to access the meter through this handle in future will panic
- }
-}
-
-// ListIDs returns a snapshot of all the managed meter IDs
-// TODO: iterating through meters safely is expensive now, since all meters are stored & locked separately
-// should avoid this where possible
-func (cache *Cache) ListIDs() map[uint32]struct{} {
- cache.lock.RLock()
- defer cache.lock.RUnlock()
- // copy the IDs so caller can safely iterate
- ret := make(map[uint32]struct{}, len(cache.meters))
- for id := range cache.meters {
- ret[id] = struct{}{}
- }
- return ret
-}
diff --git a/rw_core/core/device/meter/common.go b/rw_core/core/device/meter/common.go
new file mode 100644
index 0000000..773c0b7
--- /dev/null
+++ b/rw_core/core/device/meter/common.go
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2020-present Open Networking Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Package core Common Logger initialization
+package meter
+
+import (
+ "github.com/opencord/voltha-lib-go/v5/pkg/log"
+)
+
+var logger log.CLogger
+
+func init() {
+ // Setup this package so that it's log level can be modified at run time
+ var err error
+ logger, err = log.RegisterPackage(log.JSON, log.ErrorLevel, log.Fields{})
+ if err != nil {
+ panic(err)
+ }
+}
diff --git a/rw_core/core/device/meter/loader.go b/rw_core/core/device/meter/loader.go
new file mode 100644
index 0000000..42f4264
--- /dev/null
+++ b/rw_core/core/device/meter/loader.go
@@ -0,0 +1,172 @@
+/*
+ * Copyright 2018-present Open Networking Foundation
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package meter
+
+import (
+ "context"
+ "fmt"
+ "github.com/opencord/voltha-go/db/model"
+ "github.com/opencord/voltha-lib-go/v5/pkg/log"
+ ofp "github.com/opencord/voltha-protos/v4/go/openflow_13"
+ "google.golang.org/grpc/codes"
+ "google.golang.org/grpc/status"
+ "sync"
+)
+
+// Loader hides all low-level locking & synchronization related to meter state updates
+type Loader struct {
+ dbProxy *model.Proxy
+ // this lock protects the meters map, it does not protect individual meters
+ lock sync.RWMutex
+ meters map[uint32]*chunk
+}
+
+// chunk keeps a meter and the lock for this meter
+type chunk struct {
+ // this lock is used to synchronize all access to the meter, and also to the "deleted" variable
+ lock sync.Mutex
+ deleted bool
+ meter *ofp.OfpMeterEntry
+}
+
+func NewLoader(dbProxy *model.Proxy) *Loader {
+ return &Loader{
+ dbProxy: dbProxy,
+ meters: make(map[uint32]*chunk),
+ }
+}
+
+// Load queries existing meters from the kv,
+// and should only be called once when first created.
+func (loader *Loader) Load(ctx context.Context) {
+ loader.lock.Lock()
+ defer loader.lock.Unlock()
+ var meters []*ofp.OfpMeterEntry
+ if err := loader.dbProxy.List(ctx, &meters); err != nil {
+ logger.Errorw(ctx, "failed-to-list-meters-from-cluster-data-proxy", log.Fields{"error": err})
+ return
+ }
+ for _, meter := range meters {
+ loader.meters[meter.Config.MeterId] = &chunk{meter: meter}
+ }
+}
+
+// LockOrCreate locks this meter if it exists, or creates a new meter if it does not.
+// In the case of meter creation, the provided "meter" must not be modified afterwards.
+func (loader *Loader) LockOrCreate(ctx context.Context, meter *ofp.OfpMeterEntry) (*Handle, bool, error) {
+ // try to use read lock instead of full lock if possible
+ if handle, have := loader.Lock(meter.Config.MeterId); have {
+ return handle, false, nil
+ }
+ loader.lock.Lock()
+ entry, have := loader.meters[meter.Config.MeterId]
+ if !have {
+ entry := &chunk{meter: meter}
+ loader.meters[meter.Config.MeterId] = entry
+ entry.lock.Lock()
+ loader.lock.Unlock()
+ if err := loader.dbProxy.Set(ctx, fmt.Sprint(meter.Config.MeterId), meter); err != nil {
+ // revert the map
+ loader.lock.Lock()
+ delete(loader.meters, meter.Config.MeterId)
+ loader.lock.Unlock()
+ entry.deleted = true
+ entry.lock.Unlock()
+ return nil, false, err
+ }
+ return &Handle{loader: loader, chunk: entry}, true, nil
+ }
+ loader.lock.Unlock()
+ entry.lock.Lock()
+ if entry.deleted {
+ entry.lock.Unlock()
+ return loader.LockOrCreate(ctx, meter)
+ }
+ return &Handle{loader: loader, chunk: entry}, false, nil
+}
+
+// Lock acquires the lock for this meter, and returns a handle which can be used to access the meter until it's unlocked.
+// This handle ensures that the meter cannot be accessed if the lock is not held.
+// Returns false if the meter is not present.
+// TODO: consider accepting a ctx and aborting the lock attempt on cancellation
+func (loader *Loader) Lock(id uint32) (*Handle, bool) {
+ loader.lock.RLock()
+ entry, have := loader.meters[id]
+ loader.lock.RUnlock()
+ if !have {
+ return nil, false
+ }
+ entry.lock.Lock()
+ if entry.deleted {
+ entry.lock.Unlock()
+ return loader.Lock(id)
+ }
+ return &Handle{loader: loader, chunk: entry}, true
+}
+
+// Handle is allocated for each Lock() call, all modifications are made using it, and it is invalidated by Unlock()
+// This enforces correct Lock()-Usage()-Unlock() ordering.
+type Handle struct {
+ loader *Loader
+ chunk *chunk
+}
+
+// GetReadOnly returns an *ofp.OfpMeterEntry which MUST NOT be modified externally, but which is safe to keep indefinitely
+func (h *Handle) GetReadOnly() *ofp.OfpMeterEntry {
+ return h.chunk.meter
+}
+
+// Update updates an existing meter in the kv.
+// The provided "meter" must not be modified afterwards.
+func (h *Handle) Update(ctx context.Context, meter *ofp.OfpMeterEntry) error {
+ if err := h.loader.dbProxy.Set(ctx, fmt.Sprint(meter.Config.MeterId), meter); err != nil {
+ return status.Errorf(codes.Internal, "failed-update-meter-%v: %s", meter.Config.MeterId, err)
+ }
+ h.chunk.meter = meter
+ return nil
+}
+
+// Delete removes the device from the kv
+func (h *Handle) Delete(ctx context.Context) error {
+ if err := h.loader.dbProxy.Remove(ctx, fmt.Sprint(h.chunk.meter.Config.MeterId)); err != nil {
+ return fmt.Errorf("couldnt-delete-meter-from-store-%v", h.chunk.meter.Config.MeterId)
+ }
+ h.chunk.deleted = true
+ h.loader.lock.Lock()
+ delete(h.loader.meters, h.chunk.meter.Config.MeterId)
+ h.loader.lock.Unlock()
+ h.Unlock()
+ return nil
+}
+
+// Unlock releases the lock on the meter
+func (h *Handle) Unlock() {
+ if h.chunk != nil {
+ h.chunk.lock.Unlock()
+ h.chunk = nil // attempting to access the meter through this handle in future will panic
+ }
+}
+
+// ListIDs returns a snapshot of all the managed meter IDs
+// TODO: iterating through meters safely is expensive now, since all meters are stored & locked separately
+// should avoid this where possible
+func (loader *Loader) ListIDs() map[uint32]struct{} {
+ loader.lock.RLock()
+ defer loader.lock.RUnlock()
+ // copy the IDs so caller can safely iterate
+ ret := make(map[uint32]struct{}, len(loader.meters))
+ for id := range loader.meters {
+ ret[id] = struct{}{}
+ }
+ return ret
+}