From 20849f22ccac3c3f94c01fa3068080ee22e2b75b Mon Sep 17 00:00:00 2001 From: Vishal Thapar Date: Fri, 15 May 2015 04:40:51 +0530 Subject: [PATCH 1/1] Partial fix for remoteNextHop 1. Adds flows at time of interface creation. No need to add tunnel interface to any vpn instance. 2. Removes some superflous code 3. Adds retry mechanism: https://git.opendaylight.org/gerrit/20370 Change-Id: I17da5ade46d73abfa22878de2ccbad37803b2e8a Signed-off-by: Vishal Thapar --- .../vpnservice/nexthopmgr/NexthopManager.java | 124 +++++++++++------- .../nexthopmgr/test/NexthopManagerTest.java | 10 +- 2 files changed, 83 insertions(+), 51 deletions(-) diff --git a/nexthopmgr/nexthopmgr-impl/src/main/java/org/opendaylight/vpnservice/nexthopmgr/NexthopManager.java b/nexthopmgr/nexthopmgr-impl/src/main/java/org/opendaylight/vpnservice/nexthopmgr/NexthopManager.java index 8ea89397..e5aa70c1 100644 --- a/nexthopmgr/nexthopmgr-impl/src/main/java/org/opendaylight/vpnservice/nexthopmgr/NexthopManager.java +++ b/nexthopmgr/nexthopmgr-impl/src/main/java/org/opendaylight/vpnservice/nexthopmgr/NexthopManager.java @@ -7,16 +7,21 @@ */ package org.opendaylight.vpnservice.nexthopmgr; - +import org.opendaylight.vpnservice.mdsalutil.FlowEntity; +import org.opendaylight.vpnservice.mdsalutil.InstructionInfo; +import org.opendaylight.vpnservice.mdsalutil.InstructionType; +import org.opendaylight.vpnservice.mdsalutil.MatchFieldType; +import org.opendaylight.vpnservice.mdsalutil.MatchInfo; +import org.opendaylight.vpnservice.mdsalutil.MetaDataUtil; +import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces.Interface; +import org.opendaylight.yang.gen.v1.urn.opendaylight.vpnservice.interfacemgr.rev150331.L3tunnel; import java.math.BigInteger; import java.util.ArrayList; import java.util.List; import java.util.concurrent.Future; - import com.google.common.base.Optional; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.FutureCallback; - import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.yangtools.yang.binding.DataObject; @@ -48,9 +53,7 @@ import org.opendaylight.vpnservice.mdsalutil.GroupEntity; import org.opendaylight.vpnservice.mdsalutil.MDSALUtil; import org.opendaylight.vpnservice.mdsalutil.interfaces.IMdsalApiManager; import org.opendaylight.idmanager.IdManager; - import java.util.concurrent.ExecutionException; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,6 +63,10 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { private IMdsalApiManager mdsalManager; private IInterfaceManager interfaceManager; private IdManager idManager; + private static final short LPORT_INGRESS_TABLE = 0; + private static final short LFIB_TABLE = 20; + private static final short FIB_TABLE = 21; + private static final short DEFAULT_FLOW_PRIORITY = 10; private static final FutureCallback DEFAULT_CALLBACK = new FutureCallback() { @@ -161,7 +168,7 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { long vpnId = getVpnId(vpnName); long dpnId = interfaceManager.getDpnForInterface(ifName); - VpnNexthop nexthop = getVpnNexthop(vpnId, ipAddress); + VpnNexthop nexthop = getVpnNexthop(vpnId, ipAddress, 0); LOG.trace("nexthop: {}", nexthop); if (nexthop == null) { List listBucketInfo = new ArrayList(); @@ -196,7 +203,6 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { long dpnId = getDpnId(ofPortId); TunnelNexthop nexthop = getTunnelNexthop(dpnId, ipAddress); if (nexthop == null) { - List listBucketInfo = new ArrayList(); List listActionInfo = interfaceManager.getInterfaceEgressActions(ifName); BucketInfo bucket = new BucketInfo(listActionInfo); @@ -205,6 +211,7 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { GroupEntity groupEntity = MDSALUtil.buildGroupEntity( dpnId, groupId, ipAddress, GroupTypes.GroupIndirect, listBucketInfo); mdsalManager.installGroup(groupEntity); + addRemoteFlow(dpnId, ifName); //update MD-SAL DS addTunnelNexthopToDS(dpnId, ipAddress, groupId); @@ -213,20 +220,39 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { } } + private void addRemoteFlow(long dpId, String ifName) { + + long portNo = interfaceManager.getPortForInterface(ifName); + String flowRef = getTunnelInterfaceFlowRef(dpId, LPORT_INGRESS_TABLE, portNo); + + String flowName = ifName; + BigInteger COOKIE_VM_INGRESS_TABLE = new BigInteger("8000001", 16); + + int priority = DEFAULT_FLOW_PRIORITY; + short gotoTableId = LFIB_TABLE; + + List mkInstructions = new ArrayList(); + mkInstructions.add(new InstructionInfo(InstructionType.goto_table, new long[] { gotoTableId })); + + List matches = new ArrayList(); + matches.add(new MatchInfo(MatchFieldType.in_port, new long[] { + dpId, portNo })); + + FlowEntity flowEntity = MDSALUtil.buildFlowEntity(dpId, LPORT_INGRESS_TABLE, flowRef, + priority, flowName, 0, 0, COOKIE_VM_INGRESS_TABLE, matches, mkInstructions); + + mdsalManager.installFlow(flowEntity); + } + + private String getTunnelInterfaceFlowRef(long dpId, short tableId, long portNo) { + return new StringBuilder().append(dpId).append(tableId).append(portNo).toString(); + } + protected void addVpnNexthopToDS(long vpnId, String ipPrefix, long egressPointer) { InstanceIdentifierBuilder idBuilder = InstanceIdentifier.builder(L3nexthop.class) .child(VpnNexthops.class, new VpnNexthopsKey(vpnId)); - // check if vpn node is there or to be created - InstanceIdentifier id = idBuilder.build(); - Optional nexthops = read(LogicalDatastoreType.OPERATIONAL, id); - if (!nexthops.isPresent()) { - // create a new node - VpnNexthops node = new VpnNexthopsBuilder().setKey(new VpnNexthopsKey(vpnId)).setVpnId(vpnId).build(); - asyncWrite(LogicalDatastoreType.OPERATIONAL, id, node, DEFAULT_CALLBACK); - } - // Add nexthop to vpn node VpnNexthop nh = new VpnNexthopBuilder(). setKey(new VpnNexthopKey(ipPrefix)). @@ -236,7 +262,7 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { InstanceIdentifier id1 = idBuilder .child(VpnNexthop.class, new VpnNexthopKey(ipPrefix)).build(); LOG.trace("Adding vpnnextHop {} to Operational DS", nh); - asyncWrite(LogicalDatastoreType.OPERATIONAL, id1, nh, DEFAULT_CALLBACK); + syncWrite(LogicalDatastoreType.OPERATIONAL, id1, nh, DEFAULT_CALLBACK); } @@ -244,19 +270,6 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { InstanceIdentifierBuilder idBuilder = InstanceIdentifier.builder(L3nexthop.class) .child(TunnelNexthops.class, new TunnelNexthopsKey(dpnId)); - // check if dpn node is there or to be created - InstanceIdentifier id = idBuilder.build(); - Optional nexthops = read(LogicalDatastoreType.OPERATIONAL, id); - if (!nexthops.isPresent()) { - // create a new node - TunnelNexthops node = new TunnelNexthopsBuilder() - .setKey(new TunnelNexthopsKey(dpnId)) - .setDpnId(dpnId) - .build(); - LOG.trace("Adding tunnelnextHop {} to Operational DS for a new node", node); - asyncWrite(LogicalDatastoreType.OPERATIONAL, id, node, DEFAULT_CALLBACK); - } - // Add nexthop to dpn node TunnelNexthop nh = new TunnelNexthopBuilder(). setKey(new TunnelNexthopKey(ipPrefix)). @@ -270,26 +283,33 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { } - protected VpnNexthop getVpnNexthop(long vpnId, String ipAddress) { + protected VpnNexthop getVpnNexthop(long vpnId, String ipAddress, int retryCount) { - // check if vpn node is there - InstanceIdentifierBuilder idBuilder = InstanceIdentifier.builder(L3nexthop.class) - .child(VpnNexthops.class, new VpnNexthopsKey(vpnId)); + // check if vpn node is there + InstanceIdentifierBuilder idBuilder = + InstanceIdentifier.builder(L3nexthop.class).child(VpnNexthops.class, new VpnNexthopsKey(vpnId)); InstanceIdentifier id = idBuilder.build(); - Optional vpnNexthops = read(LogicalDatastoreType.OPERATIONAL, id); - if (vpnNexthops.isPresent()) { - - // get nexthops list for vpn - List nexthops = vpnNexthops.get().getVpnNexthop(); - for (VpnNexthop nexthop : nexthops) { - if (nexthop.getIpAddress().equals(ipAddress)) { - // return nexthop - LOG.trace("VpnNextHop : {}",nexthop); - return nexthop; + try { + for (int retry = 0; retry <= retryCount; retry++) { + Optional vpnNexthops = read(LogicalDatastoreType.OPERATIONAL, id); + if (vpnNexthops.isPresent()) { + + // get nexthops list for vpn + List nexthops = vpnNexthops.get().getVpnNexthop(); + for (VpnNexthop nexthop : nexthops) { + if (nexthop.getIpAddress().equals(ipAddress)) { + // return nexthop + LOG.trace("VpnNextHop : {}", nexthop); + return nexthop; + } + } } + Thread.sleep(100L); } + } catch (InterruptedException e) { + LOG.trace("", e); } - //return null if not found + // return null if not found return null; } @@ -316,7 +336,7 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { public long getNextHopPointer(long dpnId, long vpnId, String prefixIp, String nextHopIp) { String endpointIp = interfaceManager.getEndpointIpForDpn(dpnId); if (nextHopIp.equals(endpointIp)) { - VpnNexthop vpnNextHop = getVpnNexthop(vpnId, prefixIp); + VpnNexthop vpnNextHop = getVpnNexthop(vpnId, prefixIp, 0); return vpnNextHop.getEgressPointer(); } else { TunnelNexthop tunnelNextHop = getTunnelNexthop(dpnId, nextHopIp); @@ -350,7 +370,7 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { public void removeLocalNextHop(Long dpId, Long vpnId, String ipAddress) { - VpnNexthop nh = getVpnNexthop(vpnId, ipAddress); + VpnNexthop nh = getVpnNexthop(vpnId, ipAddress, 0); if (nh != null) { // how to inform and remove dependent FIB entries?? // we need to do it before the group is removed @@ -397,7 +417,7 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { String endpointIp = interfaceManager.getEndpointIpForDpn(input.getDpnId()); LOG.trace("getEgressPointer: input {}, endpointIp {}", input, endpointIp); if (input.getNexthopIp().equals(endpointIp)) { - VpnNexthop vpnNextHop = getVpnNexthop(input.getVpnId(), input.getIpPrefix()); + VpnNexthop vpnNextHop = getVpnNexthop(input.getVpnId(), input.getIpPrefix(), 5); output.setEgressPointer(vpnNextHop.getEgressPointer()); output.setLocalDestination(true); } else { @@ -435,6 +455,12 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { Futures.addCallback(tx.submit(), callback); } + private void syncWrite(LogicalDatastoreType datastoreType, + InstanceIdentifier path, T data, FutureCallback callback) { + WriteTransaction tx = broker.newWriteOnlyTransaction(); + tx.put(datastoreType, path, data, true); + tx.submit(); + } private void delete(LogicalDatastoreType datastoreType, InstanceIdentifier path) { WriteTransaction tx = broker.newWriteOnlyTransaction(); @@ -444,7 +470,7 @@ public class NexthopManager implements L3nexthopService, AutoCloseable { @Override public Future> removeLocalNextHop(RemoveLocalNextHopInput input) { - VpnNexthop vpnNextHop = getVpnNexthop(input.getVpnId(), input.getIpPrefix()); + VpnNexthop vpnNextHop = getVpnNexthop(input.getVpnId(), input.getIpPrefix(), 0); RpcResultBuilder rpcResultBuilder; LOG.debug("vpnnexthop is: {}", vpnNextHop); try { diff --git a/nexthopmgr/nexthopmgr-impl/src/test/java/org/opendaylight/vpnservice/nexthopmgr/test/NexthopManagerTest.java b/nexthopmgr/nexthopmgr-impl/src/test/java/org/opendaylight/vpnservice/nexthopmgr/test/NexthopManagerTest.java index 48ba2bac..d6540f9e 100644 --- a/nexthopmgr/nexthopmgr-impl/src/test/java/org/opendaylight/vpnservice/nexthopmgr/test/NexthopManagerTest.java +++ b/nexthopmgr/nexthopmgr-impl/src/test/java/org/opendaylight/vpnservice/nexthopmgr/test/NexthopManagerTest.java @@ -12,9 +12,12 @@ import static org.mockito.Matchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; +import org.opendaylight.yang.gen.v1.urn.opendaylight.vpnservice.l3nexthop.rev150409.L3nexthop; +import org.opendaylight.yang.gen.v1.urn.opendaylight.vpnservice.l3nexthop.rev150409.l3nexthop.VpnNexthopsKey; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier.InstanceIdentifierBuilder; + import com.google.common.base.Optional; import com.google.common.util.concurrent.Futures; - import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.HashMap; @@ -22,7 +25,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -189,6 +191,10 @@ public class NexthopManagerTest { doReturn(Futures.immediateCheckedFuture(vpnIf)).when(mockReadTx).read(LogicalDatastoreType.OPERATIONAL, vpnInterfaceIdent); + InstanceIdentifierBuilder idBuilder = + InstanceIdentifier.builder(L3nexthop.class).child(VpnNexthops.class, new VpnNexthopsKey(vpnId)); + InstanceIdentifier id = idBuilder.build(); + doReturn(Futures.immediateCheckedFuture(Optional.absent())).when(mockReadTx).read(LogicalDatastoreType.OPERATIONAL,id); when(interfacemanager.getDpnForInterface(ifName)).thenReturn(dpId); dataChangeEvent.created.put(adjacencies, adjacency); -- 2.36.6