Merge "Add logic to import/un-import static routes upon instance add/…

…delete" into R2.1

src/bgp/bgp_server.cc

@@ -23,6 +23,7 @@
 #include "bgp/routing-instance/routepath_replicator.h"
 #include "bgp/routing-instance/rtarget_group_mgr.h"
 #include "bgp/routing-instance/service_chaining.h"
+#include "bgp/routing-instance/static_route.h"
 #include "io/event_manager.h"
 
 using boost::system::error_code;
@@ -271,6 +272,7 @@ BgpServer::BgpServer(EventManager *evm)
 }
 
 BgpServer::~BgpServer() {
+    assert(srt_manager_list_.empty());
 }
 
 string BgpServer::ToString() const {
@@ -460,3 +462,22 @@ void BgpServer::NotifyIdentifierUpdate(Ip4Address old_identifier) {
         }
     }
 }
+
+void BgpServer::InsertStaticRouteMgr(StaticRouteMgr *srt_manager) {
+    CHECK_CONCURRENCY("bgp::Config");
+    srt_manager_list_.insert(srt_manager);
+}
+
+void BgpServer::RemoveStaticRouteMgr(StaticRouteMgr *srt_manager) {
+    CHECK_CONCURRENCY("bgp::StaticRoute");
+    srt_manager_list_.erase(srt_manager);
+}
+
+void BgpServer::NotifyAllStaticRoutes() {
+    CHECK_CONCURRENCY("bgp::Config");
+    for (StaticRouteMgrList::iterator it = srt_manager_list_.begin();
+         it != srt_manager_list_.end(); ++it) {
+        StaticRouteMgr *srt_manager = *it;
+        srt_manager->NotifyAllRoutes();
+    }
+}

src/bgp/bgp_server.h

@@ -11,6 +11,7 @@
 #include <boost/scoped_ptr.hpp>
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -36,12 +37,14 @@ class RoutingInstanceMgr;
 class RTargetGroupMgr;
 class SchedulingGroupManager;
 class ServiceChainMgr;
+class StaticRouteMgr;
 
 class BgpServer {
 public:
     typedef boost::function<void(as_t, as_t)> ASNUpdateCb;
     typedef boost::function<void(Ip4Address)> IdentifierUpdateCb;
     typedef boost::function<void(BgpPeer *)> VisitorFn;
+    typedef std::set<StaticRouteMgr *> StaticRouteMgrList;
     explicit BgpServer(EventManager *evm);
     virtual ~BgpServer();
 
@@ -137,6 +140,10 @@ class BgpServer {
     void UnregisterIdentifierUpdateCallback(int listener);
     void NotifyIdentifierUpdate(Ip4Address old_identifier);
 
+    void InsertStaticRouteMgr(StaticRouteMgr *srt_manager);
+    void RemoveStaticRouteMgr(StaticRouteMgr *srt_manager);
+    void NotifyAllStaticRoutes();
+
 private:
     class ConfigUpdater;
     class DeleteActor;
@@ -158,6 +165,7 @@ class BgpServer {
     IdentifierUpdateListenersList id_listeners_;
     boost::dynamic_bitset<> id_bmap_;      // free list.
     uint16_t hold_time_;
+    StaticRouteMgrList srt_manager_list_;
 
     DB db_;
     boost::dynamic_bitset<> peer_bmap_;

src/bgp/routing-instance/routepath_replicator.cc

@@ -200,6 +200,8 @@ void RoutePathReplicator::Join(BgpTable *table, const RouteTarget &rt,
 
     RPR_TRACE(TableJoin, table->name(), rt.ToString(), import);
     if (import) {
+        if (family_ == Address::INETVPN)
+            server_->NotifyAllStaticRoutes();
         BOOST_FOREACH(BgpTable *bgptable, group->GetExportTables(family())) {
             RequestWalk(bgptable);
         }
@@ -236,6 +238,8 @@ void RoutePathReplicator::Leave(BgpTable *table, const RouteTarget &rt,
 
     if (import) {
         group->RemoveImportTable(family(), table);
+        if (family_ == Address::INETVPN)
+            server_->NotifyAllStaticRoutes();
         BOOST_FOREACH(BgpTable *bgptable, group->GetExportTables(family())) {
             RequestWalk(bgptable);
         }

src/bgp/routing-instance/static_route.cc

@@ -142,10 +142,9 @@ class StaticRoute : public ConditionMatch {
     }
 
     void AddStaticRoute(NexthopPathIdList *list);
-
     void UpdateStaticRoute();
-
     void RemoveStaticRoute();
+    void NotifyRoute();
 
     virtual bool Match(BgpServer *server, BgpTable *table,
                        BgpRoute *route, bool deleted);
@@ -159,17 +158,6 @@ class StaticRoute : public ConditionMatch {
     }
 
 private:
-    RoutingInstance *routing_instance_;
-
-    Ip4Prefix static_route_prefix_;
-    IpAddress nexthop_;
-    BgpRoute *nexthop_route_;
-    NexthopPathIdList nexthop_path_ids_;
-
-    RouteTargetList rtarget_list_;
-
-    bool unregistered_;
-
     // Helper function to match
     bool is_nexthop_route(BgpRoute *route) {
         InetRoute *inet_route = dynamic_cast<InetRoute *>(route);
@@ -196,15 +184,25 @@ class StaticRoute : public ConditionMatch {
         return new_ext_community;
     }
 
+    RoutingInstance *routing_instance_;
+    Ip4Prefix static_route_prefix_;
+    IpAddress nexthop_;
+    BgpRoute *nexthop_route_;
+    NexthopPathIdList nexthop_path_ids_;
+    RouteTargetList rtarget_list_;
+    bool unregistered_;
+
     DISALLOW_COPY_AND_ASSIGN(StaticRoute);
 };
 
 
-StaticRoute::StaticRoute(RoutingInstance *rtinst,
-            const Ip4Prefix &static_route, const vector<string> &rtargets,
-            IpAddress nexthop)
-    : routing_instance_(rtinst), static_route_prefix_(static_route),
-    nexthop_(nexthop), nexthop_route_(NULL), unregistered_(false) {
+StaticRoute::StaticRoute(RoutingInstance *rtinst, const Ip4Prefix &static_route,
+    const vector<string> &rtargets, IpAddress nexthop)
+    : routing_instance_(rtinst),
+      static_route_prefix_(static_route),
+      nexthop_(nexthop),
+      nexthop_route_(NULL),
+      unregistered_(false) {
     for (vector<string>::const_iterator it = rtargets.begin();
         it != rtargets.end(); it++) {
         error_code ec;
@@ -462,6 +460,16 @@ StaticRoute::AddStaticRoute(NexthopPathIdList *old_path_ids) {
     }
 }
 
+void StaticRoute::NotifyRoute() {
+    InetRoute rt_key(static_route_prefix());
+    DBTablePartition *partition =
+       static_cast<DBTablePartition *>(bgp_table()->GetTablePartition(&rt_key));
+    BgpRoute *static_route = static_cast<BgpRoute *>(partition->Find(&rt_key));
+    if (!static_route)
+        return;
+    partition->Notify(static_route);
+}
+
 int StaticRouteMgr::static_route_task_id_ = -1;
 
 StaticRouteMgr::StaticRouteMgr(RoutingInstance *instance)
@@ -530,6 +538,8 @@ bool StaticRouteMgr::StaticRouteEventCallback(StaticRouteRequest *req) {
             if (!info->num_matchstate()) {
                 listener->UnregisterCondition(table, info);
                 static_route_map_.erase(info->static_route_prefix());
+                if (static_route_map_.empty())
+                    instance_->server()->RemoveStaticRouteMgr(this);
                 if (!routing_instance()->deleted() &&
                     routing_instance()->config() &&
                     routing_instance()->config()->instance_config())
@@ -551,6 +561,8 @@ bool StaticRouteMgr::StaticRouteEventCallback(StaticRouteRequest *req) {
             if (!info->num_matchstate() && info->unregistered()) {
                 listener->UnregisterCondition(table, info);
                 static_route_map_.erase(info->static_route_prefix());
+                if (static_route_map_.empty())
+                    instance_->server()->RemoveStaticRouteMgr(this);
                 if (!routing_instance()->deleted() &&
                     routing_instance()->config() &&
                     routing_instance()->config()->instance_config())
@@ -617,6 +629,8 @@ StaticRouteMgr::LocateStaticRoutePrefix(const autogen::StaticRouteType &cfg) {
         new StaticRoute(routing_instance(), prefix, cfg.route_target, nexthop);
     StaticRoutePtr static_route_match = StaticRoutePtr(match);
 
+    if (static_route_map_.empty())
+        instance_->server()->InsertStaticRouteMgr(this);
     static_route_map_.insert(make_pair(prefix, static_route_match));
 
     listener->AddMatchCondition(match->bgp_table(), static_route_match.get(),
@@ -728,6 +742,16 @@ void StaticRouteMgr::FlushStaticRouteConfig() {
     }
 }
 
+void StaticRouteMgr::NotifyAllRoutes() {
+    CHECK_CONCURRENCY("bgp::Config");
+    for (StaticRouteMap::iterator it = static_route_map_.begin();
+         it != static_route_map_.end(); ++it) {
+        StaticRoute *static_route =
+             static_cast<StaticRoute *>(it->second.get());
+        static_route->NotifyRoute();
+    }
+}
+
 class ShowStaticRouteHandler {
 public:
     static void FillStaticRoutesInfo(vector<StaticRouteEntriesInfo> *list,

src/bgp/routing-instance/static_route.h

@@ -61,6 +61,7 @@ class StaticRouteMgr {
     bool StaticRouteEventCallback(StaticRouteRequest *req);
 
     bool ResolvePendingStaticRouteConfig();
+    void NotifyAllRoutes();
 
     RoutingInstance *routing_instance() { return instance_; }
 

src/bgp/test/static_route_test.cc

@@ -343,6 +343,11 @@ class StaticRouteTest : public ::testing::Test {
         TASK_UTIL_EXPECT_EQ(1, validate_done_);
     }
 
+    void VerifyTableNoExists(const string &table_name) {
+        TASK_UTIL_EXPECT_TRUE(
+            bgp_server_->database()->FindTable(table_name) == NULL);
+    }
+
     EventManager evm_;
     DB config_db_;
     DBGraph config_graph_;
@@ -1504,6 +1509,99 @@ TEST_F(StaticRouteTest, EntryAfterStop) {
                              "Wait for Static route in blue..");
 }
 
+//
+// Delete the routing instance that imports the static route and make sure
+// the inet table gets deleted. Objective is to check that the static route
+// is removed from the table even though the static route config has not
+// changed.
+//
+TEST_F(StaticRouteTest, DeleteRoutingInstance) {
+    vector<string> instance_names = list_of("blue")("nat");
+    multimap<string, string> connections;
+    NetworkConfig(instance_names, connections);
+    task_util::WaitForIdle();
+
+    std::auto_ptr<autogen::StaticRouteEntriesType> params =
+        GetStaticRouteConfig("controller/src/bgp/testdata/static_route_1.xml");
+
+    ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance",
+                         "nat", "static-route-entries", params.release(), 0);
+    task_util::WaitForIdle();
+
+    // Add Nexthop Route
+    AddInetRoute(NULL, "nat", "192.168.1.254/32", 100, "2.3.4.5");
+    task_util::WaitForIdle();
+
+    // Check for Static route
+    TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("blue", "192.168.1.0/24"),
+                             NULL, 1000, 10000,
+                             "Wait for Static route in blue..");
+
+    // Delete the configuration for the blue instance.
+    ifmap_test_util::IFMapMsgUnlink(&config_db_, "routing-instance", "blue",
+        "virtual-network", "blue", "virtual-network-routing-instance");
+    ifmap_test_util::IFMapMsgUnlink(&config_db_, "routing-instance", "blue",
+        "route-target", "target:64496:1", "instance-target");
+    ifmap_test_util::IFMapMsgNodeDelete(
+        &config_db_, "virtual-network", "blue");
+    ifmap_test_util::IFMapMsgNodeDelete(
+        &config_db_, "routing-instance", "blue");
+    ifmap_test_util::IFMapMsgNodeDelete(
+        &config_db_, "route-target", "target:64496:1");
+    task_util::WaitForIdle();
+
+    // Make sure that the blue inet table is gone.
+    VerifyTableNoExists("blue.inet.0");
+
+    // Delete nexthop route
+    DeleteInetRoute(NULL, "nat", "192.168.1.254/32");
+    task_util::WaitForIdle();
+}
+
+//
+// Add the routing instance that imports the static route after the static
+// route has already been added. Objective is to check that the static route
+// is replicated to the table in the new instance without any triggers to
+// the static route module.
+//
+TEST_F(StaticRouteTest, AddRoutingInstance) {
+    vector<string> instance_names = list_of("nat");
+    multimap<string, string> connections;
+    NetworkConfig(instance_names, connections);
+    task_util::WaitForIdle();
+
+    std::auto_ptr<autogen::StaticRouteEntriesType> params =
+        GetStaticRouteConfig("controller/src/bgp/testdata/static_route_1.xml");
+
+    ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance",
+                         "nat", "static-route-entries", params.release(), 0);
+    task_util::WaitForIdle();
+
+    // Add Nexthop Route
+    AddInetRoute(NULL, "nat", "192.168.1.254/32", 100, "2.3.4.5");
+    task_util::WaitForIdle();
+
+    // Check for Static route
+    TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.1.0/24"),
+                             NULL, 1000, 10000,
+                             "Wait for Static route in nat..");
+
+    // Add the blue instance.
+    // Make sure that the id and route target for nat instance don't change.
+    instance_names = list_of("nat")("blue");
+    NetworkConfig(instance_names, connections);
+    task_util::WaitForIdle();
+
+    // Check for Static route
+    TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("blue", "192.168.1.0/24"),
+                             NULL, 1000, 10000,
+                             "Wait for Static route in blue..");
+
+    // Delete nexthop route
+    DeleteInetRoute(NULL, "nat", "192.168.1.254/32");
+    task_util::WaitForIdle();
+}
+
 // Sandesh introspect test
 // Verify http introspect output
 //   1. After creating the config and before nexthop route is published