Updating Intent Manager to deal with failures.
Added ids to Flow batch futures.
Adding some basic unit tests for IntentManger
Adding failedIds to the completedOperation in FlowRuleManager
Change-Id: I7645cead193299f70d319d254cd1e82d96909e7b
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 cc33f20..793c9f3 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
@@ -53,6 +53,8 @@
import org.projectfloodlight.openflow.types.VlanVid;
import org.slf4j.Logger;
+import java.util.Optional;
+
/**
* Builder for OpenFlow flow mods based on FlowRules.
*/
@@ -63,6 +65,7 @@
private final OFFactory factory;
private final FlowRule flowRule;
private final TrafficSelector selector;
+ protected final Long xid;
/**
* Creates a new flow mod builder.
@@ -71,12 +74,13 @@
* @param factory the OpenFlow factory to use to build the flow mod
* @return the new flow mod builder
*/
- public static FlowModBuilder builder(FlowRule flowRule, OFFactory factory) {
+ public static FlowModBuilder builder(FlowRule flowRule,
+ OFFactory factory, Optional<Long> xid) {
switch (factory.getVersion()) {
case OF_10:
- return new FlowModBuilderVer10(flowRule, factory);
+ return new FlowModBuilderVer10(flowRule, factory, xid);
case OF_13:
- return new FlowModBuilderVer13(flowRule, factory);
+ return new FlowModBuilderVer13(flowRule, factory, xid);
default:
throw new UnsupportedOperationException(
"No flow mod builder for protocol version " + factory.getVersion());
@@ -89,10 +93,12 @@
* @param flowRule the flow rule to transform into a flow mod
* @param factory the OpenFlow factory to use to build the flow mod
*/
- protected FlowModBuilder(FlowRule flowRule, OFFactory factory) {
+ protected FlowModBuilder(FlowRule flowRule, OFFactory factory, Optional<Long> xid) {
this.factory = factory;
this.flowRule = flowRule;
this.selector = flowRule.selector();
+ this.xid = xid.orElse((long) 0);
+
}
/**
diff --git a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer10.java b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer10.java
index ec315f5..341b718 100644
--- a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer10.java
+++ b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer10.java
@@ -18,6 +18,7 @@
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
+import java.util.Optional;
import org.onlab.onos.net.flow.FlowRule;
import org.onlab.onos.net.flow.TrafficTreatment;
@@ -62,8 +63,9 @@
* @param flowRule the flow rule to transform into a flow mod
* @param factory the OpenFlow factory to use to build the flow mod
*/
- protected FlowModBuilderVer10(FlowRule flowRule, OFFactory factory) {
- super(flowRule, factory);
+ protected FlowModBuilderVer10(FlowRule flowRule,
+ OFFactory factory, Optional<Long> xid) {
+ super(flowRule, factory, xid);
this.treatment = flowRule.treatment();
}
@@ -77,7 +79,7 @@
//TODO: what to do without bufferid? do we assume that there will be a pktout as well?
OFFlowAdd fm = factory().buildFlowAdd()
- .setXid(cookie)
+ .setXid(xid)
.setCookie(U64.of(cookie))
.setBufferId(OFBufferId.NO_BUFFER)
.setActions(actions)
@@ -98,7 +100,7 @@
//TODO: what to do without bufferid? do we assume that there will be a pktout as well?
OFFlowMod fm = factory().buildFlowModify()
- .setXid(cookie)
+ .setXid(xid)
.setCookie(U64.of(cookie))
.setBufferId(OFBufferId.NO_BUFFER)
.setActions(actions)
@@ -118,7 +120,7 @@
long cookie = flowRule().id().value();
OFFlowDelete fm = factory().buildFlowDelete()
- .setXid(cookie)
+ .setXid(xid)
.setCookie(U64.of(cookie))
.setBufferId(OFBufferId.NO_BUFFER)
.setActions(actions)
diff --git a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer13.java b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer13.java
index 6f595d0..f2ad959 100644
--- a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer13.java
+++ b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer13.java
@@ -18,6 +18,7 @@
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
+import java.util.Optional;
import org.onlab.onos.net.flow.FlowRule;
import org.onlab.onos.net.flow.TrafficTreatment;
@@ -70,8 +71,8 @@
* @param flowRule the flow rule to transform into a flow mod
* @param factory the OpenFlow factory to use to build the flow mod
*/
- protected FlowModBuilderVer13(FlowRule flowRule, OFFactory factory) {
- super(flowRule, factory);
+ protected FlowModBuilderVer13(FlowRule flowRule, OFFactory factory, Optional<Long> xid) {
+ super(flowRule, factory, xid);
this.treatment = flowRule.treatment();
}
@@ -93,7 +94,7 @@
//TODO: what to do without bufferid? do we assume that there will be a pktout as well?
OFFlowAdd fm = factory().buildFlowAdd()
- .setXid(cookie)
+ .setXid(xid)
.setCookie(U64.of(cookie))
.setBufferId(OFBufferId.NO_BUFFER)
.setActions(actions)
@@ -117,7 +118,7 @@
//TODO: what to do without bufferid? do we assume that there will be a pktout as well?
OFFlowMod fm = factory().buildFlowModify()
- .setXid(cookie)
+ .setXid(xid)
.setCookie(U64.of(cookie))
.setBufferId(OFBufferId.NO_BUFFER)
.setActions(actions)
@@ -140,7 +141,7 @@
long cookie = flowRule().id().value();
OFFlowDelete fm = factory().buildFlowDelete()
- .setXid(cookie)
+ .setXid(xid)
.setCookie(U64.of(cookie))
.setBufferId(OFBufferId.NO_BUFFER)
//.setActions(actions) //FIXME do we want to send actions in flowdel?
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 214bde3..f09748e 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
@@ -19,7 +19,6 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
-import com.google.common.util.concurrent.ExecutionList;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
@@ -76,6 +75,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
@@ -122,7 +122,7 @@
private final Map<Dpid, FlowStatsCollector> collectors = Maps.newHashMap();
- private final AtomicLong xidCounter = new AtomicLong(0);
+ private final AtomicLong xidCounter = new AtomicLong(1);
/**
* Creates an OpenFlow host provider.
@@ -164,7 +164,8 @@
private void applyRule(FlowRule flowRule) {
OpenFlowSwitch sw = controller.getSwitch(Dpid.dpid(flowRule.deviceId().uri()));
- sw.sendMsg(FlowModBuilder.builder(flowRule, sw.factory()).buildFlowAdd());
+ sw.sendMsg(FlowModBuilder.builder(flowRule, sw.factory(),
+ Optional.empty()).buildFlowAdd());
}
@@ -178,7 +179,8 @@
private void removeRule(FlowRule flowRule) {
OpenFlowSwitch sw = controller.getSwitch(Dpid.dpid(flowRule.deviceId().uri()));
- sw.sendMsg(FlowModBuilder.builder(flowRule, sw.factory()).buildFlowDel());
+ sw.sendMsg(FlowModBuilder.builder(flowRule, sw.factory(),
+ Optional.empty()).buildFlowDel());
}
@Override
@@ -211,7 +213,10 @@
return failed;
}
sws.add(new Dpid(sw.getId()));
- FlowModBuilder builder = FlowModBuilder.builder(flowRule, sw.factory());
+ Long flowModXid = xidCounter.getAndIncrement();
+ FlowModBuilder builder =
+ FlowModBuilder.builder(flowRule, sw.factory(),
+ Optional.of(flowModXid));
OFFlowMod mod = null;
switch (fbe.getOperator()) {
case ADD:
@@ -228,7 +233,7 @@
}
if (mod != null) {
mods.put(mod, sw);
- fmXids.put(xidCounter.getAndIncrement(), fbe);
+ fmXids.put(flowModXid, fbe);
} else {
log.error("Conversion of flowrule {} failed.", flowRule);
}
@@ -237,6 +242,7 @@
for (Long xid : fmXids.keySet()) {
pendingFMs.put(xid, installation);
}
+
pendingFutures.put(installation.xid(), installation);
for (Map.Entry<OFFlowMod, OpenFlowSwitch> entry : mods.entrySet()) {
OpenFlowSwitch sw = entry.getValue();
@@ -368,13 +374,13 @@
private final AtomicBoolean ok = new AtomicBoolean(true);
private final Map<Long, FlowRuleBatchEntry> fms;
+
private final Set<FlowEntry> offendingFlowMods = Sets.newHashSet();
+ private Long failedId;
private final CountDownLatch countDownLatch;
private BatchState state;
- private final ExecutionList executionList = new ExecutionList();
-
public InstallationFuture(Set<Dpid> sws, Map<Long, FlowRuleBatchEntry> fmXids) {
this.xid = xidCounter.getAndIncrement();
this.state = BatchState.STARTED;
@@ -393,6 +399,7 @@
removeRequirement(dpid);
FlowEntry fe = null;
FlowRuleBatchEntry fbe = fms.get(msg.getXid());
+ failedId = fbe.id();
FlowRule offending = fbe.getTarget();
//TODO handle specific error msgs
switch (msg.getErrType()) {
@@ -492,8 +499,11 @@
public CompletedBatchOperation get() throws InterruptedException, ExecutionException {
countDownLatch.await();
this.state = BatchState.FINISHED;
- CompletedBatchOperation result = new CompletedBatchOperation(ok.get(), offendingFlowMods);
- //FIXME do cleanup here
+ Set<Long> failedIds = (failedId != null) ? Sets.newHashSet(failedId) : Collections.emptySet();
+ CompletedBatchOperation result =
+ new CompletedBatchOperation(ok.get(), offendingFlowMods, failedIds);
+ //FIXME do cleanup here (moved by BOC)
+ cleanUp();
return result;
}
@@ -503,8 +513,11 @@
TimeoutException {
if (countDownLatch.await(timeout, unit)) {
this.state = BatchState.FINISHED;
- CompletedBatchOperation result = new CompletedBatchOperation(ok.get(), offendingFlowMods);
- // FIXME do cleanup here
+ Set<Long> failedIds = (failedId != null) ? Sets.newHashSet(failedId) : Collections.emptySet();
+ CompletedBatchOperation result =
+ new CompletedBatchOperation(ok.get(), offendingFlowMods, failedIds);
+ // FIXME do cleanup here (moved by BOC)
+ cleanUp();
return result;
}
throw new TimeoutException();
@@ -522,8 +535,8 @@
private void removeRequirement(Dpid dpid) {
countDownLatch.countDown();
sws.remove(dpid);
- //FIXME don't do cleanup here
- cleanUp();
+ //FIXME don't do cleanup here (moved by BOC)
+ //cleanUp();
}
}