Fixed VTN ARP proxy replies with private MAC for public gateways

Also added more logs for the CordVtnNodeManager device events.

Change-Id: I57337ecd972ef17d151482608814a67df226f5a9
diff --git a/src/main/java/org/opencord/cordvtn/impl/CordVtnArpProxy.java b/src/main/java/org/opencord/cordvtn/impl/CordVtnArpProxy.java
index 1d7562b..e8b4775 100644
--- a/src/main/java/org/opencord/cordvtn/impl/CordVtnArpProxy.java
+++ b/src/main/java/org/opencord/cordvtn/impl/CordVtnArpProxy.java
@@ -90,7 +90,7 @@
     protected ServiceNetworkService snetService;
 
     private final PacketProcessor packetProcessor = new InternalPacketProcessor();
-    private final Map<Ip4Address, MacAddress> gateways = Maps.newConcurrentMap();
+    private final Map<IpAddress, MacAddress> gateways = Maps.newConcurrentMap();
 
     private MacAddress privateGatewayMac = MacAddress.NONE;
     private NetworkConfigListener configListener = new InternalConfigListener();
@@ -107,11 +107,10 @@
         requestPacket();
 
         snetService.addListener(snetListener);
-        snetService.serviceNetworks().forEach(net -> {
-            if (net.serviceIp() != null) {
-                addGateway(net.serviceIp(), privateGatewayMac);
-            }
-        });
+        snetService.serviceNetworks().stream()
+                .filter(net -> net.type() == PRIVATE || net.type() == VSG)
+                .filter(net -> net.serviceIp() != null)
+                .forEach(net -> addGateway(net.serviceIp(), privateGatewayMac));
     }
 
     @Deactivate
@@ -161,10 +160,16 @@
         checkNotNull(gatewayIp);
         checkArgument(gatewayMac != null && gatewayMac != MacAddress.NONE,
                       "privateGatewayMac is not configured");
-        log.debug("Added ARP proxy entry IP:{} MAC:{}",
-                  gatewayIp,
-                  privateGatewayMac);
-        gateways.put(gatewayIp.getIp4Address(), gatewayMac);
+
+        MacAddress existing = gateways.get(gatewayIp);
+        if (existing != null && !existing.equals(privateGatewayMac) &&
+                gatewayMac.equals(privateGatewayMac)) {
+            // this is public gateway IP and MAC configured via netcfg
+            // don't update with private gateway MAC
+            return;
+        }
+        gateways.put(gatewayIp, gatewayMac);
+        log.debug("Added ARP proxy entry IP:{} MAC:{}", gatewayIp, gatewayMac);
     }
 
     /**
@@ -174,10 +179,17 @@
      */
     private void removeGateway(IpAddress gatewayIp) {
         checkNotNull(gatewayIp);
-        log.debug("Removed ARP proxy entry IP:{} MAC:{}",
-                  gatewayIp,
-                  privateGatewayMac);
-        gateways.remove(gatewayIp.getIp4Address());
+        MacAddress existing = gateways.get(gatewayIp);
+        if (existing == null) {
+            return;
+        }
+        if (!existing.equals(privateGatewayMac)) {
+            // this is public gateway IP and MAC configured via netcfg
+            // do nothing
+            return;
+        }
+        gateways.remove(gatewayIp);
+        log.debug("Removed ARP proxy entry for IP:{} MAC: {}", gatewayIp, existing);
     }
 
     /**
@@ -281,7 +293,7 @@
      * @param instances set of instances to send gratuitous ARP packet
      */
     private void sendGratuitousArp(IpAddress gatewayIp, Set<Instance> instances) {
-        MacAddress gatewayMac = gateways.get(gatewayIp.getIp4Address());
+        MacAddress gatewayMac = gateways.get(gatewayIp);
         if (gatewayMac == null) {
             log.debug("Gateway {} is not registered to ARP proxy", gatewayIp);
             return;
@@ -384,13 +396,7 @@
         @Override
         public boolean isRelevant(ServiceNetworkEvent event) {
             ServiceNetwork snet = event.subject();
-            if (snet.type() == PUBLIC ||
-                    snet.type() == MANAGEMENT_HOST ||
-                    snet.type() == MANAGEMENT_LOCAL ||
-                    snet.type() == ACCESS_AGENT) {
-                return false;
-            }
-            return true;
+            return snet.serviceIp() != null;
         }
 
         @Override
@@ -399,9 +405,7 @@
             switch (event.type()) {
                 case SERVICE_NETWORK_CREATED:
                 case SERVICE_NETWORK_UPDATED:
-                    if (snet.serviceIp() != null) {
-                        addGateway(snet.serviceIp(), privateGatewayMac);
-                    }
+                    addGateway(snet.serviceIp(), privateGatewayMac);
                     break;
                 case SERVICE_NETWORK_REMOVED:
                     removeGateway(snet.serviceIp());
@@ -422,7 +426,7 @@
             log.debug("No configuration found");
             return;
         }
-
+        // TODO handle the case that private gateway MAC is changed
         privateGatewayMac = config.privateGatewayMac();
         log.debug("Set default service IP MAC address {}", privateGatewayMac);
 
diff --git a/src/main/java/org/opencord/cordvtn/impl/CordVtnNodeManager.java b/src/main/java/org/opencord/cordvtn/impl/CordVtnNodeManager.java
index 44b70ac..eb22f8e 100644
--- a/src/main/java/org/opencord/cordvtn/impl/CordVtnNodeManager.java
+++ b/src/main/java/org/opencord/cordvtn/impl/CordVtnNodeManager.java
@@ -31,7 +31,6 @@
 import org.onosproject.cluster.NodeId;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreService;
-import org.onosproject.net.AnnotationKeys;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.Device;
 import org.onosproject.net.DeviceId;
@@ -371,7 +370,7 @@
 
     private Optional<PortNumber> getPortNumber(DeviceId deviceId, String portName) {
         PortNumber port = deviceService.getPorts(deviceId).stream()
-                .filter(p -> p.annotations().value(AnnotationKeys.PORT_NAME).equals(portName) &&
+                .filter(p -> p.annotations().value(PORT_NAME).equals(portName) &&
                         p.isEnabled())
                 .map(Port::number)
                 .findAny()
@@ -698,7 +697,7 @@
             if (node != null) {
                 setNodeState(node, getNodeState(node));
             } else {
-                log.debug("{} is detected on unregistered node, ignore it.", device.id());
+                log.info("{} is detected on unregistered node, ignore it.", device.id());
             }
         }
 
@@ -717,7 +716,7 @@
             if (node != null) {
                 setNodeState(node, getNodeState(node));
             } else {
-                log.debug("{} is detected on unregistered node, ignore it.", device.id());
+                log.info("{} is detected on unregistered node, ignore it.", device.id());
             }
         }
 
@@ -740,14 +739,11 @@
         public void portAdded(Port port) {
             CordVtnNode node = nodeByBridgeId((DeviceId) port.element().id());
             String portName = portName(port);
-
             if (node == null) {
-                log.debug("{} is added to unregistered node, ignore it.", portName);
+                log.info("{} is added to unregistered node, ignore it.", portName);
                 return;
             }
 
-            log.info("Port {} is added to {}", portName, node.hostname());
-
             if (node.systemIfaces().contains(portName)) {
                 setNodeState(node, getNodeState(node));
             } else if (isNodeStateComplete(node)) {
@@ -768,13 +764,11 @@
         public void portRemoved(Port port) {
             CordVtnNode node = nodeByBridgeId((DeviceId) port.element().id());
             String portName = portName(port);
-
             if (node == null) {
+                log.info("{} is removed from unregistered node, ignore it.", portName);
                 return;
             }
 
-            log.info("Port {} is removed from {}", portName, node.hostname());
-
             if (node.systemIfaces().contains(portName)) {
                 setNodeState(node, NodeState.INCOMPLETE);
             } else if (isNodeStateComplete(node)) {
@@ -789,33 +783,51 @@
     private class InternalDeviceListener implements DeviceListener {
 
         @Override
+        public boolean isRelevant(DeviceEvent event) {
+            NodeId leaderNodeId = leadershipService.getLeader(appId.name());
+            return Objects.equals(localNodeId, leaderNodeId);
+        }
+
+        @Override
         public void event(DeviceEvent event) {
 
-            NodeId leaderNodeId = leadershipService.getLeader(appId.name());
-            if (!Objects.equals(localNodeId, leaderNodeId)) {
-                // do not allow to proceed without leadership
-                return;
-            }
-
             Device device = event.subject();
-            ConnectionHandler<Device> handler =
-                    (device.type().equals(SWITCH) ? bridgeHandler : ovsdbHandler);
+            ConnectionHandler<Device> handler = device.type().equals(SWITCH) ?
+                    bridgeHandler : ovsdbHandler;
 
             switch (event.type()) {
                 case PORT_ADDED:
-                    eventExecutor.execute(() -> bridgeHandler.portAdded(event.port()));
+                    eventExecutor.execute(() -> {
+                        log.info("Port {} is added to {}",
+                                 event.port().annotations().value(PORT_NAME),
+                                 event.subject().id());
+                        bridgeHandler.portAdded(event.port());
+                    });
                     break;
                 case PORT_UPDATED:
                     if (!event.port().isEnabled()) {
-                        eventExecutor.execute(() -> bridgeHandler.portRemoved(event.port()));
+                        eventExecutor.execute(() -> {
+                            log.info("Port {} is removed from {}",
+                                     event.port().annotations().value(PORT_NAME),
+                                     event.subject().id());
+                            bridgeHandler.portRemoved(event.port());
+                        });
                     }
                     break;
                 case DEVICE_ADDED:
                 case DEVICE_AVAILABILITY_CHANGED:
                     if (deviceService.isAvailable(device.id())) {
-                        eventExecutor.execute(() -> handler.connected(device));
+                        eventExecutor.execute(() -> {
+                            log.info("Device {} is connected",
+                                     event.subject().id());
+                            handler.connected(device);
+                        });
                     } else {
-                        eventExecutor.execute(() -> handler.disconnected(device));
+                        eventExecutor.execute(() -> {
+                            log.info("Device {} is disconnected",
+                                     event.subject().id());
+                            handler.disconnected(device);
+                        });
                     }
                     break;
                 default: