Correct PCEP Session ID 12/104012/2
authorRobert Varga <robert.varga@pantheon.tech>
Thu, 12 Jan 2023 13:31:29 +0000 (14:31 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 12 Jan 2023 13:58:26 +0000 (14:58 +0100)
Session ID is defined to be an uint8, make sure our APIs reflect that.

Change-Id: Ibd3e09c3d22740702c3a3054066563beffb07038
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
pcep/api/src/main/java/org/opendaylight/protocol/pcep/PCEPSessionProposalFactory.java
pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/AbstractPCEPSessionNegotiatorFactory.java
pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/BasePCEPSessionProposalFactory.java
pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/DefaultPCEPSessionNegotiator.java
pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/DefaultPCEPSessionNegotiatorFactory.java
pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPPeerRegistry.java
pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPSessionNegotiator.java
pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PeerRecord.java
pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/FiniteStateMachineTest.java
pcep/topology/topology-provider/src/test/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractPCEPSessionTest.java

index d08e8a4ccaeda383bde95f1a0136a531c2cebba8..0067bc07d0bc20b5945ba5a85819905207542f54 100644 (file)
@@ -12,6 +12,7 @@ import java.util.List;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev181109.open.object.Open;
+import org.opendaylight.yangtools.yang.common.Uint8;
 
 /**
  * Factory for generating PCEP Session proposals. Used by a server.
@@ -26,7 +27,7 @@ public interface PCEPSessionProposalFactory {
      * @param peerProposal for including information from peer to our Open message
      * @return specific session proposal
      */
-    @NonNull Open getSessionProposal(@NonNull InetSocketAddress address, int sessionId,
+    @NonNull Open getSessionProposal(@NonNull InetSocketAddress address, @NonNull Uint8 sessionId,
             @Nullable PCEPPeerProposal peerProposal);
 
     /**
index 8f1e3a3d79d6dfc559f993ee491014931abb4b0c..1f7bd031e816d6450c4add42595ec19c4f007161 100644 (file)
@@ -12,6 +12,7 @@ import io.netty.util.concurrent.Promise;
 import org.opendaylight.protocol.pcep.PCEPSessionNegotiatorFactory;
 import org.opendaylight.protocol.pcep.PCEPSessionNegotiatorFactoryDependencies;
 import org.opendaylight.protocol.pcep.SessionNegotiator;
+import org.opendaylight.yangtools.yang.common.Uint8;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -37,7 +38,7 @@ public abstract class AbstractPCEPSessionNegotiatorFactory implements PCEPSessio
     protected abstract AbstractPCEPSessionNegotiator createNegotiator(
             PCEPSessionNegotiatorFactoryDependencies snd,
             Promise<PCEPSessionImpl> promise,
-            Channel channel, short sessionId);
+            Channel channel, Uint8 sessionId);
 
     @Override
     public final SessionNegotiator getSessionNegotiator(final PCEPSessionNegotiatorFactoryDependencies dependencies,
@@ -48,6 +49,6 @@ public abstract class AbstractPCEPSessionNegotiatorFactory implements PCEPSessio
     }
 
     public PCEPPeerRegistry getSessionRegistry() {
-        return this.sessionRegistry;
+        return sessionRegistry;
     }
 }
index 61f447c21bd9ab0ee0e95b7eac15bb374ce71916..2541723d9bb96656654b048f384f46ec89a86720 100644 (file)
@@ -52,7 +52,7 @@ public final class BasePCEPSessionProposalFactory implements PCEPSessionProposal
     }
 
     @Override
-    public Open getSessionProposal(final InetSocketAddress address, final int sessionId,
+    public Open getSessionProposal(final InetSocketAddress address, final Uint8 sessionId,
             final PCEPPeerProposal peerProposal) {
         final var builder = new TlvsBuilder();
         for (final var capability : capabilities) {
@@ -64,7 +64,7 @@ public final class BasePCEPSessionProposalFactory implements PCEPSessionProposal
         }
 
         return new OpenBuilder()
-            .setSessionId(Uint8.valueOf(sessionId))
+            .setSessionId(sessionId)
             .setKeepalive(keepAlive)
             .setDeadTimer(deadTimer)
             .setTlvs(builder.build())
index 30366878ff5d13c1b42ae0e4bf085694c302dcc8..1b94701f95c19dd7bc10f57aebdd036facd19f16 100644 (file)
@@ -23,7 +23,7 @@ public final class DefaultPCEPSessionNegotiator extends AbstractPCEPSessionNegot
     private final int maxUnknownMessages;
 
     public DefaultPCEPSessionNegotiator(final Promise<PCEPSessionImpl> promise, final Channel channel,
-            final PCEPSessionListener listener, final short sessionId, final int maxUnknownMessages,
+            final PCEPSessionListener listener, final Uint8 sessionId, final int maxUnknownMessages,
             final Open localPrefs, final Tls tlsConfiguration) {
         super(promise, channel, tlsConfiguration);
         this.listener = requireNonNull(listener);
@@ -31,13 +31,13 @@ public final class DefaultPCEPSessionNegotiator extends AbstractPCEPSessionNegot
         myLocalPrefs = new OpenBuilder()
                 .setKeepalive(localPrefs.getKeepalive())
                 .setDeadTimer(localPrefs.getDeadTimer())
-                .setSessionId(Uint8.valueOf(sessionId))
+                .setSessionId(requireNonNull(sessionId))
                 .setTlvs(localPrefs.getTlvs())
                 .build();
     }
 
     public DefaultPCEPSessionNegotiator(final Promise<PCEPSessionImpl> promise, final Channel channel,
-            final PCEPSessionListener listener, final short sessionId, final int maxUnknownMessages,
+            final PCEPSessionListener listener, final Uint8 sessionId, final int maxUnknownMessages,
             final Open localPrefs) {
         this(promise, channel, listener, sessionId, maxUnknownMessages, localPrefs, null);
     }
index acfc687f7dc2bdbb0cf59daffcad55d8e88a8bbd..f9e3643a5e64b6b4150476ac287331e786747b3f 100644 (file)
@@ -17,6 +17,7 @@ import org.opendaylight.protocol.pcep.PCEPSessionProposalFactory;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.pcep.app.config.rev160707.PcepDispatcherConfig;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.pcep.app.config.rev160707.pcep.dispatcher.config.Tls;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev181109.open.object.Open;
+import org.opendaylight.yangtools.yang.common.Uint8;
 
 public final class DefaultPCEPSessionNegotiatorFactory extends AbstractPCEPSessionNegotiatorFactory {
     private final PCEPSessionProposalFactory spf;
@@ -44,22 +45,22 @@ public final class DefaultPCEPSessionNegotiatorFactory extends AbstractPCEPSessi
             final PCEPSessionNegotiatorFactoryDependencies sessionNegotiatorDependencies,
             final Promise<PCEPSessionImpl> promise,
             final Channel channel,
-            final short sessionId) {
+            final Uint8 sessionId) {
 
-        final Open proposal = this.spf.getSessionProposal((InetSocketAddress) channel.remoteAddress(), sessionId,
+        final Open proposal = spf.getSessionProposal((InetSocketAddress) channel.remoteAddress(), sessionId,
                 sessionNegotiatorDependencies.getPeerProposal());
         return new DefaultPCEPSessionNegotiator(
                 promise,
                 channel,
                 sessionNegotiatorDependencies.getListenerFactory().getSessionListener(),
                 sessionId,
-                this.maxUnknownMessages,
+                maxUnknownMessages,
                 proposal,
-                this.tlsConfiguration);
+                tlsConfiguration);
     }
 
     @Override
     public PCEPSessionProposalFactory getPCEPSessionProposalFactory() {
-        return this.spf;
+        return spf;
     }
 }
index f82b73ed97ed3ec0dc02201ca91dbf09c441de92..700cf4b16235c79450fea22ad5a2799182bb9eb6 100644 (file)
@@ -17,6 +17,7 @@ import java.util.Optional;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import org.checkerframework.checker.lock.qual.GuardedBy;
+import org.opendaylight.yangtools.yang.common.Uint8;
 
 // This class is thread-safe
 final class PCEPPeerRegistry {
@@ -47,12 +48,12 @@ final class PCEPPeerRegistry {
     private final Map<ByteArrayWrapper, SessionReference> sessions = new HashMap<>();
 
     protected interface SessionReference extends AutoCloseable {
-        Short getSessionId();
+        Uint8 getSessionId();
     }
 
 
     protected synchronized Optional<SessionReference> getSessionReference(final byte[] clientAddress) {
-        final SessionReference sessionReference = this.sessions.get(new ByteArrayWrapper(clientAddress));
+        final SessionReference sessionReference = sessions.get(new ByteArrayWrapper(clientAddress));
         if (sessionReference != null) {
             return Optional.of(sessionReference);
         }
@@ -60,32 +61,27 @@ final class PCEPPeerRegistry {
     }
 
     protected synchronized Optional<SessionReference> removeSessionReference(final byte[] clientAddress) {
-        final SessionReference sessionReference = this.sessions.remove(new ByteArrayWrapper(clientAddress));
-        if (sessionReference != null) {
-            return Optional.of(sessionReference);
-        }
-        return Optional.empty();
+        return Optional.ofNullable(sessions.remove(new ByteArrayWrapper(clientAddress)));
     }
 
     protected synchronized void putSessionReference(final byte[] clientAddress,
             final SessionReference sessionReference) {
-        this.sessions.put(new ByteArrayWrapper(clientAddress), sessionReference);
+        sessions.put(new ByteArrayWrapper(clientAddress), sessionReference);
     }
 
-    protected synchronized Short nextSession(final byte[] clientAddress) throws ExecutionException {
+    protected synchronized Uint8 nextSession(final byte[] clientAddress) throws ExecutionException {
         final PeerRecord peer =
-            this.formerClients.get(new ByteArrayWrapper(clientAddress), () -> new PeerRecord(ID_CACHE_SECONDS, null));
+            formerClients.get(new ByteArrayWrapper(clientAddress), () -> new PeerRecord(ID_CACHE_SECONDS, null));
 
         return peer.allocId();
     }
 
-    protected synchronized void releaseSession(final byte[] clientAddress, final short sessionId)
+    protected synchronized void releaseSession(final byte[] clientAddress, final Uint8 sessionId)
             throws ExecutionException {
-        this.formerClients.get(new ByteArrayWrapper(clientAddress), () -> new PeerRecord(ID_CACHE_SECONDS, sessionId));
+        formerClients.get(new ByteArrayWrapper(clientAddress), () -> new PeerRecord(ID_CACHE_SECONDS, sessionId));
     }
 
     private static final class ByteArrayWrapper {
-
         private final byte[] byteArray;
 
         ByteArrayWrapper(final byte[] byteArray) {
@@ -94,18 +90,12 @@ final class PCEPPeerRegistry {
 
         @Override
         public int hashCode() {
-            return Arrays.hashCode(this.byteArray);
+            return Arrays.hashCode(byteArray);
         }
 
         @Override
         public boolean equals(final Object obj) {
-            if (this == obj) {
-                return true;
-            }
-            if (!(obj instanceof ByteArrayWrapper)) {
-                return false;
-            }
-            return Arrays.equals(this.byteArray, ((ByteArrayWrapper) obj).byteArray);
+            return this == obj || obj instanceof ByteArrayWrapper other && Arrays.equals(byteArray, other.byteArray);
         }
     }
 }
index 022e3f1ee46ff8c4e22e5f4b956f021ed4b8d873..6e156740beeac8ff2404039c1f94229cd16f5153 100644 (file)
@@ -18,6 +18,7 @@ import java.util.concurrent.ExecutionException;
 import org.opendaylight.protocol.pcep.PCEPSessionNegotiatorFactoryDependencies;
 import org.opendaylight.protocol.pcep.impl.PCEPPeerRegistry.SessionReference;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev181109.Message;
+import org.opendaylight.yangtools.yang.common.Uint8;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -33,7 +34,7 @@ public class PCEPSessionNegotiator extends AbstractSessionNegotiator {
             final PCEPSessionNegotiatorFactoryDependencies dependencies,
             final AbstractPCEPSessionNegotiatorFactory negFactory) {
         super(promise, channel);
-        this.nfd = dependencies;
+        nfd = dependencies;
         this.negFactory = negFactory;
     }
 
@@ -43,19 +44,19 @@ public class PCEPSessionNegotiator extends AbstractSessionNegotiator {
     protected void startNegotiation() throws ExecutionException {
         final Object lock = this;
 
-        LOG.debug("Bootstrap negotiation for channel {} started", this.channel);
+        LOG.debug("Bootstrap negotiation for channel {} started", channel);
 
         /*
          * We have a chance to see if there's a client session already
          * registered for this client.
          */
-        final byte[] clientAddress = ((InetSocketAddress) this.channel.remoteAddress()).getAddress().getAddress();
-        final PCEPPeerRegistry sessionReg = this.negFactory.getSessionRegistry();
+        final byte[] clientAddress = ((InetSocketAddress) channel.remoteAddress()).getAddress().getAddress();
+        final PCEPPeerRegistry sessionReg = negFactory.getSessionRegistry();
 
         synchronized (lock) {
             if (sessionReg.getSessionReference(clientAddress).isPresent()) {
                 final byte[] serverAddress =
-                    ((InetSocketAddress) this.channel.localAddress()).getAddress().getAddress();
+                    ((InetSocketAddress) channel.localAddress()).getAddress().getAddress();
                 if (COMPARATOR.compare(serverAddress, clientAddress) > 0) {
                     final Optional<SessionReference> sessionRefMaybe = sessionReg.removeSessionReference(clientAddress);
                     try {
@@ -67,14 +68,13 @@ public class PCEPSessionNegotiator extends AbstractSessionNegotiator {
                     }
                 } else {
                     negotiationFailed(new IllegalStateException("A conflicting session for address "
-                            + ((InetSocketAddress) this.channel.remoteAddress()).getAddress() + " found."));
+                            + ((InetSocketAddress) channel.remoteAddress()).getAddress() + " found."));
                     return;
                 }
             }
 
-            final Short sessionId = sessionReg.nextSession(clientAddress);
-            final AbstractPCEPSessionNegotiator n = this.negFactory
-                    .createNegotiator(this.nfd, this.promise, this.channel, sessionId);
+            final Uint8 sessionId = sessionReg.nextSession(clientAddress);
+            final AbstractPCEPSessionNegotiator n = negFactory.createNegotiator(nfd, promise, channel, sessionId);
 
             sessionReg.putSessionReference(clientAddress, new SessionReference() {
                 @Override
@@ -87,19 +87,19 @@ public class PCEPSessionNegotiator extends AbstractSessionNegotiator {
                 }
 
                 @Override
-                public Short getSessionId() {
+                public Uint8 getSessionId() {
                     return sessionId;
                 }
             });
 
-            this.channel.closeFuture().addListener((ChannelFutureListener) future -> {
+            channel.closeFuture().addListener((ChannelFutureListener) future -> {
                 synchronized (lock) {
                     sessionReg.removeSessionReference(clientAddress);
                 }
             });
 
-            LOG.info("Replacing bootstrap negotiator for channel {}", this.channel);
-            this.channel.pipeline().replace(this, "negotiator", n);
+            LOG.info("Replacing bootstrap negotiator for channel {}", channel);
+            channel.pipeline().replace(this, "negotiator", n);
             n.startNegotiation();
         }
     }
index 0911a8060d00a446c24c0e3fee845061b0890c10..a868d26d60b9baa46ae4e3e51029bf14c1f5fdb0 100644 (file)
@@ -12,6 +12,7 @@ import com.google.common.cache.CacheBuilder;
 import java.util.concurrent.TimeUnit;
 import org.checkerframework.checker.lock.qual.GuardedBy;
 import org.opendaylight.protocol.util.Values;
+import org.opendaylight.yangtools.yang.common.Uint8;
 
 // This class is thread-safe
 final class PeerRecord {
@@ -20,25 +21,27 @@ final class PeerRecord {
     private final Cache<Short, Short> pastIds;
 
     @GuardedBy("this")
-    private Short lastId;
+    private Uint8 lastId;
 
-    PeerRecord(final long idLifetimeSeconds, final Short lastId) {
+    PeerRecord(final long idLifetimeSeconds, final Uint8 lastId) {
         // Note that the cache is limited to 255 entries -- which means we will always have
         // a single entry available. That number will be the Last Recently Used ID.
-        this.pastIds = CacheBuilder.newBuilder().expireAfterWrite(idLifetimeSeconds, TimeUnit.SECONDS)
+        pastIds = CacheBuilder.newBuilder().expireAfterWrite(idLifetimeSeconds, TimeUnit.SECONDS)
                 .maximumSize(Values.UNSIGNED_BYTE_MAX_VALUE).build();
         this.lastId = lastId;
     }
 
-    synchronized Short allocId() {
-        short id = this.lastId == null ? 0 : this.lastId;
+    synchronized Uint8 allocId() {
+        short id = lastId == null ? 0 : lastId.toJava();
 
-        while (this.pastIds.getIfPresent(id) != null) {
+        while (pastIds.getIfPresent(id) != null) {
             id = (short) ((id + 1) % Values.UNSIGNED_BYTE_MAX_VALUE);
         }
 
-        this.pastIds.put(id, id);
-        this.lastId = id;
-        return id;
+        pastIds.put(id, id);
+
+        final var ret = Uint8.valueOf(id);
+        lastId = ret;
+        return ret;
     }
 }
index 9858d20de75ea0ff010536332a0096ce91c0312b..9597b11609a391ec534651560bf64e6e286a6e96 100644 (file)
@@ -47,9 +47,9 @@ public class FiniteStateMachineTest extends AbstractPCEPSessionTest {
         final org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev181109.open.object.Open
             localPrefs = new OpenBuilder().setKeepalive(Uint8.ONE).build();
         serverSession = new DefaultPCEPSessionNegotiator(new DefaultPromise<>(GlobalEventExecutor.INSTANCE),
-                channel, listener, (short) 1, 20, localPrefs);
+                channel, listener, Uint8.ONE, 20, localPrefs);
         tlsSessionNegotiator = new DefaultPCEPSessionNegotiator(new DefaultPromise<>(GlobalEventExecutor.INSTANCE),
-                channel, listener, (short) 1, 20, localPrefs, new TlsBuilder().build());
+                channel, listener, Uint8.ONE, 20, localPrefs, new TlsBuilder().build());
     }
 
     /**
@@ -75,7 +75,7 @@ public class FiniteStateMachineTest extends AbstractPCEPSessionTest {
     public void testEstablishTLS() {
         final DefaultPCEPSessionNegotiator negotiator =
             new DefaultPCEPSessionNegotiator(new DefaultPromise<>(GlobalEventExecutor.INSTANCE),
-                channel, listener, (short) 1, 20, new OpenBuilder().setKeepalive(Uint8.ONE).build(),
+                channel, listener, Uint8.ONE, 20, new OpenBuilder().setKeepalive(Uint8.ONE).build(),
                 SslContextFactoryTest.createTlsConfig());
         negotiator.channelActive(null);
         assertEquals(1, msgsSend.size());
index 56b09f58dada5abecfea3e0d48305164a6416eca..73f8856b50242ee62e40a1ef572618a0fd325540 100644 (file)
@@ -138,7 +138,7 @@ public abstract class AbstractPCEPSessionTest extends AbstractConcurrentDataBrok
         manager = customizeSessionManager(new ServerSessionManager(TOPO_IID, topologyDependencies,
                 new GraphKey("graph-test"), RPC_TIMEOUT, TimeUnit.SECONDS.toNanos(5)));
         startSessionManager();
-        neg = new DefaultPCEPSessionNegotiator(promise, clientListener, manager.getSessionListener(), (short) 1, 5,
+        neg = new DefaultPCEPSessionNegotiator(promise, clientListener, manager.getSessionListener(), Uint8.ONE, 5,
             localPrefs);
         topologyRpcs = new TopologyRPCs(manager);
     }