Merge branch 'onos-spring' of ssh://gerrit.onlab.us:29418/ONOS into onos-spring
diff --git a/src/main/java/net/onrc/onos/api/flowmanager/PacketPathFlow.java b/src/main/java/net/onrc/onos/api/flowmanager/PacketPathFlow.java
index 34d6776..5377862 100644
--- a/src/main/java/net/onrc/onos/api/flowmanager/PacketPathFlow.java
+++ b/src/main/java/net/onrc/onos/api/flowmanager/PacketPathFlow.java
@@ -3,6 +3,7 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -139,8 +140,11 @@
         List<SwitchPort> portList = new LinkedList<>();
         for (FlowLink link : path) {
             portList.add(link.getDstSwitchPort());
-            actionsList.add(Arrays.asList(
-                    (Action) new OutputAction(link.getSrcPortNumber())));
+            List<Action> l = new ArrayList<Action>();
+            l.add(new OutputAction(link.getSrcPortNumber()));
+            actionsList.add(l);
+            // Arrays.asList(
+            // (Action) new OutputAction(link.getSrcPortNumber())));
         }
 
         // The head switch's ingress port
diff --git a/src/main/java/net/onrc/onos/apps/sdnip/ISdnIpService.java b/src/main/java/net/onrc/onos/apps/sdnip/ISdnIpService.java
index a2abaa1..1e7bd08 100644
--- a/src/main/java/net/onrc/onos/apps/sdnip/ISdnIpService.java
+++ b/src/main/java/net/onrc/onos/apps/sdnip/ISdnIpService.java
@@ -54,4 +54,11 @@
      */
     public void beginRouting();
 
+    /**
+     * Start SDN-IP Routing.
+     * Before intent framework is ready, we need two methods to start the
+     * application.
+     */
+    public void beginRoutingWithNewIntent();
+
 }
diff --git a/src/main/java/net/onrc/onos/apps/sdnip/SdnIp.java b/src/main/java/net/onrc/onos/apps/sdnip/SdnIp.java
index ffa9cfa..dd0c072 100644
--- a/src/main/java/net/onrc/onos/apps/sdnip/SdnIp.java
+++ b/src/main/java/net/onrc/onos/apps/sdnip/SdnIp.java
@@ -26,6 +26,7 @@
 import net.floodlightcontroller.util.MACAddress;
 import net.onrc.onos.api.newintent.IntentService;
 import net.onrc.onos.api.newintent.MultiPointToSinglePointIntent;
+import net.onrc.onos.api.newintent.PointToPointIntent;
 import net.onrc.onos.apps.proxyarp.IArpRequester;
 import net.onrc.onos.apps.proxyarp.IProxyArpService;
 import net.onrc.onos.apps.sdnip.RibUpdate.Operation;
@@ -37,10 +38,12 @@
 import net.onrc.onos.core.intent.runtime.IPathCalcRuntimeService;
 import net.onrc.onos.core.linkdiscovery.ILinkDiscoveryService;
 import net.onrc.onos.core.main.config.IConfigInfoService;
+import net.onrc.onos.core.matchaction.action.Actions;
 import net.onrc.onos.core.matchaction.action.ModifyDstMacAction;
 import net.onrc.onos.core.matchaction.match.PacketMatch;
 import net.onrc.onos.core.matchaction.match.PacketMatchBuilder;
 import net.onrc.onos.core.newintent.IdBlockAllocatorBasedIntentIdGenerator;
+import net.onrc.onos.core.packet.Ethernet;
 import net.onrc.onos.core.registry.IControllerRegistryService;
 import net.onrc.onos.core.util.Dpid;
 import net.onrc.onos.core.util.IPv4;
@@ -100,7 +103,9 @@
     private IdBlockAllocatorBasedIntentIdGenerator intentIdGenerator;
     //private static final short ARP_PRIORITY = 20;
 
-    //private static final short BGP_PORT = 179;
+    private static final short BGP_PORT = 179;
+    public static final byte PROTOCOL_ICMP = 0x1;
+    public static final byte PROTOCOL_TCP = 0x6;
 
     // Configuration stuff
     private List<String> switches;
@@ -604,6 +609,160 @@
         pathRuntime.executeIntentOperations(operations);
     }
 
+
+    /**
+     * Setup paths for BGP Daemon and its peers. Run a loop for all the bgpPeers.
+     * Push intent for path from BGPd to the peer. Push intent for path from peer
+     * to BGPd.
+     */
+    private void setupBgpPathsWithNewIntent() {
+        for (BgpPeer bgpPeer : bgpPeers.values()) {
+
+            log.debug("Start to set up BGP paths.");
+
+            Interface peerInterface = interfaces.get(bgpPeer.getInterfaceName());
+
+            IPv4 bgpdAddress = new IPv4(InetAddresses
+                    .coerceToInteger(peerInterface.getIpAddress()));
+            IPv4 bgpdPeerAddress = new IPv4(InetAddresses
+                    .coerceToInteger(bgpPeer.getIpAddress()));
+
+            SwitchPort bgpdSwitchPort = bgpdAttachmentPoint;
+            SwitchPort bgpdPeerSwitchPort = peerInterface.getSwitchPort();
+
+            // install intent for BGP path from BGPd to BGP peer matching
+            // destination TCP port 179
+
+            // TODO: The usage of PacketMatchBuilder will be improved, then we
+            // only need to new the PacketMatchBuilder once.
+            // By then, the code here will be improved accordingly.
+            PacketMatchBuilder builderMatchDstTcpPort = new PacketMatchBuilder();
+            builderMatchDstTcpPort.setEtherType(Ethernet.TYPE_IPV4);
+            builderMatchDstTcpPort.setIpProto(PROTOCOL_TCP);
+            builderMatchDstTcpPort.setSrcIp(bgpdAddress);
+            builderMatchDstTcpPort.setDstIp(bgpdPeerAddress);
+            builderMatchDstTcpPort.setDstTcpPort(BGP_PORT);
+
+            PacketMatch packetMatch = builderMatchDstTcpPort.build();
+
+            PointToPointIntent intentMatchDstTcpPort = new PointToPointIntent(
+                    intentIdGenerator.getNewId(), packetMatch, Actions
+                    .nullAction(), bgpdSwitchPort, bgpdPeerSwitchPort);
+            intentService.submit(intentMatchDstTcpPort);
+            log.debug("Submitted BGP path intent matching dst TCP port 179 "
+                    + "from BGPd to peer {}: {}",
+                    bgpdPeerAddress, intentMatchDstTcpPort);
+
+            // install intent for BGP path from BGPd to BGP peer matching
+            // source TCP port 179
+            PacketMatchBuilder builderMatchSrcTcpPort = new PacketMatchBuilder();
+            builderMatchSrcTcpPort.setEtherType(Ethernet.TYPE_IPV4);
+            builderMatchSrcTcpPort.setIpProto(PROTOCOL_TCP);
+            builderMatchSrcTcpPort.setSrcIp(bgpdAddress);
+            builderMatchSrcTcpPort.setDstIp(bgpdPeerAddress);
+            builderMatchSrcTcpPort.setSrcTcpPort(BGP_PORT);
+
+            packetMatch = builderMatchSrcTcpPort.build();
+
+            PointToPointIntent intentMatchSrcTcpPort = new PointToPointIntent(
+                    intentIdGenerator.getNewId(), packetMatch, Actions
+                    .nullAction(), bgpdSwitchPort, bgpdPeerSwitchPort);
+            intentService.submit(intentMatchSrcTcpPort);
+            log.debug("Submitted BGP path intent matching src TCP port 179"
+                    + "from BGPd to peer {}: {}",
+                    bgpdPeerAddress, intentMatchSrcTcpPort);
+
+            // install intent for reversed BGP path from BGP peer to BGPd
+            // matching destination TCP port 179
+            PacketMatchBuilder reversedBuilderMatchDstTcpPort = new PacketMatchBuilder();
+            reversedBuilderMatchDstTcpPort.setEtherType(Ethernet.TYPE_IPV4);
+            reversedBuilderMatchDstTcpPort.setIpProto(PROTOCOL_TCP);
+            reversedBuilderMatchDstTcpPort.setSrcIp(bgpdPeerAddress);
+            reversedBuilderMatchDstTcpPort.setDstIp(bgpdAddress);
+            reversedBuilderMatchDstTcpPort.setDstTcpPort(BGP_PORT);
+
+            packetMatch = reversedBuilderMatchDstTcpPort.build();
+
+            PointToPointIntent reversedIntentMatchDstTcpPort = new PointToPointIntent(
+                    intentIdGenerator.getNewId(), packetMatch, Actions
+                    .nullAction(), bgpdPeerSwitchPort, bgpdSwitchPort);
+            intentService.submit(reversedIntentMatchDstTcpPort);
+            log.debug("Submitted BGP path intent matching dst TCP port 179"
+                    + "from BGP peer {} to BGPd: {}",
+                    bgpdPeerAddress, reversedIntentMatchDstTcpPort);
+
+            // install intent for reversed BGP path from BGP peer to BGPd
+            // matching source TCP port 179
+            PacketMatchBuilder reversedBuilderMatchSrcTcpPort = new PacketMatchBuilder();
+            reversedBuilderMatchSrcTcpPort.setEtherType(Ethernet.TYPE_IPV4);
+            reversedBuilderMatchSrcTcpPort.setIpProto(PROTOCOL_TCP);
+            reversedBuilderMatchSrcTcpPort.setSrcIp(bgpdPeerAddress);
+            reversedBuilderMatchSrcTcpPort.setDstIp(bgpdAddress);
+            reversedBuilderMatchSrcTcpPort.setSrcTcpPort(BGP_PORT);
+
+            packetMatch = reversedBuilderMatchSrcTcpPort.build();
+
+            PointToPointIntent reversedIntentMatchSrcTcpPort = new PointToPointIntent(
+                    intentIdGenerator.getNewId(), packetMatch, Actions
+                    .nullAction(), bgpdPeerSwitchPort, bgpdSwitchPort);
+            intentService.submit(reversedIntentMatchSrcTcpPort);
+            log.debug("Submitted BGP path intent matching src TCP port 179"
+                    + "from BGP peer {} to BGPd: {}",
+                    bgpdPeerAddress, reversedIntentMatchSrcTcpPort);
+
+        }
+
+    }
+
+    /**
+     * Setup ICMP paths between BGP Daemon and its peers. Run a loop for all the
+     * bgpPeers. Push intent for path from BGPd to the peer. Push intent for path
+     * from peer to BGPd.
+     */
+    private void setupIcmpPaths() {
+        for (BgpPeer bgpPeer : bgpPeers.values()) {
+
+            Interface peerInterface = interfaces.get(bgpPeer.getInterfaceName());
+
+            PacketMatchBuilder builder = new PacketMatchBuilder();
+            builder.setEtherType(Ethernet.TYPE_IPV4);
+            builder.setIpProto(PROTOCOL_ICMP);
+
+            IPv4 bgpdAddress = new IPv4(InetAddresses
+                    .coerceToInteger(peerInterface.getIpAddress()));
+            IPv4 bgpdPeerAddress = new IPv4(InetAddresses
+                    .coerceToInteger(bgpPeer.getIpAddress()));
+
+            SwitchPort bgpdSwitchPort = bgpdAttachmentPoint;
+            SwitchPort bgpdPeerSwitchPort = peerInterface.getSwitchPort();
+
+            // install intent for ICMP path from BGPd to BGP peer
+            builder.setSrcIp(bgpdAddress);
+            builder.setDstIp(bgpdPeerAddress);
+            PacketMatch packetMatch = builder.build();
+
+            PointToPointIntent intent = new PointToPointIntent(
+                    intentIdGenerator.getNewId(), packetMatch, Actions
+                    .nullAction(), bgpdSwitchPort, bgpdPeerSwitchPort);
+            intentService.submit(intent);
+            log.debug("Submitted ICMP path intent from BGPd to peer {}: {}",
+                    bgpdPeerAddress, intent);
+
+            // install intent for reversed ICMP path from BGP peer to BGPd
+            builder.setSrcIp(bgpdPeerAddress);
+            builder.setDstIp(bgpdAddress);
+            packetMatch = builder.build();
+
+            PointToPointIntent reversedIntent = new PointToPointIntent(
+                    intentIdGenerator.getNewId(), packetMatch, Actions
+                    .nullAction(), bgpdPeerSwitchPort, bgpdSwitchPort);
+            intentService.submit(reversedIntent);
+            log.debug("Submitted ICMP path intent from BGP peer {} to BGPd: {}",
+                    bgpdPeerAddress, reversedIntent);
+        }
+
+    }
+
     /**
      * This method handles the prefixes which are waiting for ARP replies for
      * MAC addresses of next hops.
@@ -761,6 +920,35 @@
     }
 
     /**
+     * The SDN-IP application is started from this method.
+     * Before intent framework is ready, we need two methods to start the
+     * application.
+     */
+    @Override
+    public void beginRoutingWithNewIntent() {
+        log.debug("Topology is now ready, beginning routing function");
+
+        // TODO
+        /*setupArpFlows();
+        setupDefaultDropFlows();*/
+
+        setupBgpPathsWithNewIntent();
+        setupIcmpPaths();
+
+        // Suppress link discovery on external-facing router ports
+        for (Interface intf : interfaces.values()) {
+            linkDiscoveryService.disableDiscoveryOnPort(intf.getDpid(), intf.getPort());
+        }
+
+        bgpUpdatesExecutor.execute(new Runnable() {
+            @Override
+            public void run() {
+                doUpdatesThread();
+            }
+        });
+    }
+
+    /**
      * Thread for handling RIB updates.
      */
     private void doUpdatesThread() {
diff --git a/src/main/java/net/onrc/onos/apps/sdnip/web/SdnIpSetup.java b/src/main/java/net/onrc/onos/apps/sdnip/web/SdnIpSetup.java
index 4bed425..5fb9714 100644
--- a/src/main/java/net/onrc/onos/apps/sdnip/web/SdnIpSetup.java
+++ b/src/main/java/net/onrc/onos/apps/sdnip/web/SdnIpSetup.java
@@ -13,8 +13,17 @@
     public String sdnipSetupMethod() {
         ISdnIpService sdnIp = (ISdnIpService) getContext()
                               .getAttributes().get(ISdnIpService.class.getCanonicalName());
-        sdnIp.beginRouting();
-        return "SdnIp SetupBgpPaths Succeeded";
+        String version = (String) getRequestAttributes().get("version");
+        if (version.equals("new")) {
+            sdnIp.beginRoutingWithNewIntent();
+            return "SdnIp SetupBgpPaths Succeeded with New intent";
+        } else if (version.equals("old")) {
+
+            sdnIp.beginRouting();
+            return "SdnIp SetupBgpPaths Succeeded";
+        }
+
+        return "URL is wrong!";
     }
 
 }
diff --git a/src/main/java/net/onrc/onos/apps/sdnip/web/SdnIpWebRoutableNew.java b/src/main/java/net/onrc/onos/apps/sdnip/web/SdnIpWebRoutableNew.java
index 5b6928a..887f1da 100644
--- a/src/main/java/net/onrc/onos/apps/sdnip/web/SdnIpWebRoutableNew.java
+++ b/src/main/java/net/onrc/onos/apps/sdnip/web/SdnIpWebRoutableNew.java
@@ -15,7 +15,8 @@
     @Override
     public Restlet getRestlet(Context context) {
         Router router = new Router(context);
-        router.attach("/beginRouting/json", SdnIpSetup.class);
+        //router.attach("/beginRouting/json", SdnIpSetup.class);
+        router.attach("/beginRouting/json/{version}", SdnIpSetup.class);
         return router;
     }
 
diff --git a/src/main/java/net/onrc/onos/core/configmanager/INetworkConfigService.java b/src/main/java/net/onrc/onos/core/configmanager/INetworkConfigService.java
index 4090879..174759b 100644
--- a/src/main/java/net/onrc/onos/core/configmanager/INetworkConfigService.java
+++ b/src/main/java/net/onrc/onos/core/configmanager/INetworkConfigService.java
@@ -5,8 +5,8 @@
 import net.floodlightcontroller.core.module.IFloodlightService;
 import net.onrc.onos.core.configmanager.NetworkConfig.LinkConfig;
 import net.onrc.onos.core.configmanager.NetworkConfig.SwitchConfig;
-import net.onrc.onos.core.linkdiscovery.Link;
 import net.onrc.onos.core.util.Dpid;
+import net.onrc.onos.core.util.LinkTuple;
 
 /**
  * Exposes methods to retrieve network configuration.
@@ -187,10 +187,10 @@
      * ConfigState contains the result of the check. The enclosed LinkConfig may
      * or may not be null, depending on the outcome of the check.
      *
-     * @param link unidirectional {@link Link} to be queried
+     * @param linkTuple unidirectional link to be queried
      * @return LinkConfigStatus with outcome of check and associated config.
      */
-    public LinkConfigStatus checkLinkConfig(Link link);
+    public LinkConfigStatus checkLinkConfig(LinkTuple linkTuple);
 
     /**
      * Retrieves a list of switches that have been configured, and have been
diff --git a/src/main/java/net/onrc/onos/core/configmanager/NetworkConfigManager.java b/src/main/java/net/onrc/onos/core/configmanager/NetworkConfigManager.java
index 0c1f933..b1d3836 100644
--- a/src/main/java/net/onrc/onos/core/configmanager/NetworkConfigManager.java
+++ b/src/main/java/net/onrc/onos/core/configmanager/NetworkConfigManager.java
@@ -19,10 +19,9 @@
 import net.floodlightcontroller.core.module.IFloodlightService;
 import net.onrc.onos.core.configmanager.NetworkConfig.LinkConfig;
 import net.onrc.onos.core.configmanager.NetworkConfig.SwitchConfig;
-import net.onrc.onos.core.linkdiscovery.Link;
 import net.onrc.onos.core.util.Dpid;
 import net.onrc.onos.core.util.LinkTuple;
-import net.onrc.onos.core.util.SwitchPort;
+import net.onrc.onos.core.util.PortNumber;
 
 import org.codehaus.jackson.JsonParseException;
 import org.codehaus.jackson.map.JsonMappingException;
@@ -99,17 +98,17 @@
 
     }
 
-    public LinkConfigStatus checkLinkConfig(Link link) {
-        LinkConfig lkc = getConfiguredLink(link);
+    public LinkConfigStatus checkLinkConfig(LinkTuple linkTuple) {
+        LinkConfig lkc = getConfiguredLink(linkTuple);
         // links are always disallowed if any one of the nodes that make up the
         // link are disallowed
-        Dpid linkNode1 = new Dpid(link.getSrc());
+        Dpid linkNode1 = linkTuple.getSrc().getDpid();
         SwitchConfigStatus scs1 = checkSwitchConfig(linkNode1);
         if (scs1.getConfigState() == NetworkConfigState.DENY) {
             return new LinkConfigStatus(NetworkConfigState.DENY, null,
                     "Link-node: " + linkNode1 + " denied by config: " + scs1.getMsg());
         }
-        Dpid linkNode2 = new Dpid(link.getDst());
+        Dpid linkNode2 = linkTuple.getDst().getDpid();
         SwitchConfigStatus scs2 = checkSwitchConfig(linkNode2);
         if (scs2.getConfigState() == NetworkConfigState.DENY) {
             return new LinkConfigStatus(NetworkConfigState.DENY, null,
@@ -349,18 +348,18 @@
 
     }
 
-    private LinkConfig getConfiguredLink(Link link) {
+    private LinkConfig getConfiguredLink(LinkTuple linkTuple) {
         LinkConfig lkc = null;
         // first try the unidirectional link with the ports assigned
-        lkc = configuredLinks.get(new LinkTuple(
-                new SwitchPort(link.getSrc(), link.getSrcPort()),
-                new SwitchPort(link.getDst(), link.getDstPort())));
+        lkc = configuredLinks.get(linkTuple);
         if (lkc == null) {
             // try without ports, as configuration may be for all links
             // between the two switches
-            lkc = configuredLinks.get(new LinkTuple(
-                    new SwitchPort(link.getSrc(), (short) 0),
-                    new SwitchPort(link.getDst(), (short) 0)));
+            LinkTuple portlessLink = new LinkTuple(linkTuple.getSrc().getDpid(),
+                    PortNumber.uint32(0), linkTuple.getDst().getDpid(),
+                    PortNumber.uint32(0));
+
+            lkc = configuredLinks.get(portlessLink);
         }
         return lkc;
     }
diff --git a/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java b/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java
index 824ed4f..27f1a3c 100644
--- a/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java
+++ b/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java
@@ -739,7 +739,7 @@
                 .setActions(actionList)
                 .setIdleTimeout(0) // hardcoded to zero for now
                 .setHardTimeout(0) // hardcoded to zero for now
-                .setCookie(U64.of(matchAction.getId().value()))
+                .setCookie(U64.of(matchAction.getId().getValue()))
                 .setBufferId(OFBufferId.NO_BUFFER)
                 .setPriority(PRIORITY_DEFAULT)
                 .setOutPort(outp);
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchAction.java b/src/main/java/net/onrc/onos/core/matchaction/MatchAction.java
index fdf1fc7..d7d5d73 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchAction.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchAction.java
@@ -42,21 +42,6 @@
     }
 
     /**
-     * Constructor. TEMPORARY
-     *
-     * @param id ID for this MatchAction object
-     * @param port switch port to apply changes to
-     * @param match the Match object as match condition on the port
-     * @param actions the list of Action objects as actions on the switch
-     */
-    public MatchAction(String id, SwitchPort port, Match match, List<Action> actions) {
-        this.id = null;
-        this.port = port;
-        this.match = match;
-        this.actions = actions;
-    }
-
-    /**
      * Gets ID for this object.
      *
      * @return the ID for this object
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchActionComponent.java b/src/main/java/net/onrc/onos/core/matchaction/MatchActionComponent.java
index da24d4d..3ee164f 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchActionComponent.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchActionComponent.java
@@ -40,29 +40,33 @@
 public class MatchActionComponent implements MatchActionService, IFloodlightService {
 
     private static final Logger log = LoggerFactory.getLogger(MatchActionService.class);
-    IFlowPusherService pusher;
-    IFloodlightProviderService provider;
+    private final IFlowPusherService pusher;
+    private final IFloodlightProviderService provider;
 
-    private ConcurrentMap<MatchActionId, MatchAction> matchActionMap = new ConcurrentHashMap<>();
-    private ConcurrentMap<MatchActionOperationsId, MatchActionOperations> matchSetMap =
+    private final ConcurrentMap<MatchActionId, MatchAction> matchActionMap = new ConcurrentHashMap<>();
+    private final ConcurrentMap<MatchActionOperationsId, MatchActionOperations> matchSetMap =
             new ConcurrentHashMap<>();
     //  TODO - want something better here for the resolved Queue
-    private BlockingQueue<MatchActionOperationsId> resolvedQueue = new ArrayBlockingQueue<>(100);
-    private BlockingQueue<MatchActionOperations> installationWorkQueue = new ArrayBlockingQueue<>(100);
+    private final BlockingQueue<MatchActionOperationsId> resolvedQueue = new ArrayBlockingQueue<>(100);
+    private final BlockingQueue<MatchActionOperations> installationWorkQueue = new ArrayBlockingQueue<>(100);
 
-    private IEventChannel<String, MatchActionOperations> installSetChannel;
+    private IEventChannel<Long, MatchActionOperations> installSetChannel;
     private IEventChannel<String, SwitchResultList> installSetReplyChannel;
 
-    // TODO Single instance for now, should be a work queue of some sort eventually
-    private Thread coordinator;
-    private Thread installer;
-    private Installer installerListener;
     private final IDatagridService datagrid;
-    private IControllerRegistryService registryService;
+    private final IControllerRegistryService registryService;
 
     private MatchActionIdGeneratorWithIdBlockAllocator matchActionIdGenerator;
     private MatchActionOperationsIdGeneratorWithIdBlockAllocator matchActionOperationsIdGenerator;
 
+    /**
+     * Constructs a MatchActionComponent given the services it depends on.
+     *
+     * @param newDatagrid datagrid dependency
+     * @param newPusher flow pusher dependency
+     * @param newProvider provider used for switch queries
+     * @param newRegistryService registry for ID block allocation
+     */
     public MatchActionComponent(final IDatagridService newDatagrid,
                                 final IFlowPusherService newPusher,
                                 final IFloodlightProviderService newProvider,
@@ -73,6 +77,10 @@
         registryService = newRegistryService;
     }
 
+    /**
+     * Starts the component.  Created channels used for communication and
+     * creates producer and consumer threads.
+     */
     public void start() {
         IdBlockAllocator idBlockAllocator = registryService;
         matchActionIdGenerator =
@@ -80,24 +88,34 @@
         matchActionOperationsIdGenerator =
                 new MatchActionOperationsIdGeneratorWithIdBlockAllocator(idBlockAllocator);
 
-        installSetChannel = datagrid.createChannel("onos.matchaction.installSetChannel",
-                String.class,
+        final Installer installerListener = new Installer();
+        installSetChannel = datagrid.addListener(
+                "onos.matchaction.installSetChannel",
+                installerListener,
+                Long.class,
                 MatchActionOperations.class);
 
-        installSetReplyChannel = datagrid.createChannel("onos.matchaction.installSetReplyChannel",
+
+        final Coordinator coordinator = new Coordinator();
+        coordinator.start();
+        installSetReplyChannel = datagrid.addListener(
+                "onos.matchaction.installSetReplyChannel",
+                coordinator,
                 String.class,
                 SwitchResultList.class);
 
-        coordinator = new Coordinator();
-        coordinator.start();
-
-        installer = new InstallerWorker();
+        // TODO Single instance for now, should be a work queue of some sort eventually
+        final InstallerWorker installer = new InstallerWorker();
         installer.start();
 
-        installerListener = new Installer();
-        installerListener.start();
     }
 
+    /**
+     * Installs a set of MatchActionOperations.
+     *
+     * @param matchSet the set of MatchActions to install
+     * @return identifier of the installed operations
+     */
     public MatchActionOperationsId installMatchActionOperations(MatchActionOperations matchSet) {
         if (checkResolved(matchSet)) {
             matchSet.setState(MatchActionOperationsState.RESOLVED);
@@ -115,12 +133,24 @@
         return matchSet.getOperationsId();
     }
 
+    /**
+     * Returns the state of a set of operations.
+     *
+     * @param matchSetId identifier of the MatchActionOperations being queried.
+     * @return state of the given operations
+     */
     public MatchActionOperationsState getMatchActionOperationsState(MatchActionOperationsId matchSetId) {
         MatchActionOperations set = matchSetMap.get(matchSetId);
         return (set == null) ? null : set.getState();
     }
 
-    protected boolean checkResolved(MatchActionOperations matchSet) {
+    /**
+     * Checks if a given set of operations has all of its dependencies resolved.
+     *
+     * @param matchSet Operations set to check
+     * @return true if all dependencies are resolved, false otherwise
+     */
+    private boolean checkResolved(MatchActionOperations matchSet) {
         boolean resolved = true;
         for (MatchActionOperationsId setId : matchSet.getDependencies()) {
             MatchActionOperations set = matchSetMap.get(setId);
@@ -132,17 +162,28 @@
         return resolved;
     }
 
-    class Coordinator extends Thread
+    /**
+     * Producer class for MatchActionOperations.  An instance of this runs on
+     * each ONOS node. Requests come in via the resolved queue, and are
+     * distributed to workers running on each ONOS instance via a channel.
+     */
+    private final class Coordinator extends Thread
             implements IEventChannelListener<String, SwitchResultList> {
 
-        private Map<MatchActionOperationsId, Map<Dpid, SwitchResult>> pendingMatchActionOperations = new HashMap<>();
+        private final Map<MatchActionOperationsId,
+                      Map<Dpid, SwitchResult>>
+                pendingMatchActionOperations = new HashMap<>();
 
-        protected Coordinator() {
-            installSetReplyChannel.addListener(this);
+        /**
+         * Default constructor.
+         */
+        Coordinator() {
+            // nothing to initialize
         }
 
         @Override
         public void run() {
+            //noinspection InfiniteLoopStatement - for IntelliJ
             while (true) {
                 // 1. Remove MatchActionOperations(s) from the Global Resolved Queue
                 try {
@@ -154,6 +195,11 @@
             }
         }
 
+        /**
+         * Processes an inbound MatchActionOperations object.
+         *
+         * @param setId Identifier of the MatchActionOperations object
+         */
         private void processSet(MatchActionOperationsId setId) {
             MatchActionOperations matchSet = matchSetMap.get(setId);
             matchSet.setState(MatchActionOperationsState.PENDING);
@@ -185,7 +231,8 @@
             pendingMatchActionOperations.put(setId, switches);
 
             // distribute apply/undo sets to cluster
-            installSetChannel.addTransientEntry(setId.toString(), matchSet);
+            log.trace("MatchAction Coordinator distributing set: {}", matchSet);
+            installSetChannel.addTransientEntry(setId.getId(), matchSet);
         }
 
         @Override
@@ -203,6 +250,11 @@
             updateSwitchResults(value);
         }
 
+        /**
+         * Processes the response from a consumer.
+         *
+         * @param results List of switches modified by the consumer
+         */
         private void updateSwitchResults(SwitchResultList results) {
             if (results == null || results.size() == 0) {
                 return;
@@ -262,11 +314,28 @@
         }
     }
 
-    class InstallerWorker extends Thread {
+    /**
+     * Worker thread that pushes MatchActionOperations to the switches via
+     * the FlowPusher.
+     */
+    private class InstallerWorker extends Thread {
+
+        /**
+         * Default constructor.
+         */
+        InstallerWorker() {
+            // nothing to initialize
+        }
 
         // Note: we should consider using an alternative representation for
         // apply sets
-        protected void install(MatchActionOperations matchSet) {
+
+        /**
+         * Installs a set of MatchActionOperations using the Flow Pusher.
+         *
+         * @param matchSet set of MatchActions to install
+         */
+        private void install(MatchActionOperations matchSet) {
             Set<Long> masterDpids = provider.getAllMasterSwitchDpids();
 
             Set<MatchActionOperationEntry> installSet = new HashSet<>();
@@ -320,6 +389,7 @@
 
         @Override
         public void run() {
+            //noinspection InfiniteLoopStatement - for IntelliJ
             while (true) {
                 // 1. Remove MatchActionOperations(s) from the Global Resolved Queue
                 try {
@@ -332,17 +402,18 @@
         }
     }
 
+    /**
+     * Consumer class for MatchActionOperations.  Listens on the MatchAction
+     * channel and places inbound requests on a queue to be handled by the
+     * InstallerWorker threads.
+     */
     class Installer
-            implements IEventChannelListener<String, MatchActionOperations> {
-
-        protected void start() {
-            installSetChannel.addListener(this);
-        }
-
+            implements IEventChannelListener<Long, MatchActionOperations> {
 
         @Override
         public void entryAdded(MatchActionOperations value) {
             try {
+                log.trace("MatchAction Installer receiving set: {}", value);
                 installationWorkQueue.put(value);
             } catch (InterruptedException e) {
                 log.warn("Error adding to installer work queue: {}",
@@ -358,6 +429,7 @@
         @Override
         public void entryUpdated(MatchActionOperations value) {
             try {
+                log.trace("MatchAction Installer receiving set: {}", value);
                 installationWorkQueue.put(value);
             } catch (InterruptedException e) {
                 log.warn("Error adding to installer work queue: {}",
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchActionId.java b/src/main/java/net/onrc/onos/core/matchaction/MatchActionId.java
index bea973e..59579a5 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchActionId.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchActionId.java
@@ -31,7 +31,7 @@
      *
      * @return MatchAction ID
      */
-    public long value() {
+    public long getValue() {
         return value;
     }
 
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchActionModule.java b/src/main/java/net/onrc/onos/core/matchaction/MatchActionModule.java
index 4970cf1..8bf34a8 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchActionModule.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchActionModule.java
@@ -4,7 +4,6 @@
 import java.util.Collection;
 import java.util.EventListener;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -31,7 +30,6 @@
 
 public class MatchActionModule implements MatchActionFloodlightService, IFloodlightModule {
 
-    private final HashSet<MatchAction> currentOperations = new HashSet<>();
     private static final Logger log = LoggerFactory
             .getLogger(MatchActionModule.class);
     private MatchActionComponent matchActionComponent;
@@ -99,7 +97,7 @@
 
     @Override
     public void setConflictDetectionPolicy(ConflictDetectionPolicy policy) {
-
+        throw new UnsupportedOperationException("conflict detection not implemented yet");
     }
 
     @Override
@@ -119,11 +117,13 @@
 
     @Override
     public void addEventListener(EventListener listener) {
-
+        // TODO
+        log.warn("Could not add MatchAction EventListener: {}", listener);
     }
 
     @Override
     public void removeEventListener(EventListener listener) {
-
+        // TODO
+        log.warn("Could not remove MatchAction EventListener: {}", listener);
     }
 }
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperations.java b/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperations.java
index 73cff28..7d9063f 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperations.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperations.java
@@ -22,7 +22,10 @@
      * The MatchAction operators.
      */
     public enum Operator {
+        /** Add a new match action. */
         ADD,
+
+        /** Remove an existing match action. */
         REMOVE,
     }
 
@@ -83,6 +86,12 @@
         return dependencies;
     }
 
+    /**
+     * Adds a dependency to this set of Operations.
+     *
+     * @param dependentOperationId Identifier of the Operations that must
+     *                             complete before this one can be installed
+     */
     public void addDependency(MatchActionOperationsId dependentOperationId) {
         dependencies.add(dependentOperationId);
     }
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationsId.java b/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationsId.java
index 93f2ec7..3bf34d8 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationsId.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationsId.java
@@ -13,6 +13,8 @@
 
     /**
      * Constructs an Operations identifier and from a unique identifier.
+     *
+     * @param newId unique identifier to use for the new Id object
      */
     public MatchActionOperationsId(final long newId) {
         id = newId;
@@ -53,4 +55,9 @@
     public int hashCode() {
         return Objects.hashCode(id);
     }
+
+    @Override
+    public String toString() {
+        return "0x" + Long.toHexString(getId());
+    }
 }
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationsState.java b/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationsState.java
index 4ccf668..f0470af 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationsState.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationsState.java
@@ -1,11 +1,22 @@
 package net.onrc.onos.core.matchaction;
 
-
+/**
+ * States for MatchActionOperations objects.
+ */
 public enum MatchActionOperationsState {
+    /** Being initialized. */
     INIT,
+
+    /** All operations that we depend on are finished. */
     RESOLVED,
+
+    /** Operation is pending waiting for dependencies. */
     PENDING,
+
+    /** Operations successfully installed. */
     INSTALLED,
+
+    /** Operations installation failed. */
     FAILED
 }
 
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchActionStatus.java b/src/main/java/net/onrc/onos/core/matchaction/MatchActionStatus.java
index 0a7c145..a1f46f4 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchActionStatus.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchActionStatus.java
@@ -1,5 +1,8 @@
 package net.onrc.onos.core.matchaction;
 
+/**
+ * Placeholder.
+ */
 public interface MatchActionStatus {
 
 }
diff --git a/src/main/java/net/onrc/onos/core/matchaction/SwitchResult.java b/src/main/java/net/onrc/onos/core/matchaction/SwitchResult.java
index b5db2e8..7f9e18a 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/SwitchResult.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/SwitchResult.java
@@ -2,17 +2,34 @@
 
 import net.onrc.onos.core.util.Dpid;
 
+/**
+ * The result of applying a MatchAction operation to a switch.
+ */
 public class SwitchResult {
     private Dpid sw;
     private Status status;
     private MatchActionOperationsId matchSetId;
 
+    /**
+     * Status of the switch operation.
+     */
     public enum Status {
+        /** Installation of the MatchAction was successful. */
         SUCCESS,
+
+        /** Installation of the MatchAction failed. */
         FAILURE,
+
+        /** No status has been assigned. */
         UNKNOWN
     }
 
+    /**
+     * Creates a new SwitchResult object.
+     *
+     * @param match identifier for MatchActionsOperations that was requested
+     * @param sw Dpid of the switch that the operations were applied to
+     */
     protected SwitchResult(MatchActionOperationsId match, Dpid sw) {
         this.sw = sw;
         this.status = Status.UNKNOWN;
@@ -23,21 +40,41 @@
      * no-arg constructor for Kryo.
      */
     protected SwitchResult() {
-
+        // Needed for Kryo
     }
 
+    /**
+     * Sets the status of the SwitchResult.
+     *
+     * @param newStatus new status
+     */
     protected void setStatus(Status newStatus) {
         this.status = newStatus;
     }
 
+    /**
+     * Gets the status of the SwitchResult.
+     *
+     * @return status
+     */
     protected Status getStatus() {
         return this.status;
     }
 
+    /**
+     * Gets the identifier for the set of operations that was requested.
+     *
+     * @return MatchActionOperationsId of the requested set of operations
+     */
     protected MatchActionOperationsId getMatchActionOperationsId() {
         return this.matchSetId;
     }
 
+    /**
+     * Gets the identifier for the switch that was requested.
+     *
+     * @return Dpid of the requested switch
+     */
     protected Dpid getSwitch() {
         return this.sw;
     }
diff --git a/src/main/java/net/onrc/onos/core/matchaction/SwitchResultList.java b/src/main/java/net/onrc/onos/core/matchaction/SwitchResultList.java
index 2523556..ff68df2 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/SwitchResultList.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/SwitchResultList.java
@@ -9,13 +9,5 @@
 
     static final long serialVersionUID = -4966789015808022563L;
 
-    /**
-     * Add a switch result to the list.
-     *
-     * @param result switch result to add to the list
-     * @return true
-     */
-    public boolean add(SwitchResult result) {
-        return super.add(result);
-    }
+
 }
diff --git a/src/main/java/net/onrc/onos/core/matchaction/match/OpticalMatch.java b/src/main/java/net/onrc/onos/core/matchaction/match/OpticalMatch.java
index 3cdec64..65c080b 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/match/OpticalMatch.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/match/OpticalMatch.java
@@ -9,7 +9,7 @@
 public class OpticalMatch implements Match {
 
     // Match fields
-    protected Integer srcLambda;
+    private Integer srcLambda;
 
     /**
      * Constructor.
diff --git a/src/main/java/net/onrc/onos/core/matchaction/match/PacketMatchBuilder.java b/src/main/java/net/onrc/onos/core/matchaction/match/PacketMatchBuilder.java
index 1ddc720..7f8be18 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/match/PacketMatchBuilder.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/match/PacketMatchBuilder.java
@@ -8,14 +8,14 @@
  * A builder to instantiate PacketMatch class.
  */
 public class PacketMatchBuilder {
-    private MACAddress srcMac = null;
-    private MACAddress dstMac = null;
-    private Short etherType = null;
-    private IPv4Net srcIp = null;
-    private IPv4Net dstIp = null;
-    private Byte ipProto = null;
-    private Short srcTcpPort = null;
-    private Short dstTcpPort = null;
+    private MACAddress srcMac;
+    private MACAddress dstMac;
+    private Short etherType;
+    private IPv4Net srcIp;
+    private IPv4Net dstIp;
+    private Byte ipProto;
+    private Short srcTcpPort;
+    private Short dstTcpPort;
 
     /**
      * Sets source MAC address.
diff --git a/src/main/java/net/onrc/onos/core/matchaction/web/MatchActionResource.java b/src/main/java/net/onrc/onos/core/matchaction/web/MatchActionResource.java
index 20e68d3..ca3f95e 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/web/MatchActionResource.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/web/MatchActionResource.java
@@ -22,7 +22,7 @@
  */
 public class MatchActionResource extends ServerResource {
 
-    CustomSerializerHelper matchActionSerializers;
+    private final CustomSerializerHelper matchActionSerializers;
 
     /**
      * Constructs a MatchActionResource.
diff --git a/src/main/java/net/onrc/onos/core/newintent/AbstractIntentInstaller.java b/src/main/java/net/onrc/onos/core/newintent/AbstractIntentInstaller.java
index b7aa6c9..d1c43c5 100644
--- a/src/main/java/net/onrc/onos/core/newintent/AbstractIntentInstaller.java
+++ b/src/main/java/net/onrc/onos/core/newintent/AbstractIntentInstaller.java
@@ -1,8 +1,12 @@
 package net.onrc.onos.core.newintent;
 
-import com.google.common.base.Optional;
-import com.google.common.base.Predicate;
-import com.google.common.collect.Iterables;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static net.onrc.onos.api.flowmanager.FlowState.FAILED;
+import static net.onrc.onos.api.flowmanager.FlowState.INSTALLED;
+import static net.onrc.onos.api.flowmanager.FlowState.WITHDRAWN;
+
+import java.util.concurrent.CountDownLatch;
+
 import net.onrc.onos.api.flowmanager.Flow;
 import net.onrc.onos.api.flowmanager.FlowBatchHandle;
 import net.onrc.onos.api.flowmanager.FlowBatchStateChangedEvent;
@@ -16,12 +20,9 @@
 import net.onrc.onos.api.newintent.Intent;
 import net.onrc.onos.api.newintent.IntentInstaller;
 
-import java.util.concurrent.CountDownLatch;
-
-import static com.google.common.base.Preconditions.checkNotNull;
-import static net.onrc.onos.api.flowmanager.FlowState.FAILED;
-import static net.onrc.onos.api.flowmanager.FlowState.INSTALLED;
-import static net.onrc.onos.api.flowmanager.FlowState.WITHDRAWN;
+import com.google.common.base.Optional;
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterables;
 
 // TODO: consider naming because to call Flow manager's API will be removed
 // in long-term refactoring
@@ -53,16 +54,19 @@
             throw new IntentInstallationException("intent installation failed: " + intent);
         }
 
-        try {
-            listener.await();
-            if (listener.getFinalState() == FAILED) {
-                throw new IntentInstallationException("intent installation failed: " + intent);
-            }
-        } catch (InterruptedException e) {
-            throw new IntentInstallationException("intent installation failed: " + intent, e);
-        } finally {
-            flowManager.removeListener(listener);
-        }
+        // TODO (BOC) this blocks a Hazelcast thread, commenting out for now
+        // try {
+        // listener.await();
+        // if (listener.getFinalState() == FAILED) {
+        // throw new IntentInstallationException("intent installation failed: "
+        // + intent);
+        // }
+        // } catch (InterruptedException e) {
+        // throw new IntentInstallationException("intent installation failed: "
+        // + intent, e);
+        // } finally {
+        // flowManager.removeListener(listener);
+        // }
     }
 
     protected void removeFlow(Intent intent, Flow flow) {
diff --git a/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java b/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
index 9cf618a..4cfb9e7 100644
--- a/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
+++ b/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
@@ -132,8 +132,11 @@
     @Override
     public void broadcastPacketOutInternalEdge(Ethernet eth, SwitchPort inSwitchPort) {
 
-        Set<SwitchPort> blacklistSwitchPorts = new HashSet<SwitchPort>(configService
-                .getExternalSwitchPorts());
+        Set<SwitchPort> blacklistSwitchPorts = new HashSet<SwitchPort>();
+        Set<SwitchPort> externalSwitchPorts = configService.getExternalSwitchPorts();
+        if (externalSwitchPorts != null) {
+            blacklistSwitchPorts.addAll(externalSwitchPorts);
+        }
         blacklistSwitchPorts.add(inSwitchPort);
 
         BroadcastPacketOutNotification notification =
diff --git a/src/main/java/net/onrc/onos/core/registry/ZookeeperRegistry.java b/src/main/java/net/onrc/onos/core/registry/ZookeeperRegistry.java
index ffb5564..3f318e1 100644
--- a/src/main/java/net/onrc/onos/core/registry/ZookeeperRegistry.java
+++ b/src/main/java/net/onrc/onos/core/registry/ZookeeperRegistry.java
@@ -580,6 +580,10 @@
                     "net.onrc.onos.core.registry.ZookeeperRegistry.connectionString",
                     DEFAULT_CONNECTION_STRING);
         }
+
+        // Remove spaces from connection string otherwise Zookeeper complains
+        connectionString = connectionString.replaceAll("\\s", "");
+
         log.info("Setting Zookeeper connection string to {}", this.connectionString);
 
         namespace = System.getProperty(ZK_NAMESPACE_KEY, DEFAULT_NAMESPACE).trim();
diff --git a/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java b/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java
index 9ef6fa9..5838483 100644
--- a/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java
+++ b/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java
@@ -42,6 +42,7 @@
 import net.onrc.onos.core.registry.IControllerRegistryService.ControlChangeCallback;
 import net.onrc.onos.core.registry.RegistryException;
 import net.onrc.onos.core.util.Dpid;
+import net.onrc.onos.core.util.LinkTuple;
 import net.onrc.onos.core.util.OnosInstanceId;
 import net.onrc.onos.core.util.PortNumberUtils;
 import net.onrc.onos.core.util.SwitchPort;
@@ -292,16 +293,18 @@
 
     @Override
     public void linkAdded(Link link) {
-        LinkConfigStatus ret = networkConfigService.checkLinkConfig(link);
+        LinkTuple linkTuple = new LinkTuple(
+                new SwitchPort(link.getSrc(), link.getSrcPort()),
+                new SwitchPort(link.getDst(), link.getDstPort()));
+
+        LinkConfigStatus ret = networkConfigService.checkLinkConfig(linkTuple);
         if (ret.getConfigState() == NetworkConfigState.DENY) {
             log.warn("Discovered {} denied by configuration. {} "
                     + "Not allowing it to proceed.", link, ret.getMsg());
             return;
         }
 
-        LinkData linkData = new LinkData(
-                new SwitchPort(link.getSrc(), link.getSrcPort()),
-                new SwitchPort(link.getDst(), link.getDstPort()));
+        LinkData linkData = new LinkData(linkTuple);
 
         // FIXME should be merging, with existing attrs, etc..
         // TODO define attr name as constant somewhere.
diff --git a/src/main/java/net/onrc/onos/core/util/serializers/KryoFactory.java b/src/main/java/net/onrc/onos/core/util/serializers/KryoFactory.java
index 134f0d7..a19636b 100644
--- a/src/main/java/net/onrc/onos/core/util/serializers/KryoFactory.java
+++ b/src/main/java/net/onrc/onos/core/util/serializers/KryoFactory.java
@@ -57,6 +57,7 @@
 import net.onrc.onos.core.matchaction.SwitchResultList;
 import net.onrc.onos.core.matchaction.action.ModifyDstMacAction;
 import net.onrc.onos.core.matchaction.action.ModifySrcMacAction;
+import net.onrc.onos.core.matchaction.action.NullAction;
 import net.onrc.onos.core.matchaction.action.OutputAction;
 import net.onrc.onos.core.matchaction.match.PacketMatch;
 import net.onrc.onos.core.newintent.IntentCompilationResult;
@@ -289,6 +290,7 @@
         kryo.register(SwitchResultList.class);
         kryo.register(SwitchResult.class);
         kryo.register(SwitchResult.Status.class);
+        kryo.register(NullAction.class);
 
         // Host-related classes
         kryo.register(HashSet.class);
diff --git a/src/test/java/net/onrc/onos/core/matchaction/MatchActionIdGeneratorTest.java b/src/test/java/net/onrc/onos/core/matchaction/MatchActionIdGeneratorTest.java
index cb4ed35..321f65d 100644
--- a/src/test/java/net/onrc/onos/core/matchaction/MatchActionIdGeneratorTest.java
+++ b/src/test/java/net/onrc/onos/core/matchaction/MatchActionIdGeneratorTest.java
@@ -17,6 +17,9 @@
 public class MatchActionIdGeneratorTest {
     private IdBlockAllocator allocator;
 
+    /**
+     * Creates a mocked ID Block allocator.
+     */
     @Before
     public void setUp() {
         allocator = createMock(IdBlockAllocator.class);
diff --git a/src/test/java/net/onrc/onos/core/matchaction/MatchActionModuleTest.java b/src/test/java/net/onrc/onos/core/matchaction/MatchActionModuleTest.java
index 42e5475..0949cca 100644
--- a/src/test/java/net/onrc/onos/core/matchaction/MatchActionModuleTest.java
+++ b/src/test/java/net/onrc/onos/core/matchaction/MatchActionModuleTest.java
@@ -29,6 +29,9 @@
     private IDatagridService datagridService;
     private FloodlightModuleContext modContext;
 
+    /**
+     * Sets up the mocks used by the test.
+     */
     @Before
     @SuppressWarnings("unchecked")
     public void setUpMocks() {
diff --git a/src/test/java/net/onrc/onos/core/matchaction/MatchActionOperationsIdGeneratorTest.java b/src/test/java/net/onrc/onos/core/matchaction/MatchActionOperationsIdGeneratorTest.java
index e95f1df..6dc04c8 100644
--- a/src/test/java/net/onrc/onos/core/matchaction/MatchActionOperationsIdGeneratorTest.java
+++ b/src/test/java/net/onrc/onos/core/matchaction/MatchActionOperationsIdGeneratorTest.java
@@ -18,6 +18,9 @@
 
     private IdBlockAllocator allocator;
 
+    /**
+     * Creates a mocked IdBlockAllocator.
+     */
     @Before
     public void setUp() {
         allocator = createMock(IdBlockAllocator.class);
diff --git a/src/test/java/net/onrc/onos/core/matchaction/MatchActionTest.java b/src/test/java/net/onrc/onos/core/matchaction/MatchActionTest.java
index 2684f19..0c8bdf6 100644
--- a/src/test/java/net/onrc/onos/core/matchaction/MatchActionTest.java
+++ b/src/test/java/net/onrc/onos/core/matchaction/MatchActionTest.java
@@ -13,8 +13,14 @@
 
 import org.junit.Test;
 
+/**
+ * Unit tests for MatchAction objects.
+ */
 public class MatchActionTest {
 
+    /**
+     * Tests that a MatchAction object can be constructed properly.
+     */
     @Test
     public void testConstructor() {
         SwitchPort port = new SwitchPort(123L, (short) 55);