When policy changes, the label of VMI is updated. The updated VMI lab…

…el was not

reflected in AAP routes. Added UT to verify this.

Remove the API InetUnicastAgentRouteTable::ReEvaluatePaths as we don’t need
separate handling for policy change because we change label on policy change.

Change-Id: Ifbb719002f53c3527efa5b161d0b37e6b372c6be
Closes-Bug: #1603958

src/vnsw/agent/oper/inet_unicast_route.cc

@@ -161,22 +161,6 @@ static void InetUnicastTableProcess(Agent *agent, const string &vrf_name,
     }
 }
 
-void InetUnicastAgentRouteTable::ReEvaluatePaths(const Agent* agent,
-                                                 const string &vrf_name,
-                                                 const IpAddress &addr,
-                                                 uint8_t plen) {
-    DBRequest  rt_req(DBRequest::DB_ENTRY_ADD_CHANGE);
-    InetUnicastRouteKey *rt_key = new InetUnicastRouteKey(agent->local_peer(),
-                                                          vrf_name,
-                                                          addr,
-                                                          plen);
-
-    rt_key->sub_op_ = AgentKey::RESYNC;
-    rt_req.key.reset(rt_key);
-    rt_req.data.reset(NULL);
-    InetUnicastTableEnqueue(Agent::GetInstance(), vrf_name, &rt_req);
-}
-
 /*
  * Traverse all smaller subnets w.r.t. route sent and mark the arp flood flag
  * accordingly.

src/vnsw/agent/oper/inet_unicast_route.h

@@ -193,10 +193,6 @@ class InetUnicastAgentRouteTable : public AgentRouteTable {
     }
 
     static DBTableBase *CreateTable(DB *db, const std::string &name);
-    static void ReEvaluatePaths(const Agent *agent,
-                                const string &vrf_name,
-                                const IpAddress &ip,
-                                uint8_t plen);
     static void DeleteReq(const Peer *peer, const string &vrf_name,
                           const IpAddress &addr, uint8_t plen,
                           AgentRouteData *data);

src/vnsw/agent/oper/test/test_aap.cc

@@ -13,6 +13,7 @@
 
 #include <cmn/agent_cmn.h>
 
+#include "test_cmn_util.h"
 #include "cfg/cfg_init.h"
 #include "cfg/cfg_interface.h"
 #include "oper/operdb_init.h"
@@ -29,7 +30,6 @@
 #include "oper/vn.h"
 #include "oper/path_preference.h"
 #include "filter/acl.h"
-#include "test_cmn_util.h"
 #include "vr_types.h"
 #include <controller/controller_export.h>
 #include <ksync/ksync_sock_user.h>
@@ -62,71 +62,6 @@ class TestAap : public ::testing::Test {
     ~TestAap() {
         DeleteBgpPeer(peer_);
     }
-    void AddAap(std::string intf_name, int intf_id,
-            std::vector<Ip4Address> aap_list) {
-        std::ostringstream buf;
-        buf << "<virtual-machine-interface-allowed-address-pairs>";
-        std::vector<Ip4Address>::iterator it = aap_list.begin();
-        while (it != aap_list.end()) {
-            buf << "<allowed-address-pair>";
-            buf << "<ip>";
-            buf << "<ip-prefix>" << it->to_string()<<"</ip-prefix>";
-            buf << "<ip-prefix-len>"<< 32 << "</ip-prefix-len>";
-            buf << "</ip>";
-            buf << "<mac><mac-address>" << "00:00:00:00:00:00"
-                << "</mac-address></mac>";
-            buf << "<flag>" << "act-stby" << "</flag>";
-            buf << "</allowed-address-pair>";
-            it++;
-        }
-        buf << "</virtual-machine-interface-allowed-address-pairs>";
-        char cbuf[10000];
-        strcpy(cbuf, buf.str().c_str());
-        AddNode("virtual-machine-interface", intf_name.c_str(),
-                intf_id, cbuf);
-        client->WaitForIdle();
-    }
-
-    void AddAap(std::string intf_name, int intf_id, Ip4Address ip,
-                const std::string &mac) {
-        std::ostringstream buf;
-        buf << "<virtual-machine-interface-allowed-address-pairs>";
-        buf << "<allowed-address-pair>";
-        buf << "<ip>";
-        buf << "<ip-prefix>" << ip.to_string() <<"</ip-prefix>";
-        buf << "<ip-prefix-len>"<< 32 << "</ip-prefix-len>";
-        buf << "</ip>";
-        buf << "<mac>" << mac << "</mac>";
-        buf << "<flag>" << "act-stby" << "</flag>";
-        buf << "</allowed-address-pair>";
-        buf << "</virtual-machine-interface-allowed-address-pairs>";
-        char cbuf[10000];
-        strcpy(cbuf, buf.str().c_str());
-        AddNode("virtual-machine-interface", intf_name.c_str(),
-                intf_id, cbuf);
-        client->WaitForIdle();
-    }
-
-    void AddEcmpAap(std::string intf_name, int intf_id, Ip4Address ip) {
-        std::ostringstream buf;
-        buf << "<virtual-machine-interface-allowed-address-pairs>";
-        buf << "<allowed-address-pair>";
-        buf << "<ip>";
-        buf << "<ip-prefix>" << ip.to_string() <<"</ip-prefix>";
-        buf << "<ip-prefix-len>"<< 32 << "</ip-prefix-len>";
-        buf << "</ip>";
-        buf << "<mac><mac-address>" << "00:00:00:00:00:00"
-            << "</mac-address></mac>";
-        buf << "<address-mode>" << "active-active" << "</address-mode>";
-        buf << "</allowed-address-pair>";
-        buf << "</virtual-machine-interface-allowed-address-pairs>";
-        char cbuf[10000];
-        strcpy(cbuf, buf.str().c_str());
-        AddNode("virtual-machine-interface", intf_name.c_str(),
-                intf_id, cbuf);
-        client->WaitForIdle();
-    }
-
 
     void AddVlan(std::string intf_name, int intf_id, uint32_t vlan) {
         std::ostringstream buf;

src/vnsw/agent/oper/test/test_intf_policy.cc

@@ -996,6 +996,64 @@ TEST_F(PolicyTest, EcmpNH_3) {
     EXPECT_FALSE(RouteFind("vrf1", ip, 32));
 }
 
+//Add and delete allowed address pair route
+TEST_F(PolicyTest, AapRoute) {
+    struct PortInfo input[] = {
+        {"vnet1", 1, "1.1.1.10", "00:00:00:01:01:01", 1, 1}
+    };
+
+    client->Reset();
+    CreateVmportEnv(input, 1, 1);
+    client->WaitForIdle();
+    EXPECT_TRUE(VmPortActive(input, 0));
+    const VmInterface *intf1 = VmInterfaceGet(input[0].intf_id);
+    EXPECT_TRUE(intf1 != NULL);
+
+   //Add an AAP route
+   Ip4Address ip = Ip4Address::from_string("10.10.10.10");
+   std::vector<Ip4Address> v;
+   v.push_back(ip);
+
+   AddAap("vnet1", 1, v);
+   EXPECT_TRUE(RouteFind("vrf1", ip, 32));
+
+   InetUnicastRouteEntry *rt = RouteGet("vrf1", ip, 32);
+   EXPECT_TRUE(rt != NULL);
+   const AgentPath *path = rt->GetActivePath();
+   EXPECT_TRUE(path != NULL);
+   EXPECT_TRUE(path->label() == intf1->label());
+   uint32_t vmi_label = intf1->label();
+
+   WAIT_FOR(100, 1000, ((VmPortPolicyEnabled(input, 0)) == true));
+
+   //Disable policy on vnet1 VMI
+   AddAapWithDisablePolicy("vnet1", 1, v, true);
+   client->WaitForIdle();
+
+   //Verify that policy status of interface
+   WAIT_FOR(100, 1000, ((VmPortPolicyEnabled(input, 0)) == false));
+
+   //Verify that vnet1's label has changed after disabling policy on it.
+   EXPECT_TRUE(vmi_label != intf1->label());
+
+   //Verify that label of aap_route's path is updated with new label of VMI
+   rt = RouteGet("vrf1", ip, 32);
+   EXPECT_TRUE(rt != NULL);
+   path = rt->GetActivePath();
+   EXPECT_TRUE(path != NULL);
+   EXPECT_TRUE(path->label() == intf1->label());
+
+   //Remove AAP route
+   v.clear();
+   AddAap("vnet1", 1, v);
+   EXPECT_FALSE(RouteFind("vrf1", ip, 32));
+
+   //cleanup
+   DeleteVmportEnv(input, 1, true, 1);
+   client->WaitForIdle();
+   WAIT_FOR(100, 1000, (VrfFind("vrf1") == false));
+}
+
 int main(int argc, char **argv) {
     GETUSERARGS();
 

src/vnsw/agent/oper/vm_interface.cc

@@ -1531,8 +1531,8 @@ void VmInterface::UpdateL3(bool old_ipv4_active, VrfEntry *old_vrf,
                            const Ip4Address &old_dhcp_addr) {
     if (ipv4_active_) {
         if (do_dhcp_relay_) {
-            UpdateIpv4InterfaceRoute(old_ipv4_active, force_update,
-                                     policy_change,
+            UpdateIpv4InterfaceRoute(old_ipv4_active,
+                                     force_update||policy_change,
                                      old_vrf, old_dhcp_addr);
         }
         UpdateIpv4InstanceIp(force_update, policy_change, false,
@@ -1549,7 +1549,7 @@ void VmInterface::UpdateL3(bool old_ipv4_active, VrfEntry *old_vrf,
                       old_ipv6_active);
     UpdateAllowedAddressPair(force_update, policy_change, false, false, false);
     UpdateVrfAssignRule();
-    UpdateStaticRoute(force_update, policy_change);
+    UpdateStaticRoute(force_update||policy_change);
 }
 
 void VmInterface::DeleteL3(bool old_ipv4_active, VrfEntry *old_vrf,
@@ -3148,16 +3148,16 @@ IpAddress VmInterface::GetServiceIp(const IpAddress &vm_ip) const {
 }
 
 // Add/Update route. Delete old route if VRF or address changed
-void VmInterface::UpdateIpv4InterfaceRoute(bool old_ipv4_active, bool force_update,
-                            bool policy_change,
-                            VrfEntry * old_vrf,
-                            const Ip4Address &old_addr) {
+void VmInterface::UpdateIpv4InterfaceRoute(bool old_ipv4_active,
+                                           bool force_update,
+                                           VrfEntry * old_vrf,
+                                           const Ip4Address &old_addr) {
     Ip4Address ip = GetServiceIp(primary_ip_addr_).to_v4();
 
     // If interface was already active earlier and there is no force_update or
     // policy_change, return
     if (old_ipv4_active == true && force_update == false
-        && policy_change == false && old_addr == primary_ip_addr_ &&
+        && old_addr == primary_ip_addr_ &&
         vm_ip_service_addr_ == ip) {
         return;
     }
@@ -3171,12 +3171,6 @@ void VmInterface::UpdateIpv4InterfaceRoute(bool old_ipv4_active, bool force_upda
             AddRoute(vrf_->GetName(), primary_ip_addr_, 32, vn_->GetName(),
                      policy_enabled_, ecmp_, false, vm_ip_service_addr_,
                      Ip4Address(0), CommunityList(), label_);
-        } else if (policy_change) {
-            // If old-l3-active and there is change in policy, invoke RESYNC of
-            // route to account for change in NH policy
-            InetUnicastAgentRouteTable::ReEvaluatePaths(agent(),
-                                                        vrf_->GetName(),
-                                                        primary_ip_addr_, 32);
         }
     }
 
@@ -3364,7 +3358,7 @@ void VmInterface::DeleteServiceVlan() {
     }
 } 
 
-void VmInterface::UpdateStaticRoute(bool force_update, bool policy_change) {
+void VmInterface::UpdateStaticRoute(bool force_update) {
     StaticRouteSet::iterator it = static_route_list_.list_.begin();
     while (it != static_route_list_.list_.end()) {
         StaticRouteSet::iterator prev = it++;
@@ -3379,7 +3373,7 @@ void VmInterface::UpdateStaticRoute(bool force_update, bool policy_change) {
             prev->DeActivate(this);
             static_route_list_.list_.erase(prev);
         } else {
-            prev->Activate(this, force_update, policy_change);
+            prev->Activate(this, force_update);
         }
     }
 }
@@ -3422,7 +3416,7 @@ void VmInterface::UpdateAllowedAddressPair(bool force_update, bool policy_change
             it->L2Activate(this, force_update, policy_change,
                     old_layer2_forwarding, old_layer3_forwarding);
         } else {
-           it->Activate(this, force_update, policy_change);
+           it->Activate(this, (force_update || policy_change));
         }
     }
 }
@@ -4498,16 +4492,15 @@ bool VmInterface::StaticRoute::IsLess(const StaticRoute *rhs) const {
 }
 
 void VmInterface::StaticRoute::Activate(VmInterface *interface,
-                                        bool force_update,
-                                        bool policy_change) const {
-    if (installed_ && force_update == false && policy_change == false)
+                                        bool force_update) const {
+    if (installed_ && force_update == false)
         return;
 
     if (vrf_ != interface->vrf()->GetName()) {
         vrf_ = interface->vrf()->GetName();
     }
 
-    if (installed_ == false || force_update || policy_change) {
+    if (installed_ == false || force_update) {
         Ip4Address gw_ip(0);
         if (gw_.is_v4() && addr_.is_v4() && gw_.to_v4() != gw_ip) {
             SecurityGroupList sg_id_list;
@@ -4744,12 +4737,11 @@ void VmInterface::AllowedAddressPair::CreateLabelAndNH(Agent *agent,
 }
 
 void VmInterface::AllowedAddressPair::Activate(VmInterface *interface,
-                                               bool force_update,
-                                               bool policy_change) const {
+                                               bool force_update) const {
     IpAddress ip = interface->GetServiceIp(addr_);
 
-    if (installed_ && force_update == false && policy_change == false &&
-        service_ip_ == ip && ecmp_config_changed_ == false) {
+    if (installed_ && force_update == false && service_ip_ == ip &&
+        ecmp_config_changed_ == false) {
         return;
     }
 
@@ -4758,11 +4750,8 @@ void VmInterface::AllowedAddressPair::Activate(VmInterface *interface,
         vrf_ = interface->vrf()->GetName();
     }
 
-    if (installed_ == true && policy_change) {
-        InetUnicastAgentRouteTable::ReEvaluatePaths(agent,
-                                                    vrf_, addr_, plen_);
-    } else if (installed_ == false || force_update || service_ip_ != ip ||
-               ecmp_config_changed_) {
+    if (installed_ == false || force_update || service_ip_ != ip ||
+        ecmp_config_changed_) {
         service_ip_ = ip;
         IpAddress dependent_rt;
         if (ecmp_ == true) {

src/vnsw/agent/oper/vm_interface.h

@@ -243,8 +243,7 @@ class VmInterface : public Interface {
 
         bool operator() (const StaticRoute &lhs, const StaticRoute &rhs) const;
         bool IsLess(const StaticRoute *rhs) const;
-        void Activate(VmInterface *interface, bool force_update,
-                      bool policy_change) const;
+        void Activate(VmInterface *interface, bool force_update) const;
         void DeActivate(VmInterface *interface) const;
 
         mutable std::string vrf_;
@@ -275,8 +274,7 @@ class VmInterface : public Interface {
         bool operator() (const AllowedAddressPair &lhs,
                          const AllowedAddressPair &rhs) const;
         bool IsLess(const AllowedAddressPair *rhs) const;
-        void Activate(VmInterface *interface, bool force_update,
-                      bool policy_change) const;
+        void Activate(VmInterface *interface, bool force_update) const;
         void DeActivate(VmInterface *interface) const;
         void L2Activate(VmInterface *interface, bool force_update,
                         bool policy_change, bool old_layer2_forwarding,
@@ -805,8 +803,8 @@ class VmInterface : public Interface {
     bool Ipv4Deactivated(bool old_ipv4_active);
     bool Ipv6Deactivated(bool old_ipv6_active);
     void UpdateIpv4InterfaceRoute(bool old_ipv4_active, bool force_update,
-                             bool policy_change, VrfEntry * old_vrf,
-                             const Ip4Address &old_addr);
+                                  VrfEntry * old_vrf,
+                                  const Ip4Address &old_addr);
     void DeleteIpv4InterfaceRoute(VrfEntry *old_vrf,
                                   const Ip4Address &old_addr);
     void UpdateResolveRoute(bool old_ipv4_active, bool force_update,
@@ -830,7 +828,7 @@ class VmInterface : public Interface {
     void UpdateServiceVlan(bool force_update, bool policy_change,
                            bool old_ipv4_active, bool old_ipv6_active);
     void DeleteServiceVlan();
-    void UpdateStaticRoute(bool force_update, bool policy_change);
+    void UpdateStaticRoute(bool force_update);
     void DeleteStaticRoute();
     void UpdateAllowedAddressPair(bool force_update, bool policy_change,
                                   bool l2, bool old_l2_forwarding,

src/vnsw/agent/port_ipc/config_stale_cleaner.cc

@@ -88,7 +88,7 @@ bool InterfaceConfigStaleCleaner::CfgIntfWalk(DBTablePartBase *partition,
         if (!pih) {
             return true;
         }
-        string msg;
+        std::string msg;
         pih->DeletePort(UuidToString(cfg_intf->GetUuid()), msg);
     }
     return true;

src/vnsw/agent/test/test_cmn_util.h

@@ -439,6 +439,14 @@ void AddInterfaceRouteTable(const char *name, int id, TestIp4Prefix *addr,
 void AddInterfaceRouteTableV6(const char *name, int id, TestIp6Prefix *addr,
                               int count);
 void ShutdownAgentController(Agent *agent);
+void AddAap(std::string intf_name, int intf_id,
+            std::vector<Ip4Address> aap_list);
+void AddEcmpAap(std::string intf_name, int intf_id, Ip4Address ip);
+void AddAap(std::string intf_name, int intf_id, Ip4Address ip,
+            const std::string &mac);
+void AddAapWithDisablePolicy(std::string intf_name, int intf_id,
+                             std::vector<Ip4Address> aap_list,
+                             bool disable_policy);
 
 class XmppChannelMock : public XmppChannel {
 public: