Internet BGPVPN traffic is not working for IPv6 18/76218/11
authorKarthikeyan Krishnan <karthikeyangceb007@gmail.com>
Tue, 18 Sep 2018 12:44:38 +0000 (18:14 +0530)
committerSam Hague <shague@redhat.com>
Tue, 2 Oct 2018 01:08:56 +0000 (01:08 +0000)
This patch is addressing the review comments
from the existing review[0]

[0] https://git.opendaylight.org/gerrit/#/c/75601/

Issue: NETVIRT-1417

Change-Id: I8ea264ad2f8a284f4471e7b4eed67086e23a460b
Signed-off-by: Karthikeyan Krishnan <karthikeyangceb007@gmail.com>
natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatUtil.java
neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/IPV6InternetDefaultRouteProgrammer.java
neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronPortChangeListener.java
neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java
neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnNatManager.java
neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java

index eb37c2feaddc1157680828e35febbeb5f0bf5d5b..fb544a028fbdd69f2972482cdc0d1a6a752cb38e 100644 (file)
@@ -698,8 +698,21 @@ public final class NatUtil {
                 if (routerIdsList == null || routerIdsList.isEmpty()) {
                     return null;
                 }
-                //Skip if current VPN is already associated with network
-                if (vpnMap.getNetworkIds() != null) {
+                //Skip router vpnId fetching from internet BGP-VPN
+                boolean isInternetBgpVpn = false;
+                if (vpnMap.getNetworkIds() != null && !vpnMap.getNetworkIds().isEmpty()) {
+                    for (Uuid netId: vpnMap.getNetworkIds()) {
+                        if (isExternalNetwork(broker, netId)) {
+                            isInternetBgpVpn = true;
+                        }
+                        /* If first network is not a external network then no need iterate
+                         * whole network list from the VPN
+                         */
+                        break;
+                    }
+                }
+                if (isInternetBgpVpn) {
+                    //skip further processing
                     continue;
                 }
                 if (routerIdsList.contains(new Uuid(routerId))) {
@@ -2404,4 +2417,15 @@ public final class NatUtil {
         LOG.debug("getSwitchStatus : Switch {} is down", nodeId);
         return false;
     }
+
+    public static boolean isExternalNetwork(DataBroker broker, Uuid networkId) {
+        InstanceIdentifier<Networks> id = buildNetworkIdentifier(networkId);
+        Optional<Networks> networkData =
+                SingleTransactionDataBroker.syncReadOptionalAndTreatReadFailedExceptionAsAbsentOptional(
+                        broker, LogicalDatastoreType.CONFIGURATION, id);
+        if (networkData.isPresent()) {
+            return true;
+        }
+        return false;
+    }
 }
index 0dd0c957b7d32450c521d806fb4f6103d025c1c1..516fbbb22132213f6b84837305d9ed4871dbb8b5 100644 (file)
@@ -41,23 +41,24 @@ public class IPV6InternetDefaultRouteProgrammer {
         this.mdsalManager = mdsalManager;
     }
 
-    private FlowEntity buildIPv6FallbacktoExternalVpn(BigInteger dpId, long bgpVpnId, long routerId, boolean add) {
+    private FlowEntity buildIPv6FallbacktoExternalVpn(BigInteger dpId, long internetBgpVpnId, long vpnId, boolean add) {
         List<MatchInfo> matches = new ArrayList<>();
         matches.add(MatchEthernetType.IPV6);
 
         //add match for router vpnId
-        matches.add(new MatchMetadata(MetaDataUtil.getVpnIdMetadata(routerId), MetaDataUtil.METADATA_MASK_VRFID));
+        matches.add(new MatchMetadata(MetaDataUtil.getVpnIdMetadata(vpnId), MetaDataUtil.METADATA_MASK_VRFID));
 
         ArrayList<ActionInfo> listActionInfo = new ArrayList<>();
         ArrayList<InstructionInfo> instructionInfo = new ArrayList<>();
         if (add) {
-            ActionSetFieldMeta actionSetFieldMeta = new ActionSetFieldMeta(MetaDataUtil.getVpnIdMetadata(bgpVpnId));
+            ActionSetFieldMeta actionSetFieldMeta = new ActionSetFieldMeta(
+                    MetaDataUtil.getVpnIdMetadata(internetBgpVpnId));
             listActionInfo.add(actionSetFieldMeta);
             listActionInfo.add(new ActionNxResubmit(NwConstants.L3_FIB_TABLE));
             instructionInfo.add(new InstructionApplyActions(listActionInfo));
         }
         String defaultIPv6 = "0:0:0:0:0:0:0:0";
-        String flowRef = getIPv6FlowRefL3(dpId, NwConstants.L3_FIB_TABLE, defaultIPv6, routerId);
+        String flowRef = getIPv6FlowRefL3(dpId, NwConstants.L3_FIB_TABLE, defaultIPv6, internetBgpVpnId);
 
         FlowEntity flowEntity = MDSALUtil.buildFlowEntity(dpId, NwConstants.L3_FIB_TABLE, flowRef,
                 NwConstants.TABLE_MISS_PRIORITY, flowRef/* "L3 ipv6 internet default route",*/, 0, 0,
@@ -70,17 +71,17 @@ public class IPV6InternetDefaultRouteProgrammer {
      * This method installs in the FIB table the default route for IPv6.
      *
      * @param dpnId of the compute node
-     * @param bgpVpnId internetVpn id as long
-     * @param routerId id of router associated to internet bgpvpn as long
+     * @param internetBgpVpnId internetVpn id as long
+     * @param vpnId id of router associated to internet bgpvpn as long
      */
-    public void installDefaultRoute(BigInteger dpnId, long bgpVpnId, long routerId) {
-        FlowEntity flowEntity = buildIPv6FallbacktoExternalVpn(dpnId, bgpVpnId, routerId, true);
+    public void installDefaultRoute(BigInteger dpnId, long internetBgpVpnId, long vpnId) {
+        FlowEntity flowEntity = buildIPv6FallbacktoExternalVpn(dpnId, internetBgpVpnId, vpnId, true);
         LOG.trace("installDefaultRoute: flowEntity: {} ", flowEntity);
         mdsalManager.installFlow(flowEntity);
     }
 
-    public void removeDefaultRoute(BigInteger dpnId, long bgpVpnId, long routerId) {
-        FlowEntity flowEntity = buildIPv6FallbacktoExternalVpn(dpnId, bgpVpnId, routerId, false);
+    public void removeDefaultRoute(BigInteger dpnId, long internetBgpVpnId, long vpnId) {
+        FlowEntity flowEntity = buildIPv6FallbacktoExternalVpn(dpnId, internetBgpVpnId, vpnId, false);
         LOG.trace("removeDefaultRoute: flowEntity: {} ", flowEntity);
         mdsalManager.removeFlow(flowEntity);
     }
index 9838454c897b04aff38456fc50793de768d2997b..c015a5de9e70eadf9b4fa7351ac5746976386ed6 100644 (file)
@@ -59,6 +59,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.elan
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.elan.interfaces.ElanInterfaceBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.elan.interfaces.ElanInterfaceKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.elan.interfaces.elan._interface.StaticMacEntries;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.vpn.instance.op.data.VpnInstanceOpDataEntry;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.natservice.rev160111.ext.routers.Routers;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.natservice.rev160111.ext.routers.RoutersBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.natservice.rev160111.floating.ip.port.info.FloatingIpIdToPortMappingBuilder;
@@ -438,6 +439,16 @@ public class NeutronPortChangeListener extends AsyncDataTreeChangeListenerBase<P
         if (isExternal) {
             Uuid vpnInternetId = neutronvpnUtils.getVpnForNetwork(networkId);
             if (vpnInternetId != null) {
+                if (isRtrGwRemoved) {
+                     //Set VPN type BGPVPNExternal from BGPVPNInternet for removal case
+                    neutronvpnUtils.updateVpnInstanceOpWithType(VpnInstanceOpDataEntry.BgpvpnType.BGPVPNExternal,
+                            vpnInternetId);
+                } else {
+                    //Set VPN type BGPVPNInternet from BGPVPNExternal for add case
+                    neutronvpnUtils.updateVpnInstanceOpWithType(VpnInstanceOpDataEntry.BgpvpnType.BGPVPNInternet,
+                            vpnInternetId);
+                    nvpnManager.updateVpnMaps(vpnInternetId, null, routerId, null, null);
+                }
                 List<Subnetmap> snList = neutronvpnUtils.getNeutronRouterSubnetMaps(routerId);
                 for (Subnetmap sn : snList) {
                     if (sn.getNetworkId() == networkId) {
@@ -447,16 +458,14 @@ public class NeutronPortChangeListener extends AsyncDataTreeChangeListenerBase<P
                         continue;
                     }
                     if (isRtrGwRemoved) {
-                        nvpnManager.removeV6PrivateSubnetToExtNetwork(vpnInternetId, sn);
+                        nvpnManager.removeV6PrivateSubnetToExtNetwork(routerId, vpnInternetId, sn);
                     } else {
-                        nvpnManager.addV6PrivateSubnetToExtNetwork(vpnInternetId, sn);
+                        nvpnManager.addV6PrivateSubnetToExtNetwork(routerId, vpnInternetId, sn);
                     }
                 }
-                //update VPN Maps with extRouterId in InternetBgpVpn
+                //Update Internet BGP-VPN
                 if (isRtrGwRemoved) {
                     nvpnManager.updateVpnMaps(vpnInternetId, null, null, null, null);
-                } else {
-                    nvpnManager.updateVpnMaps(vpnInternetId, null, routerId, null, null);
                 }
             }
         }
index 89abac66d910ee748b0c2a1e98cf8774f01eb1f9..5fdd9231c0e5ebad3655b97667a4ec7605da0186 100644 (file)
@@ -977,7 +977,7 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
         wrtConfigTxn.delete(vpnIfIdentifier);
     }
 
-    protected void removeVpnFromVpnInterface(Uuid vpnId, Port port,
+    protected void removeInternetVpnFromVpnInterface(Uuid vpnId, Port port,
                                              TypedWriteTransaction<Datastore.Configuration> writeConfigTxn,
                                              Subnetmap sm) {
         if (vpnId == null || port == null) {
@@ -1736,7 +1736,7 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
                     updateVpnInterface(vpn, null, neutronvpnUtils.getNeutronPort(
                             sm.getRouterInterfacePortId()), true, true, wrtConfigTxn);
                 } else {
-                    removeVpnFromVpnInterface(vpn,
+                    removeInternetVpnFromVpnInterface(vpn,
                             neutronvpnUtils.getNeutronPort(sm.getRouterInterfacePortId()), wrtConfigTxn, sm);
                 }
                 }
@@ -1756,7 +1756,7 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
                                 updateVpnInterface(vpn, null, neutronvpnUtils.getNeutronPort(port),
                                         true, false, tx);
                             } else {
-                                removeVpnFromVpnInterface(vpn, neutronvpnUtils.getNeutronPort(port), tx, sm);
+                                removeInternetVpnFromVpnInterface(vpn, neutronvpnUtils.getNeutronPort(port), tx, sm);
                             }
                         })));
             }
@@ -1776,7 +1776,13 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
             LOG.error("Updating subnet {} with newVpn {} failed", subnet.getValue(), newVpnId.getValue());
             return sn;
         }
-
+        /* vpnExtUuid will contain the value only on if the subnet is V6 and it is already been
+         * associated with internet BGP-VPN.
+         */
+        if (vpnExtUuid != null) {
+            //Update V6 Internet default route match with new VPN metadata
+            neutronvpnUtils.updateVpnInstanceWithFallback(vpnExtUuid, isBeingAssociated);
+        }
         //Update Router Interface first synchronously.
         //CAUTION:  Please DONOT make the router interface VPN Movement as an asynchronous commit again !
         ListenableFuture<Void> future =
@@ -2474,7 +2480,7 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
                 ipVersion = ipVersion.addVersion(ipVers);
             }
         }
-        if (ipVersion != IpVersionChoice.UNDEFINED && ipVersion != IpVersionChoice.IPV4) {
+        if (!ipVersion.isIpVersionChosen(IpVersionChoice.IPV4)) {
             neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(), IpVersionChoice.IPV6, true);
             LOG.info("associateExtNetworkToVpn: add IPv6 Internet default route in VPN {}", vpnId.getValue());
             neutronvpnUtils.updateVpnInstanceWithFallback(vpnId, true);
@@ -2602,7 +2608,7 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
                 ipVersion = ipVersion.addVersion(ipVers);
             }
         }
-        if (ipVersion != IpVersionChoice.UNDEFINED && ipVersion != IpVersionChoice.IPV4) {
+        if (!ipVersion.isIpVersionChosen(IpVersionChoice.IPV4)) {
             neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(), IpVersionChoice.IPV6, false);
             LOG.info("disassociateExtNetworkFromVpn: withdraw IPv6 Internet default route from VPN {}",
                     vpnId.getValue());
@@ -3319,29 +3325,27 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
         return message;
     }
 
-    protected void addV6PrivateSubnetToExtNetwork(@Nonnull Uuid internetVpnId, @Nonnull Subnetmap subnetMap) {
-        //Set VPN type BGPVPNInternet from BGPVPNExternal
-        LOG.info("addV6PrivateSubnetToExtNetwork: set type {} for Internet VPN {}",
-                BgpvpnType.BGPVPNInternet, internetVpnId.getValue());
-        neutronvpnUtils.updateVpnInstanceOpWithType(BgpvpnType.BGPVPNInternet, internetVpnId);
+    protected void addV6PrivateSubnetToExtNetwork(@Nonnull Uuid routerId, @Nonnull Uuid internetVpnId,
+                                                  @Nonnull Subnetmap subnetMap) {
         updateVpnInternetForSubnet(subnetMap, internetVpnId, true);
-
-        neutronvpnUtils.updateVpnInstanceWithIpFamily(internetVpnId.getValue(), IpVersionChoice.IPV6, true);
-        LOG.info("addV6PrivateSubnetToExtNetwork: Advertise IPv6 Private Subnet {} to Internet VPN {}",
-                subnetMap.getId(), internetVpnId.getValue());
+        if (neutronvpnUtils.shouldVpnHandleIpVersionChoiceChange(
+                IpVersionChoice.IPV6, routerId, true)) {
+            neutronvpnUtils.updateVpnInstanceWithIpFamily(internetVpnId.getValue(), IpVersionChoice.IPV6, true);
+            LOG.info("addV6PrivateSubnetToExtNetwork: Advertise IPv6 Private Subnet {} to Internet VPN {}",
+                    subnetMap.getId(), internetVpnId.getValue());
+        }
         neutronvpnUtils.updateVpnInstanceWithFallback(internetVpnId, true);
     }
 
-    protected void removeV6PrivateSubnetToExtNetwork(@Nonnull Uuid internetVpnId, @Nonnull Subnetmap subnetMap) {
-        //Set VPN type BGPVPNExternal from BGPVPNInternet
-        LOG.info("removeV6PrivateSubnetToExtNetwork: set type {} for Internet VPN {}",
-                VpnInstanceOpDataEntry.BgpvpnType.BGPVPNExternal, internetVpnId.getValue());
-        neutronvpnUtils.updateVpnInstanceOpWithType(VpnInstanceOpDataEntry.BgpvpnType.BGPVPNExternal, internetVpnId);
+    protected void removeV6PrivateSubnetToExtNetwork(@Nonnull Uuid routerId, @Nonnull Uuid internetVpnId,
+                                                     @Nonnull Subnetmap subnetMap) {
         updateVpnInternetForSubnet(subnetMap, internetVpnId, false);
-
-        neutronvpnUtils.updateVpnInstanceWithIpFamily(internetVpnId.getValue(), IpVersionChoice.IPV6, false);
-        LOG.info("removeV6PrivateSubnetToExtNetwork: withdraw IPv6 Private subnet {} from Internet VPN {}",
-                subnetMap.getId(), internetVpnId.getValue());
+        if (neutronvpnUtils.shouldVpnHandleIpVersionChoiceChange(
+                IpVersionChoice.IPV6, routerId, false)) {
+            neutronvpnUtils.updateVpnInstanceWithIpFamily(internetVpnId.getValue(), IpVersionChoice.IPV6, false);
+            LOG.info("removeV6PrivateSubnetToExtNetwork: withdraw IPv6 Private subnet {} from Internet VPN {}",
+                    subnetMap.getId(), internetVpnId.getValue());
+        }
         neutronvpnUtils.updateVpnInstanceWithFallback(internetVpnId, false);
     }
 }
index 6018fa9d1dc97ae181b60e4aeb02100cbfe1d1b6..c800f4988b48015206ad30b560c57e2a460bb6dc 100644 (file)
@@ -420,7 +420,7 @@ public class NeutronvpnNatManager implements AutoCloseable {
 
         // Remove the vpnInternetId fromSubnetmap
         Network net = neutronvpnUtils.getNeutronNetwork(nets.getId());
-        List<Uuid> submapIds = neutronvpnUtils.getPrivateSubnetsToExport(net, null);
+        List<Uuid> submapIds = neutronvpnUtils.getPrivateSubnetsToExport(net, /*internetVpnId*/ null);
         for (Uuid snId : submapIds) {
             Subnetmap subnetMap = neutronvpnUtils.getSubnetmap(snId);
             if (subnetMap == null || subnetMap.getInternetVpnId() == null) {
index db32ea27f2f5e13704fe4a5a4baa9b79b3051378..ba804acc39d21e73afdb1a2071f28eddd7380493 100644 (file)
@@ -284,8 +284,22 @@ public class NeutronvpnUtils {
                 if (routerIdsList == null || routerIdsList.isEmpty()) {
                     continue;
                 }
-                //Skip if current VPN is already associated with network
-                if (vpnMap.getNetworkIds() != null) {
+                boolean isInternetBgpVpn = false;
+                //Skip router vpnId fetching from internet BGP-VPN
+                if (vpnMap.getNetworkIds() != null && !vpnMap.getNetworkIds().isEmpty()) {
+                    for (Uuid netId: vpnMap.getNetworkIds()) {
+                        Network network = getNeutronNetwork(netId);
+                        if (getIsExternal(network)) {
+                            isInternetBgpVpn = true;
+                        }
+                        /* If first network is not a external network then no need iterate
+                         * whole network list from the VPN
+                         */
+                        break;
+                    }
+                }
+                if (isInternetBgpVpn) {
+                    //skip further processing
                     continue;
                 }
                 List<Uuid> rtrIdsList = routerIdsList.stream().map(routerIds -> routerIds.getRouterId())
@@ -1627,11 +1641,11 @@ public class NeutronvpnUtils {
     public @Nonnull List<Uuid> getPrivateSubnetsToExport(@Nonnull Network extNet, Uuid internetVpnId) {
         List<Uuid> subList = new ArrayList<>();
         List<Uuid> rtrList = new ArrayList<>();
-        Uuid extNwVpnId = getVpnForNetwork(extNet.getUuid());
-        if (extNwVpnId != null) {
-            rtrList.addAll(getRouterIdListforVpn(extNwVpnId));
-        } else if (internetVpnId != null) {
+        if (internetVpnId != null) {
             rtrList.addAll(getRouterIdListforVpn(internetVpnId));
+        } else {
+            Uuid extNwVpnId = getVpnForNetwork(extNet.getUuid());
+            rtrList.addAll(getRouterIdListforVpn(extNwVpnId));
         }
         if (rtrList == null || rtrList.isEmpty()) {
             return subList;
@@ -1661,7 +1675,7 @@ public class NeutronvpnUtils {
             LOG.error("updateVpnInstanceWithFallback: vpnInstanceOpDataEntry not found for vpn {}", vpnName);
             return;
         }
-        Long vpnId = vpnInstanceOpDataEntry.getVpnId();
+        Long internetBgpVpnId = vpnInstanceOpDataEntry.getVpnId();
         List<Uuid> routerIds = getRouterIdListforVpn(vpnName);
         if (routerIds == null || routerIds.isEmpty()) {
             LOG.error("updateVpnInstanceWithFallback: router not found for vpn {}", vpnName);
@@ -1677,11 +1691,19 @@ public class NeutronvpnUtils {
             }
             VpnInstanceOpDataEntry vpnOpDataEntry = getVpnInstanceOpDataEntryFromVpnId(rtrId.getValue());
             Long routerIdAsLong = vpnOpDataEntry.getVpnId();
+            long vpnId;
+            Uuid rtrVpnId = getVpnForRouter(rtrId, true);
+            if (rtrVpnId == null) {
+                //If external BGP-VPN is not associated with router then routerId is same as routerVpnId
+                vpnId = routerIdAsLong;
+            } else {
+                vpnId = getVpnId(rtrVpnId.getValue());
+            }
             for (BigInteger dpnId : dpnIds) {
                 if (add) {
-                    ipV6InternetDefRt.installDefaultRoute(dpnId, vpnId, routerIdAsLong);
+                    ipV6InternetDefRt.installDefaultRoute(dpnId, internetBgpVpnId, vpnId);
                 } else {
-                    ipV6InternetDefRt.removeDefaultRoute(dpnId, vpnId, routerIdAsLong);
+                    ipV6InternetDefRt.removeDefaultRoute(dpnId, internetBgpVpnId, vpnId);
                 }
             }
         }
@@ -1838,4 +1860,13 @@ public class NeutronvpnUtils {
                 networkId.getValue());
         return null;
     }
+
+    public long getVpnId(String vpnName) {
+        InstanceIdentifier<org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.vpn
+                .instance.to.vpn.id.VpnInstance> id = getVpnInstanceToVpnIdIdentifier(vpnName);
+        return SingleTransactionDataBroker.syncReadOptionalAndTreatReadFailedExceptionAsAbsentOptional(dataBroker,
+                LogicalDatastoreType.CONFIGURATION, id).toJavaUtil().map(
+                org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.vpn.instance.to.vpn.id
+                        .VpnInstance::getVpnId).orElse(null);
+    }
 }