ONOS-4534 Improve VLAN support in SDN-IP

Change-Id: Ib9cf64d8f896462ec18260c4371859f447e7c4de
diff --git a/apps/sdnip/src/main/java/org/onosproject/sdnip/PeerConnectivityManager.java b/apps/sdnip/src/main/java/org/onosproject/sdnip/PeerConnectivityManager.java
index 6ed06ae..8a68db2 100644
--- a/apps/sdnip/src/main/java/org/onosproject/sdnip/PeerConnectivityManager.java
+++ b/apps/sdnip/src/main/java/org/onosproject/sdnip/PeerConnectivityManager.java
@@ -202,15 +202,14 @@
     }
 
     /**
-     * Builds the required intents between the two pairs of connect points and
-     * IP addresses.
+     * Builds the required intents between a BGP speaker and an external router.
      *
-     * @param portOne the first connect point
-     * @param vlanOne the ingress VLAN
-     * @param ipOne the first IP address
-     * @param portTwo the second connect point
-     * @param vlanTwo the egress VLAN
-     * @param ipTwo the second IP address
+     * @param portOne the BGP speaker connect point
+     * @param vlanOne the BGP speaker VLAN
+     * @param ipOne the BGP speaker IP address
+     * @param portTwo the external BGP peer connect point
+     * @param vlanTwo the external BGP peer VLAN
+     * @param ipTwo the external BGP peer IP address
      * @return the intents to install
      */
     private Collection<PointToPointIntent> buildIntents(ConnectPoint portOne,
@@ -239,9 +238,13 @@
             icmpProtocol = IPv6.PROTOCOL_ICMP6;
         }
 
-        // Add treatment for VLAN for traffic going from BGP speaker to BGP peer
-        if (!vlanTwo.equals(VlanId.NONE)) {
-            treatmentToPeer.setVlanId(vlanTwo);
+        // Add VLAN treatment for traffic going from BGP speaker to BGP peer
+        if (!vlanOne.equals(vlanTwo)) {
+            if (vlanTwo.equals(VlanId.NONE)) {
+                treatmentToPeer.popVlan();
+            } else {
+                treatmentToPeer.setVlanId(vlanTwo);
+            }
         }
 
         // Path from BGP speaker to BGP peer matching destination TCP port 179
@@ -305,9 +308,13 @@
                             .priority(PRIORITY_OFFSET)
                             .build());
 
-        // Add treatment for VLAN for traffic going from BGP peer to BGP speaker
-        if (!vlanOne.equals(VlanId.NONE)) {
-            treatmentToSpeaker.setVlanId(vlanOne);
+        // Add VLAN treatment for traffic going from BGP peer to BGP speaker
+        if (!vlanTwo.equals(vlanOne)) {
+            if (vlanOne.equals(VlanId.NONE)) {
+                treatmentToSpeaker.popVlan();
+            } else {
+                treatmentToSpeaker.setVlanId(vlanOne);
+            }
         }
 
         // Path from BGP peer to BGP speaker matching destination TCP port 179
diff --git a/apps/sdnip/src/main/java/org/onosproject/sdnip/SdnIpFib.java b/apps/sdnip/src/main/java/org/onosproject/sdnip/SdnIpFib.java
index 1179c3d..f326d70 100644
--- a/apps/sdnip/src/main/java/org/onosproject/sdnip/SdnIpFib.java
+++ b/apps/sdnip/src/main/java/org/onosproject/sdnip/SdnIpFib.java
@@ -158,23 +158,38 @@
                     nextHopIpAddress);
             return null;
         }
-
-        // Generate the intent itself
-        Set<ConnectPoint> ingressPorts = new HashSet<>();
         ConnectPoint egressPort = egressInterface.connectPoint();
+
+        Set<Interface> ingressInterfaces = new HashSet<>();
+        Set<ConnectPoint> ingressPorts = new HashSet<>();
         log.debug("Generating intent for prefix {}, next hop mac {}",
                 prefix, nextHopMacAddress);
 
-        for (Interface intf : interfaceService.getInterfaces()) {
-            // TODO this should be only peering interfaces
-            if (!intf.connectPoint().equals(egressInterface.connectPoint())) {
-                ConnectPoint srcPort = intf.connectPoint();
-                ingressPorts.add(srcPort);
+        TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
+
+        // Get ingress interfaces and ports
+        // TODO this should be only peering interfaces
+        interfaceService.getInterfaces().stream()
+                .filter(intf -> !intf.equals(egressInterface))
+                .forEach(intf -> {
+                    ingressInterfaces.add(intf);
+                    ConnectPoint ingressPort = intf.connectPoint();
+                    ingressPorts.add(ingressPort);
+                });
+
+        // By default the ingress traffic is not tagged
+        VlanId ingressVlanId = VlanId.NONE;
+
+        // Match VLAN Id ANY if the source VLAN Id is not null
+        // TODO need to be able to set a different VLAN Id per ingress interface
+        for (Interface intf : ingressInterfaces) {
+            if (!intf.vlan().equals(VlanId.NONE)) {
+                selector.matchVlanId(VlanId.ANY);
+                ingressVlanId = intf.vlan();
             }
         }
 
         // Match the destination IP prefix at the first hop
-        TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
         if (prefix.isIp4()) {
             selector.matchEthType(Ethernet.TYPE_IPV4);
             // if it is default route, then we do not need match destination
@@ -189,22 +204,29 @@
             if (prefix.prefixLength() != 0) {
                 selector.matchIPv6Dst(prefix);
             }
-
         }
 
         // Rewrite the destination MAC address
         TrafficTreatment.Builder treatment = DefaultTrafficTreatment.builder()
                 .setEthDst(nextHopMacAddress);
-        if (!egressInterface.vlan().equals(VlanId.NONE)) {
-            treatment.setVlanId(egressInterface.vlan());
-            // If we set VLAN ID, we have to make sure a VLAN tag exists.
-            // TODO support no VLAN -> VLAN routing
-            selector.matchVlanId(VlanId.ANY);
+
+        // Set egress VLAN Id
+        // TODO need to make the comparison with different ingress VLAN Ids
+        if (!ingressVlanId.equals(egressInterface.vlan())) {
+            if (egressInterface.vlan().equals(VlanId.NONE)) {
+                treatment.popVlan();
+            } else {
+                treatment.setVlanId(egressInterface.vlan());
+            }
         }
 
+        // Set priority
         int priority =
                 prefix.prefixLength() * PRIORITY_MULTIPLIER + PRIORITY_OFFSET;
+
+        // Set key
         Key key = Key.of(prefix.toString(), appId);
+
         return MultiPointToSinglePointIntent.builder()
                 .appId(appId)
                 .key(key)