[SDFAB-887][SDFAB-893] Missig flowrules in the ECFlowRuleStore
Mastership changes can happen during the processing of the flow rules stats,
If there is no master while we are executing handleExistingFlow we will get
null when we try to retrieve the stored flow rule. This is wrongly handled
with the removal of the flow rule from the store which will cause lost
of connectivity. This patch basically removes the flow rule only when we are
sure of what we are doing, when we are not the code verifies if there are
proper conditions to continue the processing (check if there was a mastership
change).
Additionally, this patch improves the logging of the ECFlowRuleStore
Change-Id: I7b79e7ab3d8ccfa6edca6ba4ad5de93ada082265
diff --git a/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java b/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java
index 833a40c..d593e45 100644
--- a/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java
+++ b/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java
@@ -526,19 +526,28 @@
checkNotNull(flowEntry, FLOW_RULE_NULL);
checkValidity();
FlowEntry storedEntry = store.getFlowEntry(flowEntry);
- if ((storedEntry != null && storedEntry.state() != FlowEntry.FlowEntryState.PENDING_REMOVE)
- && checkRuleLiveness(flowEntry, storedEntry)) {
- FlowRuleEvent event = store.addOrUpdateFlowRule(flowEntry);
- if (event == null) {
- log.debug("No flow store event generated.");
- return false;
+ if (storedEntry != null) {
+ // Flow rule is still valid, let's try to update the stats
+ if (storedEntry.state() != FlowEntry.FlowEntryState.PENDING_REMOVE &&
+ checkRuleLiveness(flowEntry, storedEntry)) {
+ FlowRuleEvent event = store.addOrUpdateFlowRule(flowEntry);
+ /* Something went wrong or there is no master
+ better check if it is the latter */
+ if (event == null) {
+ log.debug("No flow store event generated for addOrUpdate of {}", flowEntry);
+ return false;
+ } else {
+ log.trace("Flow {} {}", flowEntry, event.type());
+ post(event);
+ }
} else {
- log.trace("Flow {} {}", flowEntry, event.type());
- post(event);
+ log.debug("Removing {}", flowEntry);
+ removeFlowRules(flowEntry);
}
} else {
- log.debug("Removing flow rules....");
- removeFlowRules(flowEntry);
+ /* It was already removed or there is no master
+ better check if it is the latter */
+ return false;
}
return true;
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/flow/impl/ECFlowRuleStore.java b/core/store/dist/src/main/java/org/onosproject/store/flow/impl/ECFlowRuleStore.java
index e4850d8..602f4d3 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/flow/impl/ECFlowRuleStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/flow/impl/ECFlowRuleStore.java
@@ -380,7 +380,7 @@
public int getFlowRuleCount(DeviceId deviceId, FlowEntryState state) {
NodeId master = mastershipService.getMasterFor(deviceId);
if (master == null && deviceService.isAvailable(deviceId)) {
- log.debug("Failed to getFlowRuleCount: No master for {}", deviceId);
+ log.warn("Failed to getFlowRuleCount: No master for {}", deviceId);
return 0;
}
@@ -405,7 +405,7 @@
NodeId master = mastershipService.getMasterFor(rule.deviceId());
if (master == null && deviceService.isAvailable(rule.deviceId())) {
- log.debug("Failed to getFlowEntry: No master for {}", rule.deviceId());
+ log.warn("Failed to getFlowEntry: No master for {}", rule.deviceId());
return null;
}
@@ -451,7 +451,7 @@
NodeId master = mastershipService.getMasterFor(deviceId);
if (master == null) {
- log.warn("No master for {} ", deviceId);
+ log.warn("Failed to storeBatch: No master for {}", deviceId);
Set<FlowRule> allFailures = operation.getOperations()
.stream()
@@ -960,7 +960,7 @@
NodeId master = mastershipService.getMasterFor(deviceId);
if (master == null && deviceService.isAvailable(deviceId)) {
- log.debug("Failed to getTableStats: No master for {}", deviceId);
+ log.warn("Failed to getTableStats: No master for {}", deviceId);
return Collections.emptyList();
}