Committed by
Gerrit Code Review
Simplify MP2S compiler, fix ingress/egress on same switch bug in SP2M compiler
Change-Id: Ib47bba01aac0f358f2caa5525c18e0e90a0b13ff
Showing
4 changed files
with
37 additions
and
55 deletions
| ... | @@ -27,7 +27,6 @@ import org.onosproject.core.CoreService; | ... | @@ -27,7 +27,6 @@ import org.onosproject.core.CoreService; |
| 27 | import org.onosproject.core.DefaultGroupId; | 27 | import org.onosproject.core.DefaultGroupId; |
| 28 | import org.onosproject.net.ConnectPoint; | 28 | import org.onosproject.net.ConnectPoint; |
| 29 | import org.onosproject.net.DeviceId; | 29 | import org.onosproject.net.DeviceId; |
| 30 | -import org.onosproject.net.EdgeLink; | ||
| 31 | import org.onosproject.net.Link; | 30 | import org.onosproject.net.Link; |
| 32 | import org.onosproject.net.PortNumber; | 31 | import org.onosproject.net.PortNumber; |
| 33 | import org.onosproject.net.flow.DefaultFlowRule; | 32 | import org.onosproject.net.flow.DefaultFlowRule; |
| ... | @@ -78,18 +77,8 @@ public class LinkCollectionIntentCompiler implements IntentCompiler<LinkCollecti | ... | @@ -78,18 +77,8 @@ public class LinkCollectionIntentCompiler implements IntentCompiler<LinkCollecti |
| 78 | SetMultimap<DeviceId, PortNumber> outputPorts = HashMultimap.create(); | 77 | SetMultimap<DeviceId, PortNumber> outputPorts = HashMultimap.create(); |
| 79 | 78 | ||
| 80 | for (Link link : intent.links()) { | 79 | for (Link link : intent.links()) { |
| 81 | - DeviceId srcDeviceId; | 80 | + inputPorts.put(link.dst().deviceId(), link.dst().port()); |
| 82 | - DeviceId dstDeviceId; | 81 | + outputPorts.put(link.src().deviceId(), link.src().port()); |
| 83 | - | ||
| 84 | - if (link instanceof EdgeLink) { | ||
| 85 | - EdgeLink edgeLink = (EdgeLink) link; | ||
| 86 | - dstDeviceId = edgeLink.hostLocation().deviceId(); | ||
| 87 | - srcDeviceId = dstDeviceId; | ||
| 88 | - } else { | ||
| 89 | - inputPorts.put(link.dst().deviceId(), link.dst().port()); | ||
| 90 | - srcDeviceId = link.src().deviceId(); | ||
| 91 | - } | ||
| 92 | - outputPorts.put(srcDeviceId, link.src().port()); | ||
| 93 | } | 82 | } |
| 94 | 83 | ||
| 95 | for (ConnectPoint ingressPoint : intent.ingressPoints()) { | 84 | for (ConnectPoint ingressPoint : intent.ingressPoints()) { | ... | ... |
| ... | @@ -15,12 +15,8 @@ | ... | @@ -15,12 +15,8 @@ |
| 15 | */ | 15 | */ |
| 16 | package org.onosproject.net.intent.impl.compiler; | 16 | package org.onosproject.net.intent.impl.compiler; |
| 17 | 17 | ||
| 18 | -import java.util.Collections; | 18 | +import com.google.common.collect.ImmutableSet; |
| 19 | -import java.util.HashMap; | 19 | +import com.google.common.collect.Sets; |
| 20 | -import java.util.List; | ||
| 21 | -import java.util.Map; | ||
| 22 | -import java.util.Set; | ||
| 23 | - | ||
| 24 | import org.apache.felix.scr.annotations.Activate; | 20 | import org.apache.felix.scr.annotations.Activate; |
| 25 | import org.apache.felix.scr.annotations.Component; | 21 | import org.apache.felix.scr.annotations.Component; |
| 26 | import org.apache.felix.scr.annotations.Deactivate; | 22 | import org.apache.felix.scr.annotations.Deactivate; |
| ... | @@ -40,10 +36,11 @@ import org.onosproject.net.intent.impl.PathNotFoundException; | ... | @@ -40,10 +36,11 @@ import org.onosproject.net.intent.impl.PathNotFoundException; |
| 40 | import org.onosproject.net.resource.link.LinkResourceAllocations; | 36 | import org.onosproject.net.resource.link.LinkResourceAllocations; |
| 41 | import org.onosproject.net.topology.PathService; | 37 | import org.onosproject.net.topology.PathService; |
| 42 | 38 | ||
| 43 | -import com.google.common.collect.ImmutableSet; | 39 | +import java.util.Collections; |
| 44 | -import com.google.common.collect.Sets; | 40 | +import java.util.HashMap; |
| 45 | - | 41 | +import java.util.List; |
| 46 | -import static org.onosproject.net.DefaultEdgeLink.createEdgeLink; | 42 | +import java.util.Map; |
| 43 | +import java.util.Set; | ||
| 47 | 44 | ||
| 48 | /** | 45 | /** |
| 49 | * An intent compiler for | 46 | * An intent compiler for |
| ... | @@ -73,37 +70,32 @@ public class MultiPointToSinglePointIntentCompiler | ... | @@ -73,37 +70,32 @@ public class MultiPointToSinglePointIntentCompiler |
| 73 | public List<Intent> compile(MultiPointToSinglePointIntent intent, List<Intent> installable, | 70 | public List<Intent> compile(MultiPointToSinglePointIntent intent, List<Intent> installable, |
| 74 | Set<LinkResourceAllocations> resources) { | 71 | Set<LinkResourceAllocations> resources) { |
| 75 | Map<DeviceId, Link> links = new HashMap<>(); | 72 | Map<DeviceId, Link> links = new HashMap<>(); |
| 76 | - Map<DeviceId, Link> edgeLinks = new HashMap<>(); | ||
| 77 | ConnectPoint egressPoint = intent.egressPoint(); | 73 | ConnectPoint egressPoint = intent.egressPoint(); |
| 78 | 74 | ||
| 79 | for (ConnectPoint ingressPoint : intent.ingressPoints()) { | 75 | for (ConnectPoint ingressPoint : intent.ingressPoints()) { |
| 80 | if (ingressPoint.deviceId().equals(egressPoint.deviceId())) { | 76 | if (ingressPoint.deviceId().equals(egressPoint.deviceId())) { |
| 81 | - edgeLinks.put(ingressPoint.deviceId(), createEdgeLink(ingressPoint, true)); | 77 | + continue; |
| 82 | - edgeLinks.put(egressPoint.deviceId(), createEdgeLink(egressPoint, false)); | 78 | + } |
| 83 | - } else { | 79 | + Path path = getPath(ingressPoint, intent.egressPoint()); |
| 84 | - Path path = getPath(ingressPoint, intent.egressPoint()); | 80 | + for (Link link : path.links()) { |
| 85 | - for (Link link : path.links()) { | 81 | + if (links.containsKey(link.src().deviceId())) { |
| 86 | - if (links.containsKey(link.src().deviceId())) { | 82 | + // We've already reached the existing tree with the first |
| 87 | - // We've already reached the existing tree with the first | 83 | + // part of this path. Add the merging point with different |
| 88 | - // part of this path. Add the merging point with different | 84 | + // incoming port, but don't add the remainder of the path |
| 89 | - // incoming port, but don't add the remainder of the path | 85 | + // in case it differs from the path we already have. |
| 90 | - // in case it differs from the path we already have. | ||
| 91 | - links.put(link.src().deviceId(), link); | ||
| 92 | - break; | ||
| 93 | - } | ||
| 94 | - | ||
| 95 | links.put(link.src().deviceId(), link); | 86 | links.put(link.src().deviceId(), link); |
| 87 | + break; | ||
| 96 | } | 88 | } |
| 89 | + | ||
| 90 | + links.put(link.src().deviceId(), link); | ||
| 97 | } | 91 | } |
| 98 | } | 92 | } |
| 99 | 93 | ||
| 100 | - Set<Link> allLinks = Sets.newHashSet(links.values()); | ||
| 101 | - allLinks.addAll(edgeLinks.values()); | ||
| 102 | Intent result = LinkCollectionIntent.builder() | 94 | Intent result = LinkCollectionIntent.builder() |
| 103 | .appId(intent.appId()) | 95 | .appId(intent.appId()) |
| 104 | .selector(intent.selector()) | 96 | .selector(intent.selector()) |
| 105 | .treatment(intent.treatment()) | 97 | .treatment(intent.treatment()) |
| 106 | - .links(Sets.newHashSet(allLinks)) | 98 | + .links(Sets.newHashSet(links.values())) |
| 107 | .ingressPoints(intent.ingressPoints()) | 99 | .ingressPoints(intent.ingressPoints()) |
| 108 | .egressPoints(ImmutableSet.of(intent.egressPoint())) | 100 | .egressPoints(ImmutableSet.of(intent.egressPoint())) |
| 109 | .priority(intent.priority()) | 101 | .priority(intent.priority()) | ... | ... |
| ... | @@ -15,11 +15,7 @@ | ... | @@ -15,11 +15,7 @@ |
| 15 | */ | 15 | */ |
| 16 | package org.onosproject.net.intent.impl.compiler; | 16 | package org.onosproject.net.intent.impl.compiler; |
| 17 | 17 | ||
| 18 | -import java.util.Collections; | 18 | +import com.google.common.collect.ImmutableSet; |
| 19 | -import java.util.HashSet; | ||
| 20 | -import java.util.List; | ||
| 21 | -import java.util.Set; | ||
| 22 | - | ||
| 23 | import org.apache.felix.scr.annotations.Activate; | 19 | import org.apache.felix.scr.annotations.Activate; |
| 24 | import org.apache.felix.scr.annotations.Component; | 20 | import org.apache.felix.scr.annotations.Component; |
| 25 | import org.apache.felix.scr.annotations.Deactivate; | 21 | import org.apache.felix.scr.annotations.Deactivate; |
| ... | @@ -32,7 +28,10 @@ import org.onosproject.net.intent.SinglePointToMultiPointIntent; | ... | @@ -32,7 +28,10 @@ import org.onosproject.net.intent.SinglePointToMultiPointIntent; |
| 32 | import org.onosproject.net.provider.ProviderId; | 28 | import org.onosproject.net.provider.ProviderId; |
| 33 | import org.onosproject.net.resource.link.LinkResourceAllocations; | 29 | import org.onosproject.net.resource.link.LinkResourceAllocations; |
| 34 | 30 | ||
| 35 | -import com.google.common.collect.ImmutableSet; | 31 | +import java.util.Collections; |
| 32 | +import java.util.HashSet; | ||
| 33 | +import java.util.List; | ||
| 34 | +import java.util.Set; | ||
| 36 | 35 | ||
| 37 | @Component(immediate = true) | 36 | @Component(immediate = true) |
| 38 | public class SinglePointToMultiPointIntentCompiler | 37 | public class SinglePointToMultiPointIntentCompiler |
| ... | @@ -59,8 +58,12 @@ public class SinglePointToMultiPointIntentCompiler | ... | @@ -59,8 +58,12 @@ public class SinglePointToMultiPointIntentCompiler |
| 59 | List<Intent> installable, | 58 | List<Intent> installable, |
| 60 | Set<LinkResourceAllocations> resources) { | 59 | Set<LinkResourceAllocations> resources) { |
| 61 | Set<Link> links = new HashSet<>(); | 60 | Set<Link> links = new HashSet<>(); |
| 62 | - //FIXME: need to handle the case where ingress/egress points are on same switch | 61 | + |
| 63 | for (ConnectPoint egressPoint : intent.egressPoints()) { | 62 | for (ConnectPoint egressPoint : intent.egressPoints()) { |
| 63 | + if (egressPoint.deviceId().equals(intent.ingressPoint().deviceId())) { | ||
| 64 | + continue; | ||
| 65 | + } | ||
| 66 | + | ||
| 64 | Path path = getPath(intent, intent.ingressPoint().deviceId(), egressPoint.deviceId()); | 67 | Path path = getPath(intent, intent.ingressPoint().deviceId(), egressPoint.deviceId()); |
| 65 | links.addAll(path.links()); | 68 | links.addAll(path.links()); |
| 66 | } | 69 | } | ... | ... |
| ... | @@ -15,10 +15,6 @@ | ... | @@ -15,10 +15,6 @@ |
| 15 | */ | 15 | */ |
| 16 | package org.onosproject.net.intent.impl.compiler; | 16 | package org.onosproject.net.intent.impl.compiler; |
| 17 | 17 | ||
| 18 | -import java.util.HashSet; | ||
| 19 | -import java.util.List; | ||
| 20 | -import java.util.Set; | ||
| 21 | - | ||
| 22 | import org.hamcrest.Matchers; | 18 | import org.hamcrest.Matchers; |
| 23 | import org.junit.Test; | 19 | import org.junit.Test; |
| 24 | import org.onosproject.TestApplicationId; | 20 | import org.onosproject.TestApplicationId; |
| ... | @@ -36,6 +32,10 @@ import org.onosproject.net.intent.MultiPointToSinglePointIntent; | ... | @@ -36,6 +32,10 @@ import org.onosproject.net.intent.MultiPointToSinglePointIntent; |
| 36 | import org.onosproject.net.topology.LinkWeight; | 32 | import org.onosproject.net.topology.LinkWeight; |
| 37 | import org.onosproject.net.topology.PathService; | 33 | import org.onosproject.net.topology.PathService; |
| 38 | 34 | ||
| 35 | +import java.util.HashSet; | ||
| 36 | +import java.util.List; | ||
| 37 | +import java.util.Set; | ||
| 38 | + | ||
| 39 | import static org.hamcrest.CoreMatchers.instanceOf; | 39 | import static org.hamcrest.CoreMatchers.instanceOf; |
| 40 | import static org.hamcrest.CoreMatchers.notNullValue; | 40 | import static org.hamcrest.CoreMatchers.notNullValue; |
| 41 | import static org.hamcrest.MatcherAssert.assertThat; | 41 | import static org.hamcrest.MatcherAssert.assertThat; |
| ... | @@ -255,11 +255,9 @@ public class MultiPointToSinglePointIntentCompilerTest extends AbstractIntentTes | ... | @@ -255,11 +255,9 @@ public class MultiPointToSinglePointIntentCompilerTest extends AbstractIntentTes |
| 255 | 255 | ||
| 256 | if (resultIntent instanceof LinkCollectionIntent) { | 256 | if (resultIntent instanceof LinkCollectionIntent) { |
| 257 | LinkCollectionIntent linkIntent = (LinkCollectionIntent) resultIntent; | 257 | LinkCollectionIntent linkIntent = (LinkCollectionIntent) resultIntent; |
| 258 | - assertThat(linkIntent.links(), hasSize(ingress.length + 1)); | 258 | + assertThat(linkIntent.links(), hasSize(ingress.length)); |
| 259 | 259 | ||
| 260 | assertThat(linkIntent.links(), linksHasPath("i2", "i1")); | 260 | assertThat(linkIntent.links(), linksHasPath("i2", "i1")); |
| 261 | - | ||
| 262 | - assertThat(linkIntent.links(), linksHasPath("i1", "i1")); | ||
| 263 | } | 261 | } |
| 264 | } | 262 | } |
| 265 | } | 263 | } | ... | ... |
-
Please register or login to post a comment