Select groups without any buckets for remote MIPs

Issue-1:
--------
Inconsistency observed in table-21 flows. For remote IPv4/IPv6 MIPs,
traffic for fixed IPs are sent directly to table-220. But
traffic for remote learnt/MIPs are sent to groups.

Issue-2:
--------
Sporadic issue wherein when MIPs are learnt, perform delete and add
Transport zone (ITM). The corresponding groups referred in table-21
flows for remote MIPs have Select Groups without any buckets in one
of the DPN.

Solution for both 1 and 2:
--------------------------
+ Remote learnt/MIPs were treated as extra routes, hence were sent to
  load-balancing groups (Select). With this fix now, even for remote
  learnt/MIPs, traffic is directly sent to table-220 from table-21.

+ Refactored code to minimize datastore reads.

JIRA: NETVIRT-1553

Change-Id: Idb71cc6e5e2ce3eb812e36e51a0987a204d778dc
Signed-off-by: Somashekar Byrappa <somashekar.b@altencalsoftlabs.com>

diff --git a/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/VrfEntryListener.java b/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/VrfEntryListener.java
old mode 100755 (executable)
new mode 100644 (file)
index 5431663..5838414
--- a/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/VrfEntryListener.java
+++ b/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/VrfEntryListener.java
@@ -14,6 +14,7 @@ import static org.opendaylight.genius.mdsalutil.NWUtil.isIpv4Address;
 
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
@@ -1192,59 +1193,71 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
         }
 
         String vpnName = fibUtil.getVpnNameFromId(vpnId);
-        LOG.debug("createremotefibentry: adding route {} for rd {} on remoteDpnId {}",
-                vrfEntry.getDestPrefix(), rd, remoteDpnId);
+        LOG.debug("createremotefibentry: adding route {} for rd {} on remoteDpnId {}", vrfEntry.getDestPrefix(), rd,
+                remoteDpnId);
 
-        List<AdjacencyResult> adjacencyResults = baseVrfEntryHandler.resolveAdjacency(remoteDpnId, vpnId, vrfEntry, rd);
-        if (adjacencyResults.isEmpty()) {
-            LOG.error("Could not get interface for route-paths: {} in vpn {} on DPN {}",
-                    vrfEntry.getRoutePaths(), rd, remoteDpnId);
-            LOG.error("Failed to add Route: {} in vpn: {}", vrfEntry.getDestPrefix(), rd);
+        if (RouteOrigin.value(vrfEntry.getOrigin()) != RouteOrigin.STATIC) {
+            programRemoteFibEntry(remoteDpnId, vpnId, rd, vrfEntry, tx);
             return;
         }
-
+        // Handling static VRF entries
         List<String> usedRds = VpnExtraRouteHelper.getUsedRds(dataBroker, vpnId, vrfEntry.getDestPrefix());
-        List<Routes> vpnExtraRoutes = VpnExtraRouteHelper.getAllVpnExtraRoutes(dataBroker,
-                vpnName, usedRds, vrfEntry.getDestPrefix());
-        // create loadbalancing groups for extra routes only when the extra route is present behind
-        // multiple VMs
+        List<Routes> vpnExtraRoutes =
+                VpnExtraRouteHelper.getAllVpnExtraRoutes(dataBroker, vpnName, usedRds, vrfEntry.getDestPrefix());
         if (!vpnExtraRoutes.isEmpty()) {
-            List<InstructionInfo> instructions = new ArrayList<>();
-            // Obtain the local routes for this particular dpn.
-            java.util.Optional<Routes> routes = vpnExtraRoutes
-                    .stream()
-                    .filter(route -> {
-                        Prefixes prefixToInterface = fibUtil.getPrefixToInterface(vpnId,
-                                FibUtil.getIpPrefix(route.getNexthopIpList().get(0)));
-                        if (prefixToInterface == null) {
-                            return false;
-                        }
-                        return remoteDpnId.equals(prefixToInterface.getDpnId());
-                    }).findFirst();
-            long groupId = nextHopManager.createNextHopGroups(vpnId, rd, remoteDpnId, vrfEntry,
-                    routes.isPresent() ? routes.get() : null, vpnExtraRoutes);
-            if (groupId == FibConstants.INVALID_GROUP_ID) {
-                LOG.error("Unable to create Group for local prefix {} on rd {} on Node {}",
-                        vrfEntry.getDestPrefix(), rd, remoteDpnId.toString());
-                return;
-            }
-            List<ActionInfo> actionInfos =
-                    Collections.singletonList(new ActionGroup(groupId));
-            instructions.add(new InstructionApplyActions(actionInfos));
-            String jobKey = FibUtil.getCreateRemoteNextHopJobKey(vpnId, remoteDpnId, vrfEntry.getDestPrefix());
-            jobCoordinator.enqueueJob(jobKey,
-                () ->  Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(txn -> {
-                    baseVrfEntryHandler.makeConnectedRoute(remoteDpnId, vpnId, vrfEntry, rd, instructions,
-                            NwConstants.ADD_FLOW, txn, null);
-                })));
+            programRemoteFibWithLoadBalancingGroups(remoteDpnId, vpnId, rd, vrfEntry, vpnExtraRoutes);
         } else {
-            baseVrfEntryHandler.programRemoteFib(remoteDpnId, vpnId, vrfEntry,
-                TransactionAdapter.toWriteTransaction(tx), rd, adjacencyResults, null);
+            // Program in case of other static VRF entries like floating IPs
+            programRemoteFibEntry(remoteDpnId, vpnId, rd, vrfEntry, tx);
         }
+    }
+
+    private void programRemoteFibWithLoadBalancingGroups(final BigInteger remoteDpnId, final long vpnId, String rd,
+            final VrfEntry vrfEntry, List<Routes> vpnExtraRoutes) {
+        // create loadbalancing groups for extra routes only when the extra route is
+        // present behind multiple VMs
+        // Obtain the local routes for this particular dpn.
+        java.util.Optional<Routes> routes = vpnExtraRoutes.stream().filter(route -> {
+            Prefixes prefixToInterface =
+                    fibUtil.getPrefixToInterface(vpnId, FibUtil.getIpPrefix(route.getNexthopIpList().get(0)));
+            if (prefixToInterface == null) {
+                return false;
+            }
+            return remoteDpnId.equals(prefixToInterface.getDpnId());
+        }).findFirst();
+        long groupId = nextHopManager.createNextHopGroups(vpnId, rd, remoteDpnId, vrfEntry,
+                routes.isPresent() ? routes.get() : null, vpnExtraRoutes);
+        if (groupId == FibConstants.INVALID_GROUP_ID) {
+            LOG.error("Unable to create Group for local prefix {} on rd {} on Node {}", vrfEntry.getDestPrefix(), rd,
+                    remoteDpnId);
+            return;
+        }
+        List<ActionInfo> actionInfos = Collections.singletonList(new ActionGroup(groupId));
+        List<InstructionInfo> instructions = Lists.newArrayList(new InstructionApplyActions(actionInfos));
+        String jobKey = FibUtil.getCreateRemoteNextHopJobKey(vpnId, remoteDpnId, vrfEntry.getDestPrefix());
+        jobCoordinator.enqueueJob(jobKey,
+            () -> Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(txn -> {
+                baseVrfEntryHandler.makeConnectedRoute(remoteDpnId, vpnId, vrfEntry, rd, instructions,
+                        NwConstants.ADD_FLOW, txn, null);
+            })));
 
         LOG.debug("Successfully added FIB entry for prefix {} in vpnId {}", vrfEntry.getDestPrefix(), vpnId);
     }
 
+    private void programRemoteFibEntry(final BigInteger remoteDpnId, final long vpnId, String rd,
+            final VrfEntry vrfEntry, TypedWriteTransaction<Configuration> tx) {
+        List<AdjacencyResult> adjacencyResults = baseVrfEntryHandler.resolveAdjacency(remoteDpnId, vpnId, vrfEntry, rd);
+        if (adjacencyResults.isEmpty()) {
+            LOG.error("Could not get interface for route-paths: {} in vpn {} on DPN {}", vrfEntry.getRoutePaths(), rd,
+                    remoteDpnId);
+            LOG.error("Failed to add Route: {} in vpn: {}", vrfEntry.getDestPrefix(), rd);
+            return;
+        }
+        baseVrfEntryHandler.programRemoteFib(remoteDpnId, vpnId, vrfEntry, TransactionAdapter.toWriteTransaction(tx),
+                rd, adjacencyResults, null);
+        LOG.debug("Successfully programmed FIB entry for prefix {} in vpnId {}", vrfEntry.getDestPrefix(), vpnId);
+    }
+
     protected void cleanUpOpDataForFib(Long vpnId, String primaryRd, final VrfEntry vrfEntry) {
     /* Get interface info from prefix to interface mapping;
         Use the interface info to get the corresponding vpn interface op DS entry,
@@ -1822,7 +1835,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                         //Is this fib route an extra route? If yes, get the nexthop which would be
                         //an adjacency in the vpn
                         Optional<Routes> extraRouteOptional = Optional.absent();
-                        if (usedRds.size() != 0) {
+                        if (RouteOrigin.value(vrfEntry.getOrigin()) == RouteOrigin.STATIC && usedRds.size() != 0) {
                             extraRouteOptional = VpnExtraRouteHelper.getVpnExtraroutes(dataBroker,
                                     fibUtil.getVpnNameFromId(vpnInstance.getVpnId()),
                                     usedRds.get(0), vrfEntry.getDestPrefix());

diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java
index 594e28cefce44e8e566713de30a417b33b814dfb..4f3081358b2de17d9175923c4783d314f538ba7f 100755 (executable)
--- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java
+++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java
@@ -871,8 +871,7 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
             // Please note that primary adjacency will use a subnet-gateway-mac-address that
             // can be different from the gateway-mac-address within the VRFEntry as the
             // gateway-mac-address is a superset.
-            RouteOrigin origin = nextHop.getAdjacencyType() == AdjacencyType.PrimaryAdjacency ? RouteOrigin.LOCAL
-                    : RouteOrigin.STATIC;
+            RouteOrigin origin = VpnUtil.getRouteOrigin(nextHop.getAdjacencyType());
             L3vpnInput input = new L3vpnInput().setNextHop(nextHop).setRd(rd).setVpnName(vpnName)
                 .setInterfaceName(interfaceName).setNextHopIp(nextHopIp).setPrimaryRd(primaryRd)
                 .setSubnetGatewayMacAddress(vpnInterfaceSubnetGwMacAddress).setRouteOrigin(origin);
@@ -903,8 +902,7 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
         for (Adjacency nextHop : aug.getAdjacency()) {
             // Adjacencies other than primary Adjacencies are handled in the addExtraRoute call above.
             if (nextHop.getAdjacencyType() == AdjacencyType.PrimaryAdjacency) {
-                RouteOrigin origin = nextHop.getAdjacencyType() == AdjacencyType.PrimaryAdjacency ? RouteOrigin.LOCAL
-                        : RouteOrigin.STATIC;
+                RouteOrigin origin = VpnUtil.getRouteOrigin(nextHop.getAdjacencyType());
                 input.setNextHop(nextHop).setRd(nextHop.getVrfId()).setRouteOrigin(origin);
                 registeredPopulator.populateFib(input, writeConfigTxn);
             }

diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnUtil.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnUtil.java
index 00c990f569081f1f48241190cec93a9b4eaa7895..925b14d7d1569e8fb5e40e922f66d95e0b29f561 100644 (file)
--- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnUtil.java
+++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnUtil.java
@@ -171,6 +171,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.Vpn
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.VpnInterfaceOpData;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.VpnToExtraroutes;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.adjacency.list.Adjacency;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.adjacency.list.Adjacency.AdjacencyType;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.adjacency.list.AdjacencyKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.learnt.vpn.vip.to.port.data.LearntVpnVipToPort;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.learnt.vpn.vip.to.port.data.LearntVpnVipToPortBuilder;
@@ -2359,6 +2360,24 @@ public final class VpnUtil {
         });
     }
 
+    public static RouteOrigin getRouteOrigin(AdjacencyType adjacencyType) {
+        RouteOrigin origin = RouteOrigin.LOCAL;
+        switch (adjacencyType) {
+            case PrimaryAdjacency:
+                origin = RouteOrigin.LOCAL;
+                break;
+            case ExtraRoute:
+                origin = RouteOrigin.STATIC;
+                break;
+            case LearntIp:
+                origin = RouteOrigin.DYNAMIC;
+                break;
+            default:
+                LOG.warn("Unknown adjacencyType={}", adjacencyType);
+        }
+        return origin;
+    }
+
     public static boolean isDualRouterVpnUpdate(List<String> oldVpnListCopy, List<String> newVpnListCopy) {
         return oldVpnListCopy.size() == 2 && newVpnListCopy.size() == 3
                 || oldVpnListCopy.size() == 3 && newVpnListCopy.size() == 2;