[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