VOL-4260: Fix Duplicate T-Cont AllocId assigned to two different ONU devices
- The openolt resourcemanager used a different set of locks from TechProfile
module for PON resource management resulting in corruption of resource
pool. This bug was introduced as part of VOL-3936.
Change-Id: Ic4ac0dfb5abb3603206568c56b978ce66ea1adc8
diff --git a/VERSION b/VERSION
index 40c341b..9575d51 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-3.6.0
+3.6.1
diff --git a/internal/pkg/core/device_handler.go b/internal/pkg/core/device_handler.go
index e7e530a..60bd3bd 100644
--- a/internal/pkg/core/device_handler.go
+++ b/internal/pkg/core/device_handler.go
@@ -853,6 +853,7 @@
if dh.flowMgr[i] = NewFlowManager(ctx, dh, dh.resourceMgr[i], dh.groupMgr, i); dh.flowMgr[i] == nil {
return olterrors.ErrFlowManagerInstantiating
}
+ dh.resourceMgr[i].TechprofileRef = dh.flowMgr[i].techprofile
}
/* TODO: Instantiate Alarm , stats , BW managers */
/* Instantiating Event Manager to handle Alarms and KPIs */
diff --git a/internal/pkg/core/device_handler_test.go b/internal/pkg/core/device_handler_test.go
index f041249..57103c4 100644
--- a/internal/pkg/core/device_handler_test.go
+++ b/internal/pkg/core/device_handler_test.go
@@ -213,42 +213,24 @@
dh.resourceMgr[i].PonRsrMgr = ponmgr
}
- /*
- tpMgr, err := tp.NewTechProfile(ctx, ponmgr, "etcd", "127.0.0.1", "/")
- if err != nil {
- logger.Fatal(ctx, err.Error())
- }
- */
-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
dh.groupMgr = NewGroupManager(ctx, dh, dh.resourceMgr[0])
dh.totalPonPorts = NumPonPorts
dh.flowMgr = make([]*OpenOltFlowMgr, dh.totalPonPorts)
for i = 0; i < dh.totalPonPorts; i++ {
- /*
- // Instantiate flow manager
- if dh.flowMgr[i] = NewFlowManager(ctx, dh, dh.resourceMgr[i], dh.groupMgr, uint32(i)); dh.flowMgr[i] == nil {
- return nil
- }
-
- */
dh.flowMgr[i] = &OpenOltFlowMgr{}
dh.flowMgr[i].deviceHandler = dh
dh.flowMgr[i].ponPortIdx = i
dh.flowMgr[i].grpMgr = dh.groupMgr
dh.flowMgr[i].resourceMgr = dh.resourceMgr[i]
- /*
- if err = flowMgr.populateTechProfilePerPonPort(ctx); err != nil {
- logger.Errorw(ctx, "error-while-populating-tech-profile-mgr", log.Fields{"err": err})
- return nil
- }
- */
dh.flowMgr[i].techprofile = mocks.MockTechProfile{}
dh.flowMgr[i].gemToFlowIDs = make(map[uint32][]uint64)
dh.flowMgr[i].packetInGemPort = make(map[resourcemanager.PacketInInfoKey]uint32)
dh.flowMgr[i].flowIDToGems = make(map[uint64][]uint32)
+ dh.resourceMgr[i].TechprofileRef = dh.flowMgr[i].techprofile
+
// Create a slice of buffered channels for handling concurrent flows per ONU.
// The additional entry (+1) is to handle the NNI trap flows on a separate channel from individual ONUs channel
dh.flowMgr[i].incomingFlows = make([]chan flowControlBlock, MaxOnusPerPon+1)
diff --git a/internal/pkg/resourcemanager/resourcemanager.go b/internal/pkg/resourcemanager/resourcemanager.go
index 3726ea5..830732c 100755
--- a/internal/pkg/resourcemanager/resourcemanager.go
+++ b/internal/pkg/resourcemanager/resourcemanager.go
@@ -22,6 +22,7 @@
"encoding/json"
"errors"
"fmt"
+ tp "github.com/opencord/voltha-lib-go/v6/pkg/techprofile"
"strings"
"sync"
"time"
@@ -156,9 +157,7 @@
groupInfoLock sync.RWMutex
// Local maps used for write-through-cache - end
- OnuIDMgmtLock sync.RWMutex
- GemPortIDMgmtLock sync.RWMutex
- AllocIDMgmtLock sync.RWMutex
+ TechprofileRef tp.TechProfileIf
}
func newKVClient(ctx context.Context, storeType string, address string, timeout time.Duration) (kvstore.Client, error) {
@@ -317,10 +316,8 @@
// GetONUID returns the available onuID for the given pon-port
func (rsrcMgr *OpenOltResourceMgr) GetONUID(ctx context.Context, PonIntfID uint32) (uint32, error) {
// Get ONU id for a provided pon interface ID.
- rsrcMgr.OnuIDMgmtLock.Lock()
- onuID, err := rsrcMgr.PonRsrMgr.GetResourceID(ctx, PonIntfID,
+ onuID, err := rsrcMgr.TechprofileRef.GetResourceID(ctx, PonIntfID,
ponrmgr.ONU_ID, 1)
- rsrcMgr.OnuIDMgmtLock.Unlock()
if err != nil {
logger.Errorf(ctx, "Failed to get resource for interface %d for type %s",
PonIntfID, ponrmgr.ONU_ID)
@@ -478,43 +475,13 @@
}
-// FreeResourceID frees PON resource ID like ONU ID, Alloc ID or GEM Port ID
-func (rsrcMgr *OpenOltResourceMgr) FreeResourceID(ctx context.Context, intfID uint32, resourceType string, ReleaseContent []uint32) error {
- logger.Debugw(ctx, "freeing-resource-id", log.Fields{
- "intf-id": intfID,
- "resource-type": resourceType,
- "release-content": ReleaseContent,
- })
- var err error
- switch resourceType {
- case rsrcMgr.PonRsrMgr.GetResourceTypeAllocID():
- rsrcMgr.AllocIDMgmtLock.Lock()
- err = rsrcMgr.PonRsrMgr.FreeResourceID(ctx, intfID, resourceType, ReleaseContent)
- rsrcMgr.AllocIDMgmtLock.Unlock()
- case rsrcMgr.PonRsrMgr.GetResourceTypeGemPortID():
- rsrcMgr.GemPortIDMgmtLock.Lock()
- err = rsrcMgr.PonRsrMgr.FreeResourceID(ctx, intfID, resourceType, ReleaseContent)
- rsrcMgr.GemPortIDMgmtLock.Unlock()
- case rsrcMgr.PonRsrMgr.GetResourceTypeOnuID():
- rsrcMgr.OnuIDMgmtLock.Lock()
- err = rsrcMgr.PonRsrMgr.FreeResourceID(ctx, intfID, resourceType, ReleaseContent)
- rsrcMgr.OnuIDMgmtLock.Unlock()
- default:
- return fmt.Errorf("resourceType %s not supported", resourceType)
- }
- if err != nil {
- return err
- }
- return nil
-}
-
// FreeonuID releases(make free) onu id for a particular pon-port
func (rsrcMgr *OpenOltResourceMgr) FreeonuID(ctx context.Context, intfID uint32, onuID []uint32) {
if len(onuID) == 0 {
logger.Info(ctx, "onu id slice is nil, nothing to free")
return
}
- if err := rsrcMgr.FreeResourceID(ctx, intfID, ponrmgr.ONU_ID, onuID); err != nil {
+ if err := rsrcMgr.TechprofileRef.FreeResourceID(ctx, intfID, ponrmgr.ONU_ID, onuID); err != nil {
logger.Errorw(ctx, "error-while-freeing-onu-id", log.Fields{
"intf-id": intfID,
"onu-id": onuID,
@@ -542,7 +509,7 @@
rsrcMgr.RemoveAllocIDForOnu(ctx, intfID, onuID, uniID, allocID)
allocIDs := make([]uint32, 0)
allocIDs = append(allocIDs, allocID)
- if err := rsrcMgr.FreeResourceID(ctx, intfID, ponrmgr.ALLOC_ID, allocIDs); err != nil {
+ if err := rsrcMgr.TechprofileRef.FreeResourceID(ctx, intfID, ponrmgr.ALLOC_ID, allocIDs); err != nil {
logger.Errorw(ctx, "error-while-freeing-alloc-id", log.Fields{
"intf-id": intfID,
"onu-id": onuID,
@@ -561,7 +528,7 @@
gemPortIDs := make([]uint32, 0)
gemPortIDs = append(gemPortIDs, gemPortID)
- if err := rsrcMgr.FreeResourceID(ctx, intfID, ponrmgr.GEMPORT_ID, gemPortIDs); err != nil {
+ if err := rsrcMgr.TechprofileRef.FreeResourceID(ctx, intfID, ponrmgr.GEMPORT_ID, gemPortIDs); err != nil {
logger.Errorw(ctx, "error-while-freeing-gem-port-id", log.Fields{
"intf-id": intfID,
"onu-id": onuID,
@@ -581,7 +548,7 @@
delete(rsrcMgr.allocIDsForOnu, intfOnuIDuniID)
rsrcMgr.allocIDsForOnuLock.Unlock()
- if err := rsrcMgr.FreeResourceID(ctx, intfID,
+ if err := rsrcMgr.TechprofileRef.FreeResourceID(ctx, intfID,
ponrmgr.ALLOC_ID,
AllocIDs); err != nil {
logger.Errorw(ctx, "error-while-freeing-all-alloc-ids-for-onu", log.Fields{
@@ -597,7 +564,7 @@
delete(rsrcMgr.gemPortIDsForOnu, intfOnuIDuniID)
rsrcMgr.gemPortIDsForOnuLock.Unlock()
- if err := rsrcMgr.FreeResourceID(ctx, intfID,
+ if err := rsrcMgr.TechprofileRef.FreeResourceID(ctx, intfID,
ponrmgr.GEMPORT_ID,
GEMPortIDs); err != nil {
logger.Errorw(ctx, "error-while-freeing-all-gem-port-ids-for-onu", log.Fields{
diff --git a/internal/pkg/resourcemanager/resourcemanager_test.go b/internal/pkg/resourcemanager/resourcemanager_test.go
index f8fabbc..9d8d068 100644
--- a/internal/pkg/resourcemanager/resourcemanager_test.go
+++ b/internal/pkg/resourcemanager/resourcemanager_test.go
@@ -27,6 +27,8 @@
"context"
"encoding/json"
"errors"
+ "github.com/opencord/voltha-lib-go/v6/pkg/techprofile"
+ "github.com/opencord/voltha-openolt-adapter/pkg/mocks"
"reflect"
"strconv"
"strings"
@@ -69,14 +71,15 @@
// fields mocks OpenOltResourceMgr struct.
type fields struct {
- DeviceID string
- Address string
- Args string
- KVStore *db.Backend
- DeviceType string
- DevInfo *openolt.DeviceInfo
- PonRsrMgr *ponrmgr.PONResourceManager
- NumOfPonPorts uint32
+ DeviceID string
+ Address string
+ Args string
+ KVStore *db.Backend
+ DeviceType string
+ DevInfo *openolt.DeviceInfo
+ PonRsrMgr *ponrmgr.PONResourceManager
+ NumOfPonPorts uint32
+ TechProfileRef techprofile.TechProfileIf
}
// MockKVClient mocks the AdapterProxy interface.
@@ -113,6 +116,8 @@
resMgr.PonRsrMgr.Technology = "XGS-PON"
resMgr.PonRsrMgr.PonResourceRanges = ranges
resMgr.PonRsrMgr.SharedIdxByType = sharedIdxByType
+ resMgr.TechProfileRef = mocks.MockTechProfile{}
+
/*
tpMgr, err := tp.NewTechProfile(ctx, resMgr.PonRsrMgr, "etcd", "127.0.0.1", "/")
if err != nil {
@@ -258,13 +263,14 @@
// testResMgrObject maps fields type to OpenOltResourceMgr type.
func testResMgrObject(testResMgr *fields) *OpenOltResourceMgr {
var rsrMgr = OpenOltResourceMgr{
- DeviceID: testResMgr.DeviceID,
- Args: testResMgr.Args,
- KVStore: testResMgr.KVStore,
- DeviceType: testResMgr.DeviceType,
- Address: testResMgr.Address,
- DevInfo: testResMgr.DevInfo,
- PonRsrMgr: testResMgr.PonRsrMgr,
+ DeviceID: testResMgr.DeviceID,
+ Args: testResMgr.Args,
+ KVStore: testResMgr.KVStore,
+ DeviceType: testResMgr.DeviceType,
+ Address: testResMgr.Address,
+ DevInfo: testResMgr.DevInfo,
+ PonRsrMgr: testResMgr.PonRsrMgr,
+ TechprofileRef: testResMgr.TechProfileRef,
}
rsrMgr.InitLocalCache()