Merge branch 'master' of ssh://gerrit.onlab.us:29418/onos-next
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/DefaultFlowRule.java b/core/api/src/main/java/org/onlab/onos/net/flow/DefaultFlowRule.java
index de59896..d3745b4 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/DefaultFlowRule.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/DefaultFlowRule.java
@@ -168,6 +168,7 @@
.add("selector", selector)
.add("treatment", treatment)
.add("created", created)
+ .add("state", state)
.toString();
}
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 1cee01a..51ea328 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
@@ -84,8 +84,9 @@
@Override
public void removeFlowRules(FlowRule... flowRules) {
+ FlowRule f;
for (int i = 0; i < flowRules.length; i++) {
- FlowRule f = new DefaultFlowRule(flowRules[i], FlowRuleState.PENDING_REMOVE);
+ f = new DefaultFlowRule(flowRules[i], FlowRuleState.PENDING_REMOVE);
final Device device = deviceService.getDevice(f.deviceId());
final FlowRuleProvider frp = getProvider(device.providerId());
store.deleteFlowRule(f);
@@ -134,7 +135,7 @@
public void flowMissing(FlowRule flowRule) {
checkNotNull(flowRule, FLOW_RULE_NULL);
checkValidity();
- log.info("Flow {} has not been installed.");
+ log.info("Flow {} has not been installed.", flowRule);
}
@@ -142,7 +143,7 @@
public void extraneousFlow(FlowRule flowRule) {
checkNotNull(flowRule, FLOW_RULE_NULL);
checkValidity();
- log.info("Flow {} is on switch but not in store.");
+ log.info("Flow {} is on switch but not in store.", flowRule);
}
@Override
@@ -170,8 +171,8 @@
@Override
public void pushFlowMetrics(DeviceId deviceId, Iterable<FlowRule> flowEntries) {
List<FlowRule> storedRules = Lists.newLinkedList(store.getFlowEntries(deviceId));
- //List<FlowRule> switchRules = Lists.newLinkedList(flowEntries);
- Iterator<FlowRule> switchRulesIterator = flowEntries.iterator(); //switchRules.iterator();
+
+ Iterator<FlowRule> switchRulesIterator = flowEntries.iterator();
while (switchRulesIterator.hasNext()) {
FlowRule rule = switchRulesIterator.next();
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 564bea2..8d82320 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
@@ -26,6 +26,7 @@
import org.onlab.onos.net.device.DeviceService;
import org.onlab.onos.net.flow.DefaultFlowRule;
import org.onlab.onos.net.flow.FlowRule;
+import org.onlab.onos.net.flow.FlowRule.FlowRuleState;
import org.onlab.onos.net.flow.FlowRuleEvent;
import org.onlab.onos.net.flow.FlowRuleListener;
import org.onlab.onos.net.flow.FlowRuleProvider;
@@ -57,7 +58,7 @@
protected FlowRuleService service;
protected FlowRuleProviderRegistry registry;
- protected FlowRuleProviderService providerSerivce;
+ protected FlowRuleProviderService providerService;
protected TestProvider provider;
protected TestListener listener = new TestListener();
@@ -73,7 +74,7 @@
mgr.activate();
mgr.addListener(listener);
provider = new TestProvider(PID);
- providerSerivce = registry.register(provider);
+ providerService = registry.register(provider);
assertTrue("provider should be registered",
registry.getProviders().contains(provider.id()));
}
@@ -95,10 +96,15 @@
return new DefaultFlowRule(DID, ts, tr, 0);
}
- private void addFlowRule(int hval) {
+ private FlowRule flowRule(FlowRule rule, FlowRuleState state) {
+ return new DefaultFlowRule(rule, state);
+ }
+
+ private FlowRule addFlowRule(int hval) {
FlowRule rule = flowRule(hval, hval);
- providerSerivce.flowAdded(rule);
+ providerService.flowAdded(rule);
assertNotNull("rule should be found", service.getFlowEntries(DID));
+ return rule;
}
private void validateEvents(FlowRuleEvent.Type ... events) {
@@ -135,40 +141,53 @@
validateEvents(RULE_UPDATED);
}
+
+ //backing store is sensitive to the order of additions/removals
+ private boolean validateState(FlowRuleState... state) {
+ Iterable<FlowRule> rules = service.getFlowEntries(DID);
+ int i = 0;
+ for (FlowRule f : rules) {
+ if (f.state() != state[i]) {
+ return false;
+ }
+ i++;
+ }
+ return true;
+ }
+
@Test
public void applyFlowRules() {
- TestSelector ts = new TestSelector(1);
+
FlowRule r1 = flowRule(1, 1);
FlowRule r2 = flowRule(1, 2);
FlowRule r3 = flowRule(1, 3);
- //current FlowRules always return 0. FlowEntries inherit the value
- FlowRule e1 = new DefaultFlowRule(DID, ts, r1.treatment(), 0);
- FlowRule e2 = new DefaultFlowRule(DID, ts, r2.treatment(), 0);
- FlowRule e3 = new DefaultFlowRule(DID, ts, r3.treatment(), 0);
- List<FlowRule> fel = Lists.newArrayList(e1, e2, e3);
-
assertTrue("store should be empty",
Sets.newHashSet(service.getFlowEntries(DID)).isEmpty());
mgr.applyFlowRules(r1, r2, r3);
assertEquals("3 rules should exist", 3, flowCount());
- assertTrue("3 entries should result", fel.containsAll(Lists.newArrayList(r1, r2, r3)));
+ assertTrue("Entries should be pending add.",
+ validateState(FlowRuleState.PENDING_ADD, FlowRuleState.PENDING_ADD,
+ FlowRuleState.PENDING_ADD));
}
@Test
public void removeFlowRules() {
- addFlowRule(1);
- addFlowRule(2);
+ FlowRule f1 = addFlowRule(1);
+ FlowRule f2 = addFlowRule(2);
addFlowRule(3);
assertEquals("3 rules should exist", 3, flowCount());
validateEvents(RULE_ADDED, RULE_ADDED, RULE_ADDED);
- FlowRule rem1 = flowRule(1, 1);
- FlowRule rem2 = flowRule(2, 2);
+ FlowRule rem1 = flowRule(f1, FlowRuleState.REMOVED);
+ FlowRule rem2 = flowRule(f2, FlowRuleState.REMOVED);
mgr.removeFlowRules(rem1, rem2);
//removing from north, so no events generated
validateEvents();
assertEquals("3 rule should exist", 3, flowCount());
+ assertTrue("Entries should be pending remove.",
+ validateState(FlowRuleState.CREATED, FlowRuleState.PENDING_REMOVE,
+ FlowRuleState.PENDING_REMOVE));
mgr.removeFlowRules(rem1);
assertEquals("3 rule should still exist", 3, flowCount());
@@ -176,16 +195,34 @@
@Test
public void flowRemoved() {
- addFlowRule(1);
+ FlowRule f1 = addFlowRule(1);
addFlowRule(2);
- FlowRule rem1 = flowRule(1, 1);
- providerSerivce.flowRemoved(rem1);
+ FlowRule rem1 = flowRule(f1, FlowRuleState.REMOVED);
+ providerService.flowRemoved(rem1);
validateEvents(RULE_ADDED, RULE_ADDED, RULE_REMOVED);
- providerSerivce.flowRemoved(rem1);
+ providerService.flowRemoved(rem1);
validateEvents();
}
+ @Test
+ public void flowMetrics() {
+ FlowRule f1 = flowRule(1, 1);
+ FlowRule f2 = flowRule(2, 2);
+ FlowRule f3 = flowRule(3, 3);
+
+ FlowRule updatedF1 = flowRule(f1, FlowRuleState.ADDED);
+ FlowRule updatedF2 = flowRule(f2, FlowRuleState.ADDED);
+ mgr.applyFlowRules(f1, f2, f3);
+
+ providerService.pushFlowMetrics(DID, Lists.newArrayList(updatedF1, updatedF2));
+
+ assertTrue("Entries should be added.",
+ validateState(FlowRuleState.PENDING_ADD, FlowRuleState.ADDED,
+ FlowRuleState.ADDED));
+ //TODO: add tests for flowmissing and extraneous flows
+ }
+
private static class TestListener implements FlowRuleListener {
final List<FlowRuleEvent> events = new ArrayList<>();
diff --git a/core/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleFlowRuleStore.java b/core/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleFlowRuleStore.java
index 2f0de26..fbfd0ee 100644
--- a/core/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleFlowRuleStore.java
+++ b/core/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleFlowRuleStore.java
@@ -60,6 +60,7 @@
* find the rule and mark it for deletion.
* Ultimately a flow removed will come remove it.
*/
+
if (flowEntries.containsEntry(did, rule)) {
synchronized (flowEntries) {