This issue happens when the neighbor config is removed from peer router while retaining the BGP router config.
Under this scenario, since BGP router config is still present, router is listening on BGP port. So when ODL
tries to connect, the connection succeeds. However the BGP negotiation fails as neighbor config has been removed
from router.
BGPReconnectPromise.ClosedChannelHandler#channelInactive triggers the reconnect, but the actual connection
retries as per the connection stragegy are done from BGPProtocolSessionPromise.BootstrapConnectListener#operationComplete
In failure scenario, since connection succeeds but negotiation fails, failure is not detected in BGPProtocolSessionPromise.BootstrapConnectListener#operationComplete
and so no reconnection attempts are made.
When negotiation fails, AbstractBGPSessionNegotiator#negotiationFailedCloseChannel is called, which causes BGPReconnectPromise.ClosedChannelHandler#channelInactive
to get called. Fix involves detecting negotiation failure here, and calling BGPProtocolSessionPromise#reconnect.
The scheduled job in AbstractBGPSessionNegotiator to detect hold-timer expiry during negotiation is also being cancelled
from AbstractBGPSessionNegotiator#negotiationFailedCloseChannel as the job is not needed when channel is closed.
Change-Id: Ica4add08bc5e9e44f1321d139a56cf9157cfdd69
Signed-off-by: Ajay <ajayl.bro@gmail.com>
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.util.concurrent.Promise;
+import io.netty.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import javax.annotation.concurrent.GuardedBy;
import org.opendaylight.protocol.bgp.parser.BGPDocumentedException;
private final Channel channel;
@GuardedBy("this")
private State state = State.IDLE;
+ @GuardedBy("this")
+ private ScheduledFuture pending;
@GuardedBy("this")
private BGPSessionImpl session;
if (this.state != State.FINISHED) {
this.state = State.OPEN_SENT;
- this.channel.eventLoop().schedule(new Runnable() {
+ this.pending = this.channel.eventLoop().schedule(new Runnable() {
@Override
public void run() {
- if (AbstractBGPSessionNegotiator.this.state != State.FINISHED) {
- AbstractBGPSessionNegotiator.this.sendMessage(buildErrorNotify(BGPError.HOLD_TIMER_EXPIRED));
- negotiationFailed(new BGPDocumentedException("HoldTimer expired", BGPError.FSM_ERROR));
- AbstractBGPSessionNegotiator.this.state = State.FINISHED;
+ synchronized (AbstractBGPSessionNegotiator.this) {
+ AbstractBGPSessionNegotiator.this.pending = null;
+ if (AbstractBGPSessionNegotiator.this.state != State.FINISHED) {
+ AbstractBGPSessionNegotiator.this.sendMessage(buildErrorNotify(BGPError.HOLD_TIMER_EXPIRED));
+ negotiationFailed(new BGPDocumentedException("HoldTimer expired", BGPError.FSM_ERROR));
+ AbstractBGPSessionNegotiator.this.state = State.FINISHED;
+ }
}
}
}, INITIAL_HOLDTIMER, TimeUnit.MINUTES);
private void negotiationFailedCloseChannel(final Throwable cause) {
LOG.debug("Negotiation on channel {} failed", this.channel, cause);
this.channel.close();
- this.promise.setFailure(cause);
+ synchronized (AbstractBGPSessionNegotiator.this) {
+ if (this.pending != null && this.pending.isCancellable()) {
+ this.pending.cancel(true);
+ this.pending = null;
+ }
+ }
}
private void sendMessage(final Notification msg) {
}
+ public synchronized void reconnect() {
+ final BGPProtocolSessionPromise lock = this;
+ final Future reconnectFuture = this.strategy.scheduleReconnect(this.pending.cause());
+ reconnectFuture.addListener(new BGPProtocolSessionPromise.BootstrapConnectListener(lock).new ReconnectingStrategyListener());
+ BGPProtocolSessionPromise.this.pending = reconnectFuture;
+ }
+
@Override
public synchronized boolean cancel(final boolean mayInterruptIfRunning) {
if (super.cancel(mayInterruptIfRunning)) {
BGPProtocolSessionPromise.LOG.debug("Promise {} connection successful", this.lock);
} else {
BGPProtocolSessionPromise.LOG.debug("Attempt to connect to {} failed", BGPProtocolSessionPromise.this.address, channelFuture.cause());
- final Future reconnectFuture = BGPProtocolSessionPromise.this.strategy.scheduleReconnect(channelFuture.cause());
- reconnectFuture.addListener(new BGPProtocolSessionPromise.BootstrapConnectListener.ReconnectingStrategyListener());
- BGPProtocolSessionPromise.this.pending = reconnectFuture;
+ BGPProtocolSessionPromise.this.reconnect();
}
}
}
private final ReconnectStrategyFactory strategyFactory;
private final Bootstrap bootstrap;
private final ChannelPipelineInitializer initializer;
- private Future<S> pending;
+ private BGPProtocolSessionPromise<S> pending;
public BGPReconnectPromise(final EventExecutor executor, final InetSocketAddress address,
final ReconnectStrategyFactory connectStrategyFactory, final Bootstrap bootstrap,
});
}
- public Future<S> connectSessionPromise(final InetSocketAddress address, final ReconnectStrategy strategy, final Bootstrap bootstrap,
+ public BGPProtocolSessionPromise<S> connectSessionPromise(final InetSocketAddress address, final ReconnectStrategy strategy, final Bootstrap bootstrap,
final ChannelPipelineInitializer initializer) {
final BGPProtocolSessionPromise sessionPromise = new BGPProtocolSessionPromise(address, strategy, bootstrap);
final ChannelHandler chInit = new ChannelInitializer<SocketChannel>() {
}
if (!this.promise.isInitialConnectFinished()) {
- LOG.debug("Connection to {} was dropped during negotiation", this.promise.address);
+ LOG.debug("Connection to {} was dropped during negotiation, reattempting", this.promise.address);
+ this.promise.pending.reconnect();
return;
}
Code Review / bgpcep.git / commitdiff