BUG-2109 : clear BGP session after it was already initialized

Change-Id: Ib70a725b948892a2a887a941bdcb4e8dcfdb80eb
Signed-off-by: Dana Kutenicsova <dkutenic@cisco.com>

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java
index a605c773eb8e7682548422b08167d68b6fc07e36..ca07ea75f60271918e9017946542d6ffe00b90ff 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java
@@ -171,6 +171,11 @@ public class BGPPeer implements ReusableBGPPeer, Peer, AutoCloseable, BGPPeerRun
         }
     }
 
+    @Override
+    public boolean isSessionActive() {
+        return this.session != null;
+    }
+
     @Override
     public synchronized byte[] getRawIdentifier() {
         return Arrays.copyOf(this.rawIdentifier, this.rawIdentifier.length);

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/StrictBGPPeerRegistry.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/StrictBGPPeerRegistry.java
index 3475190fa0462018a716348afe04eac8baf28a5c..d33e8be8b3b440b2753b12b96ff9530aeaab6f05 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/StrictBGPPeerRegistry.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/StrictBGPPeerRegistry.java
@@ -94,47 +94,53 @@ public final class StrictBGPPeerRegistry implements BGPPeerRegistry {
         checkPeerConfigured(ip);
 
         final BGPSessionId currentConnection = new BGPSessionId(sourceId, remoteId);
+        final BGPSessionListener p = this.peers.get(ip);
 
         if (this.sessionIds.containsKey(ip)) {
-            LOG.warn("Duplicate BGP session established with {}", ip);
-
-            final BGPSessionId previousConnection = this.sessionIds.get(ip);
-
-            // Session reestablished with different ids
-            if (!previousConnection.equals(currentConnection)) {
-                LOG.warn("BGP session with {} {} has to be dropped. Same session already present {}", ip, currentConnection, previousConnection);
-                throw new BGPDocumentedException(
-                    String.format("BGP session with %s %s has to be dropped. Same session already present %s",
-                        ip, currentConnection, previousConnection),
-                        BGPError.CONNECTION_COLLISION_RESOLUTION);
-
-                // Session reestablished with lower source bgp id, dropping current
-            } else if (previousConnection.isHigherDirection(currentConnection)) {
-                LOG.warn("BGP session with {} {} has to be dropped. Opposite session already present", ip, currentConnection);
-                throw new BGPDocumentedException(
-                    String.format("BGP session with %s initiated %s has to be dropped. Opposite session already present",
-                        ip, currentConnection),
-                        BGPError.CONNECTION_COLLISION_RESOLUTION);
-
-                // Session reestablished with higher source bgp id, dropping previous
-            } else if (currentConnection.isHigherDirection(previousConnection)) {
-                LOG.warn("BGP session with {} {} released. Replaced by opposite session", ip, previousConnection);
-                this.peers.get(ip).releaseConnection();
-                return this.peers.get(ip);
-
-                // Session reestablished with same source bgp id, dropping current as duplicate
+            if (p.isSessionActive()) {
+
+                LOG.warn("Duplicate BGP session established with {}", ip);
+
+                final BGPSessionId previousConnection = this.sessionIds.get(ip);
+
+                // Session reestablished with different ids
+                if (!previousConnection.equals(currentConnection)) {
+                    LOG.warn("BGP session with {} {} has to be dropped. Same session already present {}", ip, currentConnection, previousConnection);
+                    throw new BGPDocumentedException(
+                        String.format("BGP session with %s %s has to be dropped. Same session already present %s",
+                            ip, currentConnection, previousConnection),
+                            BGPError.CEASE);
+
+                    // Session reestablished with lower source bgp id, dropping current
+                } else if (previousConnection.isHigherDirection(currentConnection)) {
+                    LOG.warn("BGP session with {} {} has to be dropped. Opposite session already present", ip, currentConnection);
+                    throw new BGPDocumentedException(
+                        String.format("BGP session with %s initiated %s has to be dropped. Opposite session already present",
+                            ip, currentConnection),
+                            BGPError.CEASE);
+
+                    // Session reestablished with higher source bgp id, dropping previous
+                } else if (currentConnection.isHigherDirection(previousConnection)) {
+                    LOG.warn("BGP session with {} {} released. Replaced by opposite session", ip, previousConnection);
+                    this.peers.get(ip).releaseConnection();
+                    return this.peers.get(ip);
+
+                    // Session reestablished with same source bgp id, dropping current as duplicate
+                } else {
+                    LOG.warn("BGP session with %s initiated from %s to %s has to be dropped. Same session already present", ip, sourceId, remoteId);
+                    throw new BGPDocumentedException(
+                        String.format("BGP session with %s initiated %s has to be dropped. Same session already present",
+                            ip, currentConnection),
+                            BGPError.CEASE);
+                }
             } else {
-                LOG.warn("BGP session with %s initiated from %s to %s has to be dropped. Same session already present", ip, sourceId, remoteId);
-                throw new BGPDocumentedException(
-                    String.format("BGP session with %s initiated %s has to be dropped. Same session already present",
-                        ip, currentConnection),
-                        BGPError.CONNECTION_COLLISION_RESOLUTION);
+                removePeerSession(ip);
             }
         }
 
         // Map session id to peer IP address
         this.sessionIds.put(ip, currentConnection);
-        return this.peers.get(ip);
+        return p;
     }
 
     @Override

diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/SimpleSessionListener.java b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/SimpleSessionListener.java
index 7cd8908d2d738cc5fdde3e6063d35dd572211333..c6e3029252484bf7b1b10c047ba10fd5c40c99e6 100644 (file)
--- a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/SimpleSessionListener.java
+++ b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/SimpleSessionListener.java
@@ -8,9 +8,7 @@
 package org.opendaylight.protocol.bgp.rib.impl;
 
 import com.google.common.collect.Lists;
-
 import java.util.List;
-
 import org.opendaylight.protocol.bgp.rib.impl.spi.ReusableBGPPeer;
 import org.opendaylight.protocol.bgp.rib.spi.BGPSession;
 import org.opendaylight.protocol.bgp.rib.spi.BGPTerminationReason;
@@ -62,4 +60,9 @@ public class SimpleSessionListener implements ReusableBGPPeer {
     public void releaseConnection() {
         LOG.debug("Releasing connection");
     }
+
+    @Override
+    public boolean isSessionActive() {
+        return true;
+    }
 }

diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/SpeakerSessionListener.java b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/SpeakerSessionListener.java
index 4926426e48b2600adb47f7e9339aef8b5db1c9f2..4e49257f3df65a57c52b16e97e974c25e8fedea1 100644 (file)
--- a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/SpeakerSessionListener.java
+++ b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/SpeakerSessionListener.java
@@ -8,10 +8,8 @@
 package org.opendaylight.protocol.bgp.rib.impl;
 
 import com.google.common.collect.Lists;
-
 import java.util.List;
 import java.util.Set;
-
 import org.opendaylight.protocol.bgp.rib.spi.BGPSession;
 import org.opendaylight.protocol.bgp.rib.spi.BGPSessionListener;
 import org.opendaylight.protocol.bgp.rib.spi.BGPTerminationReason;
@@ -61,4 +59,9 @@ public class SpeakerSessionListener implements BGPSessionListener {
         LOG.debug("Session terminated. Cause : {}", cause.toString());
         this.up = false;
     }
+
+    @Override
+    public boolean isSessionActive() {
+        return true;
+    }
 }

diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/StrictBGPPeerRegistryTest.java b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/StrictBGPPeerRegistryTest.java
index a27ed1777517ff59780567b22c607e09eb6c2700..004ab5dada0a332dbf249320ccb0d960696743e1 100644 (file)
--- a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/StrictBGPPeerRegistryTest.java
+++ b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/StrictBGPPeerRegistryTest.java
@@ -13,7 +13,6 @@ import static org.junit.Assert.assertSame;
 import static org.junit.Assert.fail;
 
 import java.net.InetSocketAddress;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mockito;
@@ -31,8 +30,8 @@ public class StrictBGPPeerRegistryTest {
 
     @Before
     public void setUp() throws Exception {
-        droppingBGPSessionRegistry = new StrictBGPPeerRegistry();
-        mockPreferences = getMockPreferences();
+        this.droppingBGPSessionRegistry = new StrictBGPPeerRegistry();
+        this.mockPreferences = getMockPreferences();
     }
 
     @Test
@@ -49,13 +48,13 @@ public class StrictBGPPeerRegistryTest {
         final Ipv4Address to = new Ipv4Address("255.255.255.255");
 
         final ReusableBGPPeer session1 = getMockSession();
-        droppingBGPSessionRegistry.addPeer(remoteIp, session1, mockPreferences);
+        this.droppingBGPSessionRegistry.addPeer(remoteIp, session1, this.mockPreferences);
 
-        droppingBGPSessionRegistry.getPeer(remoteIp, from, to);
+        this.droppingBGPSessionRegistry.getPeer(remoteIp, from, to);
         try {
-            droppingBGPSessionRegistry.getPeer(remoteIp, from, to);
+            this.droppingBGPSessionRegistry.getPeer(remoteIp, from, to);
         } catch (final IllegalStateException e) {
-            Mockito.verifyZeroInteractions(session1);
+            Mockito.verify(session1).isSessionActive();
             return;
         }
 
@@ -69,7 +68,7 @@ public class StrictBGPPeerRegistryTest {
         final Ipv4Address to = new Ipv4Address("255.255.255.255");
 
         try {
-            droppingBGPSessionRegistry.getPeer(remoteIp, from, to);
+            this.droppingBGPSessionRegistry.getPeer(remoteIp, from, to);
         } catch (final IllegalStateException e) {
             return;
         }
@@ -86,13 +85,13 @@ public class StrictBGPPeerRegistryTest {
         final IpAddress remoteIp2 = new IpAddress(to2);
 
         final ReusableBGPPeer session1 = getMockSession();
-        droppingBGPSessionRegistry.addPeer(remoteIp, session1, mockPreferences);
+        this.droppingBGPSessionRegistry.addPeer(remoteIp, session1, this.mockPreferences);
         final ReusableBGPPeer session2 = getMockSession();
-        droppingBGPSessionRegistry.addPeer(remoteIp2, session2, mockPreferences);
+        this.droppingBGPSessionRegistry.addPeer(remoteIp2, session2, this.mockPreferences);
 
-        final BGPSessionListener returnedSession1 = droppingBGPSessionRegistry.getPeer(remoteIp, from, to);
+        final BGPSessionListener returnedSession1 = this.droppingBGPSessionRegistry.getPeer(remoteIp, from, to);
         assertSame(session1, returnedSession1);
-        final BGPSessionListener returnedSession2 = droppingBGPSessionRegistry.getPeer(remoteIp2, from, to2);
+        final BGPSessionListener returnedSession2 = this.droppingBGPSessionRegistry.getPeer(remoteIp2, from, to2);
         assertSame(session2, returnedSession2);
 
         Mockito.verifyZeroInteractions(session1);
@@ -106,13 +105,13 @@ public class StrictBGPPeerRegistryTest {
         final IpAddress remoteIp = new IpAddress(lower);
 
         final ReusableBGPPeer session1 = getMockSession();
-        droppingBGPSessionRegistry.addPeer(remoteIp, session1, mockPreferences);
+        this.droppingBGPSessionRegistry.addPeer(remoteIp, session1, this.mockPreferences);
 
-        droppingBGPSessionRegistry.getPeer(remoteIp, higher, lower);
+        this.droppingBGPSessionRegistry.getPeer(remoteIp, higher, lower);
         try {
-            droppingBGPSessionRegistry.getPeer(remoteIp, lower, higher);
+            this.droppingBGPSessionRegistry.getPeer(remoteIp, lower, higher);
         } catch (final BGPDocumentedException e) {
-            Mockito.verifyZeroInteractions(session1);
+            Mockito.verify(session1).isSessionActive();
             return;
         }
 
@@ -126,16 +125,17 @@ public class StrictBGPPeerRegistryTest {
         final IpAddress remoteIp = new IpAddress(lower);
 
         final ReusableBGPPeer session1 = getMockSession();
-        droppingBGPSessionRegistry.addPeer(remoteIp, session1, mockPreferences);
+        this.droppingBGPSessionRegistry.addPeer(remoteIp, session1, this.mockPreferences);
 
-        droppingBGPSessionRegistry.getPeer(remoteIp, lower, higher);
-        droppingBGPSessionRegistry.getPeer(remoteIp, higher, lower);
+        this.droppingBGPSessionRegistry.getPeer(remoteIp, lower, higher);
+        this.droppingBGPSessionRegistry.getPeer(remoteIp, higher, lower);
         Mockito.verify(session1).releaseConnection();
     }
 
     private ReusableBGPPeer getMockSession() {
         final ReusableBGPPeer mock = Mockito.mock(ReusableBGPPeer.class);
         Mockito.doNothing().when(mock).releaseConnection();
+        Mockito.doReturn(Boolean.TRUE).when(mock).isSessionActive();
         return mock;
     }
 

diff --git a/bgp/rib-mock/src/test/java/org/opendaylight/protocol/bgp/rib/mock/BGPListenerMock.java b/bgp/rib-mock/src/test/java/org/opendaylight/protocol/bgp/rib/mock/BGPListenerMock.java
index 92fc61080e62d2f3d00a8e77995ccd0edfe6de67..d7b4e6c0a15c3ba830da8fc5733a5dcce8c6dbc9 100644 (file)
--- a/bgp/rib-mock/src/test/java/org/opendaylight/protocol/bgp/rib/mock/BGPListenerMock.java
+++ b/bgp/rib-mock/src/test/java/org/opendaylight/protocol/bgp/rib/mock/BGPListenerMock.java
@@ -10,7 +10,6 @@ package org.opendaylight.protocol.bgp.rib.mock;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
-
 import org.opendaylight.protocol.bgp.rib.spi.BGPSession;
 import org.opendaylight.protocol.bgp.rib.spi.BGPSessionListener;
 import org.opendaylight.protocol.bgp.rib.spi.BGPTerminationReason;
@@ -51,4 +50,9 @@ public final class BGPListenerMock implements BGPSessionListener {
     public void onSessionTerminated(final BGPSession session, final BGPTerminationReason reason) {
         this.connected = false;
     }
+
+    @Override
+    public boolean isSessionActive() {
+        return true;
+    }
 }

diff --git a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/BGPSessionListener.java b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/BGPSessionListener.java
index a56cb0f16bb714caae24a54ba41a5f137a5c8cc7..a6127ba9ca1e26455db0538c8a43e0ea509b9ca1 100644 (file)
--- a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/BGPSessionListener.java
+++ b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/BGPSessionListener.java
@@ -15,4 +15,10 @@ import org.opendaylight.yangtools.yang.binding.Notification;
  */
 public interface BGPSessionListener extends SessionListener<Notification, BGPSession, BGPTerminationReason> {
 
+    /**
+     * Returns state of BGP session associated with this listener.
+     *
+     * @return false if session associated with this listener is null, true if its non-null
+     */
+    boolean isSessionActive();
 }

diff --git a/bgp/testtool/src/main/java/org/opendaylight/protocol/bgp/testtool/TestingListener.java b/bgp/testtool/src/main/java/org/opendaylight/protocol/bgp/testtool/TestingListener.java
index 3aea27e16e9a2c6cae5b4a24124cc86d5d788bbf..c66119ad62b0881948e8963144bb17ab30df5b30 100644 (file)
--- a/bgp/testtool/src/main/java/org/opendaylight/protocol/bgp/testtool/TestingListener.java
+++ b/bgp/testtool/src/main/java/org/opendaylight/protocol/bgp/testtool/TestingListener.java
@@ -45,4 +45,9 @@ public class TestingListener implements ReusableBGPPeer {
     public void releaseConnection() {
         LOG.info("Client Listener: Connection released.");
     }
+
+    @Override
+    public boolean isSessionActive() {
+        return true;
+    }
 }

diff --git a/bgp/testtool/src/test/java/org/opendaylight/protocol/bgp/testtool/SpeakerSessionListener.java b/bgp/testtool/src/test/java/org/opendaylight/protocol/bgp/testtool/SpeakerSessionListener.java
index a9510ec886edd7d7eb99664241e30e30ebc08339..4c6a42d757d26f1be71606af419394a0ccfba3ff 100644 (file)
--- a/bgp/testtool/src/test/java/org/opendaylight/protocol/bgp/testtool/SpeakerSessionListener.java
+++ b/bgp/testtool/src/test/java/org/opendaylight/protocol/bgp/testtool/SpeakerSessionListener.java
@@ -39,4 +39,9 @@ public class SpeakerSessionListener implements BGPSessionListener {
         LOG.info("Server: Message received: {}", message);
         // this.d.stop();
     }
+
+    @Override
+    public boolean isSessionActive() {
+        return true;
+    }
 }