* Copy ARP mac, interface and resolved flag in Sync() API of route

Routes and nexthops dependent on ARP route are updated, when any variable in ARP nexthop are changed. When ARP timer is expired, nexthop gets marked as invalid and route Sync is triggered, in this sync() API arp_mac and valid flags were not updated, hence eventually when ARP resolution succeds, there would be no change in agent path and the dependent nexthop would not be resolved. Test case for same Closes-bug:#1457337 Change-Id: I5c5c168367ce706a347a93f9f4852f8c5b68e95b
Juniper · Jun 15, 2015 · 27cd794 · 27cd794
1 parent 9dd4571
commit 27cd794
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 14 deletions.
diff --git a/src/vnsw/agent/oper/agent_path.cc b/src/vnsw/agent/oper/agent_path.cc
@@ -251,6 +251,12 @@ bool AgentPath::Sync(AgentRoute *sync_route) {
         }
     }
 
+    if (nh_ && nh_->GetType() == NextHop::ARP) {
+        if (CopyArpData()) {
+            ret = true;
+        }
+    }
+
     if (vrf_name_ == Agent::NullString()) {
         return ret;
     }
@@ -288,7 +294,6 @@ bool AgentPath::Sync(AgentRoute *sync_route) {
         dependant_rt_.reset(rt);
         ret = true;
     }
-
     return ret;
 }
 
@@ -310,6 +315,28 @@ void AgentPath::set_nexthop(NextHop *nh) {
     nh_ = nh;
 }
 
+bool AgentPath::CopyArpData() {
+    bool ret = false;
+    if (nh_ && nh_->GetType() == NextHop::ARP) {
+        const ArpNH *arp_nh = static_cast<const ArpNH *>(nh_.get());
+        if (arp_mac() != arp_nh->GetMac()) {
+            set_arp_mac(arp_nh->GetMac());
+            ret = true;
+        }
+
+        if (arp_interface() != arp_nh->GetInterface()) {
+            set_arp_interface(arp_nh->GetInterface());
+            ret = true;
+        }
+
+        if (arp_valid() != arp_nh->IsValid()) {
+            set_arp_valid(arp_nh->IsValid());
+            ret = true;
+        }
+    }
+    return ret;
+}
+
 EvpnDerivedPath::EvpnDerivedPath(const EvpnPeer *evpn_peer,
                    const IpAddress &ip_addr,
                    uint32_t ethernet_tag,

diff --git a/src/vnsw/agent/oper/agent_path.h b/src/vnsw/agent/oper/agent_path.h
@@ -208,6 +208,7 @@ class AgentPath : public Path {
 
     bool arp_valid() const { return arp_valid_;}
     void set_arp_valid(bool valid) { arp_valid_ = valid;}
+    bool CopyArpData();
 
 private:
     const Peer *peer_;

diff --git a/src/vnsw/agent/oper/inet_unicast_route.cc b/src/vnsw/agent/oper/inet_unicast_route.cc
@@ -753,19 +753,7 @@ bool Inet4UnicastArpRoute::AddChangePath(Agent *agent, AgentPath *path,
         ret = true;
 
     if (nh) {
-        const ArpNH *arp_nh = static_cast<const ArpNH *>(nh);
-        if (path->arp_mac() != arp_nh->GetMac()) {
-            path->set_arp_mac(arp_nh->GetMac());
-            ret = true;
-        }
-
-        if (path->arp_interface() != arp_nh->GetInterface()) {
-            path->set_arp_interface(arp_nh->GetInterface());
-            ret = true;
-        }
-
-        if (path->arp_valid() != arp_nh->IsValid()) {
-            path->set_arp_valid(arp_nh->IsValid());
+        if (path->CopyArpData()) {
             ret = true;
         }
     }

diff --git a/src/vnsw/agent/test/test_route.cc b/src/vnsw/agent/test/test_route.cc
@@ -584,6 +584,11 @@ TEST_F(RouteTest, RemoteVmRoute_4) {
     client->NHWait(6);
     EXPECT_TRUE(addr_nh->IsValid() == false);
 
+    //Readd ARP and verify tunnel NH is changed
+    AddArp(server1_ip_.to_string().c_str(), "0a:0b:0c:0d:0e:0e", eth_name_.c_str());
+    client->WaitForIdle();
+    EXPECT_TRUE(addr_nh->IsValid() == true);
+
     //Delete Remote VM route
     DeleteRoute(NULL, vrf_name_, remote_vm_ip_, 32);
     EXPECT_FALSE(RouteFind(vrf_name_, remote_vm_ip_, 32));