Support matching multiple prefixes for routing policy match

  Allow specifying multiple prefixes for routing policy match
  Test case to validate it

Change-Id: Ib79fde031aa1c3a56a87288186afdb06757728b4
Related-bug: #1500698

src/bgp/bgp_config.cc

@@ -331,11 +331,13 @@ std::string RoutingPolicyMatchConfig::ToString() const {
     ostringstream oss;
     oss << "from {" << std::endl;
     if (!community_match.empty()) {
-        oss << "    " << community_match << std::endl;
+        oss << "    community " << community_match << std::endl;
     }
-    if (!prefix_match.prefix_to_match.empty()) {
-        oss << "    " << prefix_match.prefix_to_match << " "
-            << prefix_match.prefix_match_type << std::endl;
+    if (!prefixes_to_match.empty()) {
+        BOOST_FOREACH(const PrefixMatchConfig &match, prefixes_to_match) {
+            oss << "    prefix " << match.prefix_to_match << " "
+                << match.prefix_match_type << std::endl;
+        }
     }
     oss << "}" << std::endl;
     return oss.str();

src/bgp/bgp_config.h

@@ -299,14 +299,16 @@ struct StaticRouteConfig {
 
 typedef std::vector<std::string> CommunityList;
 
-struct PrefixMatch {
+struct PrefixMatchConfig {
     std::string prefix_to_match;
     std::string prefix_match_type;
 };
 
+typedef std::vector<PrefixMatchConfig> PrefixMatchConfigList;
+
 struct RoutingPolicyMatchConfig {
     std::string community_match;
-    PrefixMatch prefix_match;
+    PrefixMatchConfigList prefixes_to_match;
     std::string ToString() const;
 };
 

src/bgp/bgp_config_ifmap.cc

@@ -1570,8 +1570,15 @@ void BgpIfmapRoutingPolicyConfig::RemoveInstance(BgpIfmapInstanceConfig *rti) {
 static void BuildPolicyTerm(autogen::PolicyTerm cfg_term,
                             RoutingPolicyTerm *term) {
     term->match.community_match = cfg_term.fromxx.community;
-    term->match.prefix_match.prefix_to_match = cfg_term.fromxx.prefix.prefix;
-    term->match.prefix_match.prefix_match_type = cfg_term.fromxx.prefix.type_;
+    BOOST_FOREACH(const autogen::PrefixMatch &prefix_match,
+                  cfg_term.fromxx.prefix) {
+        PrefixMatchConfig match;
+        match.prefix_to_match = prefix_match.prefix;
+        match.prefix_match_type =
+            prefix_match.type_.empty() ? "exact" : prefix_match.type_;
+        term->match.prefixes_to_match.push_back(match);
+    }
+
     BOOST_FOREACH(const std::string community,
                   cfg_term.then.update.community.add.community) {
         term->action.update.community_add.push_back(community);

src/bgp/routing-policy/routing_policy.cc

@@ -358,28 +358,33 @@ RoutingPolicy::PolicyTermPtr RoutingPolicy::BuildTerm(const RoutingPolicyTerm &c
          new MatchCommunity(communities_to_match);
         matches.push_back(community);
     }
-    if (!cfg_term.match.prefix_match.prefix_to_match.empty()) {
-        boost::system::error_code ec;
-        Ip4Address ip4;
-        int plen;
-        ec = Ip4PrefixParse(cfg_term.match.prefix_match.prefix_to_match,
-                            &ip4, &plen);
-        if (ec.value() == 0) {
-            PrefixMatchInet *prefix =
-                new PrefixMatchInet(cfg_term.match.prefix_match.prefix_to_match,
-                                 cfg_term.match.prefix_match.prefix_match_type);
-            matches.push_back(prefix);
-        } else {
-            Ip6Address ip6;
-            ec = Inet6PrefixParse(cfg_term.match.prefix_match.prefix_to_match,
-                                  &ip6, &plen);
+
+    if (!cfg_term.match.prefixes_to_match.empty()) {
+        PrefixMatchConfigList inet_prefix_list;
+        PrefixMatchConfigList inet6_prefix_list;
+        BOOST_FOREACH(PrefixMatchConfig match, cfg_term.match.prefixes_to_match) {
+            boost::system::error_code ec;
+            Ip4Address ip4;
+            int plen;
+            ec = Ip4PrefixParse(match.prefix_to_match, &ip4, &plen);
             if (ec.value() == 0) {
-                PrefixMatchInet6 *prefix = new
-                   PrefixMatchInet6(cfg_term.match.prefix_match.prefix_to_match,
-                                 cfg_term.match.prefix_match.prefix_match_type);
-                matches.push_back(prefix);
+                inet_prefix_list.push_back(match);
+            } else {
+                Ip6Address ip6;
+                ec = Inet6PrefixParse(match.prefix_to_match, &ip6, &plen);
+                if (ec.value() == 0) {
+                    inet6_prefix_list.push_back(match);
+                }
             }
         }
+        if (!inet_prefix_list.empty()) {
+            PrefixMatchInet *prefix = new PrefixMatchInet(inet_prefix_list);
+            matches.push_back(prefix);
+        } 
+        if (!inet6_prefix_list.empty()) {
+            PrefixMatchInet6 *prefix = new PrefixMatchInet6(inet6_prefix_list);
+            matches.push_back(prefix);
+        }
     }
 
     // Build the Action object

src/bgp/routing-policy/routing_policy_match.cc

@@ -14,6 +14,8 @@
 
 using std::ostringstream;
 using std::string;
+using std::make_pair;
+
 MatchCommunity::MatchCommunity(const std::vector<string> &communities) {
     BOOST_FOREACH(const string &community, communities) {
         uint32_t value = CommunityType::CommunityFromString(community);
@@ -65,15 +67,19 @@ bool MatchCommunity::IsEqual(const RoutingPolicyMatch &community) const {
 }
 
 template <typename T>
-MatchPrefix<T>::MatchPrefix(const string &prefix, const string &match_type) {
-    boost::system::error_code ec;
-    match_prefix_ = PrefixT::FromString(prefix, &ec);
-    if (strcmp(match_type.c_str(), "exact") == 0) {
-        match_type_ = EXACT;
-    } else if (strcmp(match_type.c_str(), "longer") == 0) {
-        match_type_ = LONGER;
-    } else if (strcmp(match_type.c_str(), "orlonger") == 0) {
-        match_type_ = ORLONGER;
+MatchPrefix<T>::MatchPrefix(const PrefixMatchConfigList &match_list) {
+    BOOST_FOREACH(const PrefixMatchConfig &match, match_list) {
+        boost::system::error_code ec;
+        PrefixT match_prefix = PrefixT::FromString(match.prefix_to_match, &ec);
+        MatchType match_type = EXACT;
+        if (strcmp(match.prefix_match_type.c_str(), "exact") == 0) {
+            match_type = EXACT;
+        } else if (strcmp(match.prefix_match_type.c_str(), "longer") == 0) {
+            match_type = LONGER;
+        } else if (strcmp(match.prefix_match_type.c_str(), "orlonger") == 0) {
+            match_type = ORLONGER;
+        }
+        match_list_.push_back(make_pair(match_prefix, match_type));
     }
 }
 
@@ -87,13 +93,15 @@ bool MatchPrefix<T>::Match(const BgpRoute *route,
     const RouteT *in_route = dynamic_cast<const RouteT *>(route);
     if (in_route == NULL) return false;
     const PrefixT &prefix = in_route->GetPrefix();
-    if (match_type_ == EXACT) {
-        if (prefix == match_prefix_) return true;
-    } else if (match_type_ == LONGER) {
-        if (prefix == match_prefix_) return false;
-        if (prefix.IsMoreSpecific(match_prefix_)) return true;
-    } else if (match_type_ == ORLONGER) {
-        if (prefix.IsMoreSpecific(match_prefix_)) return true;
+    BOOST_FOREACH(const PrefixMatch &match, match_list_) {
+        if (match.second == EXACT) {
+            if (prefix == match.first) return true;
+        } else if (match.second == LONGER) {
+            if (prefix == match.first) continue;
+            if (prefix.IsMoreSpecific(match.first)) return true;
+        } else if (match.second == ORLONGER) {
+            if (prefix.IsMoreSpecific(match.first)) return true;
+        }
     }
     return false;
 }
@@ -102,17 +110,23 @@ template <typename T>
 bool MatchPrefix<T>::IsEqual(const RoutingPolicyMatch &prefix) const {
     const MatchPrefix in_prefix =
         static_cast<const MatchPrefix&>(prefix);
-    if (match_type_ == in_prefix.match_type_)
-        return (match_prefix_ == in_prefix.match_prefix_);
+    std::equal(in_prefix.match_list_.begin(), in_prefix.match_list_.end(),
+               match_list_.begin());
     return true;
 }
 
 template <typename T>
 string MatchPrefix<T>::ToString() const {
     ostringstream oss;
-    oss << "prefix  " << match_prefix_.ToString();
-    if  (match_type_ == LONGER) oss << " longer";
-    else if  (match_type_ == ORLONGER) oss << " orlonger";
+    oss << "prefix  [";
+    BOOST_FOREACH(const PrefixMatch &match, match_list_) {
+        oss << " " << match.first.ToString();
+        if  (match.second == LONGER) oss << " longer";
+        else if  (match.second == ORLONGER) oss << " orlonger";
+        oss << ",";
+    }
+    oss.seekp(-1, oss.cur);
+    oss << " ]";
     return oss.str();
 }
 

src/bgp/routing-policy/routing_policy_match.h

@@ -11,6 +11,7 @@
 
 #include <stdint.h>
 
+#include "bgp/bgp_config.h"
 #include "bgp/inet/inet_route.h"
 #include "bgp/inet6/inet6_route.h"
 
@@ -72,15 +73,15 @@ class MatchPrefix : public RoutingPolicyMatch {
         LONGER,
         ORLONGER,
     };
-    explicit MatchPrefix(const std::string &prefix,
-                         const std::string &match_type="exact");
+    typedef std::pair<PrefixT, MatchType> PrefixMatch;
+    typedef std::vector<PrefixMatch> PrefixMatchList;
+    explicit MatchPrefix(const PrefixMatchConfigList &list);
     virtual ~MatchPrefix();
     virtual bool Match(const BgpRoute *route, const BgpAttr *attr) const;
     virtual std::string ToString() const;
     virtual bool IsEqual(const RoutingPolicyMatch &prefix) const;
 private:
-    PrefixT match_prefix_;
-    MatchType match_type_;
+    PrefixMatchList match_list_;
 };
 
 typedef MatchPrefix<InetPrefixMatch> PrefixMatchInet;

src/bgp/test/routing_policy_test.cc

@@ -334,6 +334,62 @@ TEST_F(RoutingPolicyTest, PolicyPrefixMatchUpdateLocalPref) {
     DeleteRoute<InetDefinition>(peers_[0], "test.inet.0", "10.0.1.1/32");
 }
 
+TEST_F(RoutingPolicyTest, PolicyMultiplePrefixMatchUpdateLocalPref) {
+    string content =
+        FileRead("controller/src/bgp/testdata/routing_policy_0d.xml");
+    EXPECT_TRUE(parser_.Parse(content));
+    task_util::WaitForIdle();
+
+    boost::system::error_code ec;
+    peers_.push_back(
+        new BgpPeerMock(Ip4Address::from_string("192.168.0.1", ec)));
+
+    AddRoute<InetDefinition>(peers_[0], "test.inet.0",
+                                   "10.0.1.1/32", 100);
+    AddRoute<InetDefinition>(peers_[0], "test.inet.0",
+                                   "20.1.1.1/32", 100);
+    AddRoute<InetDefinition>(peers_[0], "test.inet.0",
+                                   "30.1.1.1/32", 100);
+    task_util::WaitForIdle();
+
+    VERIFY_EQ(3, RouteCount("test.inet.0"));
+    BgpRoute *rt =
+        RouteLookup<InetDefinition>("test.inet.0", "10.0.1.1/32");
+    ASSERT_TRUE(rt != NULL);
+    VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
+    const BgpAttr *attr = rt->BestPath()->GetAttr();
+    const BgpAttr *orig_attr = rt->BestPath()->GetOriginalAttr();
+    uint32_t original_local_pref = orig_attr->local_pref();
+    uint32_t policy_local_pref = attr->local_pref();
+    ASSERT_TRUE(policy_local_pref == 102);
+    ASSERT_TRUE(original_local_pref == 100);
+
+    rt = RouteLookup<InetDefinition>("test.inet.0", "20.1.1.1/32");
+    ASSERT_TRUE(rt != NULL);
+    VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
+    attr = rt->BestPath()->GetAttr();
+    orig_attr = rt->BestPath()->GetOriginalAttr();
+    original_local_pref = orig_attr->local_pref();
+    policy_local_pref = attr->local_pref();
+    ASSERT_TRUE(policy_local_pref == 102);
+    ASSERT_TRUE(original_local_pref == 100);
+
+    rt = RouteLookup<InetDefinition>("test.inet.0", "30.1.1.1/32");
+    ASSERT_TRUE(rt != NULL);
+    VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
+    attr = rt->BestPath()->GetAttr();
+    orig_attr = rt->BestPath()->GetOriginalAttr();
+    original_local_pref = orig_attr->local_pref();
+    policy_local_pref = attr->local_pref();
+    ASSERT_TRUE(policy_local_pref == 100);
+    ASSERT_TRUE(original_local_pref == 100);
+
+    DeleteRoute<InetDefinition>(peers_[0], "test.inet.0", "10.0.1.1/32");
+    DeleteRoute<InetDefinition>(peers_[0], "test.inet.0", "20.1.1.1/32");
+    DeleteRoute<InetDefinition>(peers_[0], "test.inet.0", "30.1.1.1/32");
+}
+
+
 TEST_F(RoutingPolicyTest, PolicyCommunityMatchReject) {
     string content =
         FileRead("controller/src/bgp/testdata/routing_policy_0.xml");

src/bgp/testdata/routing_policy_0d.xml

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="utf-8"?>
+<config>
+    <routing-policy name='basic'>
+        <term>
+            <from>
+                <prefix>
+                    <prefix>10.0.1.1/32</prefix>
+                </prefix>
+                <prefix>
+                    <prefix>20.1.1.0/24</prefix>
+                    <type>longer</type>
+                </prefix>
+            </from>
+            <then>
+                <update>
+                    <local-pref>102</local-pref>
+                </update>
+                <action>accept</action>
+            </then>
+        </term>
+    </routing-policy>
+    <routing-instance name="test">
+        <routing-policy to="basic">
+            <sequence>1.0</sequence>
+        </routing-policy>
+        <vrf-target>target:1:103</vrf-target>
+    </routing-instance>
+</config>

src/bgp/testdata/routing_policy_6c.xml

@@ -7,6 +7,13 @@
                     <prefix>1.1.1.1/32</prefix>
                     <type>exact</type>
                 </prefix>
+                <prefix>
+                    <prefix>4.4.4.4/32</prefix>
+                </prefix>
+                <prefix>
+                    <prefix>5.5.5.5/32</prefix>
+                    <type>exact</type>
+                </prefix>
             </from>
             <then>
                 <update>

src/schema/routing_policy.xsd

@@ -59,7 +59,7 @@
 
 <xsd:complexType name="PolicyMatch">
     <xsd:element name="community"   type="xsd:string"/>
-    <xsd:element name="prefix"      type="PrefixMatch"/>
+    <xsd:element name="prefix"      type="PrefixMatch" maxOccurs="unbounded"/>
 </xsd:complexType>
 
 <xsd:complexType name='PolicyTerm'>