From 707e16204450dad5d6f51976d2d0ddfb550ad5b8 Mon Sep 17 00:00:00 2001 From: Dana Kutenicsova Date: Thu, 6 Nov 2014 14:24:20 +0100 Subject: [PATCH] BUG-2109 : clear BGP session after it was already initialized Change-Id: Ib70a725b948892a2a887a941bdcb4e8dcfdb80eb Signed-off-by: Dana Kutenicsova --- .../protocol/bgp/rib/impl/BGPPeer.java | 5 ++ .../bgp/rib/impl/StrictBGPPeerRegistry.java | 72 ++++++++++--------- .../bgp/rib/impl/SimpleSessionListener.java | 7 +- .../bgp/rib/impl/SpeakerSessionListener.java | 7 +- .../rib/impl/StrictBGPPeerRegistryTest.java | 38 +++++----- .../bgp/rib/mock/BGPListenerMock.java | 6 +- .../bgp/rib/spi/BGPSessionListener.java | 6 ++ .../bgp/testtool/TestingListener.java | 5 ++ .../bgp/testtool/SpeakerSessionListener.java | 5 ++ 9 files changed, 94 insertions(+), 57 deletions(-) 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 a605c773eb..ca07ea75f6 100644 --- 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 3475190fa0..d33e8be8b3 100644 --- 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 7cd8908d2d..c6e3029252 100644 --- 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 4926426e48..4e49257f3d 100644 --- 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 a27ed17775..004ab5dada 100644 --- 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 92fc61080e..d7b4e6c0a1 100644 --- 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 a56cb0f16b..a6127ba9ca 100644 --- 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 { + /** + * 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 3aea27e16e..c66119ad62 100644 --- 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 a9510ec886..4c6a42d757 100644 --- 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; + } } -- 2.36.6