Fix concurrency issue in XmppMessageBuilder

Do not access BgpPath from bgp::SendTask because the path can get
deleted from db::DBTable task which could be running in parallel.
Store vrf_originated_ attribute in RibOutAttr instead of accessing
the best path to figure out if the route is local Vrf Originated.

Change-Id: If5318153f83d1032495c7e7f0c56293745cb37bc
Closes-Bug: 1551015

src/bgp/bgp_evpn.cc

@@ -273,7 +273,8 @@ UpdateInfo *EvpnLocalMcastNode::GetUpdateInfo() {
     attr = attr_db->ReplaceLeafOListAndLocate(attr.get(), &leaf_olist_spec);
 
     UpdateInfo *uinfo = new UpdateInfo;
-    uinfo->roattr = RibOutAttr(partition_->table(), attr.get(), 0, false);
+    uinfo->roattr =
+        RibOutAttr(partition_->table(), route_, attr.get(), 0, false);
     return uinfo;
 }
 

src/bgp/bgp_multicast.cc

@@ -347,7 +347,7 @@ UpdateInfo *McastForwarder::GetUpdateInfo(ErmVpnTable *table) {
     BgpAttrPtr attr = table->server()->attr_db()->Locate(attr_spec);
 
     UpdateInfo *uinfo = new UpdateInfo;
-    uinfo->roattr = RibOutAttr(table, attr.get(), label_);
+    uinfo->roattr = RibOutAttr(table, route_, attr.get(), label_);
     return uinfo;
 }
 

src/bgp/bgp_ribout.cc

@@ -67,10 +67,8 @@ RibOutAttr::NextHop::NextHop(const BgpTable *table, IpAddress address,
         origin_vn_index_ = ext_community->GetOriginVnIndex();
     }
     if (origin_vn_index_ < 0 && vrf_originated) {
-        if (table)
-            origin_vn_index_ = table->routing_instance()->virtual_network_index();
-        else
-            origin_vn_index_ = 0;
+        origin_vn_index_ =
+            table ? table->routing_instance()->virtual_network_index() : 0;
     }
 }
 
@@ -99,14 +97,27 @@ bool RibOutAttr::NextHop::operator!=(const NextHop &rhs) const {
 }
 
 RibOutAttr::RibOutAttr(const BgpTable *table, const BgpAttr *attr,
-                       uint32_t label, bool include_nh) : attr_out_(attr) {
+    uint32_t label)
+    : attr_out_(attr),
+      vrf_originated_(false) {
+    if (attr) {
+        nexthop_list_.push_back(NextHop(table, attr->nexthop(), label,
+            attr->ext_community(), false));
+    }
+}
+
+RibOutAttr::RibOutAttr(const BgpTable *table, const BgpRoute *route,
+    const BgpAttr *attr, uint32_t label, bool include_nh)
+    : attr_out_(attr),
+      vrf_originated_(route->BestPath()->IsVrfOriginated()) {
     if (attr && include_nh) {
         nexthop_list_.push_back(NextHop(table, attr->nexthop(), label,
-                                        attr->ext_community(), false));
+            attr->ext_community(), vrf_originated_));
     }
 }
 
-RibOutAttr::RibOutAttr(BgpRoute *route, const BgpAttr *attr, bool is_xmpp) {
+RibOutAttr::RibOutAttr(const BgpRoute *route, const BgpAttr *attr,
+    bool is_xmpp) : vrf_originated_(false) {
     // Attribute should not be set already
     assert(!attr_out_);
 
@@ -123,9 +134,9 @@ RibOutAttr::RibOutAttr(BgpRoute *route, const BgpAttr *attr, bool is_xmpp) {
     set_attr(table, attr, route->BestPath()->GetLabel(),
              route->BestPath()->IsVrfOriginated());
 
-    for (Route::PathList::iterator it = route->GetPathList().begin();
-        it != route->GetPathList().end(); it++) {
-        const BgpPath *path = static_cast<BgpPath *>(it.operator->());
+    for (Route::PathList::const_iterator it = route->GetPathList().begin();
+        it != route->GetPathList().end(); ++it) {
+        const BgpPath *path = static_cast<const BgpPath *>(it.operator->());
 
         // Skip the best path.
         if (path == route->BestPath())

src/bgp/bgp_ribout.h

@@ -68,10 +68,11 @@ class RibOutAttr {
 
     typedef std::vector<NextHop> NextHopList;
 
-    RibOutAttr() : attr_out_(NULL) { }
-    RibOutAttr(const BgpTable *table, const BgpAttr *attr, uint32_t label,
-               bool include_nh = true);
-    RibOutAttr(BgpRoute *route, const BgpAttr *attr, bool is_xmpp);
+    RibOutAttr() : attr_out_(NULL), vrf_originated_(false) { }
+    RibOutAttr(const BgpTable *table, const BgpAttr *attr, uint32_t label);
+    RibOutAttr(const BgpTable *table, const BgpRoute *route,
+        const BgpAttr *attr, uint32_t label, bool include_nh = true);
+    RibOutAttr(const BgpRoute *route, const BgpAttr *attr, bool is_xmpp);
 
     bool IsReachable() const { return attr_out_.get() != NULL; }
     bool operator==(const RibOutAttr &rhs) const { return CompareTo(rhs) == 0; }
@@ -89,12 +90,14 @@ class RibOutAttr {
     uint32_t label() const {
         return nexthop_list_.empty() ? 0 : nexthop_list_.at(0).label();
     }
+    bool vrf_originated() const { return vrf_originated_; }
 
 private:
     int CompareTo(const RibOutAttr &rhs) const;
 
     BgpAttrPtr attr_out_;
     NextHopList nexthop_list_;
+    bool vrf_originated_;
 };
 
 //

src/bgp/xmpp_message_builder.cc

@@ -457,8 +457,7 @@ string BgpXmppMessage::GetVirtualNetwork(const BgpRoute *route,
     if (!is_reachable_) {
         return "unresolved";
     } else if (roattr->nexthop_list().empty()) {
-        const BgpPath *path = route->BestPath();
-        if (path && path->IsVrfOriginated()) {
+        if (roattr->vrf_originated()) {
             return table_->routing_instance()->GetVirtualNetworkName();
         } else {
             return "unresolved";