Fixing flow rule batches
Problem should now be fixed. Hashing on enums last is a bad
idea because the enum value could be 0.
Change-Id: Ib29e90b393b5285be2807729b52e69b121340f09
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/DefaultTrafficSelector.java b/core/api/src/main/java/org/onlab/onos/net/flow/DefaultTrafficSelector.java
index 413473f..673773a 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/DefaultTrafficSelector.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/DefaultTrafficSelector.java
@@ -63,7 +63,7 @@
@Override
public int hashCode() {
- return Objects.hash(criteria);
+ return criteria.hashCode();
}
@Override
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleProvider.java b/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleProvider.java
index de2c7fd..64a56ca 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleProvider.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/FlowRuleProvider.java
@@ -18,7 +18,7 @@
import org.onlab.onos.core.ApplicationId;
import org.onlab.onos.net.provider.Provider;
-import com.google.common.util.concurrent.ListenableFuture;
+import java.util.concurrent.Future;
/**
* Abstraction of a flow rule provider.
@@ -58,6 +58,6 @@
* @param batch a batch of flow rules
* @return a future indicating the status of this execution
*/
- ListenableFuture<CompletedBatchOperation> executeBatch(BatchOperation<FlowRuleBatchEntry> batch);
+ Future<CompletedBatchOperation> executeBatch(BatchOperation<FlowRuleBatchEntry> batch);
}
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/criteria/Criteria.java b/core/api/src/main/java/org/onlab/onos/net/flow/criteria/Criteria.java
index bac1bab..61fe54d 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/criteria/Criteria.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/criteria/Criteria.java
@@ -196,7 +196,7 @@
@Override
public int hashCode() {
- return Objects.hash(port, type());
+ return Objects.hash(type(), port);
}
@Override
@@ -242,7 +242,7 @@
@Override
public int hashCode() {
- return Objects.hash(mac, type);
+ return Objects.hash(type, mac);
}
@Override
@@ -288,7 +288,7 @@
@Override
public int hashCode() {
- return Objects.hash(ethType, type());
+ return Objects.hash(type(), ethType);
}
@Override
@@ -336,7 +336,7 @@
@Override
public int hashCode() {
- return Objects.hash(ip, type);
+ return Objects.hash(type, ip);
}
@Override
@@ -382,7 +382,7 @@
@Override
public int hashCode() {
- return Objects.hash(proto, type());
+ return Objects.hash(type(), proto);
}
@Override
@@ -427,7 +427,7 @@
@Override
public int hashCode() {
- return Objects.hash(vlanPcp);
+ return Objects.hash(type(), vlanPcp);
}
@Override
@@ -474,7 +474,7 @@
@Override
public int hashCode() {
- return Objects.hash(vlanId, type());
+ return Objects.hash(type(), vlanId);
}
@Override
@@ -522,7 +522,7 @@
@Override
public int hashCode() {
- return Objects.hash(tcpPort, type);
+ return Objects.hash(type, tcpPort);
}
@Override
@@ -568,7 +568,7 @@
@Override
public int hashCode() {
- return Objects.hash(lambda, type);
+ return Objects.hash(type, lambda);
}
@Override
@@ -612,7 +612,7 @@
@Override
public int hashCode() {
- return Objects.hash(signalType, type);
+ return Objects.hash(type, signalType);
}
@Override
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/instructions/Instructions.java b/core/api/src/main/java/org/onlab/onos/net/flow/instructions/Instructions.java
index 0e77f4a..7dc0f8d 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/instructions/Instructions.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/instructions/Instructions.java
@@ -190,7 +190,7 @@
@Override
public int hashCode() {
- return Objects.hash(port, type());
+ return Objects.hash(type(), port);
}
@Override
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L0ModificationInstruction.java b/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L0ModificationInstruction.java
index 0d5cd81..25fe79f 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L0ModificationInstruction.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L0ModificationInstruction.java
@@ -70,7 +70,7 @@
@Override
public int hashCode() {
- return Objects.hash(lambda, type(), subtype);
+ return Objects.hash(type(), subtype, lambda);
}
@Override
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L2ModificationInstruction.java b/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L2ModificationInstruction.java
index abe19e3..20eaf6e 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L2ModificationInstruction.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L2ModificationInstruction.java
@@ -93,7 +93,7 @@
@Override
public int hashCode() {
- return Objects.hash(mac, type(), subtype);
+ return Objects.hash(type(), subtype, mac);
}
@Override
@@ -142,7 +142,7 @@
@Override
public int hashCode() {
- return Objects.hash(vlanId, type(), subtype());
+ return Objects.hash(type(), subtype(), vlanId);
}
@Override
@@ -191,7 +191,7 @@
@Override
public int hashCode() {
- return Objects.hash(vlanPcp, type(), subtype());
+ return Objects.hash(type(), subtype(), vlanPcp);
}
@Override
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L3ModificationInstruction.java b/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L3ModificationInstruction.java
index 89a8cda..e8b72e7 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L3ModificationInstruction.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L3ModificationInstruction.java
@@ -85,7 +85,7 @@
@Override
public int hashCode() {
- return Objects.hash(ip, type(), subtype());
+ return Objects.hash(type(), subtype(), ip);
}
@Override
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 e996dfc..7e5f049 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
@@ -15,22 +15,12 @@
*/
package org.onlab.onos.net.flow.impl;
-import static com.google.common.base.Preconditions.checkNotNull;
-import static org.slf4j.LoggerFactory.getLogger;
-import static org.onlab.util.Tools.namedThreads;
-
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.CancellationException;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
-import java.util.concurrent.atomic.AtomicReference;
-
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Sets;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
@@ -64,14 +54,21 @@
import org.onlab.onos.net.provider.AbstractProviderService;
import org.slf4j.Logger;
-import com.google.common.collect.ArrayListMultimap;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Multimap;
-import com.google.common.collect.Sets;
-import com.google.common.util.concurrent.Futures;
-import com.google.common.util.concurrent.ListenableFuture;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicReference;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.onlab.util.Tools.namedThreads;
+import static org.slf4j.LoggerFactory.getLogger;
/**
* Provides implementation of the flow NB & SB APIs.
@@ -92,8 +89,7 @@
private final FlowRuleStoreDelegate delegate = new InternalStoreDelegate();
- private final ExecutorService futureListeners =
- Executors.newCachedThreadPool(namedThreads("provider-future-listeners"));
+ private ExecutorService futureService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected FlowRuleStore store;
@@ -106,6 +102,7 @@
@Activate
public void activate() {
+ futureService = Executors.newCachedThreadPool(namedThreads("provider-future-listeners"));
store.setDelegate(delegate);
eventDispatcher.addSink(FlowRuleEvent.class, listenerRegistry);
log.info("Started");
@@ -113,7 +110,7 @@
@Deactivate
public void deactivate() {
- futureListeners.shutdownNow();
+ futureService.shutdownNow();
store.unsetDelegate(delegate);
eventDispatcher.removeSink(FlowRuleEvent.class);
@@ -364,6 +361,9 @@
// Store delegate to re-post events emitted from the store.
private class InternalStoreDelegate implements FlowRuleStoreDelegate {
+
+ private static final int TIMEOUT = 5000; // ms
+
// TODO: Right now we only dispatch events at individual flowEntry level.
// It may be more efficient for also dispatch events as a batch.
@Override
@@ -384,15 +384,21 @@
FlowRuleProvider flowRuleProvider =
getProvider(batchOperation.getOperations().get(0).getTarget().deviceId());
- final ListenableFuture<CompletedBatchOperation> result =
+ final Future<CompletedBatchOperation> result =
flowRuleProvider.executeBatch(batchOperation);
- result.addListener(new Runnable() {
+ futureService.submit(new Runnable() {
@Override
public void run() {
- store.batchOperationComplete(FlowRuleBatchEvent.completed(request,
- Futures.getUnchecked(result)));
+ CompletedBatchOperation res = null;
+ try {
+ res = result.get(TIMEOUT, TimeUnit.MILLISECONDS);
+ } catch (TimeoutException | InterruptedException | ExecutionException e) {
+ log.warn("Something went wrong with the batch operation {}",
+ request.batchId());
+ }
+ store.batchOperationComplete(FlowRuleBatchEvent.completed(request, res));
}
- }, futureListeners);
+ });
break;
case BATCH_OPERATION_COMPLETED: