Fix circular route re-origination bug with transit VN

Problem:

When a service is applied between a VN and a transit VN, routes from the
VN that are re-originated into the transit VN as ServiceChain routes get
re-originated again as ServiceChain routes into the regular VN.

Cause:

Implementation prior to transit VN used to ignore routes that did belong
to the destination VN of the service chain.  Code changes for transit VN
functionality relaxed this check, but did not cover the case where route
belongs to the source VN.

Fix:

Add check to ignore routes from source VN.

Change-Id: Ifdff8e828f0d4a9bc20ab055e0e9206d2cfd347a
Partial-Bug: 1365277

src/bgp/routing-instance/service_chaining.cc

@@ -132,8 +132,11 @@ bool ServiceChain::Match(BgpServer *server, BgpTable *table,
                         deleted = true;
 
                     int vn_index = GetOriginVnIndex(route);
+                    int src_vn_index = src_->virtual_network_index();
                     int dest_vn_index = dest_->virtual_network_index();
                     if (!vn_index || dest_vn_index != vn_index) {
+                        if (src_vn_index == vn_index)
+                            deleted = true;
                         if (!dest_->virtual_network_allow_transit())
                             deleted = true;
                         if (!dest_vn_index)

src/bgp/test/bgp_test_util.cc

@@ -4,6 +4,7 @@
 
 #include "bgp/test/bgp_test_util.h"
 
+#include <assert.h>
 #include <stdio.h>
 #include <pugixml/pugixml.hpp>
 
@@ -15,7 +16,12 @@ using namespace std;
 namespace bgp_util {
 string NetworkConfigGenerate(
         const vector<string> &instance_names,
-        const multimap<string, string> &connections) {
+        const multimap<string, string> &connections,
+        const vector<string> &networks,
+        const vector<int> &network_ids) {
+    assert(networks.empty() || instance_names.size() == networks.size());
+    assert(networks.size() == network_ids.size());
+
     int index;
     xml_document xdoc;
     xml_node env = xdoc.append_child("Envelope");
@@ -28,14 +34,25 @@ string NetworkConfigGenerate(
         xml_node item = update.append_child("resultItem");
         xml_node id = item.append_child("identity");
         string vn("virtual-network:");
-        vn.append(*iter);
+        if (networks.empty()) {
+            vn.append(*iter);
+        } else {
+            vn.append(networks[index]);
+        }
         id.append_attribute("name") = vn.c_str();
         xml_node meta = item.append_child("metadata");
         xml_node vn_properties = meta.append_child("virtual-network-properties");
         xml_node net_id = vn_properties.append_child("network-id");
-        char value[16];
-        snprintf(value, sizeof(value), "%d", ++index);
-        net_id.append_child(node_pcdata).set_value(value);
+        int value;
+        if (network_ids.empty()) {
+            value = index + 1;
+        } else {
+            value = network_ids[index];
+        }
+        char value_str[16];
+        snprintf(value_str, sizeof(value), "%d", value);
+        net_id.append_child(node_pcdata).set_value(value_str);
+        index++;
     }
     index = 0;
     for (vector<string>::const_iterator iter = instance_names.begin();
@@ -47,24 +64,31 @@ string NetworkConfigGenerate(
         id1.append_attribute("name") = instance.c_str();
         xml_node id2 = item.append_child("identity");
         ostringstream target;
-        target << "route-target:target:64496:" << ++index;
+        target << "route-target:target:64496:" << (index + 1);
         id2.append_attribute("name") = target.str().c_str();
         xml_node meta = item.append_child("metadata");
         meta.append_child("instance-target");
+        index++;
     }
+    index = 0;
     for (vector<string>::const_iterator iter = instance_names.begin();
          iter != instance_names.end(); ++iter) {
         xml_node item = update.append_child("resultItem");
         xml_node id1 = item.append_child("identity");
         string vn("virtual-network:");
-        vn.append(*iter);
+        if (networks.empty()) {
+            vn.append(*iter);
+        } else {
+            vn.append(networks[index]);
+        }
         id1.append_attribute("name") = vn.c_str();
         xml_node id2 = item.append_child("identity");
         string instance("routing-instance:");
         instance.append(*iter);
         id2.append_attribute("name") = instance.c_str();
         xml_node meta = item.append_child("metadata");
         meta.append_child("virtual-network-routing-instance");
+        index++;
     }
     for (multimap<string, string>::const_iterator iter = connections.begin();
          iter != connections.end(); ++iter) {

src/bgp/test/bgp_test_util.h

@@ -11,8 +11,10 @@
 
 namespace bgp_util {
 std::string NetworkConfigGenerate(
-                const std::vector<std::string> &instance_names,
-                const std::multimap<std::string, std::string> &connections);
+    const std::vector<std::string> &instance_names,
+    const std::multimap<std::string, std::string> &connections,
+    const std::vector<std::string> &networks = std::vector<std::string>(),
+    const std::vector<int> &network_ids = std::vector<int>());
 }
 
 #endif