From e38cd7936cfa575539380e70fef9ea5bca3d8f6d Mon Sep 17 00:00:00 2001 From: Ananth Suryanarayana Date: Thu, 14 Apr 2016 14:28:01 -0700 Subject: [PATCH] Restart GR timers by offseting for already elapsed time When session comes up and closes before GR process completion, we need to mark any new routes learned in the new session again as STALE, as some routes might have gotten refreshed from the peer To keep the logic simple, we go through the close process again from the beginning. However, elapsed time is noted to adjust when timers are fired again later when we get back to GR_TIMER or LLGR_TIMER state. Change-Id: Ia9326800257f8382547a0e991f7f86cdc5caf499 Partial-Bug: #1537933 --- src/base/timer.cc | 18 +++++++++++++++++- src/base/timer.h | 3 +++ src/base/timer_impl.h | 10 +++------- src/bgp/bgp_peer_close.cc | 23 +++++++++++++++++++++-- src/bgp/bgp_peer_close.h | 2 ++ 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/base/timer.cc b/src/base/timer.cc index 8f8fbd43c13..b96f69a0067 100644 --- a/src/base/timer.cc +++ b/src/base/timer.cc @@ -115,6 +115,7 @@ bool Timer::Start(int time, Handler handler, ErrorHandler error_handler) { } // Restart the timer + time_ = time; handler_ = handler; seq_no_++; error_handler_ = error_handler; @@ -186,7 +187,6 @@ void Timer::StartTimerTask(TimerPtr reference, int time, uint32_t seq_no, // Start a task and add Task reference. assert(timer_task_ == NULL); timer_task_ = new TimerTask(reference, ec); - time_ = time; TaskScheduler::GetInstance()->Enqueue(timer_task_); } @@ -229,3 +229,19 @@ bool TimerManager::DeleteTimer(Timer *timer) { return true; } +// Get timer's already elapsed time in milliseconds. +int Timer::GetElapsedTime() const { + tbb::mutex::scoped_lock lock(mutex_); + int64_t elapsed; + +#if BOOST_VERSION >= 104900 + elapsed = + boost::chrono::nanoseconds(impl_->timer_.expires_from_now()).count(); +#else + elapsed = impl_->timer_.expires_from_now().total_nanoseconds(); +#endif + elapsed = time_ - elapsed/1000000; // Convert nanoseconds to milliseconds. + if (elapsed < 0) + elapsed = 0; + return elapsed; +} diff --git a/src/base/timer.h b/src/base/timer.h index e256ff7c0e9..83e4ac55e8e 100644 --- a/src/base/timer.h +++ b/src/base/timer.h @@ -110,6 +110,8 @@ class Timer { return delete_on_completion_; } + int GetElapsedTime() const; + // Only for state machine test // XXX: Don't use in production code void Fire() { @@ -125,6 +127,7 @@ class Timer { return name_; } private: + friend class TimerImpl; friend class TimerManager; friend class TimerTest; diff --git a/src/base/timer_impl.h b/src/base/timer_impl.h index bc0c418e4e1..8675dcf0af8 100644 --- a/src/base/timer_impl.h +++ b/src/base/timer_impl.h @@ -36,15 +36,11 @@ class TimerImpl { #endif template - void async_wait(WaitHandler handler) { - timer_.async_wait(handler); - } - - void cancel(boost::system::error_code &ec) { - timer_.cancel(ec); - } + void async_wait(WaitHandler handler) { timer_.async_wait(handler); } + void cancel(boost::system::error_code &ec) { timer_.cancel(ec); } private: + friend class Timer; TimerType timer_; }; diff --git a/src/bgp/bgp_peer_close.cc b/src/bgp/bgp_peer_close.cc index 05b2a072049..6434fba5d06 100644 --- a/src/bgp/bgp_peer_close.cc +++ b/src/bgp/bgp_peer_close.cc @@ -27,7 +27,7 @@ // Create an instance of PeerCloseManager with back reference to parent IPeer PeerCloseManager::PeerCloseManager(IPeer *peer) : peer_(peer), stale_timer_(NULL), sweep_timer_(NULL), state_(NONE), - close_again_(false) { + close_again_(false), gr_elapsed_(0), llgr_elapsed_(0) { stats_.init++; if (peer->server()) { stale_timer_ = TimerManager::CreateTimer(*peer->server()->ioservice(), @@ -119,12 +119,14 @@ void PeerCloseManager::Close() { case GR_TIMER: PEER_CLOSE_MANAGER_LOG("Nested close: Restart GR"); close_again_ = true; + gr_elapsed_ += stale_timer_->GetElapsedTime(); CloseComplete(); break; case LLGR_TIMER: PEER_CLOSE_MANAGER_LOG("Nested close: Restart LLGR"); close_again_ = true; + llgr_elapsed_ += stale_timer_->GetElapsedTime(); CloseComplete(); break; @@ -257,6 +259,8 @@ bool PeerCloseManager::ProcessSweepStateActions() { // Notify clients to trigger sweep as appropriate. peer_->peer_close()->GracefulRestartSweep(); + gr_elapsed_ = 0; + llgr_elapsed_ = 0; CloseComplete(); return false; } @@ -281,6 +285,8 @@ void PeerCloseManager::UnregisterPeerComplete(IPeer *ipeer, BgpTable *table) { if (state_ == DELETE) { peer_->peer_close()->Delete(); + gr_elapsed_ = 0; + llgr_elapsed_ = 0; MOVE_TO_STATE(NONE); stats_.init++; close_again_ = false; @@ -293,7 +299,13 @@ void PeerCloseManager::UnregisterPeerComplete(IPeer *ipeer, BgpTable *table) { peer_->peer_close()->CloseComplete(); MOVE_TO_STATE(GR_TIMER); peer_->peer_close()->GetGracefulRestartFamilies(&families_); - StartRestartTimer(1000 * peer_->peer_close()->GetGracefulRestartTime()); + + // Offset restart time with elapsed time during nested closures. + int time = peer_->peer_close()->GetGracefulRestartTime() * 1000; + time -= gr_elapsed_; + if (time < 0) + time = 0; + StartRestartTimer(time); stats_.gr_timer++; return; } @@ -306,6 +318,13 @@ void PeerCloseManager::UnregisterPeerComplete(IPeer *ipeer, BgpTable *table) { peer_->peer_close()->GetGracefulRestartFamilies(&families_); StartRestartTimer(1000 * peer_->peer_close()->GetLongLivedGracefulRestartTime()); + + // Offset restart time with elapsed time during nested closures. + int time = peer_->peer_close()->GetLongLivedGracefulRestartTime() *1000; + time -= llgr_elapsed_; + if (time < 0) + time = 0; + StartRestartTimer(time); stats_.llgr_timer++; return; } diff --git a/src/bgp/bgp_peer_close.h b/src/bgp/bgp_peer_close.h index 6adc2d1e44e..375fc0a67ae 100644 --- a/src/bgp/bgp_peer_close.h +++ b/src/bgp/bgp_peer_close.h @@ -95,6 +95,8 @@ class PeerCloseManager { Timer *sweep_timer_; State state_; bool close_again_; + int gr_elapsed_; + int llgr_elapsed_; IPeerClose::Families families_; Stats stats_; mutable tbb::recursive_mutex mutex_;