From edd0d9fa0ab45a935485b354330f2e27bb87400c Mon Sep 17 00:00:00 2001 From: Manisha Malla Date: Fri, 24 Apr 2015 17:52:57 +0530 Subject: [PATCH] BgpManager commit for code refactoring & bug fixes Change-Id: If9b1d5d49b7b527827a4c55fdd9558046383d2b8 Signed-off-by: Manisha Malla --- .../bgpmanager/BgpConfigurationManager.java | 134 +++++++----------- .../opendaylight/bgpmanager/BgpManager.java | 51 ++----- .../opendaylight/bgpmanager/FibDSWriter.java | 4 +- .../bgpmanager/globals/BgpConfiguration.java | 2 +- .../client/implementation/BgpRouter.java | 2 +- .../implementation/BgpUpdateHandler.java | 8 -- 6 files changed, 70 insertions(+), 131 deletions(-) diff --git a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpConfigurationManager.java b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpConfigurationManager.java index b12f9278..21e4cb01 100644 --- a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpConfigurationManager.java +++ b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpConfigurationManager.java @@ -40,6 +40,9 @@ public class BgpConfigurationManager { private BgpManager bgpManager; private final DataBroker broker; private static final int MAX_RETRIES_BGP_COMMUNICATION = 1; + private enum BgpOp { + START_BGP, ADD_NGHBR, DEL_NGHBR + } public BgpConfigurationManager(final DataBroker db, BgpConfiguration bgpCfg, BgpManager bgpMgr) { broker = db; @@ -68,6 +71,10 @@ public class BgpConfigurationManager { private synchronized void removeBgpRouter(BgpRouter del) { + bgpManager.disconnect(); + + bgpConfiguration.setRouterId(""); + bgpConfiguration.setAsNum(0); } @@ -93,23 +100,7 @@ public class BgpConfigurationManager { } if(bgpConfiguration.isConfigUpdated()) { - int retryCount = 0; - boolean retry = false; - do { - try { - if(retry) - LOG.info("Retrying BgpService start.." + "retry count:" + retryCount); - //bgpManager.waitForBgpInit(); - bgpManager.startBgpService(); - LOG.info("BgpService start done.."); - retry = false; - } catch (TException t) { - LOG.info("BgpService start failed.."); - retry = true; - retryCount++; - } - } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION); - + configureBgpServer(BgpOp.START_BGP); bgpConfiguration.unsetConfigUpdated(); } @@ -135,19 +126,7 @@ public class BgpConfigurationManager { if(value.getLocalAsNumber() == null || value.getLocalAsIdentifier() == null) return; - int retryCount = 0; - boolean retry = false; - - do { - try { - //bgpManager.waitForBgpInit(); - bgpManager.startBgpService(); - retry = false; - } catch (TException t) { - retry = true; - retryCount++; - } - } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION); + configureBgpServer(BgpOp.START_BGP); } @Override @@ -203,21 +182,11 @@ public class BgpConfigurationManager { if ((gateway.getPeerAddressType() != null) && (gateway.getPeerAddressType() instanceof org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.bgp.rev130715.bgp.neighbors.bgp.neighbor.peer.address.type.IpAddress)) { IpAddress neighborIPAddr = ((org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.bgp.rev130715.bgp.neighbors.bgp.neighbor.peer.address.type.IpAddress) gateway.getPeerAddressType()).getIpAddress(); LOG.info("Bgp Neighbor IP Address " + neighborIPAddr.getIpv4Address().getValue()); - bgpConfiguration.setNeighbourIp(""); - bgpConfiguration.setNeighbourAsNum(0); - int retryCount = 0; - boolean retry = false; + configureBgpServer(BgpOp.DEL_NGHBR); - do { - try { - bgpManager.deleteNeighbor(neighborIPAddr.getIpv4Address().getValue()); - retry = false; - } catch (TException t) { - retry = true; - retryCount++; - } - } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION); + bgpConfiguration.setNeighbourIp(""); + bgpConfiguration.setNeighbourAsNum(0); } } @@ -233,12 +202,24 @@ public class BgpConfigurationManager { } private synchronized void updateBgpNeighbors(BgpNeighbors original, BgpNeighbors update) { + List bgpNeighborList = update.getBgpNeighbor(); + //handle the case where there are no neighbors configured - single neighbor entry has been deleted + if(bgpNeighborList.isEmpty()) { + configureBgpServer(BgpOp.DEL_NGHBR); + return; + } + //We will always consider the first element of this list, since there can be just one DC Gateway BgpNeighbor gateway = bgpNeighborList.get(0); if(gateway != null) { + if(gateway.getAsNumber() != null || + ((gateway.getPeerAddressType() != null) && (gateway.getPeerAddressType() instanceof org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.bgp.rev130715.bgp.neighbors.bgp.neighbor.peer.address.type.IpAddress))) { + //there is an updated neighbor, so we need to delete the old neighbor + configureBgpServer(BgpOp.DEL_NGHBR); + } if(gateway.getAsNumber() != null) { LOG.info("Bgp Neighbor AS number " + gateway.getAsNumber()); if(bgpConfiguration.getNeighbourAsNum() != gateway.getAsNumber()) { @@ -257,33 +238,8 @@ public class BgpConfigurationManager { } } if(bgpConfiguration.isConfigUpdated()) { - //delete old neighbor - BgpNeighbor old = original.getBgpNeighbor().get(0); - IpAddress oldIPAddr = ((org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.bgp.rev130715.bgp.neighbors.bgp.neighbor.peer.address.type.IpAddress)old.getPeerAddressType()).getIpAddress(); - int retryCount = 0; - boolean retry = false; - do { - try { - bgpManager.deleteNeighbor(oldIPAddr.getIpv4Address().getValue()); - retry = false; - } catch (TException t) { - retry = true; - retryCount++; - } - } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION); - //add the newly configured neighbor - retryCount = 0; - retry = false; - do { - try { - bgpManager.addNeighbor(bgpConfiguration.getNeighbourIp(), bgpConfiguration.getNeighbourAsNum()); - retry = false; - } catch (TException t) { - retry = true; - retryCount++; - } - } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION); + configureBgpServer(BgpOp.ADD_NGHBR); } } @@ -315,17 +271,7 @@ public class BgpConfigurationManager { } if(bgpConfiguration.getNeighbourAsNum() != 0 && bgpConfiguration.getNeighbourIp() != null) { - int retryCount = 0; - boolean retry = false; - do { - try { - bgpManager.addNeighbor(bgpConfiguration.getNeighbourIp(), bgpConfiguration.getNeighbourAsNum()); - retry = false; - } catch (TException t) { - retry = true; - retryCount++; - } - } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION); + configureBgpServer(BgpOp.ADD_NGHBR); } } @@ -344,6 +290,7 @@ public class BgpConfigurationManager { return InstanceIdentifier.create(BgpNeighbors.class); } + @Override public void close() throws Exception { if (listenerRegistration != null) { @@ -358,4 +305,31 @@ public class BgpConfigurationManager { } } + + private void configureBgpServer(BgpOp bgpOp) { + int retryCount = 0; + boolean retry = false; + do { + try { + switch(bgpOp) { + case START_BGP: + bgpManager.startBgpService(); + break; + case ADD_NGHBR: + bgpManager.addNeighbor(bgpConfiguration.getNeighbourIp(), bgpConfiguration.getNeighbourAsNum()); + break; + case DEL_NGHBR: + bgpManager.deleteNeighbor(bgpConfiguration.getNeighbourIp()); + break; + default: + LOG.info("Invalid configuration option"); + } + + retry = false; + } catch (TException t) { + retry = true; + retryCount++; + } + } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION); + } } diff --git a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpManager.java b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpManager.java index d4fa53cc..cbcaa2e5 100644 --- a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpManager.java +++ b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpManager.java @@ -120,12 +120,13 @@ public class BgpManager implements BindingAwareProvider, AutoCloseable, IBgpMana } } catch (TException t) { - //s_logger.error("Transport error while starting bgp server ", t); s_logger.error("Could not set up thrift connection with bgp server"); + //s_logger.trace("Transport error while starting bgp server ", t); reInitConn(); throw t; } catch (Exception e) { - s_logger.error("Error while starting bgp server", e); + s_logger.error("Error while starting bgp server"); + //s_logger.trace("Bgp Service not started due to exception", e); return; } @@ -259,13 +260,12 @@ public class BgpManager implements BindingAwareProvider, AutoCloseable, IBgpMana @Override public void addPrefix(String rd, String prefix, String nextHop, int vpnLabel) throws Exception { - if(bgpThriftClient == null) { - s_logger.info("Add BGP prefix - bgpThriftClient is null. Unable to add BGP prefix."); - return; - } - if(!hasBgpServiceStarted) { + + if(bgpThriftClient == null || !hasBgpServiceStarted) { fibDSWriter.addFibEntryToDS(rd, prefix, nextHop, vpnLabel); + return; } + try { bgpThriftClient.addPrefix(rd, prefix, nextHop, vpnLabel); } catch (BgpRouterException b) { @@ -287,13 +287,11 @@ public class BgpManager implements BindingAwareProvider, AutoCloseable, IBgpMana @Override public void deletePrefix(String rd, String prefix) throws Exception { - if(bgpThriftClient == null) { - s_logger.info("Delete BGP prefix - bgpThriftClient is null. Unable to delete BGP prefix."); - return; - } - if(!hasBgpServiceStarted) { + if(bgpThriftClient == null || !hasBgpServiceStarted) { fibDSWriter.removeFibEntryFromDS(rd, prefix); + return; } + try { bgpThriftClient.delPrefix(rd, prefix); } catch (BgpRouterException b) { @@ -326,14 +324,14 @@ public class BgpManager implements BindingAwareProvider, AutoCloseable, IBgpMana s_logger.info("Connected to BGP server " + host + " on port " + port); } catch (BgpRouterException b) { s_logger.error("Failed to connect to BGP server " + host + " on port " + port + " due to BgpRouter Exception number " + b.getErrorCode()); - s_logger.error("BgpRouterException trace ", b); + //_logger.error("BgpRouterException trace ", b); throw b; } catch (TException t) { - s_logger.error("Failed to initialize BGP Connection due to Transport error ", t); + s_logger.error("Failed to initialize BGP Connection due to Transport error "); throw t; } catch (Exception e) { - s_logger.error("Failed to initialize BGP Connection ", e); + s_logger.error("Failed to initialize BGP Connection "); throw e; } } @@ -521,29 +519,6 @@ public class BgpManager implements BindingAwareProvider, AutoCloseable, IBgpMana public void disconnect() { bgpThriftClient.disconnect(); } -/* - public void setRoute(Route r) { - s_logger.info("Setting route in VPN Manager"); - //l3Manager.getVpnInstanceManager().addRoute(r.getRd(), r.getPrefix(), r.getNexthop(), r.getLabel()); - }*/ - - /* For testing purposes */ - /*public String ribGet() { - String family = "ipv4"; - String format = "json"; - - try { - List routeList = bgpThriftClient.getRoutes(); - Iterator iter = routeList.iterator(); - while(iter.hasNext()) { - Route r = iter.next(); - System.out.println("Route:: vrf:" + r.getRd() + " Prefix: " + r.getPrefix() + " Nexthop: " + r.getNexthop() + "Label: " + r.getLabel()); - } - } catch (Exception e) { - s_logger.error("Failed getting bgp routes ", e); - } - return null; - }*/ } diff --git a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/FibDSWriter.java b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/FibDSWriter.java index 6737385a..8cab6157 100644 --- a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/FibDSWriter.java +++ b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/FibDSWriter.java @@ -48,7 +48,6 @@ public class FibDSWriter { InstanceIdentifierBuilder idBuilder = InstanceIdentifier.builder(FibEntries.class).child(VrfTables.class, new VrfTablesKey(rd)); - logger.info("Created idBuilder for VrfTables"); InstanceIdentifier vrfTableId = idBuilder.build(); Optional vrfTable = read(LogicalDatastoreType.CONFIGURATION, vrfTableId); @@ -69,11 +68,10 @@ public class FibDSWriter { VrfTables vrfTableNew = new VrfTablesBuilder().setRouteDistinguisher(rd). setVrfEntry(vrfEntryList).build(); - logger.info("Created VrfTables"); InstanceIdentifier vrfTableNewId = InstanceIdentifier.builder(FibEntries.class) .child(VrfTables.class, new VrfTablesKey(rd)).build(); - logger.info("Created idBuilder for new VrfTables"); + write(LogicalDatastoreType.CONFIGURATION, vrfTableNewId, vrfTableNew); } diff --git a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/globals/BgpConfiguration.java b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/globals/BgpConfiguration.java index ae8bfb4b..65d4f0a5 100644 --- a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/globals/BgpConfiguration.java +++ b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/globals/BgpConfiguration.java @@ -10,7 +10,7 @@ package org.opendaylight.bgpmanager.globals; public class BgpConfiguration { - long asNum; + long asNum = 0; String bgpServer = ""; int bgpPort; String routerId = ""; diff --git a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/client/implementation/BgpRouter.java b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/client/implementation/BgpRouter.java index 82ac98d8..181e2664 100644 --- a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/client/implementation/BgpRouter.java +++ b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/client/implementation/BgpRouter.java @@ -78,7 +78,7 @@ public class BgpRouter { logger.info("Connecting to BGP Server " + bgpHost + " on port " + bgpPort); reInit(); } catch (Exception e) { - logger.error("Failed connecting to BGP server ", e); + logger.error("Failed connecting to BGP server "); throw e; } } diff --git a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/server/implementation/BgpUpdateHandler.java b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/server/implementation/BgpUpdateHandler.java index d0825d9d..a9aecccc 100644 --- a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/server/implementation/BgpUpdateHandler.java +++ b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/server/implementation/BgpUpdateHandler.java @@ -15,14 +15,6 @@ class BgpUpdateHandler implements BgpUpdater.Iface { public BgpUpdateHandler(BgpManager bgpMgr, FibDSWriter dsWriter) { bgpManager = bgpMgr; fibDSWriter = dsWriter; - - //Test - onUpdatePushRoute("5", "10.1.1.2", 32, "1.2.3.4", 200); - onUpdatePushRoute("5", "10.1.1.3", 32, "1.2.3.5", 400); - onUpdatePushRoute("10", "10.10.0.10", 32, "5.4.3.2", 600); - onUpdateWithdrawRoute("5", "10.1.1.3", 32); - - } public void onUpdatePushRoute(String rd, String prefix, int plen, -- 2.36.6