distributed link fixes
Change-Id: Iefede001a76834599a5629d843a4325283e42711
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