Fix: provide the ICMP connectivity to external gateway

Change-Id: I893037715c93dd228fc23eb1c706abba96cd4786
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingSnatHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingSnatHandler.java
index 07cad1e..9769203 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingSnatHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingSnatHandler.java
@@ -840,18 +840,25 @@
                     setStatefulSnatDownstreamRule(gwNode.intgBridge(),
                             IpPrefix.valueOf(natAddress, VM_PREFIX), install);
 
-                    PortRange gwPortRange = gwPortRangeMap.get(gwNode);
+                    if (install) {
+                        PortRange gwPortRange = gwPortRangeMap.get(gwNode);
 
-                    Map<String, PortRange> netPortRangeMap =
-                            getAssignedPortsForNet(getNetIdByRouterId(routerIface.getId()),
-                                    gwPortRange.min(), gwPortRange.max());
+                        Map<String, PortRange> netPortRangeMap =
+                                getAssignedPortsForNet(getNetIdByRouterId(routerIface.getId()),
+                                        gwPortRange.min(), gwPortRange.max());
 
-                    PortRange netPortRange = netPortRangeMap.get(osNet.getId());
+                        PortRange netPortRange = netPortRangeMap.get(osNet.getId());
 
-                    setStatefulSnatUpstreamRule(gwNode, natAddress,
-                            Long.parseLong(osNet.getProviderSegID()),
-                            externalPeerRouter, netPortRange.min(),
-                            netPortRange.max(), install);
+                        setStatefulSnatUpstreamRule(gwNode, natAddress,
+                                Long.parseLong(osNet.getProviderSegID()),
+                                externalPeerRouter, netPortRange.min(),
+                                netPortRange.max(), install);
+                    } else {
+                        setStatefulSnatUpstreamRule(gwNode, natAddress,
+                                Long.parseLong(osNet.getProviderSegID()),
+                                externalPeerRouter, 0, 0, install);
+                    }
+
                 });
     }
 
@@ -1022,28 +1029,31 @@
                 .matchTunnelId(vni)
                 .build();
 
-        ExtensionTreatment natTreatment = RulePopulatorUtil
-                .niciraConnTrackTreatmentBuilder(driverService, gwNode.intgBridge())
-                .commit(true)
-                .natFlag(CT_NAT_SRC_FLAG)
-                .natAction(true)
-                .natIp(gatewayIp)
-                .natPortMin(TpPort.tpPort(minPortNum))
-                .natPortMax(TpPort.tpPort(maxPortNum))
-                .build();
+        TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
 
-        TrafficTreatment treatment = DefaultTrafficTreatment.builder()
-                .extension(natTreatment, gwNode.intgBridge())
-                .setEthDst(extPeerRouter.macAddress())
-                .setEthSrc(DEFAULT_GATEWAY_MAC)
-                .setOutput(gwNode.uplinkPortNum())
-                .build();
+        // we do not consider to much like port range on removing the rules...
+        if (install) {
+            ExtensionTreatment natTreatment = RulePopulatorUtil
+                    .niciraConnTrackTreatmentBuilder(driverService, gwNode.intgBridge())
+                    .commit(true)
+                    .natFlag(CT_NAT_SRC_FLAG)
+                    .natAction(true)
+                    .natIp(gatewayIp)
+                    .natPortMin(TpPort.tpPort(minPortNum))
+                    .natPortMax(TpPort.tpPort(maxPortNum))
+                    .build();
+
+            tBuilder.extension(natTreatment, gwNode.intgBridge())
+                    .setEthDst(extPeerRouter.macAddress())
+                    .setEthSrc(DEFAULT_GATEWAY_MAC)
+                    .setOutput(gwNode.uplinkPortNum());
+        }
 
         osFlowRuleService.setRule(
                 appId,
                 gwNode.intgBridge(),
                 selector,
-                treatment,
+                tBuilder.build(),
                 PRIORITY_STATEFUL_SNAT_RULE,
                 GW_COMMON_TABLE,
                 install);
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingArpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingArpHandler.java
index 6b96dfa..feb06cc 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingArpHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingArpHandler.java
@@ -316,11 +316,10 @@
             }
 
             TrafficSelector.Builder sBuilder = DefaultTrafficSelector.builder();
-            TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
 
             if (netType == VLAN) {
                 sBuilder.matchVlanId(VlanId.vlanId(network.getProviderSegID()));
-                tBuilder.popVlan();
+
             } else if (netType == VXLAN || netType == GRE || netType == GENEVE) {
                 // do not remove fake gateway ARP rules, if there is another gateway
                 // which has the same subnet that to be removed
@@ -345,6 +344,13 @@
             if (osNode == null) {
                 osNodeService.completeNodes(COMPUTE).forEach(n -> {
                     Device device = deviceService.getDevice(n.intgBridge());
+
+                    TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
+
+                    if (netType == VLAN) {
+                        tBuilder.popVlan();
+                    }
+
                     tBuilder.extension(buildMoveEthSrcToDstExtension(device), device.id())
                             .extension(buildMoveArpShaToThaExtension(device), device.id())
                             .extension(buildMoveArpSpaToTpaExtension(device), device.id())
@@ -365,6 +371,13 @@
                 });
             } else {
                 Device device = deviceService.getDevice(osNode.intgBridge());
+
+                TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
+
+                if (netType == VLAN) {
+                    tBuilder.popVlan();
+                }
+
                 tBuilder.extension(buildMoveEthSrcToDstExtension(device), device.id())
                         .extension(buildMoveArpShaToThaExtension(device), device.id())
                         .extension(buildMoveArpSpaToTpaExtension(device), device.id())
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingIcmpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingIcmpHandler.java
index 3f8a362..1ff77a8 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingIcmpHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingIcmpHandler.java
@@ -46,6 +46,7 @@
 import org.onosproject.openstacknode.api.OpenstackNodeEvent;
 import org.onosproject.openstacknode.api.OpenstackNodeListener;
 import org.onosproject.openstacknode.api.OpenstackNodeService;
+import org.openstack4j.model.network.Network;
 import org.openstack4j.model.network.Router;
 import org.openstack4j.model.network.RouterInterface;
 import org.openstack4j.model.network.Subnet;
@@ -57,6 +58,7 @@
 import org.slf4j.Logger;
 
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.stream.Collectors;
@@ -71,6 +73,8 @@
 import static org.onosproject.openstacknetworking.api.Constants.PRIORITY_ICMP_RULE;
 import static org.onosproject.openstacknetworking.api.Constants.ROUTING_TABLE;
 import static org.onosproject.openstacknetworking.impl.OsgiPropertyConstants.USE_STATEFUL_SNAT;
+import static org.onosproject.openstacknetworking.api.OpenstackNetwork.Type.FLAT;
+import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getExternalIp;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getPropertyValueAsBoolean;
 import static org.onosproject.openstacknetworking.util.RulePopulatorUtil.NXM_NX_IP_TTL;
 import static org.onosproject.openstacknetworking.util.RulePopulatorUtil.NXM_OF_ICMP_TYPE;
@@ -169,12 +173,42 @@
 
         osNodeService.completeNodes(COMPUTE).stream()
                 .filter(cNode -> cNode.dataIp() != null)
-                .forEach(cNode -> setRoutableSubnetsIcmpRules(
-                        cNode, segId, routableSubnets, gatewayIp, netType, install));
+                .forEach(cNode -> {
+                    setRoutableSubnetsIcmpRules(cNode, segId, osSubnet,
+                            routableSubnets, gatewayIp, netType, install);
+                    setExtGatewayIcmpReplyRules(cNode, routerIface,
+                            netType, install);
+                });
+    }
+
+    private void setExtGatewayIcmpReplyRules(OpenstackNode osNode,
+                                             RouterInterface routerIface,
+                                             Type networkType, boolean install) {
+
+        if (networkType == FLAT) {
+            return;
+        }
+
+        Optional<Router> osRouter = osRouterService.routers().stream()
+                .filter(router -> osRouterService.routerInterfaces(routerIface.getId()) != null)
+                .findAny();
+
+        if (!osRouter.isPresent()) {
+            log.error("Cannot find a router for router interface {} ", routerIface);
+            return;
+        }
+
+        IpAddress natAddress = getExternalIp(osRouter.get(), osNetworkService);
+        if (natAddress == null) {
+            return;
+        }
+
+        setGatewayIcmpReplyRule(osNode, null, natAddress, networkType, install);
     }
 
     private void setRoutableSubnetsIcmpRules(OpenstackNode osNode,
                                              String segmentId,
+                                             Subnet updatedSubnet,
                                              Set<Subnet> routableSubnets,
                                              IpAddress gatewayIp,
                                              Type networkType,
@@ -184,6 +218,11 @@
         routableSubnets.forEach(subnet -> {
             setGatewayIcmpReplyRule(osNode, segmentId,
                     IpAddress.valueOf(subnet.getGateway()), networkType, install);
+
+            Network network = osNetworkService.network(subnet.getNetworkId());
+
+            setGatewayIcmpReplyRule(osNode, network.getProviderSegID(),
+                    IpAddress.valueOf(updatedSubnet.getGateway()), networkType, install);
         });
     }
 
@@ -208,17 +247,19 @@
                 .matchIcmpCode(CODE_ECHO_REQEUST)
                 .matchIPDst(gatewayIp.getIp4Address().toIpPrefix());
 
-        switch (networkType) {
-            case VXLAN:
-            case GRE:
-            case GENEVE:
-                sBuilder.matchTunnelId(Long.parseLong(segmentId));
-                break;
-            case VLAN:
-                sBuilder.matchVlanId(VlanId.vlanId(segmentId));
-                break;
-            default:
-                break;
+        if (segmentId != null) {
+            switch (networkType) {
+                case VXLAN:
+                case GRE:
+                case GENEVE:
+                    sBuilder.matchTunnelId(Long.parseLong(segmentId));
+                    break;
+                case VLAN:
+                    sBuilder.matchVlanId(VlanId.vlanId(segmentId));
+                    break;
+                default:
+                    break;
+            }
         }
 
         Device device = deviceService.getDevice(osNode.intgBridge());