Merge "Do not call BgpPeer::SetAdminState() from main thread"

Juniper · Apr 17, 2016 · 5a4a553 · 5a4a553
2 parents ee2c099 + 186c8c6
commit 5a4a553
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 3 deletions.
diff --git a/src/base/task_annotations.h b/src/base/task_annotations.h
@@ -18,6 +18,7 @@ class ConcurrencyChecker {
     ConcurrencyChecker(const char *task_ids[], size_t count);
     void Check();
     void CheckIfMainThr();
+    static bool IsInMainThr() { return (Task::Running() == NULL); }
 private:
     typedef std::set<int> TaskIdSet;
     TaskIdSet id_set_;

diff --git a/src/bgp/bgp_peer.h b/src/bgp/bgp_peer.h
@@ -99,7 +99,7 @@ class BgpPeer : public IPeer {
 
     BgpSession *CreateSession();
 
-    void SetAdminState(bool down);
+    virtual void SetAdminState(bool down);
 
     // Messages
 

diff --git a/src/bgp/test/bgp_server_test_util.cc b/src/bgp/test/bgp_server_test_util.cc
@@ -186,7 +186,9 @@ void BgpPeerTest::SetDataCollectionKey(BgpPeerInfo *peer_info) const {
 //
 BgpPeerTest::BgpPeerTest(BgpServer *server, RoutingInstance *rtinst,
                          const BgpNeighborConfig *config)
-        : BgpPeer(server, rtinst, config), id_(0) {
+        : BgpPeer(server, rtinst, config), id_(0),
+          work_queue_(TaskScheduler::GetInstance()->GetTaskId("bgp::Config"), 0,
+                      boost::bind(&BgpPeerTest::ProcessRequest, this, _1)) {
     SendUpdate_fnc_ = boost::bind(&BgpPeerTest::BgpPeerSendUpdate, this,
                                   _1, _2);
     MpNlriAllowed_fnc_ = boost::bind(&BgpPeerTest::BgpPeerMpNlriAllowed, this,
@@ -197,6 +199,26 @@ BgpPeerTest::BgpPeerTest(BgpServer *server, RoutingInstance *rtinst,
 BgpPeerTest::~BgpPeerTest() {
 }
 
+// Process requests and run them off bgp::Config exclusive task
+bool BgpPeerTest::ProcessRequest(Request *request) {
+    CHECK_CONCURRENCY("bgp::Config");
+    switch (request->type) {
+        case ADMIN_UP:
+            BgpPeer::SetAdminState(false);
+            request->result = true;
+            break;
+        case ADMIN_DOWN:
+            BgpPeer::SetAdminState(true);
+            request->result = true;
+            break;
+    }
+
+    // Notify waiting caller with the result
+    tbb::mutex::scoped_lock lock(work_mutex_);
+    cond_var_.notify_all();
+    return true;
+}
+
 //
 // Enable this if peer name uuid is also required in all bgp peer logs
 //

diff --git a/src/bgp/test/bgp_server_test_util.h b/src/bgp/test/bgp_server_test_util.h
@@ -8,9 +8,12 @@
 #include <boost/any.hpp>
 #include <boost/foreach.hpp>
 #include <boost/shared_ptr.hpp>
+#include <tbb/compat/condition_variable>
+#include <tbb/mutex.h>
 
-#include "base/util.h"
+#include "base/task_annotations.h"
 #include "base/test/task_test_util.h"
+#include "base/util.h"
 #include "bgp/bgp_config.h"
 #include "bgp/bgp_lifetime.h"
 #include "bgp/bgp_log.h"
@@ -309,15 +312,41 @@ class BgpPeerTest : public BgpPeer {
     const int id() const { return id_; }
     void set_id(int id) { id_ = id; }
 
+    virtual void SetAdminState(bool down) {
+        if (!ConcurrencyChecker::IsInMainThr()) {
+            BgpPeer::SetAdminState(down);
+            return;
+        }
+        tbb::interface5::unique_lock<tbb::mutex> lock(work_mutex_);
+
+        Request request;
+        request.type = down ? ADMIN_DOWN : ADMIN_UP;
+        work_queue_.Enqueue(&request);
+
+        // Wait for the request to get processed.
+        cond_var_.wait(lock);
+    }
+
     boost::function<bool(const uint8_t *, size_t)> SendUpdate_fnc_;
     boost::function<bool(uint16_t, uint8_t)> MpNlriAllowed_fnc_;
     boost::function<bool()> IsReady_fnc_;
 
     BgpTestUtil util_;
 
 private:
+    enum RequestType { ADMIN_UP, ADMIN_DOWN };
+    struct Request {
+        Request() : result(false) { }
+        RequestType type;
+        bool        result;
+    };
+    bool ProcessRequest(Request *request);
+
     static bool verbose_name_;
     int id_;
+    WorkQueue<Request *> work_queue_;
+    tbb::mutex work_mutex_;
+    tbb::interface5::condition_variable cond_var_;
 };
 
 class PeerManagerTest : public PeerManager {