flowrule states functional
Change-Id: Id310f146d4ef2a59993f31d60062464a24df4560
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 829db3f..de59896 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
@@ -1,39 +1,28 @@
package org.onlab.onos.net.flow;
import static com.google.common.base.MoreObjects.toStringHelper;
+import static org.slf4j.LoggerFactory.getLogger;
import java.util.Objects;
import org.onlab.onos.net.DeviceId;
+import org.slf4j.Logger;
public class DefaultFlowRule implements FlowRule {
+ private final Logger log = getLogger(getClass());
+
private final DeviceId deviceId;
private final int priority;
private final TrafficSelector selector;
private final TrafficTreatment treatment;
- private final FlowId id;
private final long created;
private final long life;
private final long packets;
private final long bytes;
private final FlowRuleState state;
-
- public DefaultFlowRule(DeviceId deviceId,
- TrafficSelector selector, TrafficTreatment treatment,
- int priority, FlowRuleState state) {
- this.deviceId = deviceId;
- this.priority = priority;
- this.selector = selector;
- this.treatment = treatment;
- this.state = state;
- this.life = 0;
- this.packets = 0;
- this.bytes = 0;
- this.id = FlowId.valueOf(this.hashCode());
- this.created = System.currentTimeMillis();
- }
+ private final FlowId id;
public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector,
TrafficTreatment treatment, int priority, FlowRuleState state,
@@ -59,7 +48,37 @@
public DefaultFlowRule(FlowRule rule, FlowRuleState state) {
this(rule.deviceId(), rule.selector(), rule.treatment(),
- rule.priority(), state);
+ rule.priority(), state, rule.id());
+ }
+
+ private DefaultFlowRule(DeviceId deviceId,
+ TrafficSelector selector, TrafficTreatment treatment,
+ int priority, FlowRuleState state) {
+ this.deviceId = deviceId;
+ this.priority = priority;
+ this.selector = selector;
+ this.treatment = treatment;
+ this.state = state;
+ this.life = 0;
+ this.packets = 0;
+ this.bytes = 0;
+ this.id = FlowId.valueOf(this.hashCode());
+ this.created = System.currentTimeMillis();
+ }
+
+ private DefaultFlowRule(DeviceId deviceId,
+ TrafficSelector selector, TrafficTreatment treatment,
+ int priority, FlowRuleState state, FlowId flowId) {
+ this.deviceId = deviceId;
+ this.priority = priority;
+ this.selector = selector;
+ this.treatment = treatment;
+ this.state = state;
+ this.life = 0;
+ this.packets = 0;
+ this.bytes = 0;
+ this.id = flowId;
+ this.created = System.currentTimeMillis();
}
@@ -128,13 +147,14 @@
* @see java.lang.Object#equals(java.lang.Object)
*/
public boolean equals(Object obj) {
+
if (this == obj) {
return true;
}
if (obj instanceof FlowRule) {
- FlowRule that = (FlowRule) obj;
- return Objects.equals(deviceId, that.deviceId()) &&
- Objects.equals(id, that.id());
+ DefaultFlowRule that = (DefaultFlowRule) obj;
+ return Objects.equals(deviceId, that.deviceId) &&
+ Objects.equals(id, that.id);
}
return false;
}
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleEvent.java b/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleEvent.java
index ce8e700..b17449d 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleEvent.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleEvent.java
@@ -19,7 +19,12 @@
/**
* Signifies that a flow rule has been removed.
*/
- RULE_REMOVED
+ RULE_REMOVED,
+
+ /**
+ * Signifies that a rule has been updated.
+ */
+ RULE_UPDATED
}
/**
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleProviderService.java b/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleProviderService.java
index df988fe..01e4372 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleProviderService.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleProviderService.java
@@ -24,6 +24,13 @@
void flowMissing(FlowRule flowRule);
/**
+ * Signals that a flow rule is on the switch but not in the store.
+ *
+ * @param flowRule the extra flow rule
+ */
+ void extraneousFlow(FlowRule flowRule);
+
+ /**
* Signals that a flow rule was indeed added.
*
* @param flowRule the added flow rule
@@ -38,4 +45,6 @@
*/
void pushFlowMetrics(DeviceId deviceId, Iterable<FlowRule> flowRules);
+
+
}
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleService.java b/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleService.java
index d2b9432..9db035a 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleService.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleService.java
@@ -1,7 +1,5 @@
package org.onlab.onos.net.flow;
-import java.util.List;
-
import org.onlab.onos.net.DeviceId;
/**
@@ -31,10 +29,8 @@
* device reconnects to the controller.
*
* @param flowRules one or more flow rules
- * throws SomeKindOfException that indicates which ones were applied and
- * which ones failed
*/
- List<FlowRule> applyFlowRules(FlowRule... flowRules);
+ void applyFlowRules(FlowRule... flowRules);
/**
* Removes the specified flow rules from their respective devices. If the
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleStore.java b/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleStore.java
index 6fdc993..0698721 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleStore.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleStore.java
@@ -16,12 +16,18 @@
Iterable<FlowRule> getFlowEntries(DeviceId deviceId);
/**
- * Stores a new flow rule, and generates a FlowRule for it.
+ * Stores a new flow rule without generating events.
*
* @param rule the flow rule to add
- * @return a flow entry
*/
- FlowRule storeFlowRule(FlowRule rule);
+ void storeFlowRule(FlowRule rule);
+
+ /**
+ * Deletes a flow rule without generating events.
+ *
+ * @param rule the flow rule to delete
+ */
+ void deleteFlowRule(FlowRule rule);
/**
* Stores a new flow rule, or updates an existing entry.
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 5bd6fed..1cee01a 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
@@ -3,7 +3,6 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static org.slf4j.LoggerFactory.getLogger;
-import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
@@ -73,18 +72,14 @@
}
@Override
- public List<FlowRule> applyFlowRules(FlowRule... flowRules) {
- List<FlowRule> entries = new ArrayList<FlowRule>();
-
+ public void applyFlowRules(FlowRule... flowRules) {
for (int i = 0; i < flowRules.length; i++) {
FlowRule f = new DefaultFlowRule(flowRules[i], FlowRuleState.PENDING_ADD);
final Device device = deviceService.getDevice(f.deviceId());
final FlowRuleProvider frp = getProvider(device.providerId());
- entries.add(store.storeFlowRule(f));
+ store.storeFlowRule(f);
frp.applyFlowRule(f);
}
-
- return entries;
}
@Override
@@ -93,7 +88,7 @@
FlowRule f = new DefaultFlowRule(flowRules[i], FlowRuleState.PENDING_REMOVE);
final Device device = deviceService.getDevice(f.deviceId());
final FlowRuleProvider frp = getProvider(device.providerId());
- store.removeFlowRule(f);
+ store.deleteFlowRule(f);
frp.removeFlowRule(f);
}
@@ -139,22 +134,30 @@
public void flowMissing(FlowRule flowRule) {
checkNotNull(flowRule, FLOW_RULE_NULL);
checkValidity();
- // TODO Auto-generated method stub
+ log.info("Flow {} has not been installed.");
}
@Override
+ public void extraneousFlow(FlowRule flowRule) {
+ checkNotNull(flowRule, FLOW_RULE_NULL);
+ checkValidity();
+ log.info("Flow {} is on switch but not in store.");
+ }
+
+ @Override
public void flowAdded(FlowRule flowRule) {
checkNotNull(flowRule, FLOW_RULE_NULL);
checkValidity();
FlowRuleEvent event = store.addOrUpdateFlowRule(flowRule);
if (event == null) {
- log.debug("Flow {} updated", flowRule);
+ log.debug("No flow store event generated.");
} else {
- log.debug("Flow {} added", flowRule);
+ log.debug("Flow {} {}", flowRule, event.type());
post(event);
}
+
}
// Posts the specified event to the local event dispatcher.
@@ -167,31 +170,23 @@
@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 = switchRules.iterator();
- List<FlowRule> extraRules = Lists.newLinkedList();
+ //List<FlowRule> switchRules = Lists.newLinkedList(flowEntries);
+ Iterator<FlowRule> switchRulesIterator = flowEntries.iterator(); //switchRules.iterator();
while (switchRulesIterator.hasNext()) {
FlowRule rule = switchRulesIterator.next();
if (storedRules.remove(rule)) {
- // we both have the rule let's update some info then.
- log.info("rule {} is added. {}", rule.id(), rule.state());
+ // we both have the rule, let's update some info then.
flowAdded(rule);
} else {
- // the device a rule the store does not have
- extraRules.add(rule);
+ // the device has a rule the store does not have
+ extraneousFlow(rule);
}
}
for (FlowRule rule : storedRules) {
// there are rules in the store that aren't on the switch
flowMissing(rule);
}
- if (extraRules.size() > 0) {
- log.warn("Device {} has extra flow rules: {}", deviceId, extraRules);
- // TODO do something with this.
- }
-
-
}
}
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 b164015..564bea2 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
@@ -6,6 +6,7 @@
import static org.junit.Assert.assertTrue;
import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_ADDED;
import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_REMOVED;
+import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_UPDATED;
import java.util.ArrayList;
import java.util.List;
@@ -37,10 +38,10 @@
import org.onlab.onos.net.flow.instructions.Instruction;
import org.onlab.onos.net.provider.AbstractProvider;
import org.onlab.onos.net.provider.ProviderId;
+import org.onlab.onos.net.trivial.impl.SimpleFlowRuleStore;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
-import org.onlab.onos.net.trivial.impl.SimpleFlowRuleStore;
/**
* Test codifying the flow rule service & flow rule provider service contracts.
@@ -131,7 +132,7 @@
addFlowRule(1);
assertEquals("should still be 2 rules", 2, flowCount());
- validateEvents();
+ validateEvents(RULE_UPDATED);
}
@Test
@@ -149,9 +150,9 @@
assertTrue("store should be empty",
Sets.newHashSet(service.getFlowEntries(DID)).isEmpty());
- List<FlowRule> ret = mgr.applyFlowRules(r1, r2, r3);
+ mgr.applyFlowRules(r1, r2, r3);
assertEquals("3 rules should exist", 3, flowCount());
- assertTrue("3 entries should result", fel.containsAll(ret));
+ assertTrue("3 entries should result", fel.containsAll(Lists.newArrayList(r1, r2, r3)));
}
@Test
@@ -167,10 +168,10 @@
mgr.removeFlowRules(rem1, rem2);
//removing from north, so no events generated
validateEvents();
- assertEquals("1 rule should exist", 1, flowCount());
+ assertEquals("3 rule should exist", 3, flowCount());
mgr.removeFlowRules(rem1);
- assertEquals("1 rule should still exist", 1, flowCount());
+ assertEquals("3 rule should still exist", 3, flowCount());
}
@Test
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 d41b58b..2f0de26 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
@@ -1,22 +1,23 @@
package org.onlab.onos.net.trivial.impl;
-import com.google.common.collect.HashMultimap;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Multimap;
+import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_ADDED;
+import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_REMOVED;
+import static org.slf4j.LoggerFactory.getLogger;
+
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
import org.apache.felix.scr.annotations.Service;
import org.onlab.onos.net.DeviceId;
-import org.onlab.onos.net.flow.DefaultFlowRule;
import org.onlab.onos.net.flow.FlowRule;
import org.onlab.onos.net.flow.FlowRuleEvent;
+import org.onlab.onos.net.flow.FlowRuleEvent.Type;
import org.onlab.onos.net.flow.FlowRuleStore;
import org.slf4j.Logger;
-import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_ADDED;
-import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_REMOVED;
-import static org.slf4j.LoggerFactory.getLogger;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Multimap;
/**
* Manages inventory of flow rules using trivial in-memory implementation.
@@ -28,7 +29,7 @@
private final Logger log = getLogger(getClass());
// store entries as a pile of rules, no info about device tables
- private final Multimap<DeviceId, FlowRule> flowEntries = HashMultimap.create();
+ private final Multimap<DeviceId, FlowRule> flowEntries = ArrayListMultimap.create();
@Activate
public void activate() {
@@ -46,12 +47,26 @@
}
@Override
- public FlowRule storeFlowRule(FlowRule rule) {
+ public void storeFlowRule(FlowRule rule) {
DeviceId did = rule.deviceId();
- FlowRule entry = new DefaultFlowRule(did,
- rule.selector(), rule.treatment(), rule.priority());
- flowEntries.put(did, entry);
- return entry;
+ flowEntries.put(did, rule);
+ }
+
+ @Override
+ public void deleteFlowRule(FlowRule rule) {
+ DeviceId did = rule.deviceId();
+
+ /*
+ * find the rule and mark it for deletion.
+ * Ultimately a flow removed will come remove it.
+ */
+ if (flowEntries.containsEntry(did, rule)) {
+ synchronized (flowEntries) {
+
+ flowEntries.remove(did, rule);
+ flowEntries.put(did, rule);
+ }
+ }
}
@Override
@@ -59,12 +74,17 @@
DeviceId did = rule.deviceId();
// check if this new rule is an update to an existing entry
- for (FlowRule fe : flowEntries.get(did)) {
- if (rule.equals(fe)) {
- // TODO update the stats on this FlowRule?
- return null;
+ if (flowEntries.containsEntry(did, rule)) {
+ synchronized (flowEntries) {
+ // Multimaps support duplicates so we have to remove our rule
+ // and replace it with the current version.
+
+ flowEntries.remove(did, rule);
+ flowEntries.put(did, rule);
}
+ return new FlowRuleEvent(Type.RULE_UPDATED, rule);
}
+
flowEntries.put(did, rule);
return new FlowRuleEvent(RULE_ADDED, rule);
}
@@ -80,4 +100,6 @@
}
}
+
+
}
diff --git a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilder.java b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilder.java
index b2e7cf4..8c009a7 100644
--- a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilder.java
+++ b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilder.java
@@ -88,6 +88,24 @@
}
+ public OFFlowMod buildFlowDel() {
+ Match match = buildMatch();
+ List<OFAction> actions = buildActions();
+
+ OFFlowMod fm = factory.buildFlowDelete()
+ .setCookie(U64.of(cookie.value()))
+ .setBufferId(OFBufferId.NO_BUFFER)
+ .setActions(actions)
+ .setMatch(match)
+ .setFlags(Collections.singleton(OFFlowModFlags.SEND_FLOW_REM))
+ .setIdleTimeout(10)
+ .setHardTimeout(10)
+ .setPriority(priority)
+ .build();
+
+ return fm;
+ }
+
private List<OFAction> buildActions() {
List<OFAction> acts = new LinkedList<>();
for (Instruction i : treatment.instructions()) {
@@ -246,4 +264,6 @@
return mBuilder.build();
}
+
+
}
diff --git a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowRuleBuilder.java b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowRuleBuilder.java
index 49ee9b9..d6c3c2d 100644
--- a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowRuleBuilder.java
+++ b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowRuleBuilder.java
@@ -53,7 +53,7 @@
this.match = entry.getMatch();
this.actions = entry.getActions();
this.dpid = dpid;
- removed = null;
+ this.removed = null;
}
public FlowRuleBuilder(Dpid dpid, OFFlowRemoved removed) {
@@ -72,14 +72,14 @@
buildSelector(), buildTreatment(), stat.getPriority(),
FlowRuleState.ADDED, stat.getDurationNsec() / 1000000,
stat.getPacketCount().getValue(), stat.getByteCount().getValue(),
- (int) (stat.getCookie().getValue() & 0xFFFFFFFF));
+ stat.getCookie().getValue());
} else {
// TODO: revisit potentially.
return new DefaultFlowRule(DeviceId.deviceId(Dpid.uri(dpid)),
buildSelector(), null, removed.getPriority(),
FlowRuleState.REMOVED, removed.getDurationNsec() / 1000000,
removed.getPacketCount().getValue(), removed.getByteCount().getValue(),
- (int) (removed.getCookie().getValue() & 0xFFFFFFFF));
+ removed.getCookie().getValue());
}
}
diff --git a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/OpenFlowRuleProvider.java b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/OpenFlowRuleProvider.java
index 7cb4fdf..7ddc65d 100644
--- a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/OpenFlowRuleProvider.java
+++ b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/OpenFlowRuleProvider.java
@@ -95,11 +95,19 @@
@Override
public void removeFlowRule(FlowRule... flowRules) {
- // TODO Auto-generated method stub
+ for (int i = 0; i < flowRules.length; i++) {
+ removeRule(flowRules[i]);
+ }
}
+ private void removeRule(FlowRule flowRule) {
+ OpenFlowSwitch sw = controller.getSwitch(Dpid.dpid(flowRule.deviceId().uri()));
+ sw.sendMsg(new FlowModBuilder(flowRule, sw.factory()).buildFlowDel());
+ }
+
+
//TODO: InternalFlowRuleProvider listening to stats and error and flowremoved.
// possibly barriers as well. May not be internal at all...
private class InternalFlowProvider
@@ -109,7 +117,7 @@
@Override
public void switchAdded(Dpid dpid) {
- FlowStatsCollector fsc = new FlowStatsCollector(controller.getSwitch(dpid), 1);
+ FlowStatsCollector fsc = new FlowStatsCollector(controller.getSwitch(dpid), 5);
fsc.start();
collectors.put(dpid, fsc);
}