From b4705322925b1eacfa3a2167bbd3e4444e3d3b21 Mon Sep 17 00:00:00 2001 From: Chetan Arakere Gowdru Date: Mon, 9 Sep 2019 18:11:40 +0530 Subject: [PATCH] Handling SNAT cluster reboot failure Description: During Cluster reboot, the ext-router listener got the add event following by update event(update of other parameters other then enable/disable SNAT). When add event is recevied, a DPN is tried to elect as NAPT switch, but since the CSS are not yet connect, Napt Switch was not elected. During update event, the Napt Switch was just elected without any installation of flow(as there is no changes in enable/disable snat) rersulting in failure. Changes are done to handle update() event properly to install SNAT related flows also when Napt Switch is elected. Change-Id: I64f47dddf077ce7818ae6c76a497419dedb975f1 Signed-off-by: Chetan Arakere Gowdru --- .../internal/ExternalRoutersListener.java | 94 ++++++++++++------- 1 file changed, 58 insertions(+), 36 deletions(-) diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ExternalRoutersListener.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ExternalRoutersListener.java index 1d978d65c1..776157108b 100644 --- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ExternalRoutersListener.java +++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ExternalRoutersListener.java @@ -344,9 +344,13 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase identifier, Routers original, Routers update) { + LOG.trace("update : origRouter: {} updatedRouter: {}", original, update); String routerName = original.getRouterName(); Long routerId = NatUtil.getVpnId(dataBroker, routerName); if (routerId == NatConstants.INVALID_ID) { LOG.error("update : external router event - Invalid routerId for routerName {}", routerName); return; } - // Check if its update on SNAT flag - boolean originalSNATEnabled = original.isEnableSnat(); - boolean updatedSNATEnabled = update.isEnableSnat(); - LOG.debug("update :called with originalFlag and updatedFlag for SNAT enabled " - + "as {} and {}", originalSNATEnabled, updatedSNATEnabled); - /* Get Primary Napt Switch for existing router from "router-to-napt-switch" DS. - * if dpnId value is null or zero then go for electing new Napt switch for existing router. - */ - long bgpVpnId = NatConstants.INVALID_ID; - Uuid bgpVpnUuid = NatUtil.getVpnForRouter(dataBroker, routerName); - if (bgpVpnUuid != null) { - bgpVpnId = NatUtil.getVpnId(dataBroker, bgpVpnUuid.getValue()); - } - BigInteger dpnId = getPrimaryNaptSwitch(routerName); - final long finalBgpVpnId = bgpVpnId; coordinator.enqueueJob(NatConstants.NAT_DJC_PREFIX + update.key(), () -> { List> futures = new ArrayList<>(); futures.add(txRunner.callWithNewReadWriteTransactionAndSubmit(CONFIGURATION, writeFlowInvTx -> { futures.add(txRunner.callWithNewReadWriteTransactionAndSubmit(CONFIGURATION, removeFlowInvTx -> { + long bgpVpnId = NatConstants.INVALID_ID; + Uuid bgpVpnUuid = NatUtil.getVpnForRouter(dataBroker, routerName); + if (bgpVpnUuid != null) { + bgpVpnId = NatUtil.getVpnId(dataBroker, bgpVpnUuid.getValue()); + } + //BigInteger dpnId = getPrimaryNaptSwitch(routerName); + /* Get Primary Napt Switch for existing router from "router-to-napt-switch" DS. + * if dpnId value is null or zero then go for electing new Napt switch for existing router. + */ + BigInteger dpnId = NatUtil.getPrimaryNaptfromRouterName(dataBroker, routerName); + boolean isPrimaryNaptSwitchNotSelected = (dpnId == null || dpnId.equals(BigInteger.ZERO)); Uuid networkId = original.getNetworkId(); - if (originalSNATEnabled != updatedSNATEnabled) { - if (originalSNATEnabled) { - if (dpnId == null || dpnId.equals(BigInteger.ZERO)) { - // Router has no interface attached + // Check if its update on SNAT flag + boolean originalSNATEnabled = original.isEnableSnat(); + boolean updatedSNATEnabled = update.isEnableSnat(); + LOG.debug("update :called with originalFlag and updatedFlag for SNAT enabled " + + "as {} and {} with Elected Dpn {}(isPrimaryNaptSwitchNotSelected:{})", + originalSNATEnabled, updatedSNATEnabled, dpnId, isPrimaryNaptSwitchNotSelected); + // Cluster Reboot Case Handling + // 1. DPN not elected during add event(due to none of the OVS connected) + // 2. Update event called with changes of parameters(but enableSnat is not changed) + // 3. First Elect dpnId and process other changes with valid dpnId + if (originalSNATEnabled != updatedSNATEnabled || isPrimaryNaptSwitchNotSelected) { + if (originalSNATEnabled && !updatedSNATEnabled) { + if (isPrimaryNaptSwitchNotSelected) { + LOG.info("No Action to be taken when SNAT is disabled " + + "with no Napt Switch Election for Router {}", routerName); return; } //SNAT disabled for the router @@ -1260,12 +1272,23 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase externalIps = NatUtil.getExternalIpsForRouter(dataBroker, routerId); handleDisableSnat(original, networkUuid, externalIps, false, null, dpnId, routerId, removeFlowInvTx); - } else { - LOG.info("update : SNAT enabled for Router {}", original.getRouterName()); - addOrDelDefFibRouteToSNAT(routerName, routerId, finalBgpVpnId, bgpVpnUuid, - true, writeFlowInvTx); - handleEnableSnat(update, routerId, dpnId, finalBgpVpnId, writeFlowInvTx); + } else if (updatedSNATEnabled) { + LOG.info("update : SNAT enabled for Router {}", routerName); + addOrDelDefFibRouteToSNAT(routerName, routerId, bgpVpnId, bgpVpnUuid, + true, writeFlowInvTx); + if (isPrimaryNaptSwitchNotSelected) { + dpnId = selectNewNAPTSwitch(routerName); + if (dpnId != null && !dpnId.equals(BigInteger.ZERO)) { + handleEnableSnat(update, routerId, dpnId, bgpVpnId, removeFlowInvTx); + } else { + LOG.error("update : Failed to elect Napt Switch During update event" + + " of router {}", routerName); + } + } } + LOG.info("update : no need to process external/subnet changes as it's will taken care" + + "in handleDisableSnat/handleEnableSnat"); + return; } if (!Objects.equals(original.getExtGwMacAddress(), update.getExtGwMacAddress())) { NatUtil.installRouterGwFlows(txRunner, vpnManager, original, dpnId, NwConstants.DEL_FLOW); @@ -1278,7 +1301,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase originalExternalIps = NatUtil.getIpsListFromExternalIps(original.getExternalIps()); List updatedExternalIps = NatUtil.getIpsListFromExternalIps(update.getExternalIps()); @@ -1286,8 +1309,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase removedExternalIps = new HashSet<>(originalExternalIps); removedExternalIps.removeAll(updatedExternalIps); if (removedExternalIps.size() > 0) { - LOG.debug("update : Start processing of the External IPs removal during the update " - + "operation"); + LOG.debug("update : Start processing of the External IPs removal for router {}", routerName); vpnManager.removeArpResponderFlowsToExternalNetworkIps(routerName, removedExternalIps, original.getExtGwMacAddress(), dpnId, networkId); @@ -1463,15 +1485,15 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase addedExternalIps = new HashSet<>(updatedExternalIps); addedExternalIps.removeAll(originalExternalIps); if (addedExternalIps.size() != 0) { - LOG.debug("update : Start processing of the External IPs addition during the update " - + "operation"); + LOG.debug("update : Start processing of the External IPs addition for router {}", + routerName); vpnManager.addArpResponderFlowsToExternalNetworkIps(routerName, addedExternalIps, update.getExtGwMacAddress(), dpnId, update.getNetworkId()); @@ -1498,7 +1520,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase originalSubnetIds = original.getSubnetIds(); List updatedSubnetIds = update.getSubnetIds(); Set addedSubnetIds = @@ -1510,7 +1532,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase