LinkStore bugfix. avoid DIRECT -> INDIRECT transition
Change-Id: If2a4c3e5e33f705a73374010cd7941167cef1aaf
Showing
2 changed files
with
23 additions
and
5 deletions
| ... | @@ -23,9 +23,9 @@ import org.onlab.onos.net.DefaultAnnotations; | ... | @@ -23,9 +23,9 @@ import org.onlab.onos.net.DefaultAnnotations; |
| 23 | import org.onlab.onos.net.DefaultLink; | 23 | import org.onlab.onos.net.DefaultLink; |
| 24 | import org.onlab.onos.net.DeviceId; | 24 | import org.onlab.onos.net.DeviceId; |
| 25 | import org.onlab.onos.net.Link; | 25 | import org.onlab.onos.net.Link; |
| 26 | -import org.onlab.onos.net.SparseAnnotations; | ||
| 27 | import org.onlab.onos.net.Link.Type; | 26 | import org.onlab.onos.net.Link.Type; |
| 28 | import org.onlab.onos.net.LinkKey; | 27 | import org.onlab.onos.net.LinkKey; |
| 28 | +import org.onlab.onos.net.SparseAnnotations; | ||
| 29 | import org.onlab.onos.net.device.DeviceClockService; | 29 | import org.onlab.onos.net.device.DeviceClockService; |
| 30 | import org.onlab.onos.net.link.DefaultLinkDescription; | 30 | import org.onlab.onos.net.link.DefaultLinkDescription; |
| 31 | import org.onlab.onos.net.link.LinkDescription; | 31 | import org.onlab.onos.net.link.LinkDescription; |
| ... | @@ -263,8 +263,8 @@ public class GossipLinkStore | ... | @@ -263,8 +263,8 @@ public class GossipLinkStore |
| 263 | ProviderId providerId, | 263 | ProviderId providerId, |
| 264 | Timestamped<LinkDescription> linkDescription) { | 264 | Timestamped<LinkDescription> linkDescription) { |
| 265 | 265 | ||
| 266 | - LinkKey key = linkKey(linkDescription.value().src(), | 266 | + final LinkKey key = linkKey(linkDescription.value().src(), |
| 267 | - linkDescription.value().dst()); | 267 | + linkDescription.value().dst()); |
| 268 | Map<ProviderId, Timestamped<LinkDescription>> descs = getOrCreateLinkDescriptions(key); | 268 | Map<ProviderId, Timestamped<LinkDescription>> descs = getOrCreateLinkDescriptions(key); |
| 269 | 269 | ||
| 270 | synchronized (descs) { | 270 | synchronized (descs) { |
| ... | @@ -275,6 +275,7 @@ public class GossipLinkStore | ... | @@ -275,6 +275,7 @@ public class GossipLinkStore |
| 275 | if (linkDescription.isNewer(linkRemovedTimestamp)) { | 275 | if (linkDescription.isNewer(linkRemovedTimestamp)) { |
| 276 | removedLinks.remove(key); | 276 | removedLinks.remove(key); |
| 277 | } else { | 277 | } else { |
| 278 | + log.trace("Link {} was already removed ignoring.", key); | ||
| 278 | return null; | 279 | return null; |
| 279 | } | 280 | } |
| 280 | } | 281 | } |
| ... | @@ -299,17 +300,25 @@ public class GossipLinkStore | ... | @@ -299,17 +300,25 @@ public class GossipLinkStore |
| 299 | // merge existing annotations | 300 | // merge existing annotations |
| 300 | Timestamped<LinkDescription> existingLinkDescription = descs.get(providerId); | 301 | Timestamped<LinkDescription> existingLinkDescription = descs.get(providerId); |
| 301 | if (existingLinkDescription != null && existingLinkDescription.isNewer(linkDescription)) { | 302 | if (existingLinkDescription != null && existingLinkDescription.isNewer(linkDescription)) { |
| 303 | + log.trace("local info is more up-to-date, ignoring {}.", linkDescription); | ||
| 302 | return null; | 304 | return null; |
| 303 | } | 305 | } |
| 304 | Timestamped<LinkDescription> newLinkDescription = linkDescription; | 306 | Timestamped<LinkDescription> newLinkDescription = linkDescription; |
| 305 | if (existingLinkDescription != null) { | 307 | if (existingLinkDescription != null) { |
| 308 | + // we only allow transition from INDIRECT -> DIRECT | ||
| 309 | + final Type newType; | ||
| 310 | + if (existingLinkDescription.value().type() == DIRECT) { | ||
| 311 | + newType = DIRECT; | ||
| 312 | + } else { | ||
| 313 | + newType = linkDescription.value().type(); | ||
| 314 | + } | ||
| 306 | SparseAnnotations merged = union(existingLinkDescription.value().annotations(), | 315 | SparseAnnotations merged = union(existingLinkDescription.value().annotations(), |
| 307 | linkDescription.value().annotations()); | 316 | linkDescription.value().annotations()); |
| 308 | newLinkDescription = new Timestamped<LinkDescription>( | 317 | newLinkDescription = new Timestamped<LinkDescription>( |
| 309 | new DefaultLinkDescription( | 318 | new DefaultLinkDescription( |
| 310 | linkDescription.value().src(), | 319 | linkDescription.value().src(), |
| 311 | linkDescription.value().dst(), | 320 | linkDescription.value().dst(), |
| 312 | - linkDescription.value().type(), merged), | 321 | + newType, merged), |
| 313 | linkDescription.timestamp()); | 322 | linkDescription.timestamp()); |
| 314 | } | 323 | } |
| 315 | return descs.put(providerId, newLinkDescription); | 324 | return descs.put(providerId, newLinkDescription); |
| ... | @@ -343,6 +352,8 @@ public class GossipLinkStore | ... | @@ -343,6 +352,8 @@ public class GossipLinkStore |
| 343 | return null; | 352 | return null; |
| 344 | } | 353 | } |
| 345 | 354 | ||
| 355 | + // Note: INDIRECT -> DIRECT transition only | ||
| 356 | + // so that BDDP discovered Link will not overwrite LDDP Link | ||
| 346 | if ((oldLink.type() == INDIRECT && newLink.type() == DIRECT) || | 357 | if ((oldLink.type() == INDIRECT && newLink.type() == DIRECT) || |
| 347 | !AnnotationsUtil.isEqual(oldLink.annotations(), newLink.annotations())) { | 358 | !AnnotationsUtil.isEqual(oldLink.annotations(), newLink.annotations())) { |
| 348 | 359 | ... | ... |
| ... | @@ -172,12 +172,19 @@ public class SimpleLinkStore | ... | @@ -172,12 +172,19 @@ public class SimpleLinkStore |
| 172 | LinkDescription oldDesc = descs.get(providerId); | 172 | LinkDescription oldDesc = descs.get(providerId); |
| 173 | LinkDescription newDesc = linkDescription; | 173 | LinkDescription newDesc = linkDescription; |
| 174 | if (oldDesc != null) { | 174 | if (oldDesc != null) { |
| 175 | + // we only allow transition from INDIRECT -> DIRECT | ||
| 176 | + final Type newType; | ||
| 177 | + if (oldDesc.type() == DIRECT) { | ||
| 178 | + newType = DIRECT; | ||
| 179 | + } else { | ||
| 180 | + newType = linkDescription.type(); | ||
| 181 | + } | ||
| 175 | SparseAnnotations merged = union(oldDesc.annotations(), | 182 | SparseAnnotations merged = union(oldDesc.annotations(), |
| 176 | linkDescription.annotations()); | 183 | linkDescription.annotations()); |
| 177 | newDesc = new DefaultLinkDescription( | 184 | newDesc = new DefaultLinkDescription( |
| 178 | linkDescription.src(), | 185 | linkDescription.src(), |
| 179 | linkDescription.dst(), | 186 | linkDescription.dst(), |
| 180 | - linkDescription.type(), merged); | 187 | + newType, merged); |
| 181 | } | 188 | } |
| 182 | return descs.put(providerId, newDesc); | 189 | return descs.put(providerId, newDesc); |
| 183 | } | 190 | } | ... | ... |
-
Please register or login to post a comment