Cleanup a few of the CORD apps.

* Removed or turned per-packet logs down to trace in the PIM app
* Can now reconfigure remote sync host in CordMcast
* CordMcast catches REST exceptions rather than bombing

Change-Id: Iae027d5ce1d9047827ea80b071dc77ca49c65206

@@ -21,6 +21,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Maps;
 import com.sun.jersey.api.client.Client;
 import com.sun.jersey.api.client.Client;
+import com.sun.jersey.api.client.ClientHandlerException;
 import com.sun.jersey.api.client.WebResource;
 import com.sun.jersey.api.client.WebResource;
 import com.sun.jersey.api.client.filter.HTTPBasicAuthFilter;
 import com.sun.jersey.api.client.filter.HTTPBasicAuthFilter;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Activate;
@@ -83,6 +84,7 @@ import static org.slf4j.LoggerFactory.getLogger;
 @Component(immediate = true)
 @Component(immediate = true)
 public class CordMcast {
 public class CordMcast {
 
 
+    private static final int DEFAULT_REST_TIMEOUT_MS = 2000;
     private static final int DEFAULT_PRIORITY = 1000;
     private static final int DEFAULT_PRIORITY = 1000;
     private static final short DEFAULT_MCAST_VLAN = 4000;
     private static final short DEFAULT_MCAST_VLAN = 4000;
     private static final String DEFAULT_SYNC_HOST = "10.90.0.8:8181";
     private static final String DEFAULT_SYNC_HOST = "10.90.0.8:8181";
@@ -149,8 +151,6 @@ public class CordMcast {
 
 
         appId = coreService.registerApplication("org.onosproject.cordmcast");
         appId = coreService.registerApplication("org.onosproject.cordmcast");
 
 
-        fabricOnosUrl = "http://" + syncHost + "/onos/v1/mcast";
-
         clearRemoteRoutes();
         clearRemoteRoutes();
 
 
         mcastService.addListener(listener);
         mcastService.addListener(listener);
@@ -192,7 +192,7 @@ public class CordMcast {
             s = get(properties, "priority");
             s = get(properties, "priority");
             priority = isNullOrEmpty(s) ? DEFAULT_PRIORITY : Integer.parseInt(s.trim());
             priority = isNullOrEmpty(s) ? DEFAULT_PRIORITY : Integer.parseInt(s.trim());
 
 
-            s = get(properties, syncHost);
+            s = get(properties, "syncHost");
             syncHost = isNullOrEmpty(s) ? DEFAULT_SYNC_HOST : s.trim();
             syncHost = isNullOrEmpty(s) ? DEFAULT_SYNC_HOST : s.trim();
         } catch (Exception e) {
         } catch (Exception e) {
             user = DEFAULT_USER;
             user = DEFAULT_USER;
@@ -202,6 +202,11 @@ public class CordMcast {
             vlanEnabled = false;
             vlanEnabled = false;
             priority = DEFAULT_PRIORITY;
             priority = DEFAULT_PRIORITY;
         }
         }
+        fabricOnosUrl = createRemoteUrl(syncHost);
+    }
+
+    private static String createRemoteUrl(String remoteHost) {
+        return "http://" + remoteHost + "/onos/v1/mcast";
     }
     }
 
 
     private class InternalMulticastListener implements McastListener {
     private class InternalMulticastListener implements McastListener {
@@ -369,13 +374,18 @@ public class CordMcast {
             return;
             return;
         }
         }
 
 
-        log.debug("Sending route to other ONOS: {}", route);
+        log.debug("Sending route {} to other ONOS {}", route, fabricOnosUrl);
 
 
         WebResource.Builder builder = getClientBuilder(fabricOnosUrl);
         WebResource.Builder builder = getClientBuilder(fabricOnosUrl);
 
 
         ObjectNode json = codecService.getCodec(McastRoute.class)
         ObjectNode json = codecService.getCodec(McastRoute.class)
                 .encode(route, new AbstractWebResource());
                 .encode(route, new AbstractWebResource());
-        builder.post(json.toString());
+
+        try {
+            builder.post(json.toString());
+        } catch (ClientHandlerException e) {
+            log.warn("Unable to send route to remote controller: {}", e.getMessage());
+        }
     }
     }
 
 
     private void removeRemoteRoute(McastRoute route) {
     private void removeRemoteRoute(McastRoute route) {
@@ -384,13 +394,17 @@ public class CordMcast {
             return;
             return;
         }
         }
 
 
-        log.debug("Removing route from other ONOS: {}", route);
+        log.debug("Removing route {} from other ONOS {}", route, fabricOnosUrl);
 
 
         WebResource.Builder builder = getClientBuilder(fabricOnosUrl);
         WebResource.Builder builder = getClientBuilder(fabricOnosUrl);
 
 
         ObjectNode json = codecService.getCodec(McastRoute.class)
         ObjectNode json = codecService.getCodec(McastRoute.class)
                 .encode(route, new AbstractWebResource());
                 .encode(route, new AbstractWebResource());
-        builder.delete(json.toString());
+        try {
+            builder.delete(json.toString());
+        } catch (ClientHandlerException e) {
+            log.warn("Unable to delete route from remote controller: {}", e.getMessage());
+        }
     }
     }
 
 
     private void clearRemoteRoutes() {
     private void clearRemoteRoutes() {
@@ -399,23 +413,28 @@ public class CordMcast {
             return;
             return;
         }
         }
 
 
-        log.debug("Clearing remote multicast routes");
+        log.debug("Clearing remote multicast routes from {}", fabricOnosUrl);
 
 
         WebResource.Builder builder = getClientBuilder(fabricOnosUrl);
         WebResource.Builder builder = getClientBuilder(fabricOnosUrl);
-
-        String response = builder
-                .accept(MediaType.APPLICATION_JSON_TYPE)
-                .get(String.class);
-
-        JsonCodec<McastRoute> routeCodec = codecService.getCodec(McastRoute.class);
-        ObjectMapper mapper = new ObjectMapper();
         List<McastRoute> mcastRoutes = Lists.newArrayList();
         List<McastRoute> mcastRoutes = Lists.newArrayList();
+
         try {
         try {
+            String response = builder
+                    .accept(MediaType.APPLICATION_JSON_TYPE)
+                    .get(String.class);
+
+            JsonCodec<McastRoute> routeCodec = codecService.getCodec(McastRoute.class);
+            ObjectMapper mapper = new ObjectMapper();
+
+
             ObjectNode node = (ObjectNode) mapper.readTree(response);
             ObjectNode node = (ObjectNode) mapper.readTree(response);
             ArrayNode list = (ArrayNode) node.path("routes");
             ArrayNode list = (ArrayNode) node.path("routes");
 
 
             list.forEach(n -> mcastRoutes.add(
             list.forEach(n -> mcastRoutes.add(
                     routeCodec.decode((ObjectNode) n, new AbstractWebResource())));
                     routeCodec.decode((ObjectNode) n, new AbstractWebResource())));
+
+        } catch (ClientHandlerException e) {
+            log.warn("Unable to clear routes from remote controller: {}", e.getMessage());
         } catch (IOException e) {
         } catch (IOException e) {
             log.warn("Error clearing remote routes", e);
             log.warn("Error clearing remote routes", e);
         }
         }
@@ -429,6 +448,8 @@ public class CordMcast {
 
 
     private WebResource.Builder getClientBuilder(String uri) {
     private WebResource.Builder getClientBuilder(String uri) {
         Client client = Client.create();
         Client client = Client.create();
+        client.setConnectTimeout(DEFAULT_REST_TIMEOUT_MS);
+        client.setReadTimeout(DEFAULT_REST_TIMEOUT_MS);
         client.addFilter(new HTTPBasicAuthFilter(user, password));
         client.addFilter(new HTTPBasicAuthFilter(user, password));
         WebResource resource = client.resource(uri);
         WebResource resource = client.resource(uri);
         return resource.accept(JSON_UTF_8.toString())
         return resource.accept(JSON_UTF_8.toString())

@@ -403,10 +403,8 @@ public class IgmpSnoop {
                 return;
                 return;
             }
             }
 
 
-            /*
-             * IPv6 MLD packets are handled by ICMP6. We'll only deal
-             * with IPv4.
-             */
+
+            // IPv6 MLD packets are handled by ICMP6. We'll only deal with IPv4.
             if (ethPkt.getEtherType() != Ethernet.TYPE_IPV4) {
             if (ethPkt.getEtherType() != Ethernet.TYPE_IPV4) {
                 return;
                 return;
             }
             }
@@ -414,29 +412,22 @@ public class IgmpSnoop {
             IPv4 ip = (IPv4) ethPkt.getPayload();
             IPv4 ip = (IPv4) ethPkt.getPayload();
             IpAddress gaddr = IpAddress.valueOf(ip.getDestinationAddress());
             IpAddress gaddr = IpAddress.valueOf(ip.getDestinationAddress());
             IpAddress saddr = Ip4Address.valueOf(ip.getSourceAddress());
             IpAddress saddr = Ip4Address.valueOf(ip.getSourceAddress());
-            log.debug("Packet ({}, {}) -> ingress port: {}", saddr, gaddr,
+            log.trace("Packet ({}, {}) -> ingress port: {}", saddr, gaddr,
                       context.inPacket().receivedFrom());
                       context.inPacket().receivedFrom());
 
 
 
 
-            if (ip.getProtocol() != IPv4.PROTOCOL_IGMP) {
-                log.debug("IGMP Picked up a non IGMP packet.");
-                return;
-            }
-
-            IpPrefix mcast = IpPrefix.valueOf(DEFAULT_MCAST_ADDR);
-            if (!mcast.contains(gaddr)) {
-                log.debug("IGMP Picked up a non multicast packet.");
+            if (ip.getProtocol() != IPv4.PROTOCOL_IGMP ||
+                    !IpPrefix.MULTICAST_RANGE.contains(gaddr)) {
                 return;
                 return;
             }
             }
 
 
-            if (mcast.contains(saddr)) {
+            if (IpPrefix.MULTICAST_RANGE.contains(saddr)) {
                 log.debug("IGMP Picked up a packet with a multicast source address.");
                 log.debug("IGMP Picked up a packet with a multicast source address.");
                 return;
                 return;
             }
             }
 
 
             IGMP igmp = (IGMP) ip.getPayload();
             IGMP igmp = (IGMP) ip.getPayload();
             switch (igmp.getIgmpType()) {
             switch (igmp.getIgmpType()) {
-
                 case IGMP.TYPE_IGMPV3_MEMBERSHIP_REPORT:
                 case IGMP.TYPE_IGMPV3_MEMBERSHIP_REPORT:
                     processMembership(igmp, pkt.receivedFrom());
                     processMembership(igmp, pkt.receivedFrom());
                     break;
                     break;
@@ -453,7 +444,7 @@ public class IgmpSnoop {
                               igmp.getIgmpType());
                               igmp.getIgmpType());
                     break;
                     break;
                 default:
                 default:
-                    log.debug("Unknown IGMP message type: {}", igmp.getIgmpType());
+                    log.warn("Unknown IGMP message type: {}", igmp.getIgmpType());
                     break;
                     break;
             }
             }
         }
         }
@@ -551,6 +542,5 @@ public class IgmpSnoop {
                 .filter(p -> !oltData.get(p.element().id()).uplink().equals(p.number()))
                 .filter(p -> !oltData.get(p.element().id()).uplink().equals(p.number()))
                 .filter(p -> p.isEnabled())
                 .filter(p -> p.isEnabled())
                 .forEach(p -> processFilterObjective((DeviceId) p.element().id(), p, false));
                 .forEach(p -> processFilterObjective((DeviceId) p.element().id(), p, false));
-
     }
     }
 }
 }

@@ -109,7 +109,6 @@ public class PIMApplication {
      * they arrived on, then forward them on to be processed by the appropriate entity.
      * they arrived on, then forward them on to be processed by the appropriate entity.
      */
      */
     public class PIMPacketProcessor implements PacketProcessor {
     public class PIMPacketProcessor implements PacketProcessor {
-        private final Logger log = getLogger(getClass());
 
 
         @Override
         @Override
         public void process(PacketContext context) {
         public void process(PacketContext context) {
@@ -131,14 +130,13 @@ public class PIMApplication {
             Ethernet eth = pkt.parsed();
             Ethernet eth = pkt.parsed();
             if (eth == null) {
             if (eth == null) {
                 // problem getting the ethernet pkt.  Log it debug to avoid spamming log file
                 // problem getting the ethernet pkt.  Log it debug to avoid spamming log file
-                log.debug("Could not retrieve ethnernet packet from the parsed packet");
+                log.debug("Could not retrieve ethernet packet from the parsed packet");
                 return;
                 return;
             }
             }
 
 
             // Get the PIM Interface the packet was received on.
             // Get the PIM Interface the packet was received on.
             PIMInterface pimi = pimInterfaceManager.getPIMInterface(pkt.receivedFrom());
             PIMInterface pimi = pimInterfaceManager.getPIMInterface(pkt.receivedFrom());
             if (pimi == null) {
             if (pimi == null) {
-                log.debug("We received PIM packet from a non PIM interface: " + pkt.receivedFrom().toString());
                 return;
                 return;
             }
             }
 
 
@@ -148,8 +146,7 @@ public class PIMApplication {
              * TODO: Is it possible that PIM interface processing should move to the
              * TODO: Is it possible that PIM interface processing should move to the
              * PIMInterfaceManager directly?
              * PIMInterfaceManager directly?
              */
              */
-            PIMPacketHandler ph = new PIMPacketHandler();
-            ph.processPacket(eth, pimi);
+            pimPacketHandler.processPacket(eth, pimi);
         }
         }
     }
     }
 
 

@@ -15,6 +15,7 @@
  */
  */
 package org.onosproject.pim.impl;
 package org.onosproject.pim.impl;
 
 
+import com.google.common.collect.ImmutableList;
 import org.onlab.packet.Ethernet;
 import org.onlab.packet.Ethernet;
 import org.onlab.packet.IPv4;
 import org.onlab.packet.IPv4;
 import org.onlab.packet.Ip4Address;
 import org.onlab.packet.Ip4Address;
@@ -38,7 +39,6 @@ import org.slf4j.Logger;
 
 
 import java.nio.ByteBuffer;
 import java.nio.ByteBuffer;
 import java.util.Collection;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.List;
 import java.util.List;
 import java.util.Map;
 import java.util.Map;
 import java.util.Random;
 import java.util.Random;
@@ -87,7 +87,7 @@ public final class PIMInterface {
     private IpAddress drIpaddress;
     private IpAddress drIpaddress;
 
 
     // A map of all our PIM neighbors keyed on our neighbors IP address
     // A map of all our PIM neighbors keyed on our neighbors IP address
-    private Map<IpAddress, PIMNeighbor> pimNeighbors = new HashMap<>();
+    private Map<IpAddress, PIMNeighbor> pimNeighbors = new ConcurrentHashMap<>();
 
 
     private Map<McastRoute, RouteData> routes = new ConcurrentHashMap<>();
     private Map<McastRoute, RouteData> routes = new ConcurrentHashMap<>();
 
 
@@ -224,7 +224,7 @@ public final class PIMInterface {
      * @return PIM neighbors
      * @return PIM neighbors
      */
      */
     public Collection<PIMNeighbor> getNeighbors() {
     public Collection<PIMNeighbor> getNeighbors() {
-        return pimNeighbors.values();
+        return ImmutableList.copyOf(pimNeighbors.values());
     }
     }
 
 
     public Collection<McastRoute> getRoutes() {
     public Collection<McastRoute> getRoutes() {
@@ -297,6 +297,9 @@ public final class PIMInterface {
      * @param ethPkt the Ethernet packet header
      * @param ethPkt the Ethernet packet header
      */
      */
     public void processHello(Ethernet ethPkt) {
     public void processHello(Ethernet ethPkt) {
+        if (log.isTraceEnabled()) {
+            log.trace("Received a PIM hello packet");
+        }
 
 
         // We'll need to save our neighbors MAC address
         // We'll need to save our neighbors MAC address
         MacAddress nbrmac = ethPkt.getSourceMAC();
         MacAddress nbrmac = ethPkt.getSourceMAC();
@@ -476,39 +479,6 @@ public final class PIMInterface {
         data.timestamp = System.currentTimeMillis();
         data.timestamp = System.currentTimeMillis();
     }
     }
 
 
-    /*private void sendPrune(McastRoute route, RouteData data) {
-        PIMJoinPrune jp = new PIMJoinPrune();
-
-        jp.addJoinPrune(route.source().toIpPrefix(), route.group().toIpPrefix(), false);
-        jp.setHoldTime((short) 0);
-        jp.setUpstreamAddr(new PIMAddrUnicast(data.ipAddress.toString()));
-
-        PIM pim = new PIM();
-        pim.setPIMType(PIM.TYPE_JOIN_PRUNE_REQUEST);
-        pim.setPayload(jp);
-
-        IPv4 ipv4 = new IPv4();
-        ipv4.setDestinationAddress(PIM.PIM_ADDRESS.getIp4Address().toInt());
-        ipv4.setSourceAddress(getIpAddress().getIp4Address().toInt());
-        ipv4.setProtocol(IPv4.PROTOCOL_PIM);
-        ipv4.setTtl((byte) 1);
-        ipv4.setDiffServ((byte) 0xc0);
-        ipv4.setPayload(pim);
-
-        Ethernet eth = new Ethernet();
-        eth.setSourceMACAddress(onosInterface.mac());
-        eth.setDestinationMACAddress(MacAddress.valueOf("01:00:5E:00:00:0d"));
-        eth.setEtherType(Ethernet.TYPE_IPV4);
-        eth.setPayload(ipv4);
-
-        TrafficTreatment treatment = DefaultTrafficTreatment.builder()
-                .setOutput(onosInterface.connectPoint().port())
-                .build();
-
-        packetService.emit(new DefaultOutboundPacket(onosInterface.connectPoint().deviceId(),
-                treatment, ByteBuffer.wrap(eth.serialize())));
-    }*/
-
     /**
     /**
      * Returns a builder for a PIM interface.
      * Returns a builder for a PIM interface.
      *
      *

@@ -177,17 +177,11 @@ public class PIMInterfaceManager implements PIMInterfaceService {
         log.info("Stopped");
         log.info("Stopped");
     }
     }
 
 
-    /**
-     * Return the PIMInterface that corresponds to the given ConnectPoint.
-     *
-     * @param cp The ConnectPoint we want to get the PIMInterface for
-     * @return The PIMInterface if it exists, NULL if it does not exist.
-     */
     @Override
     @Override
     public PIMInterface getPIMInterface(ConnectPoint cp) {
     public PIMInterface getPIMInterface(ConnectPoint cp) {
-        PIMInterface pi = pimInterfaces.getOrDefault(cp, null);
-        if (pi == null) {
-            log.warn("We have been asked for an Interface we don't have: " + cp.toString());
+        PIMInterface pi = pimInterfaces.get(cp);
+        if (pi == null && log.isTraceEnabled()) {
+            log.trace("We have been asked for an Interface we don't have: {}", cp);
         }
         }
         return pi;
         return pi;
     }
     }

@@ -29,12 +29,12 @@ import java.util.Set;
 public interface PIMInterfaceService {
 public interface PIMInterfaceService {
 
 
     /**
     /**
-     * Return the PIMInterface associated with the given ConnectPoint.
+     * Returns the PIM interface associated with the given connect point.
      *
      *
-     * @param cp The ConnectPoint we want to get the PIMInterface for.
-     * @return the PIMInterface if it exists, NULL if it does not exist.
+     * @param cp the connect point we want to get the PIM interface for
+     * @return the PIM interface if it exists, otherwise null
      */
      */
-    public PIMInterface getPIMInterface(ConnectPoint cp);
+    PIMInterface getPIMInterface(ConnectPoint cp);
 
 
     /**
     /**
      * Retrieves the set of all interfaces running PIM.
      * Retrieves the set of all interfaces running PIM.

@@ -50,20 +50,20 @@ public class PIMPacketHandler {
 
 
         // Sanitize the ethernet header to ensure it is IPv4.  IPv6 we'll deal with later
         // Sanitize the ethernet header to ensure it is IPv4.  IPv6 we'll deal with later
         if (ethPkt.getEtherType() != Ethernet.TYPE_IPV4) {
         if (ethPkt.getEtherType() != Ethernet.TYPE_IPV4) {
-            log.debug("Recieved a non IPv4 packet");
             return;
             return;
         }
         }
 
 
         // Get the IP header
         // Get the IP header
         IPv4 ip = (IPv4) ethPkt.getPayload();
         IPv4 ip = (IPv4) ethPkt.getPayload();
         if (ip.getProtocol() != IPv4.PROTOCOL_PIM) {
         if (ip.getProtocol() != IPv4.PROTOCOL_PIM) {
-            log.debug("Received a non PIM IP packet");
             return;
             return;
         }
         }
 
 
         // Get the address of our the neighbor that sent this packet to us.
         // Get the address of our the neighbor that sent this packet to us.
         IpAddress nbraddr = IpAddress.valueOf(ip.getDestinationAddress());
         IpAddress nbraddr = IpAddress.valueOf(ip.getDestinationAddress());
-        log.debug("Packet " + nbraddr.toString() + " received on port " + pimi.toString());
+        if (log.isTraceEnabled()) {
+            log.trace("Packet {} received on port {}", nbraddr, pimi);
+        }
 
 
         // Get the PIM header
         // Get the PIM header
         PIM pim = (PIM) ip.getPayload();
         PIM pim = (PIM) ip.getPayload();
@@ -71,19 +71,15 @@ public class PIMPacketHandler {
 
 
         // Process the pim packet
         // Process the pim packet
         switch (pim.getPimMsgType()) {
         switch (pim.getPimMsgType()) {
-
             case PIM.TYPE_HELLO:
             case PIM.TYPE_HELLO:
                 pimi.processHello(ethPkt);
                 pimi.processHello(ethPkt);
-                log.debug("Received a PIM hello packet");
                 break;
                 break;
-
             case PIM.TYPE_JOIN_PRUNE_REQUEST:
             case PIM.TYPE_JOIN_PRUNE_REQUEST:
                 pimi.processJoinPrune(ethPkt);
                 pimi.processJoinPrune(ethPkt);
                 log.debug("Received a PIM Join/Prune message");
                 log.debug("Received a PIM Join/Prune message");
                 break;
                 break;
-
             default:
             default:
-                log.debug("Recieved unsupported PIM type: " + pim.getPimMsgType());
+                log.debug("Received unsupported PIM type: {}", pim.getPimMsgType());
                 break;
                 break;
         }
         }
     }
     }

@@ -31,6 +31,8 @@ public class IpPrefix {
     public static final int MAX_INET_MASK_LENGTH = IpAddress.INET_BIT_LENGTH;
     public static final int MAX_INET_MASK_LENGTH = IpAddress.INET_BIT_LENGTH;
     public static final int MAX_INET6_MASK_LENGTH = IpAddress.INET6_BIT_LENGTH;
     public static final int MAX_INET6_MASK_LENGTH = IpAddress.INET6_BIT_LENGTH;
 
 
+    public static final IpPrefix MULTICAST_RANGE = IpPrefix.valueOf("224.0.0.0/4");
+
     private final IpAddress address;
     private final IpAddress address;
     private final short prefixLength;
     private final short prefixLength;