From e1cb19aa3b8f963a896222f8b4efdfddd340143f Mon Sep 17 00:00:00 2001 From: Andreas Wundsam <andreas.wundsam@bigswitch.com> Date: Tue, 12 Feb 2013 16:19:20 -0800 Subject: [PATCH] OFSwitchBase: fix broadcast portBroadcastCacheHitMap --- .../core/OFSwitchBase.java | 56 +++++++++++-------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java b/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java index 17bbcf07e..dba286927 100644 --- a/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java +++ b/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java @@ -30,14 +30,11 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantReadWriteLock; -import net.floodlightcontroller.core.FloodlightContext; -import net.floodlightcontroller.core.IFloodlightProviderService; -import net.floodlightcontroller.core.IOFMessageListener; import net.floodlightcontroller.core.IFloodlightProviderService.Role; -import net.floodlightcontroller.core.IOFSwitch; import net.floodlightcontroller.core.annotations.LogMessageDoc; import net.floodlightcontroller.core.internal.Controller; import net.floodlightcontroller.core.internal.OFFeaturesReplyFuture; @@ -56,11 +53,11 @@ import org.openflow.protocol.OFFlowMod; import org.openflow.protocol.OFMatch; import org.openflow.protocol.OFMessage; import org.openflow.protocol.OFPhysicalPort; -import org.openflow.protocol.OFPort; -import org.openflow.protocol.OFType; import org.openflow.protocol.OFPhysicalPort.OFPortConfig; import org.openflow.protocol.OFPhysicalPort.OFPortState; +import org.openflow.protocol.OFPort; import org.openflow.protocol.OFStatisticsRequest; +import org.openflow.protocol.OFType; import org.openflow.protocol.statistics.OFStatistics; import org.openflow.util.HexString; import org.openflow.util.U16; @@ -93,8 +90,8 @@ public abstract class OFSwitchBase implements IOFSwitch { * Members hidden from subclasses */ private Channel channel; - private AtomicInteger transactionIdSource; - // Lock to protect modification of the port maps. We only need to + private final AtomicInteger transactionIdSource; + // Lock to protect modification of the port maps. We only need to // synchronize on modifications. For read operations we are fine since // we rely on ConcurrentMaps which works for our use case. protected Object portLock; @@ -102,18 +99,18 @@ public abstract class OFSwitchBase implements IOFSwitch { protected ConcurrentHashMap<Short, OFPhysicalPort> portsByNumber; // Map port names to the appropriate OFPhyiscalPort // XXX: The OF spec doesn't specify if port names need to be unique but - // according it's always the case in practice. - private ConcurrentHashMap<String, OFPhysicalPort> portsByName; - private Map<Integer,OFStatisticsFuture> statsFutureMap; - private Map<Integer, IOFMessageListener> iofMsgListenersMap; - private Map<Integer,OFFeaturesReplyFuture> featuresFutureMap; + // according it's always the case in practice. + private final ConcurrentHashMap<String, OFPhysicalPort> portsByName; + private final Map<Integer,OFStatisticsFuture> statsFutureMap; + private final Map<Integer, IOFMessageListener> iofMsgListenersMap; + private final Map<Integer,OFFeaturesReplyFuture> featuresFutureMap; private volatile boolean connected; - private TimedCache<Long> timedCache; - private ReentrantReadWriteLock listenerLock; - private ConcurrentMap<Short, Long> portBroadcastCacheHitMap; - + private final TimedCache<Long> timedCache; + private final ReentrantReadWriteLock listenerLock; + private final ConcurrentMap<Short, AtomicLong> portBroadcastCacheHitMap; - protected static ThreadLocal<Map<IOFSwitch,List<OFMessage>>> local_msg_buffer = + + protected final static ThreadLocal<Map<IOFSwitch,List<OFMessage>>> local_msg_buffer = new ThreadLocal<Map<IOFSwitch,List<OFMessage>>>() { @Override protected Map<IOFSwitch,List<OFMessage>> initialValue() { @@ -139,8 +136,8 @@ public abstract class OFSwitchBase implements IOFSwitch { this.role = null; this.timedCache = new TimedCache<Long>(100, 5*1000 ); // 5 seconds interval this.listenerLock = new ReentrantReadWriteLock(); - this.portBroadcastCacheHitMap = new ConcurrentHashMap<Short, Long>(); - + this.portBroadcastCacheHitMap = new ConcurrentHashMap<Short, AtomicLong>(); + // Defaults properties for an ideal switch this.setAttribute(PROP_FASTWILDCARDS, OFMatch.OFPFW_ALL); this.setAttribute(PROP_SUPPORTS_OFPP_FLOOD, new Boolean(true)); @@ -524,10 +521,17 @@ public abstract class OFSwitchBase implements IOFSwitch { @Override public boolean updateBroadcastCache(Long entry, Short port) { if (timedCache.update(entry)) { - Long count = portBroadcastCacheHitMap.putIfAbsent(port, new Long(1)); - if (count != null) { - count++; + AtomicLong count = portBroadcastCacheHitMap.get(port); + if(count == null) { + AtomicLong newCount = new AtomicLong(0); + AtomicLong retrieved; + if((retrieved = portBroadcastCacheHitMap.putIfAbsent(port, newCount)) == null ) { + count = newCount; + } else { + count = retrieved; + } } + count.incrementAndGet(); return true; } else { return false; @@ -537,7 +541,11 @@ public abstract class OFSwitchBase implements IOFSwitch { @Override @JsonIgnore public Map<Short, Long> getPortBroadcastHits() { - return this.portBroadcastCacheHitMap; + Map<Short, Long> res = new HashMap<Short, Long>(); + for (Map.Entry<Short, AtomicLong> entry : portBroadcastCacheHitMap.entrySet()) { + res.put(entry.getKey(), entry.getValue().get()); + } + return res; } -- GitLab