From c370ebddf78c194232a8e486dd16a06e1388fc9b Mon Sep 17 00:00:00 2001 From: tom Date: Tue, 16 Sep 2014 01:25:21 -0700 Subject: [PATCH] Added javadocs and simplified the reactive fwd app in attempt to find the glitch; no dice --- .../onlab/onos/fwd/ReactiveForwarding.java | 104 +++++++++++++++++- .../onos/fwd/ReactivePacketProcessor.java | 74 ------------- .../java/org/onlab/onos/fwd/package-info.java | 4 + .../main/java/org/onlab/onos/net/HostId.java | 11 +- .../packet/impl/SimplePacketManager.java | 8 +- pom.xml | 2 +- .../main/java/org/onlab/packet/VLANID.java | 1 + 7 files changed, 120 insertions(+), 84 deletions(-) delete mode 100644 apps/fwd/src/main/java/org/onlab/onos/fwd/ReactivePacketProcessor.java create mode 100644 apps/fwd/src/main/java/org/onlab/onos/fwd/package-info.java diff --git a/apps/fwd/src/main/java/org/onlab/onos/fwd/ReactiveForwarding.java b/apps/fwd/src/main/java/org/onlab/onos/fwd/ReactiveForwarding.java index 9cb36e0cc7..98c14f506b 100644 --- a/apps/fwd/src/main/java/org/onlab/onos/fwd/ReactiveForwarding.java +++ b/apps/fwd/src/main/java/org/onlab/onos/fwd/ReactiveForwarding.java @@ -5,14 +5,31 @@ import org.apache.felix.scr.annotations.Component; import org.apache.felix.scr.annotations.Deactivate; import org.apache.felix.scr.annotations.Reference; import org.apache.felix.scr.annotations.ReferenceCardinality; +import org.onlab.onos.net.Host; +import org.onlab.onos.net.HostId; +import org.onlab.onos.net.Path; +import org.onlab.onos.net.PortNumber; +import org.onlab.onos.net.flow.Instructions; import org.onlab.onos.net.host.HostService; +import org.onlab.onos.net.packet.InboundPacket; +import org.onlab.onos.net.packet.PacketContext; import org.onlab.onos.net.packet.PacketProcessor; import org.onlab.onos.net.packet.PacketService; import org.onlab.onos.net.topology.TopologyService; +import org.slf4j.Logger; -@Component +import java.util.Set; + +import static org.slf4j.LoggerFactory.getLogger; + +/** + * Sample reactive forwarding application. + */ +@Component(immediate = true) public class ReactiveForwarding { + private final Logger log = getLogger(getClass()); + @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) protected TopologyService topologyService; @@ -22,18 +39,99 @@ public class ReactiveForwarding { @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) protected HostService hostService; - private ReactivePacketProcessor processor; + private ReactivePacketProcessor processor = new ReactivePacketProcessor(); @Activate public void activate() { - processor = new ReactivePacketProcessor(topologyService, hostService); packetService.addProcessor(processor, PacketProcessor.ADVISOR_MAX + 1); + log.info("Started"); } @Deactivate public void deactivate() { packetService.removeProcessor(processor); processor = null; + log.info("Stopped"); } + + + /** + * Packet processor responsible for forwarding packets along their paths. + */ + private class ReactivePacketProcessor implements PacketProcessor { + + @Override + public void process(PacketContext context) { + InboundPacket pkt = context.inPacket(); + HostId id = HostId.hostId(pkt.parsed().getDestinationMAC()); + + // Do we know who this is for? If not, flood and bail. + Host dst = hostService.getHost(id); + if (dst == null) { + flood(context); + return; + } + + // Are we on an edge switch that our destination is on? If so, + // simply forward out to the destination and bail. + if (pkt.receivedFrom().deviceId().equals(dst.location().deviceId())) { + forward(context, dst.location().port()); + return; + } + + // Otherwise, get a set of paths that lead from here to the + // destination edge switch. + Set paths = topologyService.getPaths(topologyService.currentTopology(), + context.inPacket().receivedFrom().deviceId(), + dst.location().deviceId()); + if (paths.isEmpty()) { + // If there are no paths, flood and bail. + flood(context); + return; + } + + // Otherwise, pick a path that does not lead back to where we + // came from; if no such path, flood and bail. + Path path = pickForwardPath(paths, pkt.receivedFrom().port()); + if (path == null) { + log.warn("Doh... don't know where to go..."); + flood(context); + return; + } + + // Otherwise forward and be done with it. + forward(context, path.src().port()); + } + } + + // Selects a path from the given set that does not lead back to the + // specified port. + private Path pickForwardPath(Set paths, PortNumber notToPort) { + for (Path path : paths) { + if (!path.src().port().equals(notToPort)) { + return path; + } + } + return null; + } + + // Floods the specified packet. + private void flood(PacketContext context) { + boolean canBcast = topologyService.isBroadcastPoint(topologyService.currentTopology(), + context.inPacket().receivedFrom()); + if (canBcast) { + forward(context, PortNumber.FLOOD); + } else { + context.block(); + } + } + + // Forwards the packet to the specified port. + private void forward(PacketContext context, PortNumber portNumber) { + context.treatmentBuilder().add(Instructions.createOutput(portNumber)); + context.send(); + } + } + diff --git a/apps/fwd/src/main/java/org/onlab/onos/fwd/ReactivePacketProcessor.java b/apps/fwd/src/main/java/org/onlab/onos/fwd/ReactivePacketProcessor.java deleted file mode 100644 index 4ecb129479..0000000000 --- a/apps/fwd/src/main/java/org/onlab/onos/fwd/ReactivePacketProcessor.java +++ /dev/null @@ -1,74 +0,0 @@ -package org.onlab.onos.fwd; - -import static org.slf4j.LoggerFactory.getLogger; - -import java.util.Set; - -import org.onlab.onos.net.Host; -import org.onlab.onos.net.HostId; -import org.onlab.onos.net.Path; -import org.onlab.onos.net.PortNumber; -import org.onlab.onos.net.flow.Instructions; -import org.onlab.onos.net.host.HostService; -import org.onlab.onos.net.packet.InboundPacket; -import org.onlab.onos.net.packet.PacketContext; -import org.onlab.onos.net.packet.PacketProcessor; -import org.onlab.onos.net.topology.TopologyService; -import org.onlab.packet.VLANID; -import org.slf4j.Logger; - -public class ReactivePacketProcessor implements PacketProcessor { - - private final Logger log = getLogger(getClass()); - private final TopologyService topologyService; - private final HostService hostService; - - - public ReactivePacketProcessor(TopologyService topologyService, HostService hostService) { - this.topologyService = topologyService; - this.hostService = hostService; - } - - - @Override - public void process(PacketContext context) { - InboundPacket pkt = context.inPacket(); - HostId id = HostId.hostId(pkt.parsed().getDestinationMAC(), VLANID.vlanId((short) -1)); - Host dst = hostService.getHost(id); - if (dst == null) { - flood(context); - return; - } - - Set p = null; - if (pkt.receivedFrom().deviceId().equals(dst.location().deviceId())) { - context.treatmentBuilder().add(Instructions.createOutput(dst.location().port())); - context.send(); - return; - } else { - p = topologyService.getPaths(topologyService.currentTopology(), - context.inPacket().receivedFrom().deviceId(), dst.location().deviceId()); - } - - if (p.isEmpty()) { - flood(context); - } else { - Path p1 = p.iterator().next(); - context.treatmentBuilder().add(Instructions.createOutput(p1.src().port())); - context.send(); - } - - } - - private void flood(PacketContext context) { - boolean canBcast = topologyService.isBroadcastPoint(topologyService.currentTopology(), - context.inPacket().receivedFrom()); - if (canBcast) { - context.treatmentBuilder().add(Instructions.createOutput(PortNumber.FLOOD)); - context.send(); - } else { - context.block(); - } - } - -} diff --git a/apps/fwd/src/main/java/org/onlab/onos/fwd/package-info.java b/apps/fwd/src/main/java/org/onlab/onos/fwd/package-info.java new file mode 100644 index 0000000000..9624edb093 --- /dev/null +++ b/apps/fwd/src/main/java/org/onlab/onos/fwd/package-info.java @@ -0,0 +1,4 @@ +/** + * Trivial application that provides simple form of reactive forwarding. + */ +package org.onlab.onos.fwd; \ No newline at end of file diff --git a/core/api/src/main/java/org/onlab/onos/net/HostId.java b/core/api/src/main/java/org/onlab/onos/net/HostId.java index c4069549c3..170e2a22a6 100644 --- a/core/api/src/main/java/org/onlab/onos/net/HostId.java +++ b/core/api/src/main/java/org/onlab/onos/net/HostId.java @@ -42,10 +42,19 @@ public final class HostId extends ElementId { * @param vlanId vlan identifier * @return host identifier */ - // FIXME: replace vlanId long with a rich data-type, e.g. VLanId or something like that public static HostId hostId(MACAddress mac, VLANID vlanId) { // FIXME: use more efficient means of encoding return hostId("nic" + ":" + mac + "/" + vlanId); } + /** + * Creates a device id using the supplied MAC and default VLAN. + * + * @param mac mac address + * @return host identifier + */ + public static HostId hostId(MACAddress mac) { + return hostId(mac, VLANID.vlanId(VLANID.UNTAGGED)); + } + } diff --git a/core/trivial/src/main/java/org/onlab/onos/net/trivial/packet/impl/SimplePacketManager.java b/core/trivial/src/main/java/org/onlab/onos/net/trivial/packet/impl/SimplePacketManager.java index 637ada8f7c..53c3fbcf73 100644 --- a/core/trivial/src/main/java/org/onlab/onos/net/trivial/packet/impl/SimplePacketManager.java +++ b/core/trivial/src/main/java/org/onlab/onos/net/trivial/packet/impl/SimplePacketManager.java @@ -25,10 +25,8 @@ import org.onlab.onos.net.provider.AbstractProviderRegistry; import org.onlab.onos.net.provider.AbstractProviderService; import org.slf4j.Logger; - /** * Provides a basic implementation of the packet SB & NB APIs. - * */ @Component(immediate = true) @Service @@ -43,7 +41,6 @@ implements PacketService, PacketProviderRegistry { private final Map processors = new TreeMap<>(); - @Activate public void activate() { log.info("Started"); @@ -62,19 +59,20 @@ implements PacketService, PacketProviderRegistry { @Override public void removeProcessor(PacketProcessor processor) { + checkNotNull(processor, "Processor cannot be null"); processors.values().remove(processor); } @Override public void emit(OutboundPacket packet) { + checkNotNull(packet, "Packet cannot be null"); final Device device = deviceService.getDevice(packet.sendThrough()); final PacketProvider packetProvider = getProvider(device.providerId()); packetProvider.emit(packet); } @Override - protected PacketProviderService createProviderService( - PacketProvider provider) { + protected PacketProviderService createProviderService(PacketProvider provider) { return new InternalPacketProviderService(provider); } diff --git a/pom.xml b/pom.xml index 51f1a5ad91..f461357d55 100644 --- a/pom.xml +++ b/pom.xml @@ -366,7 +366,7 @@ Sample Applications - org.onlab.onos.tvue + org.onlab.onos.tvue:org.onlab.onos.fwd diff --git a/utils/misc/src/main/java/org/onlab/packet/VLANID.java b/utils/misc/src/main/java/org/onlab/packet/VLANID.java index d2a09fa45f..fa59a82170 100644 --- a/utils/misc/src/main/java/org/onlab/packet/VLANID.java +++ b/utils/misc/src/main/java/org/onlab/packet/VLANID.java @@ -3,6 +3,7 @@ package org.onlab.packet; /** * Representation of a VLAN ID. */ +// FIXME: This will end-up looking like a constant; we should name it 'VlanId', 'IpAddress', 'MacAddress'. public class VLANID { private final short value;