handshake - simultaneous hello fix, improved exception throwing

Change-Id: Ife63e7dcba169fdc82a2e414e85b948edc222b3e
Signed-off-by: Michal Rehak <mirehak@cisco.com>

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/ConnectionConductorImpl.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/ConnectionConductorImpl.java
index 1249855749e2b919eb466b368b8604635da71a50..cdb2b5c35fcb1f0d6ffaeacbdc1a9daf39419ec3 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/ConnectionConductorImpl.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/ConnectionConductorImpl.java
@@ -8,24 +8,17 @@
 
 package org.opendaylight.openflowplugin.openflow.md.core;
 
-import java.math.BigInteger;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 
-import org.opendaylight.controller.sal.binding.api.NotificationProviderService;
 import org.opendaylight.openflowjava.protocol.api.connection.ConnectionAdapter;
 import org.opendaylight.openflowjava.protocol.api.connection.ConnectionReadyListener;
 import org.opendaylight.openflowplugin.openflow.md.core.session.OFSessionUtil;
 import org.opendaylight.openflowplugin.openflow.md.core.session.SessionContext;
 import org.opendaylight.openflowplugin.openflow.md.core.session.SessionManager;
 import org.opendaylight.openflowplugin.openflow.md.queue.QueueKeeper;
-import org.opendaylight.openflowplugin.openflow.md.util.InventoryDataServiceUtil;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNodeUpdated;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNodeUpdatedBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeUpdated;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeUpdatedBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.MultipartRequestFlags;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.MultipartType;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.EchoInputBuilder;
@@ -43,7 +36,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.OpenflowProtocolListener;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.PacketInMessage;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.PortStatusMessage;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.multipart.reply.multipart.reply.body.MultipartReplyDesc;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.multipart.request.multipart.request.body.MultipartRequestDescBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.multipart.request.multipart.request.body.MultipartRequestPortDescBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.system.rev130927.DisconnectEvent;
@@ -70,7 +62,7 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
      * it will help while testing and isolating issues related to processing of
      * BitMaps from switches.
      */
-    protected boolean isBitmapNegotiationEnable = true;
+    private boolean isBitmapNegotiationEnable = true;
     protected ErrorHandler errorHandler;
 
     private final ConnectionAdapter connectionAdapter;
@@ -81,13 +73,12 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
 
     private SessionContext sessionContext;
 
-    protected boolean isFirstHelloNegotiation = true;
-    protected Short lastProposedVersion = null;
-
     private QueueKeeper<OfHeader, DataObject> queueKeeper;
     private ExecutorService hsPool;
     private HandshakeManager handshakeManager;
 
+    private boolean firstHelloProcessed;
+
     /**
      * @param connectionAdapter
      */
@@ -95,6 +86,7 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
         this.connectionAdapter = connectionAdapter;
         conductorState = CONDUCTOR_STATE.HANDSHAKING;
         hsPool = Executors.newFixedThreadPool(1);
+        firstHelloProcessed = false;
         handshakeManager = new HandshakeManagerImpl(connectionAdapter,
                 ConnectionConductor.versionOrder.get(0), ConnectionConductor.versionOrder);
         handshakeManager.setUseVersionBitmap(isBitmapNegotiationEnable);
@@ -168,9 +160,11 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
     @Override
     public synchronized void onHelloMessage(final HelloMessage hello) {
         LOG.debug("processing HELLO.xid{}", hello.getXid());
+        firstHelloProcessed = true;
         checkState(CONDUCTOR_STATE.HANDSHAKING);
-        handshakeManager.setReceivedHello(hello);
-        hsPool.execute(handshakeManager);
+        HandshakeStepWrapper handshakeStepWrapper = new HandshakeStepWrapper(
+                hello, handshakeManager, connectionAdapter);
+        hsPool.execute(handshakeStepWrapper);
     }
 
     /**
@@ -241,7 +235,8 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
                         }
                     } catch (Exception e) {
                         LOG.error("while waiting for echoReply in TIMEOUTING state: "
-                                + e.getMessage(), e);
+                                + e.getMessage());
+                        errorHandler.handleException(e, sessionContext);
                         //switch is not responding
                         disconnect();
                         OFSessionUtil.getSessionManager().invalidateOnDisconnect(ConnectionConductorImpl.this);
@@ -328,9 +323,16 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
     }
 
     @Override
-    public void onConnectionReady() {
+    public synchronized void onConnectionReady() {
         LOG.debug("connection is ready-to-use");
-        hsPool.execute(handshakeManager);
+        if (! firstHelloProcessed) {
+            HandshakeStepWrapper handshakeStepWrapper = new HandshakeStepWrapper(
+                    null, handshakeManager, connectionAdapter);
+            hsPool.execute(handshakeStepWrapper);
+            firstHelloProcessed = true;
+        } else {
+            LOG.debug("already touched by hello message");
+        }
     }
 
     @Override

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/HandshakeManager.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/HandshakeManager.java
index d2e0c14a6801d43b7dcb9fa8064e86687f353318..3a8995a6e10111d0e61db60cff6ff84df6a7ba35 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/HandshakeManager.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/HandshakeManager.java
@@ -14,7 +14,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731
  * @author mirehak
  *
  */
-public interface HandshakeManager extends Runnable {
+public interface HandshakeManager {
 
     /**
      * @return negotiated version
@@ -45,4 +45,9 @@ public interface HandshakeManager extends Runnable {
      * @param isBitmapNegotiationEnable
      */
     void setUseVersionBitmap(boolean isBitmapNegotiationEnable);
+
+    /**
+     * process current handshake step
+     */
+    void shake();
 }

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/HandshakeManagerImpl.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/HandshakeManagerImpl.java
index e7e0a98eff9da18ec28012c2ed85ca3340ea62ab..50567adefd06df6eed3dd3e1a2854184328cfb26 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/HandshakeManagerImpl.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/HandshakeManagerImpl.java
@@ -73,28 +73,28 @@ public class HandshakeManagerImpl implements HandshakeManager {
     }
 
     @Override
-    public synchronized void run() {
+    public synchronized void shake() {
         LOG.info("handshake STARTED");
         setActiveXid(20L);
         HelloMessage receivedHelloLoc = receivedHello;
         
-        if (receivedHelloLoc == null) {
-            // first Hello sending
-            sendHelloMessage(highestVersion, getNextXid());
-            lastProposedVersion = highestVersion;
-            LOG.debug("ret - firstHello+wait");
-            return;
-        }
-        
-        // process the 2. and later hellos
-        Short remoteVersion = receivedHelloLoc.getVersion();
-        List<Elements> elements = receivedHelloLoc.getElements();
-        setActiveXid(receivedHelloLoc.getXid());
-        List<Boolean> remoteVersionBitmap = MessageFactory.digVersions(elements);
-        LOG.debug("Hello message: version={}, bitmap={}, xid={}", remoteVersion, 
-                remoteVersionBitmap, receivedHelloLoc.getXid());
-        
         try {
+            if (receivedHelloLoc == null) {
+                // first Hello sending
+                sendHelloMessage(highestVersion, getNextXid());
+                lastProposedVersion = highestVersion;
+                LOG.debug("ret - firstHello+wait");
+                return;
+            }
+
+            // process the 2. and later hellos
+            Short remoteVersion = receivedHelloLoc.getVersion();
+            List<Elements> elements = receivedHelloLoc.getElements();
+            setActiveXid(receivedHelloLoc.getXid());
+            List<Boolean> remoteVersionBitmap = MessageFactory.digVersions(elements);
+            LOG.debug("Hello message: version={}, bitmap={}, xid={}", remoteVersion, 
+                    remoteVersionBitmap, receivedHelloLoc.getXid());
+        
             if (useVersionBitmap && remoteVersionBitmap != null) {
                 // versionBitmap on both sides -> ONE STEP DECISION
                 handleVersionBitmapNegotiation(elements);
@@ -102,7 +102,7 @@ public class HandshakeManagerImpl implements HandshakeManager {
                 // versionBitmap missing at least on one side -> STEP-BY-STEP NEGOTIATION applying 
                 handleStepByStepVersionNegotiation(remoteVersion);
             }
-        } catch (Exception ex) {
+        } catch (Throwable ex) {
             errorHandler.handleException(ex, null);
             connectionAdapter.disconnect();
             LOG.debug("ret - "+ex.getMessage());
@@ -111,8 +111,9 @@ public class HandshakeManagerImpl implements HandshakeManager {
 
     /**
      * @param remoteVersion
+     * @throws Throwable 
      */
-    private void handleStepByStepVersionNegotiation(Short remoteVersion) {
+    private void handleStepByStepVersionNegotiation(Short remoteVersion) throws Throwable {
         LOG.debug("remoteVersion:{} lastProposedVersion:{}, highestVersion:{}", 
                 remoteVersion, lastProposedVersion, highestVersion);
         if (remoteVersion == lastProposedVersion) {
@@ -133,8 +134,9 @@ public class HandshakeManagerImpl implements HandshakeManager {
 
     /**
      * @param remoteVersion
+     * @throws Throwable 
      */
-    private void handleLowerVersionProposal(Short remoteVersion) {
+    private void handleLowerVersionProposal(Short remoteVersion) throws Throwable {
         Short proposedVersion;
         // find the version from header version field
         proposedVersion = proposeNextVersion(remoteVersion);
@@ -151,8 +153,9 @@ public class HandshakeManagerImpl implements HandshakeManager {
 
     /**
      * @param elements
+     * @throws Throwable 
      */
-    private void handleVersionBitmapNegotiation(List<Elements> elements) {
+    private void handleVersionBitmapNegotiation(List<Elements> elements) throws Throwable {
         Short proposedVersion;
         proposedVersion = proposeCommonBitmapVersion(elements);
         postHandshake(proposedVersion, getNextXid());
@@ -246,8 +249,9 @@ public class HandshakeManagerImpl implements HandshakeManager {
      * send hello reply without versionBitmap
      * @param helloVersion
      * @param helloXid
+     * @throws Throwable 
      */
-    protected void sendHelloMessage(Short helloVersion, Long helloXid) {
+    private void sendHelloMessage(Short helloVersion, Long helloXid) throws Throwable {
         //Short highestVersion = ConnectionConductor.versionOrder.get(0);
         //final Long helloXid = 21L;
         HelloInput helloInput = MessageFactory.createHelloInput(helloVersion, helloXid, versionOrder);
@@ -261,7 +265,7 @@ public class HandshakeManagerImpl implements HandshakeManager {
             LOG.debug("FIRST HELLO sent.");
         } catch (Throwable e) {
             LOG.debug("FIRST HELLO sending failed.");
-            errorHandler.handleException(e, null);
+            throw e;
         }
     }
 
@@ -269,8 +273,9 @@ public class HandshakeManagerImpl implements HandshakeManager {
      * after handshake set features, register to session
      * @param proposedVersion
      * @param xId
+     * @throws Throwable 
      */
-    protected void postHandshake(Short proposedVersion, Long xid) {
+    protected void postHandshake(Short proposedVersion, Long xid) throws Throwable {
         // set version
         version = proposedVersion;
         LOG.debug("version set: " + proposedVersion);
@@ -299,8 +304,8 @@ public class HandshakeManagerImpl implements HandshakeManager {
         } catch (Throwable e) {
             //handshake failed
             LOG.error("issuing disconnect during handshake, reason: "+e.getMessage());
-            errorHandler.handleException(e, null);
             connectionAdapter.disconnect();
+            throw e;
         }
         
         LOG.debug("postHandshake DONE");

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/HandshakeStepWrapper.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/HandshakeStepWrapper.java
new file mode 100644 (file)
index 0000000..4f2f523
--- /dev/null
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/HandshakeStepWrapper.java
@@ -0,0 +1,53 @@
+/**
+ * Copyright (c) 2013 Cisco Systems, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+
+package org.opendaylight.openflowplugin.openflow.md.core;
+
+import org.opendaylight.openflowjava.protocol.api.connection.ConnectionAdapter;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.HelloMessage;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * @author mirehak
+ *
+ */
+public class HandshakeStepWrapper implements Runnable {
+    
+    private static final Logger LOG = LoggerFactory
+            .getLogger(HandshakeStepWrapper.class);
+    
+    private HelloMessage helloMessage;
+    private HandshakeManager handshakeManager;
+    private ConnectionAdapter connectionAdapter;
+    
+    
+    
+    /**
+     * @param helloMessage
+     * @param handshakeManager
+     * @param connectionAdapter 
+     */
+    public HandshakeStepWrapper(HelloMessage helloMessage,
+            HandshakeManager handshakeManager, ConnectionAdapter connectionAdapter) {
+        this.helloMessage = helloMessage;
+        this.handshakeManager = handshakeManager;
+        this.connectionAdapter = connectionAdapter;
+    }
+
+    @Override
+    public void run() {
+        if (connectionAdapter.isAlive()) {
+            handshakeManager.setReceivedHello(helloMessage);
+            handshakeManager.shake();
+        } else {
+            LOG.debug("connection is down - skipping handshake step");
+        }
+    }
+
+}