Showing
8 changed files
with
57 additions
and
29 deletions
| 1 | package org.onlab.onos.net.link; | 1 | package org.onlab.onos.net.link; |
| 2 | 2 | ||
| 3 | +import com.google.common.base.MoreObjects; | ||
| 3 | import org.onlab.onos.net.AbstractDescription; | 4 | import org.onlab.onos.net.AbstractDescription; |
| 4 | import org.onlab.onos.net.ConnectPoint; | 5 | import org.onlab.onos.net.ConnectPoint; |
| 5 | import org.onlab.onos.net.Link; | 6 | import org.onlab.onos.net.Link; |
| ... | @@ -46,4 +47,11 @@ public class DefaultLinkDescription extends AbstractDescription | ... | @@ -46,4 +47,11 @@ public class DefaultLinkDescription extends AbstractDescription |
| 46 | return type; | 47 | return type; |
| 47 | } | 48 | } |
| 48 | 49 | ||
| 50 | + @Override | ||
| 51 | + public String toString() { | ||
| 52 | + return MoreObjects.toStringHelper("Link").add("src", src()) | ||
| 53 | + .add("dst", dst()) | ||
| 54 | + .add("type", type()).toString(); | ||
| 55 | + } | ||
| 56 | + | ||
| 49 | } | 57 | } | ... | ... |
| ... | @@ -201,10 +201,10 @@ public class LinkManager | ... | @@ -201,10 +201,10 @@ public class LinkManager |
| 201 | ConnectPoint dst = linkDescription.dst(); | 201 | ConnectPoint dst = linkDescription.dst(); |
| 202 | // if we aren't master for the device associated with the ConnectPoint | 202 | // if we aren't master for the device associated with the ConnectPoint |
| 203 | // we probably shouldn't be doing this. | 203 | // we probably shouldn't be doing this. |
| 204 | - if ((deviceService.getRole(src.deviceId()) != MastershipRole.MASTER) || | 204 | + |
| 205 | - (deviceService.getRole(dst.deviceId()) != MastershipRole.MASTER)) { | 205 | +// if (deviceService.getRole(dst.deviceId()) != MastershipRole.MASTER) { |
| 206 | - return; | 206 | +// return; |
| 207 | - } | 207 | +// } |
| 208 | LinkEvent event = store.createOrUpdateLink(provider().id(), | 208 | LinkEvent event = store.createOrUpdateLink(provider().id(), |
| 209 | linkDescription); | 209 | linkDescription); |
| 210 | if (event != null) { | 210 | if (event != null) { |
| ... | @@ -220,14 +220,8 @@ public class LinkManager | ... | @@ -220,14 +220,8 @@ public class LinkManager |
| 220 | 220 | ||
| 221 | ConnectPoint src = linkDescription.src(); | 221 | ConnectPoint src = linkDescription.src(); |
| 222 | ConnectPoint dst = linkDescription.dst(); | 222 | ConnectPoint dst = linkDescription.dst(); |
| 223 | - // if we aren't master for the device associated with the ConnectPoint | 223 | + |
| 224 | - // we probably shouldn't be doing this. | 224 | + LinkEvent event = store.removeLink(src, dst); |
| 225 | - if ((deviceService.getRole(src.deviceId()) != MastershipRole.MASTER) || | ||
| 226 | - (deviceService.getRole(dst.deviceId()) != MastershipRole.MASTER)) { | ||
| 227 | - return; | ||
| 228 | - } | ||
| 229 | - LinkEvent event = store.removeLink(linkDescription.src(), | ||
| 230 | - linkDescription.dst()); | ||
| 231 | if (event != null) { | 225 | if (event != null) { |
| 232 | log.info("Link {} vanished", linkDescription); | 226 | log.info("Link {} vanished", linkDescription); |
| 233 | post(event); | 227 | post(event); | ... | ... |
| ... | @@ -239,11 +239,14 @@ public class GossipLinkStore | ... | @@ -239,11 +239,14 @@ public class GossipLinkStore |
| 239 | LinkKey key = linkKey(linkDescription.src(), linkDescription.dst()); | 239 | LinkKey key = linkKey(linkDescription.src(), linkDescription.dst()); |
| 240 | final LinkEvent event; | 240 | final LinkEvent event; |
| 241 | final Timestamped<LinkDescription> mergedDesc; | 241 | final Timestamped<LinkDescription> mergedDesc; |
| 242 | - synchronized (getOrCreateLinkDescriptions(key)) { | 242 | + Map<ProviderId, Timestamped<LinkDescription>> map = getOrCreateLinkDescriptions(key); |
| 243 | + synchronized (map) { | ||
| 243 | event = createOrUpdateLinkInternal(providerId, deltaDesc); | 244 | event = createOrUpdateLinkInternal(providerId, deltaDesc); |
| 244 | - mergedDesc = getOrCreateLinkDescriptions(key).get(providerId); | 245 | + mergedDesc = map.get(providerId); |
| 245 | } | 246 | } |
| 246 | 247 | ||
| 248 | + | ||
| 249 | + | ||
| 247 | if (event != null) { | 250 | if (event != null) { |
| 248 | log.info("Notifying peers of a link update topology event from providerId: " | 251 | log.info("Notifying peers of a link update topology event from providerId: " |
| 249 | + "{} between src: {} and dst: {}", | 252 | + "{} between src: {} and dst: {}", |
| ... | @@ -252,8 +255,8 @@ public class GossipLinkStore | ... | @@ -252,8 +255,8 @@ public class GossipLinkStore |
| 252 | notifyPeers(new InternalLinkEvent(providerId, mergedDesc)); | 255 | notifyPeers(new InternalLinkEvent(providerId, mergedDesc)); |
| 253 | } catch (IOException e) { | 256 | } catch (IOException e) { |
| 254 | log.info("Failed to notify peers of a link update topology event from providerId: " | 257 | log.info("Failed to notify peers of a link update topology event from providerId: " |
| 255 | - + "{} between src: {} and dst: {}", | 258 | + + "{} between src: {} and dst: {}", |
| 256 | - providerId, linkDescription.src(), linkDescription.dst()); | 259 | + providerId, linkDescription.src(), linkDescription.dst()); |
| 257 | } | 260 | } |
| 258 | } | 261 | } |
| 259 | return event; | 262 | return event; | ... | ... |
| ... | @@ -288,6 +288,7 @@ public abstract class AbstractOpenFlowSwitch implements OpenFlowSwitchDriver { | ... | @@ -288,6 +288,7 @@ public abstract class AbstractOpenFlowSwitch implements OpenFlowSwitchDriver { |
| 288 | // The message wasn't really a Nicira role reply. We just | 288 | // The message wasn't really a Nicira role reply. We just |
| 289 | // dispatch it to the OFMessage listeners in this case. | 289 | // dispatch it to the OFMessage listeners in this case. |
| 290 | this.handleMessage(m); | 290 | this.handleMessage(m); |
| 291 | + return; | ||
| 291 | } | 292 | } |
| 292 | 293 | ||
| 293 | RoleRecvStatus rrs = this.roleMan.deliverRoleReply( | 294 | RoleRecvStatus rrs = this.roleMan.deliverRoleReply( |
| ... | @@ -301,8 +302,7 @@ public abstract class AbstractOpenFlowSwitch implements OpenFlowSwitchDriver { | ... | @@ -301,8 +302,7 @@ public abstract class AbstractOpenFlowSwitch implements OpenFlowSwitchDriver { |
| 301 | this.transitionToEqualSwitch(); | 302 | this.transitionToEqualSwitch(); |
| 302 | } | 303 | } |
| 303 | } else { | 304 | } else { |
| 304 | - return; | 305 | + this.disconnectSwitch(); |
| 305 | - //TODO: tell people that we failed. | ||
| 306 | } | 306 | } |
| 307 | } | 307 | } |
| 308 | 308 | ... | ... |
| ... | @@ -71,6 +71,15 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider { | ... | @@ -71,6 +71,15 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider { |
| 71 | providerService = providerRegistry.register(this); | 71 | providerService = providerRegistry.register(this); |
| 72 | deviceService.addListener(listener); | 72 | deviceService.addListener(listener); |
| 73 | packetSevice.addProcessor(listener, 0); | 73 | packetSevice.addProcessor(listener, 0); |
| 74 | + LinkDiscovery ld; | ||
| 75 | + for (Device device : deviceService.getDevices()) { | ||
| 76 | + ld = new LinkDiscovery(device, packetSevice, masterService, | ||
| 77 | + providerService, useBDDP); | ||
| 78 | + discoverers.put(device.id(), ld); | ||
| 79 | + for (Port p : deviceService.getPorts(device.id())) { | ||
| 80 | + ld.addPort(p); | ||
| 81 | + } | ||
| 82 | + } | ||
| 74 | 83 | ||
| 75 | log.info("Started"); | 84 | log.info("Started"); |
| 76 | } | 85 | } |
| ... | @@ -96,6 +105,10 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider { | ... | @@ -96,6 +105,10 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider { |
| 96 | LinkDiscovery ld = null; | 105 | LinkDiscovery ld = null; |
| 97 | Device device = event.subject(); | 106 | Device device = event.subject(); |
| 98 | Port port = event.port(); | 107 | Port port = event.port(); |
| 108 | + if (device == null) { | ||
| 109 | + log.error("Device is null."); | ||
| 110 | + return; | ||
| 111 | + } | ||
| 99 | switch (event.type()) { | 112 | switch (event.type()) { |
| 100 | case DEVICE_ADDED: | 113 | case DEVICE_ADDED: |
| 101 | discoverers.put(device.id(), | 114 | discoverers.put(device.id(), |
| ... | @@ -144,6 +157,11 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider { | ... | @@ -144,6 +157,11 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider { |
| 144 | break; | 157 | break; |
| 145 | case DEVICE_UPDATED: | 158 | case DEVICE_UPDATED: |
| 146 | case DEVICE_MASTERSHIP_CHANGED: | 159 | case DEVICE_MASTERSHIP_CHANGED: |
| 160 | + if (!discoverers.containsKey(device.id())) { | ||
| 161 | + discoverers.put(device.id(), | ||
| 162 | + new LinkDiscovery(device, packetSevice, masterService, | ||
| 163 | + providerService, useBDDP)); | ||
| 164 | + } | ||
| 147 | break; | 165 | break; |
| 148 | default: | 166 | default: |
| 149 | log.debug("Unknown event {}", event); | 167 | log.debug("Unknown event {}", event); | ... | ... |
| ... | @@ -257,10 +257,10 @@ public class LinkDiscovery implements TimerTask { | ... | @@ -257,10 +257,10 @@ public class LinkDiscovery implements TimerTask { |
| 257 | sendProbes(portNumber); | 257 | sendProbes(portNumber); |
| 258 | } else { | 258 | } else { |
| 259 | // Update fast and slow ports | 259 | // Update fast and slow ports |
| 260 | - //fastIterator.remove(); | 260 | + fastIterator.remove(); |
| 261 | - //this.slowPorts.add(portNumber); | 261 | + this.slowPorts.add(portNumber); |
| 262 | - //this.portProbeCount.remove(portNumber); | 262 | + this.portProbeCount.remove(portNumber); |
| 263 | - this.portProbeCount.get(portNumber).set(0); | 263 | + |
| 264 | 264 | ||
| 265 | ConnectPoint cp = new ConnectPoint( | 265 | ConnectPoint cp = new ConnectPoint( |
| 266 | device.id(), | 266 | device.id(), | ... | ... |
| ... | @@ -134,7 +134,7 @@ public class LLDPLinkProviderTest { | ... | @@ -134,7 +134,7 @@ public class LLDPLinkProviderTest { |
| 134 | deviceListener.event(portEvent(DeviceEvent.Type.PORT_ADDED, DID2, port(DID2, 1, false))); | 134 | deviceListener.event(portEvent(DeviceEvent.Type.PORT_ADDED, DID2, port(DID2, 1, false))); |
| 135 | 135 | ||
| 136 | 136 | ||
| 137 | - assertNull("DPID exists", | 137 | + assertNull("DeviceId exists", |
| 138 | provider.discoverers.get(DID2)); | 138 | provider.discoverers.get(DID2)); |
| 139 | } | 139 | } |
| 140 | 140 | ||
| ... | @@ -394,7 +394,7 @@ public class LLDPLinkProviderTest { | ... | @@ -394,7 +394,7 @@ public class LLDPLinkProviderTest { |
| 394 | 394 | ||
| 395 | @Override | 395 | @Override |
| 396 | public Iterable<Device> getDevices() { | 396 | public Iterable<Device> getDevices() { |
| 397 | - return devices.values(); | 397 | + return Collections.EMPTY_LIST; |
| 398 | } | 398 | } |
| 399 | 399 | ||
| 400 | @Override | 400 | @Override | ... | ... |
providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/OpenFlowRuleProvider.java
| ... | @@ -205,7 +205,7 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr | ... | @@ -205,7 +205,7 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr |
| 205 | pendingFMs.put(xid, installation); | 205 | pendingFMs.put(xid, installation); |
| 206 | } | 206 | } |
| 207 | pendingFutures.put(U32.f(batch.hashCode()), installation); | 207 | pendingFutures.put(U32.f(batch.hashCode()), installation); |
| 208 | - installation.verify(batch.hashCode()); | 208 | + installation.verify(U32.f(batch.hashCode())); |
| 209 | return installation; | 209 | return installation; |
| 210 | } | 210 | } |
| 211 | 211 | ||
| ... | @@ -226,7 +226,10 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr | ... | @@ -226,7 +226,10 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr |
| 226 | 226 | ||
| 227 | @Override | 227 | @Override |
| 228 | public void switchRemoved(Dpid dpid) { | 228 | public void switchRemoved(Dpid dpid) { |
| 229 | - collectors.remove(dpid).stop(); | 229 | + FlowStatsCollector collector = collectors.remove(dpid); |
| 230 | + if (collector != null) { | ||
| 231 | + collector.stop(); | ||
| 232 | + } | ||
| 230 | } | 233 | } |
| 231 | 234 | ||
| 232 | @Override | 235 | @Override |
| ... | @@ -321,7 +324,7 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr | ... | @@ -321,7 +324,7 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr |
| 321 | private final List<FlowEntry> offendingFlowMods = Lists.newLinkedList(); | 324 | private final List<FlowEntry> offendingFlowMods = Lists.newLinkedList(); |
| 322 | 325 | ||
| 323 | private final CountDownLatch countDownLatch; | 326 | private final CountDownLatch countDownLatch; |
| 324 | - private Integer pendingXid; | 327 | + private Long pendingXid; |
| 325 | private BatchState state; | 328 | private BatchState state; |
| 326 | 329 | ||
| 327 | public InstallationFuture(Set<Dpid> sws, Map<Long, FlowRuleBatchEntry> fmXids) { | 330 | public InstallationFuture(Set<Dpid> sws, Map<Long, FlowRuleBatchEntry> fmXids) { |
| ... | @@ -391,7 +394,7 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr | ... | @@ -391,7 +394,7 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr |
| 391 | } | 394 | } |
| 392 | 395 | ||
| 393 | 396 | ||
| 394 | - public void verify(Integer id) { | 397 | + public void verify(Long id) { |
| 395 | pendingXid = id; | 398 | pendingXid = id; |
| 396 | for (Dpid dpid : sws) { | 399 | for (Dpid dpid : sws) { |
| 397 | OpenFlowSwitch sw = controller.getSwitch(dpid); | 400 | OpenFlowSwitch sw = controller.getSwitch(dpid); |
| ... | @@ -449,7 +452,9 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr | ... | @@ -449,7 +452,9 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr |
| 449 | 452 | ||
| 450 | private void cleanUp() { | 453 | private void cleanUp() { |
| 451 | if (isDone() || isCancelled()) { | 454 | if (isDone() || isCancelled()) { |
| 452 | - pendingFutures.remove(pendingXid); | 455 | + if (pendingXid != null) { |
| 456 | + pendingFutures.remove(pendingXid); | ||
| 457 | + } | ||
| 453 | for (Long xid : fms.keySet()) { | 458 | for (Long xid : fms.keySet()) { |
| 454 | pendingFMs.remove(xid); | 459 | pendingFMs.remove(xid); |
| 455 | } | 460 | } | ... | ... |
-
Please register or login to post a comment