Session ref. entries stored in bi-map were identifed by byte array (raw IP Address of client),
casuing that already existing session in bi-map were not look-up properly => allowing to create duplicate sessions.
Changed type of bi-map's key to wrapper of byte array.
Fixed also removing of session refs. from map on channel close. Turned bi-map to map, since inverse map is not used anymore.
pcc-mock is reusing this code - need to create session negotiator factory per pcc.
Change-Id: I85670b083b6ea832f8b9a4891c812845174f03ff
Signed-off-by: Milos Fabian <milfabia@cisco.com>
Signed-off-by: Robert Varga <rovarga@cisco.com>
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
-import com.google.common.collect.BiMap;
-import com.google.common.collect.HashBiMap;
import com.google.common.primitives.UnsignedBytes;
-
import io.netty.channel.Channel;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelFutureListener;
import io.netty.util.concurrent.Promise;
-
import java.net.InetSocketAddress;
+import java.util.Arrays;
import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
-
import javax.annotation.concurrent.GuardedBy;
-
import org.opendaylight.protocol.framework.AbstractSessionNegotiator;
import org.opendaylight.protocol.framework.SessionListenerFactory;
import org.opendaylight.protocol.framework.SessionNegotiator;
private static final long ID_CACHE_SECONDS = 3 * 3600;
@GuardedBy("this")
- private final BiMap<byte[], SessionReference> sessions = HashBiMap.create();
+ private final Map<ByteArrayWrapper, SessionReference> sessions = new HashMap<>();
@GuardedBy("this")
private final Cache<byte[], PeerRecord> formerClients = CacheBuilder.newBuilder().expireAfterAccess(PEER_CACHE_SECONDS,
* registered for this client.
*/
final byte[] clientAddress = ((InetSocketAddress) this.channel.remoteAddress()).getAddress().getAddress();
+ final ByteArrayWrapper clientAddressWrapper = new ByteArrayWrapper(clientAddress);
synchronized (lock) {
- if (AbstractPCEPSessionNegotiatorFactory.this.sessions.containsKey(clientAddress)) {
+ if (AbstractPCEPSessionNegotiatorFactory.this.sessions.containsKey(clientAddressWrapper)) {
final byte[] serverAddress = ((InetSocketAddress) this.channel.localAddress()).getAddress().getAddress();
if (COMPARATOR.compare(serverAddress, clientAddress) > 0) {
- final SessionReference n = AbstractPCEPSessionNegotiatorFactory.this.sessions.remove(clientAddress);
+ final SessionReference n = AbstractPCEPSessionNegotiatorFactory.this.sessions.remove(clientAddressWrapper);
try {
n.close();
} catch (final Exception e) {
final Short sessionId = nextSession(clientAddress);
final AbstractPCEPSessionNegotiator n = createNegotiator(promise, factory.getSessionListener(), this.channel, sessionId);
- AbstractPCEPSessionNegotiatorFactory.this.sessions.put(clientAddress, new SessionReference() {
+ AbstractPCEPSessionNegotiatorFactory.this.sessions.put(clientAddressWrapper, new SessionReference() {
@Override
public void close() throws ExecutionException {
try {
@Override
public void operationComplete(final ChannelFuture future) {
synchronized (lock) {
- AbstractPCEPSessionNegotiatorFactory.this.sessions.inverse().remove(this);
+ AbstractPCEPSessionNegotiatorFactory.this.sessions.remove(clientAddressWrapper);
}
}
});
return peer.allocId();
}
+
+ private static final class ByteArrayWrapper {
+
+ private final byte[] byteArray;
+
+ ByteArrayWrapper(final byte[] byteArray) {
+ this.byteArray = byteArray;
+ }
+
+ @Override
+ public int hashCode() {
+ return Arrays.hashCode(byteArray);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof ByteArrayWrapper)) {
+ return false;
+ }
+ return Arrays.equals(byteArray, ((ByteArrayWrapper) obj).byteArray);
+ }
+ }
}
public void setUp() {
final Open open = new OpenBuilder().setSessionId((short) 0).setDeadTimer(DEAD_TIMER).setKeepalive(KEEP_ALIVE)
.build();
- final SessionNegotiatorFactory<Message, PCEPSessionImpl, PCEPSessionListener> snf = new DefaultPCEPSessionNegotiatorFactory(
- open, 0);
final EventLoopGroup eventLoopGroup = new NioEventLoopGroup();
final MessageRegistry msgReg = ServiceLoaderPCEPExtensionProviderContext.getSingletonInstance()
.getMessageHandlerRegistry();
- this.dispatcher = new PCEPDispatcherImpl(msgReg, snf, eventLoopGroup, eventLoopGroup);
- this.pccMock = new PCCMock<>(snf, new PCEPHandlerFactory(msgReg), new DefaultPromise<PCEPSessionImpl>(
- GlobalEventExecutor.INSTANCE));
+ this.dispatcher = new PCEPDispatcherImpl(msgReg, new DefaultPCEPSessionNegotiatorFactory(open, 0),
+ eventLoopGroup, eventLoopGroup);
+ this.pccMock = new PCCMock<>(new DefaultPCEPSessionNegotiatorFactory(open, 0),
+ new PCEPHandlerFactory(msgReg), new DefaultPromise<PCEPSessionImpl>(
+ GlobalEventExecutor.INSTANCE));
}
@Test
session1.close();
session2.close();
Assert.assertTrue(futureChannel.channel().isActive());
+ }
+
+ @Test
+ public void testCreateDuplicateClient() throws InterruptedException, ExecutionException {
+ final ChannelFuture futureChannel = this.dispatcher.createServer(new InetSocketAddress("0.0.0.0", PORT),
+ new SessionListenerFactory<PCEPSessionListener>() {
+ @Override
+ public PCEPSessionListener getSessionListener() {
+ return new SimpleSessionListener();
+ }
+ });
+ final PCEPSessionImpl session1 = pccMock.createClient(CLIENT1_ADDRESS,
+ new NeverReconnectStrategy(GlobalEventExecutor.INSTANCE, 500),
+ new SessionListenerFactory<PCEPSessionListener>() {
+ @Override
+ public PCEPSessionListener getSessionListener() {
+ return new SimpleSessionListener();
+ }
+ }).get();
+
+ try {
+ pccMock.createClient(CLIENT1_ADDRESS,
+ new NeverReconnectStrategy(GlobalEventExecutor.INSTANCE, 500),
+ new SessionListenerFactory<PCEPSessionListener>() {
+ @Override
+ public PCEPSessionListener getSessionListener() {
+ return new SimpleSessionListener();
+ }
+ }).get();
+ Assert.fail();
+ } catch(ExecutionException e) {
+ Assert.assertTrue(e.getMessage().contains("A conflicting session for address"));
+ } finally {
+ session1.close();
+ }
+ }
- futureChannel.channel().close();
+ @Test
+ public void testReconectClient() throws InterruptedException, ExecutionException {
+ final ChannelFuture futureChannel = this.dispatcher.createServer(new InetSocketAddress("0.0.0.0", PORT),
+ new SessionListenerFactory<PCEPSessionListener>() {
+ @Override
+ public PCEPSessionListener getSessionListener() {
+ return new SimpleSessionListener();
+ }
+ });
+ final PCEPSessionImpl session1 = pccMock.createClient(CLIENT1_ADDRESS,
+ new NeverReconnectStrategy(GlobalEventExecutor.INSTANCE, 500),
+ new SessionListenerFactory<PCEPSessionListener>() {
+ @Override
+ public PCEPSessionListener getSessionListener() {
+ return new SimpleSessionListener();
+ }
+ }).get();
+
+ Assert.assertEquals(CLIENT1_ADDRESS.getAddress().getHostAddress(), session1.getPeerAddress());
+ Assert.assertEquals(DEAD_TIMER, session1.getDeadTimerValue().shortValue());
+ Assert.assertEquals(KEEP_ALIVE, session1.getKeepAliveTimerValue().shortValue());
+ session1.close();
+
+ final PCEPSessionImpl session2 = pccMock.createClient(CLIENT1_ADDRESS,
+ new NeverReconnectStrategy(GlobalEventExecutor.INSTANCE, 500),
+ new SessionListenerFactory<PCEPSessionListener>() {
+ @Override
+ public PCEPSessionListener getSessionListener() {
+ return new SimpleSessionListener();
+ }
+ }).get();
+
+ Assert.assertEquals(CLIENT1_ADDRESS.getAddress().getHostAddress(), session2.getPeerAddress());
+ Assert.assertEquals(DEAD_TIMER, session2.getDeadTimerValue().shortValue());
+ Assert.assertEquals(KEEP_ALIVE, session2.getKeepAliveTimerValue().shortValue());
+
+ session2.close();
}
@After
public static void createPCCs(final int lspsPerPcc, final boolean pcerr, final int pccCount,
final InetAddress localAddress, final List<InetAddress> remoteAddress, final short keepalive, final short deadtimer) throws InterruptedException, ExecutionException {
- final SessionNegotiatorFactory<Message, PCEPSessionImpl, PCEPSessionListener> snf = new DefaultPCEPSessionNegotiatorFactory(
- new OpenBuilder().setKeepalive(keepalive).setDeadTimer(deadtimer).setSessionId((short) 0).build(), 0);
-
final StatefulActivator activator07 = new StatefulActivator();
activator07.start(ServiceLoaderPCEPExtensionProviderContext.getSingletonInstance());
- final PCCMock<Message, PCEPSessionImpl, PCEPSessionListener> pcc = new PCCMock<>(snf, new PCEPHandlerFactory(
- ServiceLoaderPCEPExtensionProviderContext.getSingletonInstance().getMessageHandlerRegistry()),
- new DefaultPromise<PCEPSessionImpl>(GlobalEventExecutor.INSTANCE));
-
for (final InetAddress pceAddress : remoteAddress) {
InetAddress currentAddress = localAddress;
int i = 0;
while (i < pccCount) {
final InetAddress pccAddress = currentAddress;
+ final SessionNegotiatorFactory<Message, PCEPSessionImpl, PCEPSessionListener> snf = new DefaultPCEPSessionNegotiatorFactory(
+ new OpenBuilder().setKeepalive(keepalive).setDeadTimer(deadtimer).setSessionId((short) 0).build(), 0);
+
+ final PCCMock<Message, PCEPSessionImpl, PCEPSessionListener> pcc = new PCCMock<>(snf, new PCEPHandlerFactory(
+ ServiceLoaderPCEPExtensionProviderContext.getSingletonInstance().getMessageHandlerRegistry()),
+ new DefaultPromise<PCEPSessionImpl>(GlobalEventExecutor.INSTANCE));
pcc.createClient(new InetSocketAddress(pccAddress, 0), new InetSocketAddress(pceAddress, DEFAULT_PORT),
new NeverReconnectStrategy(GlobalEventExecutor.INSTANCE, RECONNECT_STRATEGY_TIMEOUT),
new SessionListenerFactory<PCEPSessionListener>() {
org.opendaylight.protocol.pcep.testtool.Main.main(new String[]{"-a", "127.0.4.0:4189", "-ka", "10", "-d", "0", "--stateful", "--active"});
org.opendaylight.protocol.pcep.testtool.Main.main(new String[]{"-a", "127.0.2.0:4189", "-ka", "10", "-d", "0", "--stateful", "--active"});
org.opendaylight.protocol.pcep.testtool.Main.main(new String[]{"-a", "127.0.3.0:4189", "-ka", "10", "-d", "0", "--stateful", "--active"});
- Main.main(new String[] {"--local-address", "127.0.0.1", "--remote-address", "127.0.4.0,127.0.2.0,127.0.3.0"});
+ Main.main(new String[] {"--local-address", "127.0.0.1", "--remote-address", "127.0.4.0,127.0.2.0,127.0.3.0", "--pcc", "3"});
} catch (Exception e) {
Assert.fail(e.getMessage());
}
Code Review / bgpcep.git / commitdiff