[SDFAB-1100] Fix flow removal logic
If the store is already in sync re-issueing a batch
request is dangerous (race with conflictual writes)
and wrong. In this case, if the device is still reporting
the flow, it is enough to send a flow removal directly.
Instead, if the flow is expired it is important to update
the store accordingly and then issue a flow removal as
consequence of the batch request.
Change-Id: I11a595e0a91f0efdc1cac3bf1d589a5108f1ae06
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 9bc5abc..670103b 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
@@ -534,9 +534,8 @@
return false;
}
FlowRuleEvent event = store.addOrUpdateFlowRule(flowEntry);
- // Something went wrong or there is no master or
- // the device is not available better check if it
- // is the latter cases
+ // Something went wrong or there is no master or the device
+ // is not available better check if it is the latter cases
if (event == null) {
log.debug("No flow store event generated for addOrUpdate of {}", flowEntry);
return false;
@@ -544,7 +543,14 @@
log.trace("Flow {} {}", flowEntry, event.type());
post(event);
}
- } else {
+ } else if (storedEntry.state() == FlowEntry.FlowEntryState.PENDING_REMOVE) {
+ // Store is already in sync, let's re-issue flow removal only
+ log.debug("Removing {} from the device", flowEntry);
+ FlowRuleProvider frp = getProvider(flowEntry.deviceId());
+ frp.removeFlowRule(flowEntry);
+ } else if (!checkRuleLiveness(flowEntry, storedEntry)) {
+ // Update store first as the flow entry is expired. Then,
+ // as consequence of this a flow removal will be sent.
log.debug("Removing {}", flowEntry);
removeFlowRules(flowEntry);
}