Committed by
Gerrit Code Review
ONOS-1636 Added code to mark devices offline when openflow provider is deactivat…
…ed; fixed a few NPEs. Change-Id: I5f0e90b14bf1f00abd58e12590a3339b93695122
Showing
4 changed files
with
50 additions
and
34 deletions
| ... | @@ -396,7 +396,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider { | ... | @@ -396,7 +396,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider { |
| 396 | synchronized (discoverers) { | 396 | synchronized (discoverers) { |
| 397 | ld = discoverers.get(deviceId); | 397 | ld = discoverers.get(deviceId); |
| 398 | if (ld == null) { | 398 | if (ld == null) { |
| 399 | - if (rules.isSuppressed(device)) { | 399 | + if (rules != null && rules.isSuppressed(device)) { |
| 400 | log.debug("LinkDiscovery from {} disabled by configuration", device.id()); | 400 | log.debug("LinkDiscovery from {} disabled by configuration", device.id()); |
| 401 | return; | 401 | return; |
| 402 | } | 402 | } | ... | ... |
| ... | @@ -119,6 +119,7 @@ import com.google.common.collect.Sets; | ... | @@ -119,6 +119,7 @@ import com.google.common.collect.Sets; |
| 119 | public class OpenFlowDeviceProvider extends AbstractProvider implements DeviceProvider { | 119 | public class OpenFlowDeviceProvider extends AbstractProvider implements DeviceProvider { |
| 120 | 120 | ||
| 121 | private static final Logger LOG = getLogger(OpenFlowDeviceProvider.class); | 121 | private static final Logger LOG = getLogger(OpenFlowDeviceProvider.class); |
| 122 | + | ||
| 122 | private static final long MBPS = 1_000 * 1_000; | 123 | private static final long MBPS = 1_000 * 1_000; |
| 123 | private static final Frequency FREQ100 = Frequency.ofGHz(100); | 124 | private static final Frequency FREQ100 = Frequency.ofGHz(100); |
| 124 | private static final Frequency FREQ193_1 = Frequency.ofTHz(193.1); | 125 | private static final Frequency FREQ193_1 = Frequency.ofTHz(193.1); |
| ... | @@ -158,27 +159,16 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr | ... | @@ -158,27 +159,16 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr |
| 158 | providerService = providerRegistry.register(this); | 159 | providerService = providerRegistry.register(this); |
| 159 | controller.addListener(listener); | 160 | controller.addListener(listener); |
| 160 | controller.addEventListener(listener); | 161 | controller.addEventListener(listener); |
| 161 | - for (OpenFlowSwitch sw : controller.getSwitches()) { | 162 | + connectInitialDevices(); |
| 162 | - try { | ||
| 163 | - listener.switchAdded(new Dpid(sw.getId())); | ||
| 164 | - } catch (Exception e) { | ||
| 165 | - LOG.warn("Failed initially adding {} : {}", sw.getStringId(), e.getMessage()); | ||
| 166 | - LOG.debug("Error details:", e); | ||
| 167 | - // disconnect to trigger switch-add later | ||
| 168 | - sw.disconnectSwitch(); | ||
| 169 | - } | ||
| 170 | - PortStatsCollector psc = new PortStatsCollector(sw, portStatsPollFrequency); | ||
| 171 | - psc.start(); | ||
| 172 | - collectors.put(new Dpid(sw.getId()), psc); | ||
| 173 | - } | ||
| 174 | LOG.info("Started"); | 163 | LOG.info("Started"); |
| 175 | } | 164 | } |
| 176 | 165 | ||
| 177 | @Deactivate | 166 | @Deactivate |
| 178 | public void deactivate(ComponentContext context) { | 167 | public void deactivate(ComponentContext context) { |
| 179 | cfgService.unregisterProperties(getClass(), false); | 168 | cfgService.unregisterProperties(getClass(), false); |
| 180 | - providerRegistry.unregister(this); | ||
| 181 | controller.removeListener(listener); | 169 | controller.removeListener(listener); |
| 170 | + disconnectDevices(); | ||
| 171 | + providerRegistry.unregister(this); | ||
| 182 | collectors.values().forEach(PortStatsCollector::stop); | 172 | collectors.values().forEach(PortStatsCollector::stop); |
| 183 | providerService = null; | 173 | providerService = null; |
| 184 | LOG.info("Stopped"); | 174 | LOG.info("Stopped"); |
| ... | @@ -204,13 +194,31 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr | ... | @@ -204,13 +194,31 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr |
| 204 | LOG.info("Settings: portStatsPollFrequency={}", portStatsPollFrequency); | 194 | LOG.info("Settings: portStatsPollFrequency={}", portStatsPollFrequency); |
| 205 | } | 195 | } |
| 206 | 196 | ||
| 197 | + private void connectInitialDevices() { | ||
| 198 | + for (OpenFlowSwitch sw : controller.getSwitches()) { | ||
| 199 | + try { | ||
| 200 | + listener.switchAdded(new Dpid(sw.getId())); | ||
| 201 | + } catch (Exception e) { | ||
| 202 | + LOG.warn("Failed initially adding {} : {}", sw.getStringId(), e.getMessage()); | ||
| 203 | + LOG.debug("Error details:", e); | ||
| 204 | + // disconnect to trigger switch-add later | ||
| 205 | + sw.disconnectSwitch(); | ||
| 206 | + } | ||
| 207 | + PortStatsCollector psc = new PortStatsCollector(sw, portStatsPollFrequency); | ||
| 208 | + psc.start(); | ||
| 209 | + collectors.put(new Dpid(sw.getId()), psc); | ||
| 210 | + } | ||
| 211 | + } | ||
| 212 | + | ||
| 213 | + private void disconnectDevices() { | ||
| 214 | + // Only disconnect the devices for which we are currently master. | ||
| 215 | + controller.getMasterSwitches().forEach(sw -> listener.switchRemoved(new Dpid(sw.getId()))); | ||
| 216 | + } | ||
| 217 | + | ||
| 207 | @Override | 218 | @Override |
| 208 | public boolean isReachable(DeviceId deviceId) { | 219 | public boolean isReachable(DeviceId deviceId) { |
| 209 | OpenFlowSwitch sw = controller.getSwitch(dpid(deviceId.uri())); | 220 | OpenFlowSwitch sw = controller.getSwitch(dpid(deviceId.uri())); |
| 210 | - if (sw == null || !sw.isConnected()) { | 221 | + return sw != null && sw.isConnected(); |
| 211 | - return false; | ||
| 212 | - } | ||
| 213 | - return true; | ||
| 214 | } | 222 | } |
| 215 | 223 | ||
| 216 | @Override | 224 | @Override | ... | ... |
| ... | @@ -16,6 +16,7 @@ | ... | @@ -16,6 +16,7 @@ |
| 16 | package org.onosproject.provider.of.device.impl; | 16 | package org.onosproject.provider.of.device.impl; |
| 17 | 17 | ||
| 18 | import com.google.common.collect.HashMultimap; | 18 | import com.google.common.collect.HashMultimap; |
| 19 | +import com.google.common.collect.ImmutableSet; | ||
| 19 | import com.google.common.collect.Lists; | 20 | import com.google.common.collect.Lists; |
| 20 | import com.google.common.collect.Multimap; | 21 | import com.google.common.collect.Multimap; |
| 21 | 22 | ||
| ... | @@ -236,7 +237,7 @@ public class OpenFlowDeviceProviderTest { | ... | @@ -236,7 +237,7 @@ public class OpenFlowDeviceProviderTest { |
| 236 | 237 | ||
| 237 | @Override | 238 | @Override |
| 238 | public Iterable<OpenFlowSwitch> getMasterSwitches() { | 239 | public Iterable<OpenFlowSwitch> getMasterSwitches() { |
| 239 | - return null; | 240 | + return ImmutableSet.of(); |
| 240 | } | 241 | } |
| 241 | 242 | ||
| 242 | @Override | 243 | @Override | ... | ... |
| ... | @@ -273,7 +273,10 @@ public class OpenFlowRuleProvider extends AbstractProvider | ... | @@ -273,7 +273,10 @@ public class OpenFlowRuleProvider extends AbstractProvider |
| 273 | 273 | ||
| 274 | if (adaptiveFlowSampling) { | 274 | if (adaptiveFlowSampling) { |
| 275 | // Add TypedFlowEntry to deviceFlowEntries in NewAdaptiveFlowStatsCollector | 275 | // Add TypedFlowEntry to deviceFlowEntries in NewAdaptiveFlowStatsCollector |
| 276 | - afsCollectors.get(dpid).addWithFlowRule(flowRule); | 276 | + NewAdaptiveFlowStatsCollector collector = afsCollectors.get(dpid); |
| 277 | + if (collector != null) { | ||
| 278 | + collector.addWithFlowRule(flowRule); | ||
| 279 | + } | ||
| 277 | } | 280 | } |
| 278 | } | 281 | } |
| 279 | 282 | ||
| ... | @@ -299,7 +302,10 @@ public class OpenFlowRuleProvider extends AbstractProvider | ... | @@ -299,7 +302,10 @@ public class OpenFlowRuleProvider extends AbstractProvider |
| 299 | 302 | ||
| 300 | if (adaptiveFlowSampling) { | 303 | if (adaptiveFlowSampling) { |
| 301 | // Remove TypedFlowEntry to deviceFlowEntries in NewAdaptiveFlowStatsCollector | 304 | // Remove TypedFlowEntry to deviceFlowEntries in NewAdaptiveFlowStatsCollector |
| 302 | - afsCollectors.get(dpid).removeFlows(flowRule); | 305 | + NewAdaptiveFlowStatsCollector collector = afsCollectors.get(dpid); |
| 306 | + if (collector != null) { | ||
| 307 | + collector.removeFlows(flowRule); | ||
| 308 | + } | ||
| 303 | } | 309 | } |
| 304 | } | 310 | } |
| 305 | 311 | ||
| ... | @@ -327,32 +333,30 @@ public class OpenFlowRuleProvider extends AbstractProvider | ... | @@ -327,32 +333,30 @@ public class OpenFlowRuleProvider extends AbstractProvider |
| 327 | sw.sendMsg(msg); | 333 | sw.sendMsg(msg); |
| 328 | continue; | 334 | continue; |
| 329 | } | 335 | } |
| 330 | - FlowModBuilder builder = FlowModBuilder.builder(fbe.target(), sw | 336 | + FlowModBuilder builder = |
| 331 | - .factory(), Optional.of(batch.id())); | 337 | + FlowModBuilder.builder(fbe.target(), sw.factory(), Optional.of(batch.id())); |
| 338 | + NewAdaptiveFlowStatsCollector collector = afsCollectors.get(dpid); | ||
| 332 | switch (fbe.operator()) { | 339 | switch (fbe.operator()) { |
| 333 | case ADD: | 340 | case ADD: |
| 334 | mod = builder.buildFlowAdd(); | 341 | mod = builder.buildFlowAdd(); |
| 335 | - | 342 | + if (adaptiveFlowSampling && collector != null) { |
| 336 | - if (adaptiveFlowSampling) { | ||
| 337 | // Add TypedFlowEntry to deviceFlowEntries in NewAdaptiveFlowStatsCollector | 343 | // Add TypedFlowEntry to deviceFlowEntries in NewAdaptiveFlowStatsCollector |
| 338 | - afsCollectors.get(dpid).addWithFlowRule(fbe.target()); | 344 | + collector.addWithFlowRule(fbe.target()); |
| 339 | } | 345 | } |
| 340 | break; | 346 | break; |
| 341 | case REMOVE: | 347 | case REMOVE: |
| 342 | mod = builder.buildFlowDel(); | 348 | mod = builder.buildFlowDel(); |
| 343 | - | 349 | + if (adaptiveFlowSampling && collector != null) { |
| 344 | - if (adaptiveFlowSampling) { | ||
| 345 | // Remove TypedFlowEntry to deviceFlowEntries in NewAdaptiveFlowStatsCollector | 350 | // Remove TypedFlowEntry to deviceFlowEntries in NewAdaptiveFlowStatsCollector |
| 346 | - afsCollectors.get(dpid).removeFlows(fbe.target()); | 351 | + collector.removeFlows(fbe.target()); |
| 347 | } | 352 | } |
| 348 | break; | 353 | break; |
| 349 | case MODIFY: | 354 | case MODIFY: |
| 350 | mod = builder.buildFlowMod(); | 355 | mod = builder.buildFlowMod(); |
| 351 | - | 356 | + if (adaptiveFlowSampling && collector != null) { |
| 352 | - if (adaptiveFlowSampling) { | ||
| 353 | // Add or Update TypedFlowEntry to deviceFlowEntries in NewAdaptiveFlowStatsCollector | 357 | // Add or Update TypedFlowEntry to deviceFlowEntries in NewAdaptiveFlowStatsCollector |
| 354 | // afsCollectors.get(dpid).addWithFlowRule(fbe.target()); //check if add is good or not | 358 | // afsCollectors.get(dpid).addWithFlowRule(fbe.target()); //check if add is good or not |
| 355 | - afsCollectors.get(dpid).addOrUpdateFlows((FlowEntry) fbe.target()); | 359 | + collector.addOrUpdateFlows((FlowEntry) fbe.target()); |
| 356 | } | 360 | } |
| 357 | break; | 361 | break; |
| 358 | default: | 362 | default: |
| ... | @@ -424,7 +428,10 @@ public class OpenFlowRuleProvider extends AbstractProvider | ... | @@ -424,7 +428,10 @@ public class OpenFlowRuleProvider extends AbstractProvider |
| 424 | 428 | ||
| 425 | if (adaptiveFlowSampling) { | 429 | if (adaptiveFlowSampling) { |
| 426 | // Removed TypedFlowEntry to deviceFlowEntries in NewAdaptiveFlowStatsCollector | 430 | // Removed TypedFlowEntry to deviceFlowEntries in NewAdaptiveFlowStatsCollector |
| 427 | - afsCollectors.get(dpid).flowRemoved(fr); | 431 | + NewAdaptiveFlowStatsCollector collector = afsCollectors.get(dpid); |
| 432 | + if (collector != null) { | ||
| 433 | + collector.flowRemoved(fr); | ||
| 434 | + } | ||
| 428 | } | 435 | } |
| 429 | break; | 436 | break; |
| 430 | case STATS_REPLY: | 437 | case STATS_REPLY: | ... | ... |
-
Please register or login to post a comment