Removed old codes from FlowPusher which leads unnecessary overhead.
Fixed FlowPusherTest to fit modification above.
Change-Id: Ifd72ceca52dc2e290c412df3aa6d973f36a8e478
diff --git a/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java b/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java
index b27e0fd..a660cd6 100644
--- a/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java
+++ b/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java
@@ -6,12 +6,12 @@
import java.util.Collection;
import java.util.EnumSet;
import java.util.HashMap;
-import java.util.HashSet;
+import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Queue;
-import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.locks.Condition;
@@ -324,14 +324,12 @@
}
}
- // for safety of concurrent access, copy set of key objects
- Set<IOFSwitch> keys = new HashSet<IOFSwitch>(assignedQueues.size());
- for (IOFSwitch sw : assignedQueues.keySet()) {
- keys.add(sw);
- }
-
- for (IOFSwitch sw : keys) {
- SwitchQueue queue = assignedQueues.get(sw);
+ for (Iterator<Entry<IOFSwitch, SwitchQueue>> it = assignedQueues.entrySet().iterator();
+ it.hasNext();
+ ) {
+ Entry<IOFSwitch, SwitchQueue> entry = it.next();
+ IOFSwitch sw = entry.getKey();
+ SwitchQueue queue = entry.getValue();
if (queue == null) {
continue;
@@ -341,7 +339,7 @@
processQueue(sw, queue, MAX_MESSAGE_SEND);
if (queue.toBeDeleted && !queue.hasMessageToSend()) {
// remove queue if flagged to be.
- assignedQueues.remove(sw);
+ it.remove();
}
}
}
@@ -362,7 +360,7 @@
long currentTime = System.currentTimeMillis();
long size = 0;
- if (queue.isSendable(currentTime)) {
+ if (sw.isConnected() && queue.isSendable(currentTime)) {
int i = 0;
while (queue.hasMessageToSend()) {
// Number of messages excess the limit
diff --git a/src/test/java/net/onrc/onos/core/flowprogrammer/FlowPusherTest.java b/src/test/java/net/onrc/onos/core/flowprogrammer/FlowPusherTest.java
index cf86358..89f188d 100644
--- a/src/test/java/net/onrc/onos/core/flowprogrammer/FlowPusherTest.java
+++ b/src/test/java/net/onrc/onos/core/flowprogrammer/FlowPusherTest.java
@@ -1,5 +1,18 @@
package net.onrc.onos.core.flowprogrammer;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
import net.floodlightcontroller.core.FloodlightContext;
import net.floodlightcontroller.core.IFloodlightProviderService;
import net.floodlightcontroller.core.IOFSwitch;
@@ -16,6 +29,7 @@
import net.onrc.onos.core.util.FlowId;
import net.onrc.onos.core.util.IntegrationTest;
import net.onrc.onos.core.util.PortNumber;
+
import org.easymock.EasyMock;
import org.easymock.IAnswer;
import org.junit.Test;
@@ -28,19 +42,6 @@
import org.openflow.protocol.action.OFAction;
import org.openflow.protocol.factory.BasicFactory;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
@Category(IntegrationTest.class)
public class FlowPusherTest {
private FlowPusher pusher;
@@ -63,10 +64,7 @@
EasyMock.expect(msg.getLength()).andReturn((short) 100).anyTimes();
EasyMock.replay(msg);
- IOFSwitch sw = EasyMock.createMock(IOFSwitch.class);
- EasyMock.expect(sw.getId()).andReturn((long) 1).anyTimes();
- sw.flush();
- EasyMock.expectLastCall().once();
+ IOFSwitch sw = createConnectedSwitchMock(1, false);
EasyMock.replay(sw);
try {
@@ -105,10 +103,7 @@
beginInitMock();
- IOFSwitch sw = EasyMock.createMock(IOFSwitch.class);
- EasyMock.expect(sw.getId()).andReturn((long) 1).anyTimes();
- sw.flush();
- EasyMock.expectLastCall().atLeastOnce();
+ IOFSwitch sw = createConnectedSwitchMock(1, false);
EasyMock.replay(sw);
List<OFMessage> messages = new ArrayList<OFMessage>();
@@ -164,10 +159,7 @@
Map<IOFSwitch, List<OFMessage>> swMap = new HashMap<IOFSwitch, List<OFMessage>>();
for (int i = 0; i < numSwitch; ++i) {
- IOFSwitch sw = EasyMock.createMock(IOFSwitch.class);
- EasyMock.expect(sw.getId()).andReturn((long) i).anyTimes();
- sw.flush();
- EasyMock.expectLastCall().atLeastOnce();
+ IOFSwitch sw = createConnectedSwitchMock(i, false);
EasyMock.replay(sw);
List<OFMessage> messages = new ArrayList<OFMessage>();
@@ -230,10 +222,7 @@
Map<IOFSwitch, List<OFMessage>> swMap = new HashMap<IOFSwitch, List<OFMessage>>();
for (int i = 0; i < numThreads; ++i) {
- IOFSwitch sw = EasyMock.createMock(IOFSwitch.class);
- EasyMock.expect(sw.getId()).andReturn((long) i).anyTimes();
- sw.flush();
- EasyMock.expectLastCall().atLeastOnce();
+ IOFSwitch sw = createConnectedSwitchMock(i, false);
EasyMock.replay(sw);
List<OFMessage> messages = new ArrayList<OFMessage>();
@@ -302,11 +291,7 @@
beginInitMock();
- IOFSwitch sw = EasyMock.createMock(IOFSwitch.class);
- EasyMock.expect(sw.getId()).andReturn((long) 1).anyTimes();
- sw.flush();
- EasyMock.expectLastCall().atLeastOnce();
- prepareBarrier(sw);
+ IOFSwitch sw = createConnectedSwitchMock(1, true);
EasyMock.replay(sw);
List<OFMessage> messages = new ArrayList<OFMessage>();
@@ -384,11 +369,7 @@
public void testBarrierMessage() {
beginInitMock();
- IOFSwitch sw = EasyMock.createMock(IOFSwitch.class);
- EasyMock.expect(sw.getId()).andReturn((long) 1).anyTimes();
- sw.flush();
- EasyMock.expectLastCall().atLeastOnce();
- prepareBarrier(sw);
+ IOFSwitch sw = createConnectedSwitchMock(1, true);
EasyMock.replay(sw);
try {
@@ -457,11 +438,8 @@
EasyMock.expect(factory.getMessage(EasyMock.eq(OFType.FLOW_MOD))).andReturn(msg);
- IOFSwitch sw = EasyMock.createMock(IOFSwitch.class);
- EasyMock.expect(sw.getId()).andReturn(DPID_TO_VERIFY).anyTimes();
+ IOFSwitch sw = createConnectedSwitchMock(DPID_TO_VERIFY, false);
EasyMock.expect(sw.getStringId()).andReturn("1").anyTimes();
- sw.flush();
- EasyMock.expectLastCall().once();
try {
EasyMock.expect(damper.write(EasyMock.eq(sw), EasyMock.anyObject(OFMessage.class), EasyMock.eq(context)))
@@ -545,6 +523,19 @@
pusher.start();
}
+ private IOFSwitch createConnectedSwitchMock(long dpid, boolean useBarrier) {
+ IOFSwitch sw = EasyMock.createMock(IOFSwitch.class);
+ EasyMock.expect(sw.isConnected()).andReturn(true).anyTimes();
+ EasyMock.expect(sw.getId()).andReturn(dpid).anyTimes();
+ sw.flush();
+ EasyMock.expectLastCall().atLeastOnce();
+ if (useBarrier) {
+ prepareBarrier(sw);
+ }
+
+ return sw;
+ }
+
private void prepareBarrier(IOFSwitch sw) {
OFBarrierRequest req = EasyMock.createMock(OFBarrierRequest.class);
req.setXid(EasyMock.anyInt());