[ONOS-6113] BGP hold time expiry handling

Change-Id: I1fe437790b07a43c596c1d93b2017564a54ce3a5
diff --git a/protocols/bgp/ctl/src/main/java/org/onosproject/bgp/controller/impl/BgpChannelHandler.java b/protocols/bgp/ctl/src/main/java/org/onosproject/bgp/controller/impl/BgpChannelHandler.java
index 1f6e5bc..31a16f9 100644
--- a/protocols/bgp/ctl/src/main/java/org/onosproject/bgp/controller/impl/BgpChannelHandler.java
+++ b/protocols/bgp/ctl/src/main/java/org/onosproject/bgp/controller/impl/BgpChannelHandler.java
@@ -24,8 +24,10 @@
 import org.jboss.netty.channel.ExceptionEvent;
 import org.jboss.netty.channel.MessageEvent;
 import org.jboss.netty.handler.timeout.IdleStateAwareChannelHandler;
-import org.jboss.netty.handler.timeout.ReadTimeoutException;
-import org.jboss.netty.handler.timeout.ReadTimeoutHandler;
+import org.jboss.netty.util.HashedWheelTimer;
+import org.jboss.netty.util.Timeout;
+import org.jboss.netty.util.Timer;
+import org.jboss.netty.util.TimerTask;
 import org.onlab.packet.Ip4Address;
 import org.onlab.packet.IpAddress;
 import org.onosproject.bgp.controller.BgpCfg;
@@ -60,6 +62,9 @@
 import java.util.ListIterator;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.RejectedExecutionException;
+import java.util.concurrent.TimeUnit;
+
+import static org.onlab.util.Tools.groupedThreads;
 
 /**
  * Channel handler deals with the bgp peer connection and dispatches messages from peer to the appropriate locations.
@@ -73,8 +78,8 @@
     private BgpId thisbgpId;
     private Channel channel;
     private BgpKeepAliveTimer keepAliveTimer = null;
+    private short minHoldTime = 0;
     private short peerHoldTime = 0;
-    private short negotiatedHoldTime = 0;
     private long peerAsNum;
     private int peerIdentifier;
     private BgpPacketStatsImpl bgpPacketStats;
@@ -112,6 +117,8 @@
     private String peerAddr;
     private BgpCfg bgpconfig;
     List<BgpValueType> remoteBgpCapability;
+    private Timer timer = new HashedWheelTimer(groupedThreads("BgpChannel", "timer-%d", log));
+    private volatile Timeout holdTimerTimeout;
 
     /**
      * Create a new unconnected BGPChannelHandler.
@@ -183,14 +190,9 @@
                          * received in the OPEN message
                          */
                         h.peerHoldTime = pOpenmsg.getHoldTime();
-                        if (h.peerHoldTime < h.bgpconfig.getHoldTime()) {
-                            h.channel.getPipeline().replace("holdTime",
-                                                            "holdTime",
-                                                            new ReadTimeoutHandler(BgpPipelineFactory.TIMER,
-                                                                                   h.peerHoldTime));
-                        }
-
-                        log.info("Hold Time : " + h.peerHoldTime);
+                        h.minHoldTime = (short) Math.min(h.bgpconfig.getHoldTime(), pOpenmsg.getHoldTime());
+                        h.restartHoldTimeoutTimer();
+                        log.info("Hold Time : " + h.minHoldTime);
 
                         // update AS number
                         h.peerAsNum = pOpenmsg.getAsNumber();
@@ -238,14 +240,9 @@
                          * received in the OPEN message
                          */
                         h.peerHoldTime = pOpenmsg.getHoldTime();
-                        if (h.peerHoldTime < h.bgpconfig.getHoldTime()) {
-                            h.channel.getPipeline().replace("holdTime",
-                                                            "holdTime",
-                                                            new ReadTimeoutHandler(BgpPipelineFactory.TIMER,
-                                                                                   h.peerHoldTime));
-                        }
-
-                        log.debug("Hold Time : " + h.peerHoldTime);
+                        h.minHoldTime = (short) Math.min(h.bgpconfig.getHoldTime(), pOpenmsg.getHoldTime());
+                        h.restartHoldTimeoutTimer();
+                        log.debug("Hold Time : " + h.minHoldTime);
 
                         // update AS number
                         h.peerAsNum = pOpenmsg.getAsNumber();
@@ -279,11 +276,8 @@
                     final InetSocketAddress inetAddress = (InetSocketAddress) h.address;
                     h.thisbgpId = BgpId.bgpId(IpAddress.valueOf(inetAddress.getAddress()));
 
-                    // set session parameters
-                    h.negotiatedHoldTime = (h.peerHoldTime < h.bgpconfig.getHoldTime()) ? h.peerHoldTime
-                                                                                        : h.bgpconfig.getHoldTime();
                     h.sessionInfo = new BgpSessionInfoImpl(h.thisbgpId, h.bgpVersion, h.peerAsNum, h.peerHoldTime,
-                                                           h.peerIdentifier, h.negotiatedHoldTime, h.isIbgpSession,
+                                                           h.peerIdentifier, h.minHoldTime, h.isIbgpSession,
                                                            h.remoteBgpCapability);
 
                     h.bgpPeer = h.peerManager.getBgpPeerInstance(h.bgpController, h.sessionInfo, h.bgpPacketStats);
@@ -297,9 +291,9 @@
                      * time between KEEPALIVE messages would be one third of the Hold Time interval.
                      */
 
-                    if (h.negotiatedHoldTime != 0) {
+                    if (h.minHoldTime != 0) {
                         h.keepAliveTimer = new BgpKeepAliveTimer(h,
-                                                                (h.negotiatedHoldTime / BGP_MAX_KEEPALIVE_INTERVAL));
+                                                                (h.minHoldTime / BGP_MAX_KEEPALIVE_INTERVAL));
                     } else {
                         h.sendKeepAliveMessage();
                     }
@@ -308,6 +302,7 @@
 
                     // set the state handshake completion.
                     h.setHandshakeComplete(true);
+                    h.restartHoldTimeoutTimer();
 
                     if (!h.peerManager.addConnectedPeer(h.thisbgpId, h.bgpPeer)) {
                         disconnectDuplicate(h);
@@ -324,6 +319,7 @@
             void processBgpMessage(BgpChannelHandler h, BgpMessage m) throws IOException, BgpParseException {
                 log.debug("Message received in established state " + m.getType());
                 // dispatch the message
+                h.restartHoldTimeoutTimer();
                 h.dispatchMessage(m);
             }
         };
@@ -375,6 +371,17 @@
         }
     }
 
+    //Stop hold timer
+    private void stopSessionTimers() {
+        if ((keepAliveTimer != null) && (keepAliveTimer.getKeepAliveTimer() != null)) {
+            keepAliveTimer.getKeepAliveTimer().cancel();
+        }
+
+        if (holdTimerTimeout != null) {
+            holdTimerTimeout.cancel();
+        }
+    }
+
     // *************************
     // Channel handler methods
     // *************************
@@ -485,7 +492,7 @@
                 duplicateBgpIdFound = Boolean.FALSE;
             }
 
-           stopKeepAliveTimer();
+            stopSessionTimers();
         } else {
             bgpconfig.setPeerConnState(peerAddr, BgpPeerCfg.State.IDLE);
             log.warn("No bgp ip in channelHandler registered for " + "disconnected peer {}", getPeerInfoString());
@@ -497,16 +504,7 @@
 
         log.error("[exceptionCaught]: " + e.toString());
 
-        if (e.getCause() instanceof ReadTimeoutException) {
-            // device timeout
-            bgpController.closedSessionExceptionAdd(peerAddr, e.getCause().toString());
-            log.error("Disconnecting device {} due to read timeout", getPeerInfoString());
-            sendNotification(BgpErrorType.HOLD_TIMER_EXPIRED, (byte) 0, null);
-            state = ChannelState.IDLE;
-            stopKeepAliveTimer();
-            ctx.getChannel().close();
-            return;
-        } else if (e.getCause() instanceof ClosedChannelException) {
+        if (e.getCause() instanceof ClosedChannelException) {
             bgpController.activeSessionExceptionAdd(peerAddr, e.getCause().toString());
             log.debug("Channel for bgp {} already closed", getPeerInfoString());
         } else if (e.getCause() instanceof IOException) {
@@ -516,7 +514,7 @@
                 // still print stack trace if debug is enabled
                 log.debug("StackTrace for previous Exception: ", e.getCause());
             }
-            stopKeepAliveTimer();
+            stopSessionTimers();
             ctx.getChannel().close();
         } else if (e.getCause() instanceof BgpParseException) {
             byte[] data = new byte[] {};
@@ -535,7 +533,7 @@
             log.warn("Could not process message: queue full");
             bgpController.activeSessionExceptionAdd(peerAddr, e.getCause().toString());
         } else {
-            stopKeepAliveTimer();
+            stopSessionTimers();
             log.error("Error while processing message from peer " + getPeerInfoString() + "state " + this.state);
             bgpController.closedSessionExceptionAdd(peerAddr, e.getCause().toString());
             ctx.getChannel().close();
@@ -1002,4 +1000,41 @@
         }
         return true;
     }
+
+    /**
+     * Restarts the BGP hold Timeout Timer.
+     */
+    void restartHoldTimeoutTimer() {
+        if (peerHoldTime == 0) {
+            return;
+        }
+        if (holdTimerTimeout != null) {
+            holdTimerTimeout.cancel();
+        }
+        holdTimerTimeout = timer.newTimeout(new HoldTimerTimeout(), minHoldTime, TimeUnit.SECONDS);
+    }
+
+    /**
+     * Timer class for BGP hold timer timeout.
+     */
+    private final class HoldTimerTimeout implements TimerTask {
+
+        @Override
+        public void run(Timeout timeout) throws Exception {
+            if (timeout.isCancelled()) {
+                return;
+            }
+            if (!channel.isOpen()) {
+                return;
+            }
+
+            log.debug("BGP hold timer expired: peer {}", channel.getRemoteAddress());
+
+            sendNotification(BgpErrorType.HOLD_TIMER_EXPIRED, (byte) 0, null);
+            bgpController.closedSessionExceptionAdd(peerAddr, "BGP hold timer expired");
+            stopSessionTimers();
+            state = ChannelState.IDLE;
+            channel.close();
+        }
+    }
 }
diff --git a/protocols/bgp/ctl/src/main/java/org/onosproject/bgp/controller/impl/BgpPipelineFactory.java b/protocols/bgp/ctl/src/main/java/org/onosproject/bgp/controller/impl/BgpPipelineFactory.java
index ae6aef8..0a108e7 100644
--- a/protocols/bgp/ctl/src/main/java/org/onosproject/bgp/controller/impl/BgpPipelineFactory.java
+++ b/protocols/bgp/ctl/src/main/java/org/onosproject/bgp/controller/impl/BgpPipelineFactory.java
@@ -19,10 +19,7 @@
 import org.jboss.netty.channel.ChannelPipeline;
 import org.jboss.netty.channel.ChannelPipelineFactory;
 import org.jboss.netty.channel.Channels;
-import org.jboss.netty.handler.timeout.ReadTimeoutHandler;
 import org.jboss.netty.util.ExternalResourceReleasable;
-import org.jboss.netty.util.HashedWheelTimer;
-import org.jboss.netty.util.Timer;
 import org.onosproject.bgp.controller.BgpController;
 
 /**
@@ -31,8 +28,6 @@
 public class BgpPipelineFactory
     implements ChannelPipelineFactory, ExternalResourceReleasable {
 
-    static final Timer TIMER = new HashedWheelTimer();
-    protected ReadTimeoutHandler readTimeoutHandler;
     private boolean isBgpServ;
     private BgpController bgpController;
 
@@ -46,8 +41,6 @@
         super();
         this.isBgpServ = isBgpServ;
         this.bgpController = bgpController;
-        /* hold time */
-        this.readTimeoutHandler = new ReadTimeoutHandler(TIMER, bgpController.getConfig().getHoldTime());
     }
 
     @Override
@@ -57,7 +50,6 @@
         ChannelPipeline pipeline = Channels.pipeline();
         pipeline.addLast("bgpmessagedecoder", new BgpMessageDecoder());
         pipeline.addLast("bgpmessageencoder", new BgpMessageEncoder());
-        pipeline.addLast("holdTime", readTimeoutHandler);
         if (isBgpServ) {
             pipeline.addLast("PassiveHandler", handler);
         } else {
@@ -69,6 +61,6 @@
 
     @Override
     public void releaseExternalResources() {
-        TIMER.stop();
+        // TODO:
     }
 }
\ No newline at end of file