Added javadocs and simplified the reactive fwd app in attempt to find the glitch; no dice

@@ -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<Path> 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<Path> 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();
     }
+
 }
 
+

-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<Path> 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();
-        }
-    }
-
-}

+/**
+ * Trivial application that provides simple form of reactive forwarding.
+ */
+package org.onlab.onos.fwd;
\ No newline at end of file

@@ -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));
+    }
+
 }

@@ -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<Integer, PacketProcessor> 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);
     }
 

@@ -366,7 +366,7 @@
                         <group>
                             <title>Sample Applications</title>
                             <packages>
-                                org.onlab.onos.tvue
+                                org.onlab.onos.tvue:org.onlab.onos.fwd
                             </packages>
                         </group>
                     </groups>

@@ -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;