OpenConfig BGP more defensive

Add null checks and set default values to allow to configure BGP with
minimalistic configuration.

Change-Id: Id7f4c4014832054100e9148f575e215d493c3bb7
Signed-off-by: Milos Fabian <milfabia@cisco.com>
(cherry picked from commit 6efa3c0d6a4490a7e898519e3905847284803f2c)

diff --git a/bgp/openconfig-impl/src/main/java/org/opendaylight/protocol/bgp/openconfig/impl/util/OpenConfigUtil.java b/bgp/openconfig-impl/src/main/java/org/opendaylight/protocol/bgp/openconfig/impl/util/OpenConfigUtil.java
index fd2ed204996ed8d26b3a9c15604c7aabf3256675..e93547288bb7817de1bc1811714f58bc34817ec5 100644 (file)
--- a/bgp/openconfig-impl/src/main/java/org/opendaylight/protocol/bgp/openconfig/impl/util/OpenConfigUtil.java
+++ b/bgp/openconfig-impl/src/main/java/org/opendaylight/protocol/bgp/openconfig/impl/util/OpenConfigUtil.java
@@ -31,6 +31,7 @@ import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.AfiSa
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.AfiSafi1Builder;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.AfiSafi2;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.AfiSafi2Builder;
+import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.neighbor.group.Config;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.neighbor.group.RouteReflector;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.neighbors.Neighbor;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.top.Bgp;
@@ -215,9 +216,13 @@ public final class OpenConfigUtil {
     }
 
     public static boolean isAppNeighbor(final Neighbor neighbor) {
-        final Config2 config1 = neighbor.getConfig().getAugmentation(Config2.class);
-        if (config1 != null) {
-            return config1.getPeerGroup() != null && config1.getPeerGroup().equals(OpenConfigUtil.APPLICATION_PEER_GROUP_NAME);
+        final Config config = neighbor.getConfig();
+        if (config != null) {
+            final Config2 config1 = config.getAugmentation(Config2.class);
+            if (config1 != null) {
+                final String peerGroup = config1.getPeerGroup();
+                return peerGroup != null && peerGroup.equals(OpenConfigUtil.APPLICATION_PEER_GROUP_NAME);
+            }
         }
         return false;
     }

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
index 97247230c97d17a8a5b3c12758dd3994246ff908..1db6f2339676694724e76018b1de4d52a37eafcb 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
@@ -137,14 +137,15 @@ public class BgpPeer implements PeerBean, BGPPeerRuntimeMXBean {
         caps.add(new OptionalCapabilitiesBuilder().setCParameters(BgpExtendedMessageUtil.EXTENDED_MESSAGE_CAPABILITY).build());
         caps.add(new OptionalCapabilitiesBuilder().setCParameters(MultiprotocolCapabilitiesUtil.RR_CAPABILITY).build());
 
-        final List<AddressFamilies> addPathCapability = mappingService.toAddPathCapability(neighbor.getAfiSafis().getAfiSafi());
+        final List<AfiSafi> afiSafi = OpenConfigMappingUtil.getAfiSafiWithDefault(neighbor.getAfiSafis(), false);
+        final List<AddressFamilies> addPathCapability = mappingService.toAddPathCapability(afiSafi);
         if (!addPathCapability.isEmpty()) {
             caps.add(new OptionalCapabilitiesBuilder().setCParameters(new CParametersBuilder().addAugmentation(CParameters1.class,
                     new CParameters1Builder().setAddPathCapability(
                             new AddPathCapabilityBuilder().setAddressFamilies(addPathCapability).build()).build()).build()).build());
         }
 
-        final List<BgpTableType> tableTypes = mappingService.toTableTypes(neighbor.getAfiSafis().getAfiSafi());
+        final List<BgpTableType> tableTypes = mappingService.toTableTypes(afiSafi);
         for (final BgpTableType tableType : tableTypes) {
             if (!rib.getLocalTables().contains(tableType)) {
                 LOG.info("RIB instance does not list {} in its local tables. Incoming data will be dropped.", tableType);
@@ -207,7 +208,7 @@ public class BgpPeer implements PeerBean, BGPPeerRuntimeMXBean {
         private BgpPeerSingletonService(final RIB rib, final Neighbor neighbor, final BGPOpenConfigMappingService mappingService,
             final WriteConfiguration configurationWriter) {
             this.neighborAddress = neighbor.getNeighborAddress();
-            this.bgpPeer = new BGPPeer(Ipv4Util.toStringIP(this.neighborAddress), rib, mappingService.toPeerRole(neighbor), rpcRegistry);
+            this.bgpPeer = new BGPPeer(Ipv4Util.toStringIP(this.neighborAddress), rib, mappingService.toPeerRole(neighbor), BgpPeer.this.rpcRegistry);
             final List<BgpParameters> bgpParameters = getBgpParameters(neighbor, rib, mappingService);
             final KeyMapping key = OpenConfigMappingUtil.getNeighborKey(neighbor);
             this.prefs = new BGPSessionPreferences(rib.getLocalAs(), getHoldTimer(neighbor), rib.getBgpIdentifier(), getPeerAs(neighbor, rib),
@@ -239,22 +240,22 @@ public class BgpPeer implements PeerBean, BGPPeerRuntimeMXBean {
             }
             LOG.info("Peer Singleton Service {} instantiated", getIdentifier());
             this.bgpPeer.instantiateServiceInstance();
-            peerRegistry.addPeer(this.neighborAddress, this.bgpPeer, prefs);
+            BgpPeer.this.peerRegistry.addPeer(this.neighborAddress, this.bgpPeer, this.prefs);
             if (this.activeConnection) {
-                this.connection = this.dispatcher.createReconnectingClient(this.inetAddress, peerRegistry, this.retryTimer, this.key);
+                this.connection = this.dispatcher.createReconnectingClient(this.inetAddress, BgpPeer.this.peerRegistry, this.retryTimer, this.key);
             }
         }
 
         @Override
         public ListenableFuture<Void> closeServiceInstance() {
-            LOG.info("Close Peer Singleton Service {}", this.getIdentifier());
+            LOG.info("Close Peer Singleton Service {}", getIdentifier());
             if (this.connection != null) {
                 this.connection.cancel(true);
                 this.connection = null;
             }
             this.bgpPeer.close();
-            if(currentConfiguration != null) {
-                peerRegistry.removePeer(currentConfiguration.getNeighborAddress());
+            if(BgpPeer.this.currentConfiguration != null) {
+                BgpPeer.this.peerRegistry.removePeer(BgpPeer.this.currentConfiguration.getNeighborAddress());
             }
             return Futures.immediateFuture(null);
         }

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/OpenConfigMappingUtil.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/OpenConfigMappingUtil.java
index 4ce3f9bb9d07b2a66fbc56e5a99b26a8b3d96c70..cc0564c8117ff06d01510c1a44bf1f768754e342 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/OpenConfigMappingUtil.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/OpenConfigMappingUtil.java
@@ -10,13 +10,24 @@ package org.opendaylight.protocol.bgp.rib.impl.config;
 
 import static org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IetfInetUtil.INSTANCE;
 
+import com.google.common.base.Predicate;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
+import java.util.Collections;
+import java.util.List;
 import org.opendaylight.protocol.bgp.rib.impl.spi.RIB;
 import org.opendaylight.protocol.concepts.KeyMapping;
 import org.opendaylight.protocol.util.Ipv4Util;
+import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.multiprotocol.rev151009.BgpCommonAfiSafiList;
+import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.multiprotocol.rev151009.bgp.common.afi.safi.list.AfiSafi;
+import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.multiprotocol.rev151009.bgp.common.afi.safi.list.AfiSafiBuilder;
+import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.neighbor.group.Timers;
+import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.neighbor.group.transport.Config;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.neighbors.Neighbor;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.neighbors.NeighborKey;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.top.Bgp;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.top.bgp.Neighbors;
+import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.types.rev151009.IPV4UNICAST;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.network.instance.rev151018.network.instance.top.network.instances.network.instance.protocols.Protocol;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.AsNumber;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.PortNumber;
@@ -25,6 +36,12 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
 final class OpenConfigMappingUtil {
 
+    private static final AfiSafi IPV4_AFISAFI = new AfiSafiBuilder().setAfiSafiName(IPV4UNICAST.class).build();
+    private static final List<AfiSafi> DEFAULT_AFISAFI = ImmutableList.of(IPV4_AFISAFI);
+    private static final int HOLDTIMER = 90;
+    private static final int CONNECT_RETRY = 30;
+    private static final PortNumber PORT = new PortNumber(179);
+
     private OpenConfigMappingUtil() {
         throw new UnsupportedOperationException();
     }
@@ -34,29 +51,49 @@ final class OpenConfigMappingUtil {
     }
 
     public static int getHoldTimer(final Neighbor neighbor) {
-        return neighbor.getTimers().getConfig().getHoldTime().intValue();
+        final org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.neighbor.group.timers.Config config =
+                getTimersConfig(neighbor);
+        if (config != null && config.getHoldTime() != null) {
+            return config.getHoldTime().intValue();
+        }
+        return HOLDTIMER;
     }
 
     public static AsNumber getPeerAs(final Neighbor neighbor, final RIB rib) {
-        final AsNumber peerAs = neighbor.getConfig().getPeerAs();
-        if (peerAs != null) {
-            return peerAs;
+        if (neighbor.getConfig() != null) {
+            final AsNumber peerAs = neighbor.getConfig().getPeerAs();
+            if (peerAs != null) {
+                return peerAs;
+            }
         }
         return rib.getLocalAs();
     }
 
     public static boolean isActive(final Neighbor neighbor) {
-        return !neighbor.getTransport().getConfig().isPassiveMode();
+        if (neighbor.getTransport() != null) {
+            final Config config = neighbor.getTransport().getConfig();
+            if (config != null && config.isPassiveMode() != null) {
+                return !config.isPassiveMode();
+            }
+        }
+        return true;
     }
 
     public static int getRetryTimer(final Neighbor neighbor) {
-        return neighbor.getTimers().getConfig().getConnectRetry().intValue();
+        final org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.neighbor.group.timers.Config config =
+                getTimersConfig(neighbor);
+        if (config != null && config.getConnectRetry() != null) {
+            return config.getConnectRetry().intValue();
+        }
+        return CONNECT_RETRY;
     }
 
     public static KeyMapping getNeighborKey(final Neighbor neighbor) {
-        final String authPassword = neighbor.getConfig().getAuthPassword();
-        if (authPassword != null) {
-            KeyMapping.getKeyMapping(INSTANCE.inetAddressFor(neighbor.getNeighborAddress()), authPassword);
+        if (neighbor.getConfig() != null) {
+            final String authPassword = neighbor.getConfig().getAuthPassword();
+            if (authPassword != null) {
+                return KeyMapping.getKeyMapping(INSTANCE.inetAddressFor(neighbor.getNeighborAddress()), authPassword);
+            }
         }
         return null;
     }
@@ -71,7 +108,41 @@ final class OpenConfigMappingUtil {
     }
 
     public static PortNumber getPort(final Neighbor neighbor) {
-        return neighbor.getTransport().getConfig().getAugmentation(Config1.class).getRemotePort();
+        if (neighbor.getTransport() != null) {
+            final Config config = neighbor.getTransport().getConfig();
+            if (config != null && config.getAugmentation(Config1.class) != null) {
+                final PortNumber remotePort = config.getAugmentation(Config1.class).getRemotePort();
+                if (remotePort != null) {
+                    return remotePort;
+                }
+            }
+        }
+        return PORT;
+    }
+
+    //make sure IPv4 Unicast (RFC 4271) when required
+    public static List<AfiSafi> getAfiSafiWithDefault(final BgpCommonAfiSafiList afiSAfis, final boolean setDeafultIPv4) {
+        if (afiSAfis == null || afiSAfis.getAfiSafi() == null) {
+            return setDeafultIPv4 ? DEFAULT_AFISAFI : Collections.emptyList();
+        }
+        final List<AfiSafi> afiSafi = afiSAfis.getAfiSafi();
+        if (setDeafultIPv4) {
+            final boolean anyMatch = FluentIterable.from(afiSafi).anyMatch(new Predicate<AfiSafi>() {
+                @Override
+                public boolean apply(final AfiSafi input) {
+                    return input.getAfiSafiName().getName().equals(IPV4UNICAST.class);
+                }
+            });
+            if (!anyMatch) {
+                afiSafi.add(IPV4_AFISAFI);
+            }
+        }
+        return afiSafi;
+    }
+
+    private static org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.rev151009.bgp.neighbor.group.timers.Config getTimersConfig(final Neighbor neighbor) {
+        final Timers timers = neighbor.getTimers();
+        return timers != null ? timers.getConfig() : null;
     }
 
 }

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/RibImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/RibImpl.java
index b297195012d9ae8248b206c3087fcf56d5d91f24..b618d693c433a3ff99d332c084bfd4431cff259e 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/RibImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/RibImpl.java
@@ -8,6 +8,8 @@
 
 package org.opendaylight.protocol.bgp.rib.impl.config;
 
+import static org.opendaylight.protocol.bgp.rib.impl.config.OpenConfigMappingUtil.getAfiSafiWithDefault;
+
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import java.util.List;
@@ -73,7 +75,6 @@ public final class RibImpl implements RIB, AutoCloseable {
     private AsNumber asNumber;
     private Ipv4Address routerId;
 
-    @SuppressWarnings("deprecation")
     public RibImpl(final ClusterSingletonServiceProvider provider, final RIBExtensionConsumerContext contextProvider, final BGPDispatcher dispatcher,
             final BindingCodecTreeFactory codecTreeFactory, final DOMDataBroker domBroker, final SchemaService schemaService) {
         this.provider = Preconditions.checkNotNull(provider);
@@ -87,12 +88,12 @@ public final class RibImpl implements RIB, AutoCloseable {
     void start(final Global global, final String instanceName, final BGPOpenConfigMappingService mappingService,
         final BgpDeployer.WriteConfiguration configurationWriter) {
         Preconditions.checkState(this.ribImpl == null, "Previous instance %s was not closed.", this);
-        this.ribImpl = createRib(provider, global, instanceName, mappingService, configurationWriter);
+        this.ribImpl = createRib(this.provider, global, instanceName, mappingService, configurationWriter);
         this.schemaContextRegistration = this.schemaService.registerSchemaContextListener(this.ribImpl);
     }
 
     Boolean isGlobalEqual(final Global global) {
-        final List<AfiSafi> globalAfiSafi = global.getAfiSafis().getAfiSafi();
+        final List<AfiSafi> globalAfiSafi = getAfiSafiWithDefault(global.getAfiSafis(), true);
         final AsNumber globalAs = global.getConfig().getAs();
         final Ipv4Address globalRouterId = global.getConfig().getRouterId();
         return this.afiSafi.containsAll(globalAfiSafi) && globalAfiSafi.containsAll(this.afiSafi)
@@ -169,7 +170,7 @@ public final class RibImpl implements RIB, AutoCloseable {
         if (this.ribImpl != null) {
             try {
                 this.ribImpl.close();
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 LOG.warn("Failed to close {} rib instance", this, e);
             }
             this.ribImpl = null;
@@ -219,7 +220,7 @@ public final class RibImpl implements RIB, AutoCloseable {
 
     private RIBImpl createRib(final ClusterSingletonServiceProvider provider, final Global global, final String bgpInstanceName,
         final BGPOpenConfigMappingService mappingService, final BgpDeployer.WriteConfiguration configurationWriter) {
-        this.afiSafi = global.getAfiSafis().getAfiSafi();
+        this.afiSafi = getAfiSafiWithDefault(global.getAfiSafis(), true);
         this.asNumber = global.getConfig().getAs();
         this.routerId = global.getConfig().getRouterId();
         final Map<TablesKey, PathSelectionMode> pathSelectionModes = mappingService.toPathSelectionMode(this.afiSafi).entrySet()