Tests for FlowRuleStore plus bugfixes
Change-Id: Ib5b0bd9d41fbbcac1cf09684e70446326887caf7
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/DefaultFlowEntry.java b/core/api/src/main/java/org/onlab/onos/net/flow/DefaultFlowEntry.java
index 3f86697..295517b 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/DefaultFlowEntry.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/DefaultFlowEntry.java
@@ -1,5 +1,7 @@
package org.onlab.onos.net.flow;
+import static com.google.common.base.MoreObjects.toStringHelper;
+
import org.onlab.onos.net.DeviceId;
public class DefaultFlowEntry extends DefaultFlowRule implements FlowEntry {
@@ -63,7 +65,9 @@
public int hashCode() {
final int prime = 31;
int result = prime * this.deviceId().hashCode();
- result = prime * result + Long.valueOf(this.created).hashCode();
+ result = prime * result + this.priority;
+ result = prime * result + this.selector().hashCode();
+ result = prime * result + this.treatment().hashCode();
return result;
}
@@ -77,7 +81,10 @@
public boolean equals(Object obj) {
if (obj instanceof DefaultFlowEntry) {
DefaultFlowEntry that = (DefaultFlowEntry) obj;
- if (!this.id.equals(that.id())) {
+ if (!this.deviceId().equals(that.deviceId())) {
+ return false;
+ }
+ if (!(this.priority == that.priority)) {
return false;
}
return super.equals(obj);
@@ -85,4 +92,15 @@
return false;
}
+ @Override
+ public String toString() {
+ return toStringHelper(this)
+ .add("id", id)
+ .add("deviceId", deviceId())
+ .add("priority", priority)
+ .add("selector", selector())
+ .add("treatment", treatment())
+ .toString();
+ }
+
}
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 9a24091..d20e79e 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
@@ -17,6 +17,7 @@
@Override
public int priority() {
+ // is this supposed to be 0?
return 0;
}
@@ -63,8 +64,9 @@
if (!this.selector().equals(that.selector())) {
return false;
}
+ return true;
}
- return true;
+ return false;
}
diff --git a/core/trivial/src/main/java/org/onlab/onos/net/trivial/flow/impl/SimpleFlowRuleManager.java b/core/trivial/src/main/java/org/onlab/onos/net/trivial/flow/impl/SimpleFlowRuleManager.java
index 7540ec8..038a0d9 100644
--- a/core/trivial/src/main/java/org/onlab/onos/net/trivial/flow/impl/SimpleFlowRuleManager.java
+++ b/core/trivial/src/main/java/org/onlab/onos/net/trivial/flow/impl/SimpleFlowRuleManager.java
@@ -45,10 +45,10 @@
private final SimpleFlowRuleStore store = new SimpleFlowRuleStore();
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
- private EventDeliveryService eventDispatcher;
+ protected EventDeliveryService eventDispatcher;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
- private DeviceService deviceService;
+ protected DeviceService deviceService;
@Activate
public void activate() {
diff --git a/core/trivial/src/main/java/org/onlab/onos/net/trivial/flow/impl/SimpleFlowRuleStore.java b/core/trivial/src/main/java/org/onlab/onos/net/trivial/flow/impl/SimpleFlowRuleStore.java
index f839396..4f23ff4 100644
--- a/core/trivial/src/main/java/org/onlab/onos/net/trivial/flow/impl/SimpleFlowRuleStore.java
+++ b/core/trivial/src/main/java/org/onlab/onos/net/trivial/flow/impl/SimpleFlowRuleStore.java
@@ -53,17 +53,20 @@
FlowRuleEvent addOrUpdateFlowRule(FlowRule rule) {
DeviceId did = rule.deviceId();
+ FlowEntry entry = new DefaultFlowEntry(
+ did,
+ rule.selector(),
+ rule.treatment(),
+ rule.priority());
+
// check if this new rule is an update to an existing entry
for (FlowEntry fe : flowEntries.get(did)) {
- if (rule.equals(fe)) {
+ if (entry.equals(fe)) {
// TODO update the stats on this flowEntry?
return null;
}
}
-
- FlowEntry newfe = new DefaultFlowEntry(did,
- rule.selector(), rule.treatment(), rule.priority());
- flowEntries.put(did, newfe);
+ flowEntries.put(did, entry);
return new FlowRuleEvent(RULE_ADDED, rule);
}
@@ -73,8 +76,11 @@
* @return flow_removed event, or null if nothing removed
*/
FlowRuleEvent removeFlowRule(FlowRule rule) {
+
+ FlowEntry rem = new DefaultFlowEntry(rule.deviceId(),
+ rule.selector(), rule.treatment(), rule.priority());
synchronized (this) {
- if (flowEntries.remove(rule.deviceId(), rule)) {
+ if (flowEntries.remove(rem.deviceId(), rem)) {
return new FlowRuleEvent(RULE_REMOVED, rule);
} else {
return null;
diff --git a/core/trivial/src/test/java/org/onlab/onos/net/trivial/flow/impl/SimpleFlowRuleManagerTest.java b/core/trivial/src/test/java/org/onlab/onos/net/trivial/flow/impl/SimpleFlowRuleManagerTest.java
new file mode 100644
index 0000000..bb0a496
--- /dev/null
+++ b/core/trivial/src/test/java/org/onlab/onos/net/trivial/flow/impl/SimpleFlowRuleManagerTest.java
@@ -0,0 +1,322 @@
+package org.onlab.onos.net.trivial.flow.impl;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.onlab.onos.event.impl.TestEventDispatcher;
+import org.onlab.onos.net.DefaultDevice;
+import org.onlab.onos.net.Device;
+import org.onlab.onos.net.Device.Type;
+import org.onlab.onos.net.DeviceId;
+import org.onlab.onos.net.MastershipRole;
+import org.onlab.onos.net.Port;
+import org.onlab.onos.net.PortNumber;
+import org.onlab.onos.net.device.DeviceListener;
+import org.onlab.onos.net.device.DeviceService;
+import org.onlab.onos.net.flow.DefaultFlowEntry;
+import org.onlab.onos.net.flow.DefaultFlowRule;
+import org.onlab.onos.net.flow.FlowEntry;
+import org.onlab.onos.net.flow.FlowRule;
+import org.onlab.onos.net.flow.FlowRuleEvent;
+import org.onlab.onos.net.flow.FlowRuleListener;
+import org.onlab.onos.net.flow.FlowRuleProvider;
+import org.onlab.onos.net.flow.FlowRuleProviderRegistry;
+import org.onlab.onos.net.flow.FlowRuleProviderService;
+import org.onlab.onos.net.flow.FlowRuleService;
+import org.onlab.onos.net.flow.TrafficSelector;
+import org.onlab.onos.net.flow.TrafficTreatment;
+import org.onlab.onos.net.flow.criteria.Criterion;
+import org.onlab.onos.net.flow.instructions.Instruction;
+import org.onlab.onos.net.provider.AbstractProvider;
+import org.onlab.onos.net.provider.ProviderId;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+
+import static org.onlab.onos.net.flow.FlowRuleEvent.Type.*;
+
+/**
+ * Test codifying the flow rule service & flow rule provider service contracts.
+ */
+public class SimpleFlowRuleManagerTest {
+
+ private static final ProviderId PID = new ProviderId("foo");
+ private static final DeviceId DID = DeviceId.deviceId("of:001");
+ private static final Device DEV = new DefaultDevice(
+ PID, DID, Type.SWITCH, "", "", "", "");
+
+ private SimpleFlowRuleManager mgr;
+
+ protected FlowRuleService service;
+ protected FlowRuleProviderRegistry registry;
+ protected FlowRuleProviderService providerSerivce;
+ protected TestProvider provider;
+ protected TestListener listener = new TestListener();
+
+ @Before
+ public void setUp() {
+ mgr = new SimpleFlowRuleManager();
+ mgr.eventDispatcher = new TestEventDispatcher();
+ mgr.deviceService = new TestDeviceService();
+ service = mgr;
+ registry = mgr;
+
+ mgr.activate();
+ mgr.addListener(listener);
+ provider = new TestProvider(PID);
+ providerSerivce = registry.register(provider);
+ assertTrue("provider should be registered",
+ registry.getProviders().contains(provider.id()));
+ }
+
+ @After
+ public void tearDown() {
+ registry.unregister(provider);
+ assertFalse("provider should not be registered",
+ registry.getProviders().contains(provider.id()));
+ service.removeListener(listener);
+ mgr.deactivate();
+ mgr.eventDispatcher = null;
+ mgr.deviceService = null;
+ }
+
+ private FlowRule flowRule(int tsval, int trval) {
+ TestSelector ts = new TestSelector(tsval);
+ TestTreatment tr = new TestTreatment(trval);
+ return new DefaultFlowRule(DID, ts, tr);
+ }
+
+ private void addFlowRule(int hval) {
+ FlowRule rule = flowRule(hval, hval);
+ providerSerivce.flowAdded(rule);
+ assertNotNull("rule should be found", service.getFlowEntries(DID));
+ }
+
+ private void validateEvents(FlowRuleEvent.Type ... events) {
+ if (events == null) {
+ assertTrue("events generated", listener.events.isEmpty());
+ }
+
+ int i = 0;
+ for (FlowRuleEvent e : listener.events) {
+ assertTrue("unexpected event", e.type().equals(events[i]));
+ i++;
+ }
+
+ assertEquals("mispredicted number of events",
+ events.length, listener.events.size());
+
+ listener.events.clear();
+ }
+
+ private int flowCount() {
+ return Sets.newHashSet(service.getFlowEntries(DID)).size();
+ }
+ @Test
+ public void getFlowEntries() {
+ assertTrue("store should be empty",
+ Sets.newHashSet(service.getFlowEntries(DID)).isEmpty());
+ addFlowRule(1);
+ addFlowRule(2);
+ assertEquals("2 rules should exist", 2, flowCount());
+ validateEvents(RULE_ADDED, RULE_ADDED);
+
+ addFlowRule(1);
+ assertEquals("should still be 2 rules", 2, flowCount());
+ validateEvents();
+ }
+
+ @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
+ FlowEntry e1 = new DefaultFlowEntry(DID, ts, r1.treatment(), 0);
+ FlowEntry e2 = new DefaultFlowEntry(DID, ts, r2.treatment(), 0);
+ FlowEntry e3 = new DefaultFlowEntry(DID, ts, r3.treatment(), 0);
+ List<FlowEntry> fel = Lists.newArrayList(e1, e2, e3);
+
+ assertTrue("store should be empty",
+ Sets.newHashSet(service.getFlowEntries(DID)).isEmpty());
+ List<FlowEntry> ret = mgr.applyFlowRules(r1, r2, r3);
+ assertEquals("3 rules should exist", 3, flowCount());
+ assertTrue("3 entries should result", fel.containsAll(ret));
+ }
+
+ @Test
+ public void removeFlowRules() {
+ addFlowRule(1);
+ 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);
+ mgr.removeFlowRules(rem1, rem2);
+ //removing from north, so no events generated
+ validateEvents();
+ assertEquals("1 rule should exist", 1, flowCount());
+
+ mgr.removeFlowRules(rem1);
+ assertEquals("1 rule should still exist", 1, flowCount());
+ }
+
+ @Test
+ public void flowRemoved() {
+ addFlowRule(1);
+ addFlowRule(2);
+ FlowRule rem1 = flowRule(1, 1);
+ providerSerivce.flowRemoved(rem1);
+ validateEvents(RULE_ADDED, RULE_ADDED, RULE_REMOVED);
+
+ providerSerivce.flowRemoved(rem1);
+ validateEvents();
+ }
+
+ private static class TestListener implements FlowRuleListener {
+ final List<FlowRuleEvent> events = new ArrayList<>();
+
+ @Override
+ public void event(FlowRuleEvent event) {
+ events.add(event);
+ }
+ }
+
+ private static class TestDeviceService implements DeviceService {
+
+ @Override
+ public int getDeviceCount() {
+ return 0;
+ }
+
+ @Override
+ public Iterable<Device> getDevices() {
+ return null;
+ }
+
+ @Override
+ public Device getDevice(DeviceId deviceId) {
+ return DEV;
+ }
+
+ @Override
+ public MastershipRole getRole(DeviceId deviceId) {
+ return null;
+ }
+
+ @Override
+ public List<Port> getPorts(DeviceId deviceId) {
+ return null;
+ }
+
+ @Override
+ public Port getPort(DeviceId deviceId, PortNumber portNumber) {
+ return null;
+ }
+
+ @Override
+ public boolean isAvailable(DeviceId deviceId) {
+ return false;
+ }
+
+ @Override
+ public void addListener(DeviceListener listener) {
+ }
+
+ @Override
+ public void removeListener(DeviceListener listener) {
+ }
+
+ }
+
+ private class TestProvider extends AbstractProvider implements FlowRuleProvider {
+
+ protected TestProvider(ProviderId id) {
+ super(PID);
+ }
+
+ @Override
+ public void applyFlowRule(FlowRule... flowRules) {
+ }
+
+ @Override
+ public void removeFlowRule(FlowRule... flowRules) {
+ }
+
+ @Override
+ public Iterable<FlowEntry> getFlowMetrics(DeviceId deviceId) {
+ return null;
+ }
+
+ }
+
+ private class TestSelector implements TrafficSelector {
+
+ //for controlling hashcode uniqueness;
+ private int testval;
+
+ public TestSelector(int val) {
+ testval = val;
+ }
+
+ @Override
+ public List<Criterion> criteria() {
+ return null;
+ }
+
+ @Override
+ public int hashCode() {
+ return testval;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (o instanceof TestSelector) {
+ return this.testval == ((TestSelector) o).testval;
+ }
+ return false;
+ }
+ }
+
+ private class TestTreatment implements TrafficTreatment {
+
+ //for controlling hashcode uniqueness;
+ private int testval;
+
+ public TestTreatment(int val) {
+ testval = val;
+ }
+
+ @Override
+ public List<Instruction> instructions() {
+ return null;
+ }
+
+ @Override
+ public int hashCode() {
+ return testval;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (o instanceof TestTreatment) {
+ return this.testval == ((TestTreatment) o).testval;
+ }
+ return false;
+ }
+
+ }
+
+}
diff --git a/utils/misc/src/main/java/org/onlab/packet/IpAddress.java b/utils/misc/src/main/java/org/onlab/packet/IpAddress.java
index 7669cc5..3fc1429 100644
--- a/utils/misc/src/main/java/org/onlab/packet/IpAddress.java
+++ b/utils/misc/src/main/java/org/onlab/packet/IpAddress.java
@@ -18,6 +18,7 @@
//maximum CIDR value
public static final int MAX_INET_MASK = 32;
+ //no mask (no network), e.g. a simple address
public static final int DEFAULT_MASK = 0;
/**
@@ -112,7 +113,7 @@
final String [] parts = address.split("\\/");
if (parts.length > 2) {
throw new IllegalArgumentException("Malformed IP address string; "
- + "Addres must take form \"x.x.x.x\" or \"x.x.x.x/y\"");
+ + "Address must take form \"x.x.x.x\" or \"x.x.x.x/y\"");
}
int mask = DEFAULT_MASK;
@@ -128,7 +129,7 @@
final String [] net = parts[0].split("\\.");
if (net.length != INET_LEN) {
throw new IllegalArgumentException("Malformed IP address string; "
- + "Addres must have four decimal values separated by dots (.)");
+ + "Address must have four decimal values separated by dots (.)");
}
final byte [] bytes = new byte[INET_LEN];
for (int i = 0; i < INET_LEN; i++) {