Merge "Invert non-graceful logic to graceful for better code read-abi…

…lity"

src/bgp/bgp_peer.cc

@@ -368,10 +368,8 @@ bool BgpPeer::MembershipPathCallback(DBTablePartBase *tpart, BgpRoute *route,
 }
 
 bool BgpPeer::ResumeClose() {
-    peer_close_->Close(non_graceful_close_);
-
-    // No need to track graceful_closure once the close process is resumed.
-    non_graceful_close_ = false;
+    peer_close_->Close(graceful_close_);
+    graceful_close_ = true;
     return true;
 }
 
@@ -401,7 +399,7 @@ BgpPeer::BgpPeer(BgpServer *server, RoutingInstance *instance,
           resolve_paths_(config->router_type() == "bgpaas-client"),
           as_override_(config->as_override()),
           defer_close_(false),
-          non_graceful_close_(false),
+          graceful_close_(true),
           vpn_tables_registered_(false),
           hold_time_(config->hold_time()),
           local_as_(config->local_as()),
@@ -905,7 +903,7 @@ void BgpPeer::CustomClose() {
 //
 // Close this peer by closing all of it's RIBs.
 //
-void BgpPeer::Close(bool non_graceful) {
+void BgpPeer::Close(bool graceful) {
     send_ready_ = true;
     if (membership_req_pending_ && !close_manager_->IsMembershipInUse()) {
         BGP_LOG_PEER(Event, this, SandeshLevel::SYS_INFO, BGP_LOG_FLAG_ALL,
@@ -914,11 +912,11 @@ void BgpPeer::Close(bool non_graceful) {
 
         // Note down non-graceful closures. Once a close is non-graceful,
         // it shall remain as non-graceful.
-        non_graceful_close_ |= non_graceful;
+        graceful_close_ &= graceful;
         return;
     }
 
-    peer_close_->Close(non_graceful);
+    peer_close_->Close(graceful);
 }
 
 IPeerClose *BgpPeer::peer_close() {
@@ -1020,7 +1018,7 @@ void BgpPeer::Register(BgpTable *table, const RibExportPolicy &policy) {
     if (close_manager_->IsMembershipInUse()) {
         BGP_LOG_PEER(Config, this, SandeshLevel::SYS_NOTICE, BGP_LOG_FLAG_ALL,
                      BGP_PEER_DIR_IN, "Session cleared due to GR not ready");
-        Close(false);
+        Close(true);
     }
 
     if (close_manager_->IsMembershipInWait())
@@ -1037,7 +1035,7 @@ void BgpPeer::Register(BgpTable *table) {
     if (close_manager_->IsMembershipInUse()) {
         BGP_LOG_PEER(Config, this, SandeshLevel::SYS_NOTICE, BGP_LOG_FLAG_ALL,
                      BGP_PEER_DIR_IN, "Session cleared due to GR not ready");
-        Close(false);
+        Close(true);
     }
 
     if (close_manager_->IsMembershipInWait())
@@ -1529,7 +1527,7 @@ bool BgpPeer::SetCapabilities(const BgpProto::OpenMessage *msg) {
     if (!peer_close_->SetGRCapabilities(&peer_info)) {
         BGP_LOG_PEER(Message, this, SandeshLevel::SYS_INFO, BGP_LOG_FLAG_ALL,
                      BGP_PEER_DIR_IN, "Close non-gracefully");
-        Close(true);
+        Close(false);
         return false;
     }
     return true;

src/bgp/bgp_peer.h

@@ -217,7 +217,7 @@ class BgpPeer : public IPeer {
     virtual uint64_t GetEorSendTimerElapsedTimeUsecs() const;
     virtual bool send_ready() const { return send_ready_; }
 
-    void Close(bool non_graceful);
+    void Close(bool graceful);
     void Clear(int subcode);
 
     virtual IPeerClose *peer_close();
@@ -451,7 +451,7 @@ class BgpPeer : public IPeer {
 
     tbb::atomic<int> membership_req_pending_;
     bool defer_close_;
-    bool non_graceful_close_;
+    bool graceful_close_;
     bool vpn_tables_registered_;
     std::vector<BgpProto::OpenMessage::Capability *> capabilities_;
     uint16_t hold_time_;

src/bgp/bgp_peer.sandesh

@@ -58,7 +58,7 @@ struct PeerCloseInfo {
     1: string state;
     17: string membership_state;
     2: bool close_again;
-    16: bool non_graceful;
+    16: bool graceful;
     3: u64 init;
     4: u64 close;
     5: u64 nested;

src/bgp/bgp_peer_close.cc

@@ -125,16 +125,16 @@ void BgpPeerClose::UpdateRouteStats(Address::Family family,
     GetManager()->UpdateRouteStats(family, old_path, path_flags);
 }
 
-void BgpPeerClose::Close(bool non_graceful) {
+void BgpPeerClose::Close(bool graceful) {
     // Abort GR-Closure if this request is for non-graceful closure.
     // Reset GR-Closure if previous closure is still in progress or if
     // this is a flip (from established state).
-    if (non_graceful || flap_count_ != peer_->total_flap_count()) {
+    if (!graceful || flap_count_ != peer_->total_flap_count()) {
         if (flap_count_ != peer_->total_flap_count()) {
             flap_count_++;
             assert(peer_->total_flap_count() == flap_count_);
         }
-        GetManager()->Close(non_graceful);
+        GetManager()->Close(graceful);
         return;
     }
 

src/bgp/bgp_peer_close.h

@@ -28,7 +28,7 @@ class BgpPeerClose : public IPeerClose {
     virtual void GracefulRestartSweep();
     virtual bool IsReady() const;
     virtual IPeer *peer() const;
-    virtual void Close(bool non_graceful);
+    virtual void Close(bool graceful);
     virtual void Delete();
     virtual int GetGracefulRestartTime() const;
     virtual int GetLongLivedGracefulRestartTime() const;

src/bgp/bgp_xmpp_channel.cc

@@ -333,7 +333,7 @@ class BgpXmppChannel::XmppPeer : public IPeer {
     virtual bool IsXmppPeer() const {
         return true;
     }
-    virtual void Close(bool non_graceful);
+    virtual void Close(bool graceful);
 
     const bool IsDeleted() const { return is_closed_; }
     void SetPeerClosed(bool closed) {
@@ -440,7 +440,7 @@ bool BgpXmppChannel::XmppPeer::SendUpdate(const uint8_t *msg, size_t msgsize,
     }
 }
 
-void BgpXmppChannel::XmppPeer::Close(bool non_graceful) {
+void BgpXmppChannel::XmppPeer::Close(bool graceful) {
     send_ready_ = true;
     parent_->set_peer_closed(true);
     if (server_ == NULL)
@@ -454,7 +454,7 @@ void BgpXmppChannel::XmppPeer::Close(bool non_graceful) {
         // Clear EOR state.
         parent_->ClearEndOfRibState();
 
-        parent_->peer_close_->Close(non_graceful);
+        parent_->peer_close_->Close(graceful);
     }
 }
 
@@ -1751,7 +1751,7 @@ void BgpXmppChannel::DequeueRequest(const string &table_name,
 }
 
 bool BgpXmppChannel::ResumeClose() {
-    peer_->Close(false);
+    peer_->Close(true);
     return true;
 }
 
@@ -2773,7 +2773,7 @@ void BgpXmppChannel::Close() {
         defer_peer_close_ = true;
         return;
     }
-    peer_->Close(false);
+    peer_->Close(true);
 }
 
 //

src/bgp/bgp_xmpp_peer_close.cc

@@ -125,12 +125,12 @@ void BgpXmppPeerClose::Delete() {
     channel_ = NULL;
 }
 
-void BgpXmppPeerClose::Close(bool non_graceful) {
+void BgpXmppPeerClose::Close(bool graceful) {
     if (channel_) {
         assert(channel_->peer_deleted());
         assert(channel_->channel()->IsCloseInProgress());
         if (!IsCloseGraceful())
-            non_graceful = true;
-        GetManager()->Close(non_graceful);
+            graceful = false;
+        GetManager()->Close(graceful);
     }
 }

src/bgp/bgp_xmpp_peer_close.h

@@ -37,7 +37,7 @@ class BgpXmppPeerClose : public IPeerClose {
     virtual void CustomClose();
     virtual void CloseComplete();
     virtual void Delete();
-    virtual void Close(bool non_graceful);
+    virtual void Close(bool graceful);
     virtual PeerCloseManager *GetManager() const;
     virtual void UpdateRouteStats(Address::Family family,
         const BgpPath *old_path, uint32_t path_flags) const;

src/bgp/ipeer.h

@@ -159,7 +159,7 @@ class IPeerClose {
     typedef std::set<Address::Family> Families;
 
     virtual ~IPeerClose() { }
-    virtual void Close(bool non_graceful) = 0;
+    virtual void Close(bool graceful) = 0;
     virtual bool IsCloseGraceful() const = 0;
     virtual bool IsCloseLongLivedGraceful() const = 0;
     virtual void CustomClose() = 0;
@@ -202,7 +202,7 @@ class IPeer : public IPeerUpdate {
     // Control is mostly required in MockPeer in unit tests.
     //
     virtual bool IsRegistrationRequired() const = 0;
-    virtual void Close(bool non_graceful) = 0;
+    virtual void Close(bool graceful) = 0;
     virtual BgpProto::BgpPeerType PeerType() const = 0;
     virtual uint32_t bgp_identifier() const = 0;
     virtual const std::string GetStateName() const = 0;

src/bgp/l3vpn/test/inetvpn_table_test.cc

@@ -30,7 +30,7 @@ class BgpPeerMock : public IPeer {
     virtual const IPeerDebugStats *peer_stats() const { return NULL; }
     virtual bool IsReady() const { return true; }
     virtual bool IsXmppPeer() const { return false; }
-    virtual void Close(bool non_graceful) { }
+    virtual void Close(bool graceful) { }
     BgpProto::BgpPeerType PeerType() const { return BgpProto::IBGP; }
     virtual uint32_t bgp_identifier() const { return 0; }
     virtual const std::string GetStateName() const { return "UNKNOWN"; }

src/bgp/peer_close_manager.cc

@@ -50,7 +50,7 @@ PeerCloseManager::PeerCloseManager(IPeerClose *peer_close,
                          peer_close_->GetTaskName()),
                      peer_close_->GetTaskInstance(),
                      boost::bind(&PeerCloseManager::EventCallback, this, _1))),
-        state_(NONE), close_again_(false), non_graceful_(false), gr_elapsed_(0),
+        state_(NONE), close_again_(false), graceful_(true), gr_elapsed_(0),
         llgr_elapsed_(0), membership_state_(MEMBERSHIP_NONE) {
     stats_.init++;
     membership_req_pending_ = 0;
@@ -66,7 +66,7 @@ PeerCloseManager::PeerCloseManager(IPeerClose *peer_close) :
                          peer_close_->GetTaskName()),
                      peer_close_->GetTaskInstance(),
                      boost::bind(&PeerCloseManager::EventCallback, this, _1))),
-        state_(NONE), close_again_(false), non_graceful_(false), gr_elapsed_(0),
+        state_(NONE), close_again_(false), graceful_(true), gr_elapsed_(0),
         llgr_elapsed_(0), membership_state_(MEMBERSHIP_NONE) {
     stats_.init++;
     membership_req_pending_ = 0;
@@ -139,7 +139,7 @@ std::string PeerCloseManager::GetEventName(EventType eventType) const {
 void PeerCloseManager::EnqueueEvent(Event *event) {
     PEER_CLOSE_MANAGER_LOG("Enqueued event " <<
             GetEventName(event->event_type) <<
-            ", non_graceful " << event->non_graceful <<
+            ", graceful " << event->graceful <<
             ", family " << Address::FamilyToString(event->family));
     event_queue_->Enqueue(event);
 }
@@ -180,17 +180,17 @@ void PeerCloseManager::EnqueueEvent(Event *event) {
 //
 // If Close is restarted, account for GR timer's elapsed time.
 //
-// Use non_graceful as true for non-graceful closure
-void PeerCloseManager::Close(bool non_graceful) {
-    EnqueueEvent(new Event(CLOSE, non_graceful));
+// Use graceful as true to close gracefully.
+void PeerCloseManager::Close(bool graceful) {
+    EnqueueEvent(new Event(CLOSE, graceful));
 }
 
 void PeerCloseManager::Close(Event *event) {
 
     // Note down non-graceful close trigger. Once non-graceful closure is
     // triggered, it should remain so until close process is complete. Further
     // graceful closure calls until then should remain non-graceful.
-    non_graceful_ |= event->non_graceful;
+    graceful_ &= event->graceful;
     CloseInternal();
 }
 
@@ -298,7 +298,7 @@ void PeerCloseManager::ProcessClosure() {
     // sweep old paths which may not have come back in the new session
     switch (state_) {
         case NONE:
-            if (non_graceful_ || !peer_close_->IsCloseGraceful()) {
+            if (!graceful_ || !peer_close_->IsCloseGraceful()) {
                 MOVE_TO_STATE(DELETE);
                 stats_.deletes++;
             } else {
@@ -545,7 +545,7 @@ bool PeerCloseManager::MembershipRequestCallback(Event *event) {
         llgr_elapsed_ = 0;
         stats_.init++;
         close_again_ = false;
-        non_graceful_ = false;
+        graceful_ = true;
         return result;
     }
 
@@ -618,7 +618,7 @@ void PeerCloseManager::FillCloseInfo(BgpNeighborResp *resp) const {
     peer_close_info.set_membership_state(
         GetMembershipStateName(membership_state_));
     peer_close_info.set_close_again(close_again_);
-    peer_close_info.set_non_graceful(non_graceful_);
+    peer_close_info.set_graceful(graceful_);
     peer_close_info.set_init(stats_.init);
     peer_close_info.set_close(stats_.close);
     peer_close_info.set_nested(stats_.nested);

src/bgp/peer_close_manager.h

@@ -58,7 +58,7 @@ class PeerCloseManager {
     bool IsInLlgrTimerWaitState() const { return state_ == LLGR_TIMER; }
     bool IsQueueEmpty() const { return event_queue_->IsQueueEmpty(); }
 
-    void Close(bool non_graceful);
+    void Close(bool graceful);
     void ProcessEORMarkerReceived(Address::Family family);
     void MembershipRequest();
     void MembershipRequestCallback();
@@ -105,20 +105,20 @@ class PeerCloseManager {
     };
 
     struct Event {
-        Event(EventType event_type, bool non_graceful) :
-                event_type(event_type), non_graceful(non_graceful),
+        Event(EventType event_type, bool graceful) :
+                event_type(event_type), graceful(graceful),
                 family(Address::UNSPEC) { }
         Event(EventType event_type, Address::Family family) :
-                event_type(event_type), non_graceful(false),
+                event_type(event_type), graceful(true),
                 family(family) { }
         explicit Event(EventType event_type) : event_type(event_type),
-                                               non_graceful(false),
+                                               graceful(true),
                                                family(Address::UNSPEC) { }
-        Event() : event_type(EVENT_NONE), non_graceful(false),
+        Event() : event_type(EVENT_NONE), graceful(true),
                              family(Address::UNSPEC) { }
 
         EventType event_type;
-        bool non_graceful;
+        bool graceful;
         Address::Family family;
     };
 
@@ -207,7 +207,7 @@ class PeerCloseManager {
     boost::scoped_ptr<WorkQueue<Event *> > event_queue_;
     State state_;
     bool close_again_;
-    bool non_graceful_;
+    bool graceful_;
     int gr_elapsed_;
     int llgr_elapsed_;
     MembershipState membership_state_;

src/bgp/state_machine.cc

@@ -1360,8 +1360,8 @@ void StateMachine::SendNotificationAndClose(BgpSession *session, int code,
     set_idle_hold_time(idle_hold_time() ? idle_hold_time() : kIdleHoldTime);
     reset_hold_time();
 
-    bool non_graceful = code && !peer_->SkipNotificationReceive(code, subcode);
-    peer_->Close(non_graceful);
+    bool graceful = !code || peer_->SkipNotificationReceive(code, subcode);
+    peer_->Close(graceful);
 }
 
 //

src/bgp/test/bgp_evpn_manager_test.cc

@@ -105,7 +105,7 @@ class PeerMock : public IPeer {
     virtual bool IsRegistrationRequired() const {
         return false;
     }
-    virtual void Close(bool non_graceful) { }
+    virtual void Close(bool graceful) { }
     BgpProto::BgpPeerType PeerType() const {
         return BgpProto::IBGP;
     }

src/bgp/test/bgp_ip_test.cc

@@ -44,7 +44,7 @@ class PeerMock : public IPeer {
     virtual const IPeerDebugStats *peer_stats() const { return NULL; }
     virtual bool IsReady() const { return true; }
     virtual bool IsXmppPeer() const { return false; }
-    virtual void Close(bool non_graceful) { }
+    virtual void Close(bool graceful) { }
     BgpProto::BgpPeerType PeerType() const { return BgpProto::EBGP; }
     virtual uint32_t bgp_identifier() const {
         return htonl(address_.to_ulong());

src/bgp/test/bgp_multicast_test.cc

@@ -79,7 +79,7 @@ class XmppPeerMock : public IPeer {
     virtual bool IsReady() const { return true; }
     virtual bool IsXmppPeer() const { return true; }
     virtual bool IsRegistrationRequired() const { return false; }
-    virtual void Close(bool non_graceful) { }
+    virtual void Close(bool graceful) { }
     virtual BgpProto::BgpPeerType PeerType() const { return BgpProto::IBGP; }
     virtual uint32_t bgp_identifier() const { return address_.to_ulong(); }
     virtual const std::string GetStateName() const { return ""; }

src/bgp/test/bgp_peer_close_test.cc

@@ -796,7 +796,7 @@ void BgpPeerCloseTest::XmppPeerClose() {
 
     if (xmpp_close_from_control_node_) {
         BOOST_FOREACH(BgpXmppChannel *peer, xmpp_peers_) {
-            peer->Peer()->Close(true);
+            peer->Peer()->Close(false);
         }
     } else {
         BOOST_FOREACH(test::NetworkAgentMock *agent, xmpp_agents_) {