Skip to content
Snippets Groups Projects
Commit 3aec000e authored by abat's avatar abat
Browse files

Merge into master from pull request #21:

Move switch locking code from IOFSwitch interface to OFSwitchImpl. (https://github.com/floodlight/floodlight/pull/21)
parents 80146aad b9f11a65
No related branches found
No related tags found
No related merge requests found
...@@ -22,8 +22,6 @@ import java.util.Date; ...@@ -22,8 +22,6 @@ import java.util.Date;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.concurrent.Future; import java.util.concurrent.Future;
import java.util.concurrent.locks.Lock;
import net.floodlightcontroller.core.types.MacVlanPair; import net.floodlightcontroller.core.types.MacVlanPair;
import net.floodlightcontroller.util.TimedCache; import net.floodlightcontroller.util.TimedCache;
...@@ -308,18 +306,4 @@ public interface IOFSwitch { ...@@ -308,18 +306,4 @@ public interface IOFSwitch {
*/ */
public TimedCache<Long> getTimedCache(); public TimedCache<Long> getTimedCache();
/**
* Return a lock that need to be held while processing a message. Multiple threads
* can hold this lock.
* @return
*/
public Lock processMessageLock();
/**
* Return a lock that needs to be held while the switch is removed asynchronously, i.e.,
* the removing is not triggered by events on this switch's channel.
* Mutex with processMessageLock
* @return
*/
public Lock asyncRemoveSwitchLock();
} }
...@@ -885,7 +885,7 @@ public class Controller ...@@ -885,7 +885,7 @@ public class Controller
protected void addSwitch(IOFSwitch sw) { protected void addSwitch(IOFSwitch sw) {
// TODO: is it save to modify the HashMap without holding // TODO: is it save to modify the HashMap without holding
// the old switch's lock // the old switch's lock
IOFSwitch oldSw = this.switches.put(sw.getId(), sw); OFSwitchImpl oldSw = (OFSwitchImpl) this.switches.put(sw.getId(), sw);
if (sw == oldSw) { if (sw == oldSw) {
// Note == for object equality, not .equals for value // Note == for object equality, not .equals for value
log.info("New add switch for pre-existing switch {}", sw); log.info("New add switch for pre-existing switch {}", sw);
......
...@@ -365,12 +365,21 @@ public class OFSwitchImpl implements IOFSwitch { ...@@ -365,12 +365,21 @@ public class OFSwitchImpl implements IOFSwitch {
return timedCache; return timedCache;
} }
@Override /**
* Return a lock that need to be held while processing a message. Multiple threads
* can hold this lock.
* @return
*/
public Lock processMessageLock() { public Lock processMessageLock() {
return lock.readLock(); return lock.readLock();
} }
@Override /**
* Return a lock that needs to be held while the switch is removed asynchronously, i.e.,
* the removing is not triggered by events on this switch's channel.
* Mutex with processMessageLock
* @return
*/
public Lock asyncRemoveSwitchLock() { public Lock asyncRemoveSwitchLock() {
return lock.writeLock(); return lock.writeLock();
} }
......
...@@ -27,7 +27,6 @@ import java.util.List; ...@@ -27,7 +27,6 @@ import java.util.List;
import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Future; import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import net.floodlightcontroller.core.FloodlightContext; import net.floodlightcontroller.core.FloodlightContext;
import net.floodlightcontroller.core.IFloodlightProvider; import net.floodlightcontroller.core.IFloodlightProvider;
...@@ -391,17 +390,22 @@ public class ControllerTest extends FloodlightTestCase { ...@@ -391,17 +390,22 @@ public class ControllerTest extends FloodlightTestCase {
@Test @Test
public void testAddSwitch() throws Exception { public void testAddSwitch() throws Exception {
controller.switches = new ConcurrentHashMap<Long, IOFSwitch>(); controller.switches = new ConcurrentHashMap<Long, IOFSwitch>();
ReentrantReadWriteLock rwlock = new ReentrantReadWriteLock();
IOFSwitch oldsw = createMock(IOFSwitch.class); //OFSwitchImpl oldsw = createMock(OFSwitchImpl.class);
expect(oldsw.getId()).andReturn(0L).anyTimes(); OFSwitchImpl oldsw = new OFSwitchImpl();
expect(oldsw.asyncRemoveSwitchLock()).andReturn(rwlock.writeLock()).anyTimes(); OFFeaturesReply featuresReply = new OFFeaturesReply();
oldsw.setConnected(false); featuresReply.setDatapathId(0L);
expect(oldsw.getFeaturesReply()).andReturn(new OFFeaturesReply()).anyTimes(); featuresReply.setPorts(new ArrayList<OFPhysicalPort>());
expect(oldsw.getStringId()).andReturn("00:00:00:00:00:00:00").anyTimes(); oldsw.setFeaturesReply(featuresReply);
//expect(oldsw.getId()).andReturn(0L).anyTimes();
//expect(oldsw.asyncRemoveSwitchLock()).andReturn(rwlock.writeLock()).anyTimes();
//oldsw.setConnected(false);
//expect(oldsw.getFeaturesReply()).andReturn(new OFFeaturesReply()).anyTimes();
//expect(oldsw.getStringId()).andReturn("00:00:00:00:00:00:00").anyTimes();
Channel channel = createMock(Channel.class); Channel channel = createMock(Channel.class);
expect(oldsw.getChannel()).andReturn(channel); //expect(oldsw.getChannel()).andReturn(channel);
oldsw.setChannel(channel);
expect(channel.close()).andReturn(null); expect(channel.close()).andReturn(null);
IOFSwitch newsw = createMock(IOFSwitch.class); IOFSwitch newsw = createMock(IOFSwitch.class);
...@@ -415,10 +419,10 @@ public class ControllerTest extends FloodlightTestCase { ...@@ -415,10 +419,10 @@ public class ControllerTest extends FloodlightTestCase {
expect(newsw.getPorts()).andReturn(new HashMap<Short,OFPhysicalPort>()); expect(newsw.getPorts()).andReturn(new HashMap<Short,OFPhysicalPort>());
controller.switches.put(0L, oldsw); controller.switches.put(0L, oldsw);
replay(oldsw, newsw, channel, channel2); replay(newsw, channel, channel2);
controller.addSwitch(newsw); controller.addSwitch(newsw);
verify(oldsw, newsw, channel, channel2); verify(newsw, channel, channel2);
} }
} }
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment