[VOL-4492] sporadic issue ensuring the expected device reason (mib state) in Multi-UNI/Multi-ONU deployments
Signed-off-by: mpagenko <michael.pagenkopf@adtran.com>
Change-Id: I8bba7f190799e81045f78a3299cf7645baf23893
diff --git a/VERSION b/VERSION
index 7a1a884..5fca835 100755
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-2.1.2-dev253
+2.1.2-dev254
diff --git a/internal/pkg/common/interfaces.go b/internal/pkg/common/interfaces.go
index a4612f7..7c0d835 100755
--- a/internal/pkg/common/interfaces.go
+++ b/internal/pkg/common/interfaces.go
@@ -61,7 +61,7 @@
GetUniVlanConfigFsm(uint8) IuniVlanConfigFsm
GetDeviceReasonString() string
- SetDeviceReason(uint8)
+ ReasonUpdate(context.Context, uint8, bool) error
GetCollectorIsRunning() bool
StartCollector(context.Context)
diff --git a/internal/pkg/core/device_handler.go b/internal/pkg/core/device_handler.go
index 27c0f52..d651df5 100755
--- a/internal/pkg/core/device_handler.go
+++ b/internal/pkg/core/device_handler.go
@@ -787,7 +787,7 @@
// DeviceReason to update acc.to modified py code as per beginning of Sept 2020
//TODO with VOL-3045/VOL-3046: catch and return error, valid for all occurrences in the codebase
- _ = dh.deviceReasonUpdate(ctx, cmn.DrOmciAdminLock, true)
+ _ = dh.ReasonUpdate(ctx, cmn.DrOmciAdminLock, true)
}
}
}
@@ -960,11 +960,11 @@
return
}
if abTechProfInstLoadFailed {
- dh.SetDeviceReason(cmn.DrTechProfileConfigDownloadFailed)
+ _ = dh.ReasonUpdate(ctx, cmn.DrTechProfileConfigDownloadFailed, false)
dh.stopReconciling(ctx, false, cWaitReconcileFlowNoActivity)
return
} else if dh.IsSkipOnuConfigReconciling() {
- dh.SetDeviceReason(cmn.DrTechProfileConfigDownloadSuccess)
+ _ = dh.ReasonUpdate(ctx, cmn.DrTechProfileConfigDownloadSuccess, false)
}
if !abFlowsFound {
logger.Debugw(ctx, "reconciling - no flows have been stored before adapter restart - terminate reconcilement",
@@ -1064,7 +1064,7 @@
}
return
}
- dh.SetDeviceReason(cmn.DrOmciFlowsPushed)
+ _ = dh.ReasonUpdate(ctx, cmn.DrOmciFlowsPushed, false)
}
}
@@ -1267,7 +1267,7 @@
logger.Errorw(ctx, "error-updating-device-state", log.Fields{"device-id": dh.DeviceID, "error": err})
return
}
- if err := dh.deviceReasonUpdate(ctx, cmn.DrRebooting, true); err != nil {
+ if err := dh.ReasonUpdate(ctx, cmn.DrRebooting, true); err != nil {
return
}
dh.SetReadyForOmciConfig(false)
@@ -1639,7 +1639,7 @@
dh.device.Vendor = "OpenONU"
dh.device.Model = "go"
dh.device.Reason = cmn.DeviceReasonMap[cmn.DrActivatingOnu]
- dh.SetDeviceReason(cmn.DrActivatingOnu)
+ _ = dh.ReasonUpdate(ctx, cmn.DrActivatingOnu, false)
dh.logicalDeviceID = dh.DeviceID // really needed - what for ??? //TODO!!!
@@ -1990,7 +1990,7 @@
return err
}
- _ = dh.deviceReasonUpdate(ctx, cmn.DrStartingOpenomci, !dh.IsReconciling())
+ _ = dh.ReasonUpdate(ctx, cmn.DrStartingOpenomci, !dh.IsReconciling())
/* this might be a good time for Omci Verify message? */
verifyExec := make(chan bool)
@@ -2163,7 +2163,7 @@
dh.SetReadyForOmciConfig(false)
- if err := dh.deviceReasonUpdate(ctx, cmn.DrStoppingOpenomci, true); err != nil {
+ if err := dh.ReasonUpdate(ctx, cmn.DrStoppingOpenomci, true); err != nil {
// abort: system behavior is just unstable ...
return err
}
@@ -2285,7 +2285,7 @@
logger.Warnw(ctx, "store persistent data error - continue as there will be additional write attempts",
log.Fields{"device-id": dh.DeviceID, "err": err})
}
- _ = dh.deviceReasonUpdate(ctx, cmn.DrDiscoveryMibsyncComplete, !dh.IsReconciling())
+ _ = dh.ReasonUpdate(ctx, cmn.DrDiscoveryMibsyncComplete, !dh.IsReconciling())
dh.AddAllUniPorts(ctx)
/* 200605: lock processing after initial MIBUpload removed now as the ONU should be in the lock state per default here */
@@ -2369,7 +2369,7 @@
logger.Debugw(ctx, "reconciling - don't notify core about DeviceStateUpdate to ACTIVE",
log.Fields{"device-id": dh.DeviceID})
}
- _ = dh.deviceReasonUpdate(ctx, cmn.DrInitialMibDownloaded, !dh.IsReconciling())
+ _ = dh.ReasonUpdate(ctx, cmn.DrInitialMibDownloaded, !dh.IsReconciling())
if !dh.GetCollectorIsRunning() {
// Start PM collector routine
@@ -2465,7 +2465,7 @@
logger.Debugw(ctx, "DeviceReasonUpdate upon disable", log.Fields{"reason": cmn.DeviceReasonMap[cmn.DrOmciAdminLock], "device-id": dh.DeviceID})
// DeviceReason to update acc.to modified py code as per beginning of Sept 2020
- _ = dh.deviceReasonUpdate(ctx, cmn.DrOmciAdminLock, true)
+ _ = dh.ReasonUpdate(ctx, cmn.DrOmciAdminLock, true)
//transfer the modified logical uni port state
dh.DisableUniPortStateUpdate(ctx)
@@ -2499,7 +2499,7 @@
logger.Debugw(ctx, "DeviceReasonUpdate upon re-enable", log.Fields{
"reason": cmn.DeviceReasonMap[cmn.DrOnuReenabled], "device-id": dh.DeviceID})
// DeviceReason to update acc.to modified py code as per beginning of Sept 2020
- _ = dh.deviceReasonUpdate(ctx, cmn.DrOnuReenabled, true)
+ _ = dh.ReasonUpdate(ctx, cmn.DrOnuReenabled, true)
//transfer the modified logical uni port state
dh.EnableUniPortStateUpdate(ctx)
@@ -2536,8 +2536,8 @@
// attention: the device reason update is done based on ONU-UNI-Port related activity
// - which may cause some inconsistency
if dh.getDeviceReason() != cmn.DrTechProfileConfigDownloadSuccess {
- // which may be the case from some previous actvity even on this UNI Port (but also other UNI ports)
- _ = dh.deviceReasonUpdate(ctx, cmn.DrTechProfileConfigDownloadSuccess, !dh.IsReconciling())
+ // which may be the case from some previous activity even on this UNI Port (but also other UNI ports)
+ _ = dh.ReasonUpdate(ctx, cmn.DrTechProfileConfigDownloadSuccess, !dh.IsReconciling())
}
if dh.IsReconciling() {
go dh.ReconcileDeviceFlowConfig(ctx)
@@ -2547,8 +2547,8 @@
// attention: the device reason update is done based on ONU-UNI-Port related activity
// - which may cause some inconsistency
if dh.getDeviceReason() != cmn.DrTechProfileConfigDeleteSuccess {
- // which may be the case from some previous actvity even on this ONU port (but also other UNI ports)
- _ = dh.deviceReasonUpdate(ctx, cmn.DrTechProfileConfigDeleteSuccess, true)
+ // which may be the case from some previous activity even on this ONU port (but also other UNI ports)
+ _ = dh.ReasonUpdate(ctx, cmn.DrTechProfileConfigDeleteSuccess, true)
}
}
}
@@ -2561,9 +2561,9 @@
if aDevEvent == cmn.OmciVlanFilterAddDone || aDevEvent == cmn.OmciVlanFilterAddDoneNoKvStore {
if dh.getDeviceReason() != cmn.DrOmciFlowsPushed {
- // which may be the case from some previous actvity on another UNI Port of the ONU
+ // which may be the case from some previous activity on another UNI Port of the ONU
// or even some previous flow add activity on the same port
- _ = dh.deviceReasonUpdate(ctx, cmn.DrOmciFlowsPushed, !dh.IsReconciling())
+ _ = dh.ReasonUpdate(ctx, cmn.DrOmciFlowsPushed, !dh.IsReconciling())
if dh.IsReconciling() {
go dh.reconcileEnd(ctx)
}
@@ -2571,7 +2571,7 @@
} else {
if dh.getDeviceReason() != cmn.DrOmciFlowsDeleted {
//not relevant for reconcile
- _ = dh.deviceReasonUpdate(ctx, cmn.DrOmciFlowsDeleted, true)
+ _ = dh.ReasonUpdate(ctx, cmn.DrOmciFlowsDeleted, true)
}
}
@@ -3538,23 +3538,29 @@
"device-id": dh.DeviceID, "called from": aCallerIdent})
}
-func (dh *deviceHandler) deviceReasonUpdate(ctx context.Context, deviceReason uint8, notifyCore bool) error {
-
- dh.SetDeviceReason(deviceReason)
+//ReasonUpdate set the internally store device reason and if requested in notifyCore updates this state in the core
+// (renamed from previous deviceReasonUpdate to avoid confusing with the core function DeviceReasonUpdate)
+func (dh *deviceHandler) ReasonUpdate(ctx context.Context, deviceReason uint8, notifyCore bool) error {
+ // acquire the deviceReason semaphore throughout this function including the possible update processing in core
+ // in order to avoid reversion of the state sequence within core in case of quasi-parallel calls (eg. in multi UNI processing)
+ dh.mutexDeviceReason.Lock()
+ defer dh.mutexDeviceReason.Unlock()
if notifyCore {
//TODO with VOL-3045/VOL-3046: return the error and stop further processing at calling position
if err := dh.updateDeviceReasonInCore(ctx, &ca.DeviceReason{
DeviceId: dh.DeviceID,
Reason: cmn.DeviceReasonMap[deviceReason],
}); err != nil {
- logger.Errorf(ctx, "DeviceReasonUpdate error: %s",
+ logger.Errorf(ctx, "updating reason in core failed for: %s",
log.Fields{"device-id": dh.DeviceID, "error": err}, cmn.DeviceReasonMap[deviceReason])
return err
}
- logger.Infof(ctx, "DeviceReasonUpdate success: %s - device-id: %s", cmn.DeviceReasonMap[deviceReason], dh.DeviceID)
- return nil
+ } else {
+ logger.Debugf(ctx, "update reason in core not requested: %s - device-id: %s", cmn.DeviceReasonMap[deviceReason], dh.DeviceID)
}
- logger.Infof(ctx, "Don't notify core about DeviceReasonUpdate: %s - device-id: %s", cmn.DeviceReasonMap[deviceReason], dh.DeviceID)
+ dh.deviceReason = deviceReason
+ logger.Infof(ctx, "reason update done for: %s - device-id: %s - with core update: %v",
+ cmn.DeviceReasonMap[deviceReason], dh.DeviceID, notifyCore)
return nil
}
@@ -4080,12 +4086,6 @@
return dh.reconciling == cSkipOnuConfigReconciling
}
-func (dh *deviceHandler) SetDeviceReason(value uint8) {
- dh.mutexDeviceReason.Lock()
- dh.deviceReason = value
- dh.mutexDeviceReason.Unlock()
-}
-
func (dh *deviceHandler) getDeviceReason() uint8 {
dh.mutexDeviceReason.RLock()
value := dh.deviceReason
@@ -4110,7 +4110,7 @@
}
func (dh *deviceHandler) deviceReconcileFailedUpdate(ctx context.Context, deviceReason uint8, connectStatus voltha.ConnectStatus_Types) {
- if err := dh.deviceReasonUpdate(ctx, deviceReason, true); err != nil {
+ if err := dh.ReasonUpdate(ctx, deviceReason, true); err != nil {
logger.Errorw(ctx, "unable to update device reason to core",
log.Fields{"device-id": dh.DeviceID, "Err": err})
}
diff --git a/internal/pkg/mib/mib_sync.go b/internal/pkg/mib/mib_sync.go
index 6e2378e..6ff7425 100755
--- a/internal/pkg/mib/mib_sync.go
+++ b/internal/pkg/mib/mib_sync.go
@@ -305,7 +305,7 @@
if oo.getMibFromTemplate(ctx) {
oo.baseDeviceHandler.StartReconciling(ctx, true)
oo.baseDeviceHandler.AddAllUniPorts(ctx)
- oo.baseDeviceHandler.SetDeviceReason(cmn.DrInitialMibDownloaded)
+ _ = oo.baseDeviceHandler.ReasonUpdate(ctx, cmn.DrInitialMibDownloaded, false)
oo.baseDeviceHandler.SetReadyForOmciConfig(true)
if !oo.baseDeviceHandler.GetCollectorIsRunning() {
@@ -328,7 +328,7 @@
if oo.SOnuPersistentData.PersUniDisableDone {
oo.MutexPersOnuConfig.RUnlock()
oo.baseDeviceHandler.DisableUniPortStateUpdate(ctx)
- oo.baseDeviceHandler.SetDeviceReason(cmn.DrOmciAdminLock)
+ _ = oo.baseDeviceHandler.ReasonUpdate(ctx, cmn.DrOmciAdminLock, false)
} else {
oo.MutexPersOnuConfig.RUnlock()
oo.baseDeviceHandler.EnableUniPortStateUpdate(ctx)