flowrules no longer install a timeout but are monitored by onos in order to be expired
Change-Id: Ibd1a5952349d7ccb27c92b4982d04574f31424c0
diff --git a/core/net/src/main/java/org/onlab/onos/net/flow/impl/FlowRuleManager.java b/core/net/src/main/java/org/onlab/onos/net/flow/impl/FlowRuleManager.java
index a6f5ebb..00619b3 100644
--- a/core/net/src/main/java/org/onlab/onos/net/flow/impl/FlowRuleManager.java
+++ b/core/net/src/main/java/org/onlab/onos/net/flow/impl/FlowRuleManager.java
@@ -5,6 +5,9 @@
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
@@ -59,6 +62,8 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected DeviceService deviceService;
+ private final Map<FlowRule, AtomicInteger> deadRounds = new ConcurrentHashMap<>();
+
@Activate
public void activate() {
store.setDelegate(delegate);
@@ -84,6 +89,7 @@
FlowRule f = flowRules[i];
final Device device = deviceService.getDevice(f.deviceId());
final FlowRuleProvider frp = getProvider(device.providerId());
+ deadRounds.put(f, new AtomicInteger(0));
store.storeFlowRule(f);
frp.applyFlowRule(f);
}
@@ -98,6 +104,7 @@
f = flowRules[i];
device = deviceService.getDevice(f.deviceId());
frp = getProvider(device.providerId());
+ deadRounds.remove(f);
store.deleteFlowRule(f);
frp.removeFlowRule(f);
}
@@ -161,11 +168,7 @@
switch (stored.state()) {
case ADDED:
case PENDING_ADD:
- if (flowRule.expired()) {
- event = store.removeFlowRule(flowRule);
- } else {
frp.applyFlowRule(stored);
- }
break;
case PENDING_REMOVE:
case REMOVED:
@@ -181,8 +184,8 @@
}
}
- @Override
- public void flowMissing(FlowRule flowRule) {
+
+ private void flowMissing(FlowRule flowRule) {
checkNotNull(flowRule, FLOW_RULE_NULL);
checkValidity();
Device device = deviceService.getDevice(flowRule.deviceId());
@@ -209,29 +212,47 @@
}
- @Override
- public void extraneousFlow(FlowRule flowRule) {
+
+ private void extraneousFlow(FlowRule flowRule) {
checkNotNull(flowRule, FLOW_RULE_NULL);
checkValidity();
removeFlowRules(flowRule);
log.debug("Flow {} is on switch but not in store.", flowRule);
}
- @Override
- public void flowAdded(FlowRule flowRule) {
+
+ private void flowAdded(FlowRule flowRule) {
checkNotNull(flowRule, FLOW_RULE_NULL);
checkValidity();
- FlowRuleEvent event = store.addOrUpdateFlowRule(flowRule);
- if (event == null) {
- log.debug("No flow store event generated.");
+ if (deadRounds.containsKey(flowRule) &&
+ checkRuleLiveness(flowRule, store.getFlowRule(flowRule))) {
+
+ FlowRuleEvent event = store.addOrUpdateFlowRule(flowRule);
+ if (event == null) {
+ log.debug("No flow store event generated.");
+ } else {
+ log.debug("Flow {} {}", flowRule, event.type());
+ post(event);
+ }
} else {
- log.debug("Flow {} {}", flowRule, event.type());
- post(event);
+ removeFlowRules(flowRule);
}
}
+ private boolean checkRuleLiveness(FlowRule swRule, FlowRule storedRule) {
+ int timeout = storedRule.timeout();
+ if (storedRule.packets() != swRule.packets()) {
+ deadRounds.get(swRule).set(0);
+ return true;
+ }
+
+ return (deadRounds.get(swRule).getAndIncrement() *
+ FlowRuleProvider.POLL_INTERVAL) <= timeout;
+
+ }
+
// Posts the specified event to the local event dispatcher.
private void post(FlowRuleEvent event) {
if (event != null) {
diff --git a/core/net/src/test/java/org/onlab/onos/net/flow/impl/FlowRuleManagerTest.java b/core/net/src/test/java/org/onlab/onos/net/flow/impl/FlowRuleManagerTest.java
index 5ff72a2..0b451c0 100644
--- a/core/net/src/test/java/org/onlab/onos/net/flow/impl/FlowRuleManagerTest.java
+++ b/core/net/src/test/java/org/onlab/onos/net/flow/impl/FlowRuleManagerTest.java
@@ -9,7 +9,9 @@
import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_UPDATED;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
+import java.util.Set;
import org.junit.After;
import org.junit.Before;
@@ -42,6 +44,7 @@
import org.onlab.onos.net.provider.ProviderId;
import org.onlab.onos.store.trivial.impl.SimpleFlowRuleStore;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
@@ -52,6 +55,7 @@
private static final ProviderId PID = new ProviderId("of", "foo");
private static final DeviceId DID = DeviceId.deviceId("of:001");
+ private static final int TIMEOUT = 10;
private static final Device DEV = new DefaultDevice(
PID, DID, Type.SWITCH, "", "", "", "");
@@ -96,7 +100,7 @@
private FlowRule flowRule(int tsval, int trval) {
TestSelector ts = new TestSelector(tsval);
TestTreatment tr = new TestTreatment(trval);
- return new DefaultFlowRule(DID, ts, tr, 0, appId);
+ return new DefaultFlowRule(DID, ts, tr, 0, appId, TIMEOUT);
}
private FlowRule flowRule(FlowRule rule, FlowRuleState state) {
@@ -105,7 +109,8 @@
private FlowRule addFlowRule(int hval) {
FlowRule rule = flowRule(hval, hval);
- providerService.flowAdded(rule);
+ service.applyFlowRules(rule);
+
assertNotNull("rule should be found", service.getFlowEntries(DID));
return rule;
}
@@ -135,13 +140,18 @@
public void getFlowEntries() {
assertTrue("store should be empty",
Sets.newHashSet(service.getFlowEntries(DID)).isEmpty());
- addFlowRule(1);
- addFlowRule(2);
+ FlowRule f1 = addFlowRule(1);
+ FlowRule f2 = addFlowRule(2);
+
assertEquals("2 rules should exist", 2, flowCount());
+
+ providerService.pushFlowMetrics(DID, ImmutableList.of(f1, f2));
validateEvents(RULE_ADDED, RULE_ADDED);
addFlowRule(1);
assertEquals("should still be 2 rules", 2, flowCount());
+
+ providerService.pushFlowMetrics(DID, ImmutableList.of(f1));
validateEvents(RULE_UPDATED);
}
@@ -179,8 +189,10 @@
public void removeFlowRules() {
FlowRule f1 = addFlowRule(1);
FlowRule f2 = addFlowRule(2);
- addFlowRule(3);
+ FlowRule f3 = addFlowRule(3);
assertEquals("3 rules should exist", 3, flowCount());
+
+ providerService.pushFlowMetrics(DID, ImmutableList.of(f1, f2, f3));
validateEvents(RULE_ADDED, RULE_ADDED, RULE_ADDED);
FlowRule rem1 = flowRule(f1, FlowRuleState.REMOVED);
@@ -200,8 +212,9 @@
@Test
public void flowRemoved() {
FlowRule f1 = addFlowRule(1);
+ FlowRule f2 = addFlowRule(2);
+ providerService.pushFlowMetrics(f1.deviceId(), ImmutableList.of(f1, f2));
service.removeFlowRules(f1);
- addFlowRule(2);
FlowRule rem1 = flowRule(f1, FlowRuleState.REMOVED);
providerService.flowRemoved(rem1);
validateEvents(RULE_ADDED, RULE_ADDED, RULE_REMOVED);
@@ -209,9 +222,11 @@
providerService.flowRemoved(rem1);
validateEvents();
- FlowRule f3 = flowRule(flowRule(3, 3), FlowRuleState.ADDED);
- providerService.flowAdded(f3);
+ FlowRule f3 = flowRule(3, 3);
+ service.applyFlowRules(f3);
+ providerService.pushFlowMetrics(f3.deviceId(), Collections.singletonList(f3));
validateEvents(RULE_ADDED);
+
providerService.flowRemoved(f3);
validateEvents();
}
@@ -223,9 +238,10 @@
FlowRule f3 = flowRule(3, 3);
+
+ mgr.applyFlowRules(f1, f2, f3);
FlowRule updatedF1 = flowRule(f1, FlowRuleState.ADDED);
FlowRule updatedF2 = flowRule(f2, FlowRuleState.ADDED);
- mgr.applyFlowRules(f1, f2, f3);
providerService.pushFlowMetrics(DID, Lists.newArrayList(updatedF1, updatedF2));
@@ -233,7 +249,7 @@
validateState(FlowRuleState.PENDING_ADD, FlowRuleState.ADDED,
FlowRuleState.ADDED));
- validateEvents(RULE_UPDATED, RULE_UPDATED);
+ validateEvents(RULE_ADDED, RULE_ADDED);
}
@Test
@@ -241,15 +257,15 @@
FlowRule f1 = flowRule(1, 1);
FlowRule f2 = flowRule(2, 2);
FlowRule f3 = flowRule(3, 3);
+ mgr.applyFlowRules(f1, f2);
FlowRule updatedF1 = flowRule(f1, FlowRuleState.ADDED);
FlowRule updatedF2 = flowRule(f2, FlowRuleState.ADDED);
FlowRule updatedF3 = flowRule(f3, FlowRuleState.ADDED);
- mgr.applyFlowRules(f1, f2);
providerService.pushFlowMetrics(DID, Lists.newArrayList(updatedF1, updatedF2, updatedF3));
- validateEvents(RULE_UPDATED, RULE_UPDATED);
+ validateEvents(RULE_ADDED, RULE_ADDED);
}
@@ -271,7 +287,7 @@
providerService.pushFlowMetrics(DID, Lists.newArrayList(updatedF1, updatedF2));
- validateEvents(RULE_UPDATED, RULE_UPDATED, RULE_REMOVED);
+ validateEvents(RULE_ADDED, RULE_ADDED, RULE_REMOVED);
}
@@ -386,7 +402,7 @@
}
@Override
- public List<Criterion> criteria() {
+ public Set<Criterion> criteria() {
return null;
}