Committed by
Gerrit Code Review
Misc fixes in openflow provider subsystem
Change-Id: I5e6ab619f66ca71badc25efc7be7560070639051
Showing
5 changed files
with
22 additions
and
5 deletions
| ... | @@ -787,7 +787,7 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr | ... | @@ -787,7 +787,7 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr |
| 787 | OFPortStatsReply portStatsReply = (OFPortStatsReply) msg; | 787 | OFPortStatsReply portStatsReply = (OFPortStatsReply) msg; |
| 788 | List<OFPortStatsEntry> portStatsReplyList = portStatsReplies.get(dpid); | 788 | List<OFPortStatsEntry> portStatsReplyList = portStatsReplies.get(dpid); |
| 789 | if (portStatsReplyList == null) { | 789 | if (portStatsReplyList == null) { |
| 790 | - portStatsReplyList = Lists.newArrayList(); | 790 | + portStatsReplyList = Lists.newCopyOnWriteArrayList(); |
| 791 | } | 791 | } |
| 792 | portStatsReplyList.addAll(portStatsReply.getEntries()); | 792 | portStatsReplyList.addAll(portStatsReply.getEntries()); |
| 793 | portStatsReplies.put(dpid, portStatsReplyList); | 793 | portStatsReplies.put(dpid, portStatsReplyList); | ... | ... |
| ... | @@ -15,6 +15,7 @@ | ... | @@ -15,6 +15,7 @@ |
| 15 | */ | 15 | */ |
| 16 | package org.onosproject.provider.of.flow.impl; | 16 | package org.onosproject.provider.of.flow.impl; |
| 17 | 17 | ||
| 18 | +import static com.google.common.base.Preconditions.checkNotNull; | ||
| 18 | import org.onlab.util.SharedExecutors; | 19 | import org.onlab.util.SharedExecutors; |
| 19 | import org.onosproject.openflow.controller.OpenFlowSwitch; | 20 | import org.onosproject.openflow.controller.OpenFlowSwitch; |
| 20 | import org.onosproject.openflow.controller.RoleState; | 21 | import org.onosproject.openflow.controller.RoleState; |
| ... | @@ -52,7 +53,7 @@ class FlowStatsCollector implements SwitchDataCollector { | ... | @@ -52,7 +53,7 @@ class FlowStatsCollector implements SwitchDataCollector { |
| 52 | */ | 53 | */ |
| 53 | FlowStatsCollector(Timer timer, OpenFlowSwitch sw, int pollInterval) { | 54 | FlowStatsCollector(Timer timer, OpenFlowSwitch sw, int pollInterval) { |
| 54 | this.timer = timer; | 55 | this.timer = timer; |
| 55 | - this.sw = sw; | 56 | + this.sw = checkNotNull(sw, "Null switch"); |
| 56 | this.pollInterval = pollInterval; | 57 | this.pollInterval = pollInterval; |
| 57 | } | 58 | } |
| 58 | 59 | ... | ... |
| ... | @@ -19,8 +19,11 @@ import com.google.common.cache.Cache; | ... | @@ -19,8 +19,11 @@ import com.google.common.cache.Cache; |
| 19 | import com.google.common.cache.CacheBuilder; | 19 | import com.google.common.cache.CacheBuilder; |
| 20 | import com.google.common.cache.RemovalCause; | 20 | import com.google.common.cache.RemovalCause; |
| 21 | import com.google.common.cache.RemovalNotification; | 21 | import com.google.common.cache.RemovalNotification; |
| 22 | +import com.google.common.collect.ImmutableSet; | ||
| 23 | +import com.google.common.collect.Lists; | ||
| 22 | import com.google.common.collect.Maps; | 24 | import com.google.common.collect.Maps; |
| 23 | import com.google.common.collect.Sets; | 25 | import com.google.common.collect.Sets; |
| 26 | + | ||
| 24 | import org.apache.felix.scr.annotations.Activate; | 27 | import org.apache.felix.scr.annotations.Activate; |
| 25 | import org.apache.felix.scr.annotations.Component; | 28 | import org.apache.felix.scr.annotations.Component; |
| 26 | import org.apache.felix.scr.annotations.Deactivate; | 29 | import org.apache.felix.scr.annotations.Deactivate; |
| ... | @@ -217,6 +220,7 @@ public class OpenFlowRuleProvider extends AbstractProvider | ... | @@ -217,6 +220,7 @@ public class OpenFlowRuleProvider extends AbstractProvider |
| 217 | } | 220 | } |
| 218 | 221 | ||
| 219 | private void createCollector(OpenFlowSwitch sw) { | 222 | private void createCollector(OpenFlowSwitch sw) { |
| 223 | + checkNotNull(sw, "Null switch"); | ||
| 220 | if (adaptiveFlowSampling) { | 224 | if (adaptiveFlowSampling) { |
| 221 | // NewAdaptiveFlowStatsCollector Constructor | 225 | // NewAdaptiveFlowStatsCollector Constructor |
| 222 | NewAdaptiveFlowStatsCollector fsc = | 226 | NewAdaptiveFlowStatsCollector fsc = |
| ... | @@ -339,10 +343,17 @@ public class OpenFlowRuleProvider extends AbstractProvider | ... | @@ -339,10 +343,17 @@ public class OpenFlowRuleProvider extends AbstractProvider |
| 339 | public void executeBatch(FlowRuleBatchOperation batch) { | 343 | public void executeBatch(FlowRuleBatchOperation batch) { |
| 340 | checkNotNull(batch); | 344 | checkNotNull(batch); |
| 341 | 345 | ||
| 342 | - pendingBatches.put(batch.id(), new InternalCacheEntry(batch)); | ||
| 343 | - | ||
| 344 | Dpid dpid = Dpid.dpid(batch.deviceId().uri()); | 346 | Dpid dpid = Dpid.dpid(batch.deviceId().uri()); |
| 345 | OpenFlowSwitch sw = controller.getSwitch(dpid); | 347 | OpenFlowSwitch sw = controller.getSwitch(dpid); |
| 348 | + | ||
| 349 | + // If switch no longer exists, simply return. | ||
| 350 | + if (sw == null) { | ||
| 351 | + Set<FlowRule> failures = ImmutableSet.copyOf(Lists.transform(batch.getOperations(), e -> e.target())); | ||
| 352 | + providerService.batchOperationCompleted(batch.id(), | ||
| 353 | + new CompletedBatchOperation(false, failures, batch.deviceId())); | ||
| 354 | + return; | ||
| 355 | + } | ||
| 356 | + pendingBatches.put(batch.id(), new InternalCacheEntry(batch)); | ||
| 346 | OFFlowMod mod; | 357 | OFFlowMod mod; |
| 347 | for (FlowRuleBatchEntry fbe : batch.getOperations()) { | 358 | for (FlowRuleBatchEntry fbe : batch.getOperations()) { |
| 348 | // flow is the third party privacy flow | 359 | // flow is the third party privacy flow | ... | ... |
| ... | @@ -56,6 +56,11 @@ public class MeterStatsCollector implements TimerTask { | ... | @@ -56,6 +56,11 @@ public class MeterStatsCollector implements TimerTask { |
| 56 | 56 | ||
| 57 | @Override | 57 | @Override |
| 58 | public void run(Timeout timeout) throws Exception { | 58 | public void run(Timeout timeout) throws Exception { |
| 59 | + if (!sw.isConnected()) { | ||
| 60 | + log.debug("Switch {} disconnected. Aborting meter stats collection", sw.getStringId()); | ||
| 61 | + return; | ||
| 62 | + } | ||
| 63 | + | ||
| 59 | log.trace("Collecting stats for {}", sw.getStringId()); | 64 | log.trace("Collecting stats for {}", sw.getStringId()); |
| 60 | 65 | ||
| 61 | sendMeterStatistic(); | 66 | sendMeterStatistic(); | ... | ... |
| ... | @@ -204,7 +204,7 @@ public class OpenFlowMeterProvider extends AbstractProvider implements MeterProv | ... | @@ -204,7 +204,7 @@ public class OpenFlowMeterProvider extends AbstractProvider implements MeterProv |
| 204 | } | 204 | } |
| 205 | 205 | ||
| 206 | private void createStatsCollection(OpenFlowSwitch sw) { | 206 | private void createStatsCollection(OpenFlowSwitch sw) { |
| 207 | - if (isMeterSupported(sw)) { | 207 | + if (sw != null && isMeterSupported(sw)) { |
| 208 | MeterStatsCollector msc = new MeterStatsCollector(sw, POLL_INTERVAL); | 208 | MeterStatsCollector msc = new MeterStatsCollector(sw, POLL_INTERVAL); |
| 209 | msc.start(); | 209 | msc.start(); |
| 210 | stopCollectorIfNeeded(collectors.put(new Dpid(sw.getId()), msc)); | 210 | stopCollectorIfNeeded(collectors.put(new Dpid(sw.getId()), msc)); | ... | ... |
-
Please register or login to post a comment