[VOL-3628] ATT scenario subscriber add/remove: synchronized flow and tech profile handling + correction clear persistent data
Signed-off-by: mpagenko <michael.pagenkopf@adtran.com>
Change-Id: Id77ba0e8b537b49663439f86b0d84734dba6cf23
diff --git a/VERSION b/VERSION
index 52cd72d..1713aea 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-0.1.13-dev137
+0.1.13-dev138
diff --git a/internal/pkg/onuadaptercore/device_handler.go b/internal/pkg/onuadaptercore/device_handler.go
index 4dce9b5..49bdaaa 100644
--- a/internal/pkg/onuadaptercore/device_handler.go
+++ b/internal/pkg/onuadaptercore/device_handler.go
@@ -652,7 +652,7 @@
logger.Errorw("error-updating-device-state", log.Fields{"device-id": dh.deviceID, "error": err})
}
- logger.Debugw("DeviceReasonUpdate upon re-enable", log.Fields{
+ logger.Debugw("DeviceReasonUpdate upon disable", log.Fields{
"reason": "omci-admin-lock", "device-id": dh.deviceID})
// DeviceReason to update acc.to modified py code as per beginning of Sept 2020
if err := dh.coreProxy.DeviceReasonUpdate(context.TODO(), dh.deviceID, "omci-admin-lock"); err != nil {
@@ -697,7 +697,11 @@
return
}
if err := pDevEntry.restoreDataFromOnuKvStore(context.TODO()); err != nil {
- logger.Errorw("reconciling - restoring OnuTp-data failed - abort", log.Fields{"err": err, "device-id": dh.deviceID})
+ if err == fmt.Errorf("no-ONU-data-found") {
+ logger.Debugw("no persistent data found - abort reconciling", log.Fields{"device-id": dh.deviceID})
+ } else {
+ logger.Errorw("reconciling - restoring OnuTp-data failed - abort", log.Fields{"err": err, "device-id": dh.deviceID})
+ }
dh.reconciling = false
return
}
@@ -783,13 +787,14 @@
dh.reconciling = false
}
-func (dh *deviceHandler) deleteDevice(device *voltha.Device) error {
- logger.Debugw("delete-device", log.Fields{"device-id": device.Id, "SerialNumber": device.SerialNumber})
+func (dh *deviceHandler) deleteDevicePersistencyData() error {
+ logger.Debugw("delete device persistency data", log.Fields{"device-id": dh.deviceID})
- pDevEntry := dh.getOnuDeviceEntry(true)
+ pDevEntry := dh.getOnuDeviceEntry(false)
if pDevEntry == nil {
- logger.Errorw("No valid OnuDevice - aborting", log.Fields{"device-id": dh.deviceID})
- return fmt.Errorf("no valid OnuDevice: %s", dh.deviceID)
+ //IfDevEntry does not exist here, no problem - no persistent data should have been stored
+ logger.Debugw("OnuDevice does not exist - nothing to delete", log.Fields{"device-id": dh.deviceID})
+ return nil
}
pDevEntry.lockOnuKVStoreMutex()
defer pDevEntry.unlockOnuKVStoreMutex()
@@ -1339,6 +1344,7 @@
// (but note that the deviceReason may also have changed to e.g. TechProf*Delete_Success in between)
if dh.deviceReason != "stopping-openomci" {
logger.Debugw("updateInterface-started - stopping-device", log.Fields{"device-id": dh.deviceID})
+
//stop all running FSM processing - make use of the DH-state as mirrored in the deviceReason
//here no conflict with aborted FSM's should arise as a complete OMCI initialization is assumed on ONU-Up
//but that might change with some simple MDS check on ONU-Up treatment -> attention!!!
@@ -1348,6 +1354,8 @@
// abort: system behavior is just unstable ...
return err
}
+ //all stored persitent data are not valid anymore (loosing knowledge abouit the connected ONU)
+ _ = dh.deleteDevicePersistencyData() //ignore possible errors here and continue, hope is that data is synchronized with new ONU-Up
//deviceEntry stop without omciCC reset here, regarding the OMCI_CC still valid for this ONU
// - in contrary to disableDevice - compare with processUniDisableStateDoneEvent
@@ -1441,6 +1449,10 @@
//VlanFilterFsm exists and was already started
pVlanFilterStatemachine := pVlanFilterFsm.pAdaptFsm.pFsm
if pVlanFilterStatemachine != nil {
+ //reset of all Fsm is always accompagnied by global persistency data removal
+ // no need to remove specific data
+ pVlanFilterFsm.RequestClearPersistency(false)
+ //and reset the UniVlanConfig FSM
_ = pVlanFilterStatemachine.Event(vlanEvReset)
}
}
@@ -2313,7 +2325,7 @@
logger.Debugw("reconciling - don't store persistent UniFlowConfig", log.Fields{"device-id": dh.deviceID})
return nil
}
- logger.Debugw("Store persistent UniFlowConfig", log.Fields{"device-id": dh.deviceID})
+ logger.Debugw("Store or clear persistent UniFlowConfig", log.Fields{"device-id": dh.deviceID})
pDevEntry := dh.getOnuDeviceEntry(true)
if pDevEntry == nil {
diff --git a/internal/pkg/onuadaptercore/omci_vlan_config.go b/internal/pkg/onuadaptercore/omci_vlan_config.go
index 2fbc5db..36efa13 100644
--- a/internal/pkg/onuadaptercore/omci_vlan_config.go
+++ b/internal/pkg/onuadaptercore/omci_vlan_config.go
@@ -140,6 +140,7 @@
omciMIdsResponseReceived chan bool //seperate channel needed for checking multiInstance OMCI message responses
pAdaptFsm *AdapterFsm
acceptIncrementalEvtoOption bool
+ clearPersistency bool
mutexFlowParams sync.Mutex
uniVlanFlowParamsSlice []uniVlanFlowParams
uniRemoveFlowsSlice []uniRemoveVlanFlowParams
@@ -173,6 +174,7 @@
numUniFlows: 0,
configuredUniFlow: 0,
numRemoveFlows: 0,
+ clearPersistency: true,
}
instFsm.pAdaptFsm = NewAdapterFsm(aName, instFsm.deviceID, aCommChannel)
@@ -299,6 +301,14 @@
return nil
}
+//RequestClearPersistency sets the internal flag to not clear persistency data (false=NoClear)
+func (oFsm *UniVlanConfigFsm) RequestClearPersistency(aClear bool) {
+ //mutex protection is required for possible concurrent access to FSM members
+ oFsm.mutexFlowParams.Lock()
+ defer oFsm.mutexFlowParams.Unlock()
+ oFsm.clearPersistency = aClear
+}
+
//SetUniFlowParams verifies on existence of flow parameters to be configured,
// optionally udates the cookie list or appends a new flow if there is space
// if possible the FSM is trigggerd to start with the processing
@@ -470,6 +480,9 @@
oFsm.numUniFlows = 0 //no more flows
oFsm.configuredUniFlow = 0 //no more flows configured
oFsm.uniVlanFlowParamsSlice = nil //reset the slice
+ //at this point it is evident that no flow anymore refers to a still possibly active Techprofile
+ //request that this profile gets deleted before a new flow add is allowed
+ oFsm.pUniTechProf.setProfileToDelete(oFsm.pOnuUniPort.uniID, true)
logger.Debugw("UniVlanConfigFsm flow removal - no more flows", log.Fields{
"device-id": oFsm.deviceID})
} else {
@@ -479,9 +492,28 @@
//TODO!! might be needed to consider still outstanding configure requests ..
// so a flow at removal might still not be configured !?!
}
+ usedTpID := storedUniFlowParams.VlanRuleParams.TpID
//cut off the requested flow by slicing out this element
oFsm.uniVlanFlowParamsSlice = append(
oFsm.uniVlanFlowParamsSlice[:flow], oFsm.uniVlanFlowParamsSlice[flow+1:]...)
+ //here we have to check, if there are still other flows referencing to the actual ProfileId
+ // before we can request that this profile gets deleted before a new flow add is allowed
+ tpIDInOtherFlows := false
+ for _, tpUniFlowParams := range oFsm.uniVlanFlowParamsSlice {
+ if tpUniFlowParams.VlanRuleParams.TpID == usedTpID {
+ tpIDInOtherFlows = true
+ break // search loop can be left
+ }
+ }
+ if tpIDInOtherFlows {
+ logger.Debugw("UniVlanConfigFsm tp-id used in deleted flow is still used in other flows", log.Fields{
+ "device-id": oFsm.deviceID, "tp-id": usedTpID})
+ } else {
+ logger.Debugw("UniVlanConfigFsm tp-id used in deleted flow is not used anymore", log.Fields{
+ "device-id": oFsm.deviceID, "tp-id": usedTpID})
+ //request that this profile gets deleted before a new flow add is allowed
+ oFsm.pUniTechProf.setProfileToDelete(oFsm.pOnuUniPort.uniID, true)
+ }
logger.Debugw("UniVlanConfigFsm flow removal - specific flow removed from data", log.Fields{
"device-id": oFsm.deviceID})
}
@@ -943,7 +975,20 @@
logger.Debugw("UniVlanConfigFsm enters disabled state", log.Fields{"device-id": oFsm.deviceID})
oFsm.pLastTxMeInstance = nil
if oFsm.pDeviceHandler != nil {
- //request removal of 'reference' in the Handler (completely clear the FSM)
+ //TODO: to clarify with improved error treatment for VlanConfigFsm (timeout,reception) errors
+ // current code removes the complete FSM including all flow/rule configuration done so far
+ // this might be a bit to much, it would require fully new flow config from rwCore (at least on OnuDown/up)
+ // maybe a more sophisticated approach is possible without clearing the data
+ if oFsm.clearPersistency {
+ //permanently remove possibly stored persistent data
+ if len(oFsm.uniVlanFlowParamsSlice) > 0 {
+ var emptySlice = make([]uniVlanFlowParams, 0)
+ _ = oFsm.pDeviceHandler.storePersUniFlowConfig(oFsm.pOnuUniPort.uniID, &emptySlice) //ignore errors
+ }
+ } else {
+ logger.Debugw("UniVlanConfigFsm persistency data not cleared", log.Fields{"device-id": oFsm.deviceID})
+ }
+ //request removal of 'reference' in the Handler (completely clear the FSM and its data)
go oFsm.pDeviceHandler.RemoveVlanFilterFsm(oFsm.pOnuUniPort)
}
}
diff --git a/internal/pkg/onuadaptercore/onu_device_entry.go b/internal/pkg/onuadaptercore/onu_device_entry.go
index ad683b0..66dfabd 100644
--- a/internal/pkg/onuadaptercore/onu_device_entry.go
+++ b/internal/pkg/onuadaptercore/onu_device_entry.go
@@ -542,8 +542,8 @@
logger.Debugw("ONU-data", log.Fields{"sOnuPersistentData": oo.sOnuPersistentData,
"device-id": oo.deviceID})
} else {
- logger.Errorw("no ONU-data found", log.Fields{"path": oo.onuKVStorePath, "device-id": oo.deviceID})
- return fmt.Errorf(fmt.Sprintf("no-ONU-data-found-%s", oo.deviceID))
+ logger.Debugw("no ONU-data found", log.Fields{"path": oo.onuKVStorePath, "device-id": oo.deviceID})
+ return fmt.Errorf("no-ONU-data-found")
}
} else {
logger.Errorw("unable to read from KVstore", log.Fields{"device-id": oo.deviceID})
@@ -572,6 +572,10 @@
func (oo *OnuDeviceEntry) deletePersistentData(ctx context.Context, aProcessingStep uint8) {
+ logger.Debugw("delete and clear internal persistency data", log.Fields{"device-id": oo.deviceID})
+ oo.sOnuPersistentData.PersUniConfig = nil //releasing all UniConfig entries to garbage collector
+ oo.sOnuPersistentData = onuPersistentData{0, 0, "", "", "", make([]uniPersConfig, 0)} //default entry
+
logger.Debugw("delete ONU-data from KVStore", log.Fields{"device-id": oo.deviceID})
err := oo.onuKVStore.Delete(ctx, oo.onuKVStorePath)
if err != nil {
@@ -669,6 +673,11 @@
// (during which the flows are removed/re-assigned but the techProf is left active)
//and as the TechProfile is regarded as active we have to verify, if some flow configuration still waits on it
// (should not be the case, but should not harm or be more robust ...)
+ // and to be sure, that for some reason the corresponding TpDelete was lost somewhere in history
+ // we also reset a possibly outstanding delete request - repeated TpConfig is regarded as valid for waiting flow config
+ if oo.baseDeviceHandler.pOnuTP != nil {
+ oo.baseDeviceHandler.pOnuTP.setProfileToDelete(aUniID, false)
+ }
go oo.baseDeviceHandler.VerifyVlanConfigRequest(aUniID)
}
return false //indicate 'no change' - nothing more to do, TechProf inter-adapter message is return with success anyway here
diff --git a/internal/pkg/onuadaptercore/onu_uni_tp.go b/internal/pkg/onuadaptercore/onu_uni_tp.go
index 97b4dbd..2e19274 100644
--- a/internal/pkg/onuadaptercore/onu_uni_tp.go
+++ b/internal/pkg/onuadaptercore/onu_uni_tp.go
@@ -54,6 +54,7 @@
techProfileType string
techProfileID uint16
techProfileConfigDone bool
+ techProfileToDelete bool
}
type tcontParamStruct struct {
@@ -297,6 +298,7 @@
//note the limitation on ID range (probably even more limited) - based on usage within OMCI EntityID
onuTP.mapUniTpIndication[aUniID].techProfileID = uint16(profID)
onuTP.mapUniTpIndication[aUniID].techProfileConfigDone = false
+ onuTP.mapUniTpIndication[aUniID].techProfileToDelete = false
logger.Debugw("tech-profile path indications",
log.Fields{"device-id": onuTP.deviceID, "uni-id": aUniID,
"profType": onuTP.mapUniTpIndication[aUniID].techProfileType,
@@ -569,9 +571,23 @@
if onuTP.mapUniTpIndication[aUniID].techProfileID == aTpID {
onuTP.mutexTPState.Lock()
defer onuTP.mutexTPState.Unlock()
+ if onuTP.mapUniTpIndication[aUniID].techProfileToDelete {
+ logger.Debugw("TechProfile not relevant for requested flow config - waiting on delete",
+ log.Fields{"device-id": onuTP.deviceID, "uni-id": aUniID})
+ return false //still waiting for removal of this techProfile first
+ }
return onuTP.mapUniTpIndication[aUniID].techProfileConfigDone
}
}
//for all other constellations indicate false = Config not done
return false
}
+
+// setProfileToDelete sets the requested techProfile toDelete state (if possible)
+func (onuTP *onuUniTechProf) setProfileToDelete(aUniID uint8, aState bool) {
+ if _, existTP := onuTP.mapUniTpIndication[aUniID]; existTP {
+ onuTP.mutexTPState.Lock()
+ onuTP.mapUniTpIndication[aUniID].techProfileToDelete = aState
+ onuTP.mutexTPState.Unlock()
+ } //else: the state is just ignored (does not exist)
+}
diff --git a/internal/pkg/onuadaptercore/openonu.go b/internal/pkg/onuadaptercore/openonu.go
index 5fbaa31..cdce427 100644
--- a/internal/pkg/onuadaptercore/openonu.go
+++ b/internal/pkg/onuadaptercore/openonu.go
@@ -326,19 +326,15 @@
// Delete_device deletes the given device
func (oo *OpenONUAC) Delete_device(device *voltha.Device) error {
- logger.Debugw("Delete_device", log.Fields{"device-id": device.Id})
+ logger.Debugw("Delete_device", log.Fields{"device-id": device.Id, "SerialNumber": device.SerialNumber})
if handler := oo.getDeviceHandler(device.Id, false); handler != nil {
- err := handler.deleteDevice(device)
+ err := handler.deleteDevicePersistencyData()
//don't leave any garbage - even in error case
oo.deleteDeviceHandlerToMap(handler)
- if err != nil {
- return err
- }
- } else {
- logger.Warnw("no handler found for device-deletion", log.Fields{"device-id": device.Id})
- return fmt.Errorf(fmt.Sprintf("handler-not-found-%s", device.Id))
+ return err
}
- return nil
+ logger.Warnw("no handler found for device-deletion", log.Fields{"device-id": device.Id})
+ return fmt.Errorf(fmt.Sprintf("handler-not-found-%s", device.Id))
}
//Get_device_details unimplemented