Do not add entries to ARP cache when VRF is deleted.

As ARP entries were getting added even when VRF is delete marked, state was not deleted on the VRF. closes-bug: 1572520 (cherry picked from commit 68472f29e47a3c37ba5da8dff7bf663831a1a691) Conflicts: src/vnsw/agent/services/arp_proto.cc Change-Id: Ie9e3d7b6cc93cbd281efb8a1bbcf1dbc08815d47
Juniper · May 10, 2016 · b9f6aa7 · b9f6aa7
1 parent 60c72cf
commit b9f6aa7
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 14 deletions.
diff --git a/src/vnsw/agent/services/arp_entry.cc b/src/vnsw/agent/services/arp_entry.cc
@@ -40,7 +40,10 @@ void ArpEntry::HandleDerivedArpRequest() {
     } else {
         entry = new ArpEntry(io_, handler_.get(), key, nh_vrf_, ArpEntry::INITING,
                              interface_);
-        arp_proto->AddArpEntry(entry);
+        if (arp_proto->AddArpEntry(entry) == false) {
+            delete entry;
+            return;
+        }
         entry->HandleArpRequest();
     }
 }

diff --git a/src/vnsw/agent/services/arp_handler.cc b/src/vnsw/agent/services/arp_handler.cc
@@ -148,7 +148,10 @@ bool ArpHandler::HandlePacket() {
             } else {
                 entry = new ArpEntry(io_, this, key, nh_vrf, ArpEntry::INITING,
                                      itf);
-                arp_proto->AddArpEntry(entry);
+                if (arp_proto->AddArpEntry(entry) == false) {
+                    delete entry;
+                    return true;
+                }
                 entry->HandleArpRequest();
                 return false;
             }
@@ -177,7 +180,10 @@ bool ArpHandler::HandlePacket() {
             } else { 
                 entry = new ArpEntry(io_, this, key, nh_vrf, ArpEntry::INITING,
                                      itf);
-                arp_proto->AddArpEntry(entry);
+                if (arp_proto->AddArpEntry(entry) == false) {
+                    delete entry;
+                    return true;
+                }
                 entry->HandleArpReply(MacAddress(arp_->arp_sha));
                 arp_ = NULL;
                 return false;
@@ -229,7 +235,10 @@ bool ArpHandler::HandleMessage() {
             if (!entry) {
                 entry = new ArpEntry(io_, this, ipc->key, ipc->key.vrf,
                                      ArpEntry::INITING, ipc->interface);
-                arp_proto->AddArpEntry(entry);
+                if (arp_proto->AddArpEntry(entry) == false) {
+                    delete entry;
+                    break;
+                }
                 ret = false;
             }
             arp_proto->IncrementStatsArpReq();

diff --git a/src/vnsw/agent/services/arp_proto.cc b/src/vnsw/agent/services/arp_proto.cc
@@ -317,7 +317,8 @@ void ArpVrfState::WalkDone(DBTableBase *partition, ArpVrfState *state) {
 }
 
 void ArpVrfState::PreWalkDone(DBTableBase *partition) {
-    arp_proto->ValidateAndClearVrfState(vrf);
+    if (arp_proto->ValidateAndClearVrfState(vrf, this) == false)
+        return;
     rt_table->Unregister(route_table_listener_id);
     table_delete_ref.Reset(NULL);
 }
@@ -473,6 +474,14 @@ void ArpProto::SendArpIpc(ArpProto::ArpMsgType type, ArpKey &key,
 }
 
 bool ArpProto::AddArpEntry(ArpEntry *entry) {
+    const VrfEntry *vrf = entry->key().vrf;
+    const ArpVrfState *state = static_cast<const ArpVrfState *>
+                         (vrf->GetState(vrf->get_table_partition()->parent(),
+                          vrf_table_listener_id_));
+    // If VRF is delete marked, do not add ARP entries to cache
+    if (state == NULL || state->deleted == true)
+        return false;
+
     bool ret = arp_cache_.insert(ArpCachePair(entry->key(), entry)).second;
     uint32_t intf_id = entry->interface()->id();
     InterfaceArpMap::iterator it = interface_arp_map_.find(intf_id);
@@ -518,14 +527,12 @@ ArpEntry *ArpProto::FindArpEntry(const ArpKey &key) {
     return it->second;
 }
 
-void ArpProto::ValidateAndClearVrfState(VrfEntry *vrf) {
-    if (!vrf->IsDeleted())
-        return;
-
-    ArpKey key(0, vrf);
-    ArpProto::ArpIterator it = arp_cache_.upper_bound(key);
-    if (it != arp_cache_.end() && it->first.vrf == vrf) {
-        return;
+bool ArpProto::ValidateAndClearVrfState(VrfEntry *vrf,
+                                        const ArpVrfState *vrf_state) {
+    if (!vrf_state->deleted) {
+        ARP_TRACE(Trace, "ARP state not cleared - VRF is not delete marked",
+                  "", vrf->GetName(), "");
+        return false;
     }
 
     DBState *state = static_cast<DBState *>
@@ -535,6 +542,7 @@ void ArpProto::ValidateAndClearVrfState(VrfEntry *vrf) {
         vrf->ClearState(vrf->get_table_partition()->parent(),
                         vrf_table_listener_id_);
     }
+    return true;
 }
 
 ArpProto::ArpIterator

diff --git a/src/vnsw/agent/services/arp_proto.h b/src/vnsw/agent/services/arp_proto.h
@@ -14,6 +14,8 @@ do {                                                                        \
     Arp##obj::TraceMsg(ArpTraceBuf, __FILE__, __LINE__, ##__VA_ARGS__);     \
 } while (false)                                                             \
 
+struct ArpVrfState;
+
 class ArpProto : public Proto {
 public:
     static const uint16_t kGratRetries = 2;
@@ -154,7 +156,7 @@ class ArpProto : public Proto {
     void set_aging_timeout(uint32_t timeout) { aging_timeout_ = timeout; }
     void SendArpIpc(ArpProto::ArpMsgType type, in_addr_t ip,
                     const VrfEntry *vrf, const Interface* itf);
-    void ValidateAndClearVrfState(VrfEntry *vrf);
+    bool ValidateAndClearVrfState(VrfEntry *vrf, const ArpVrfState *vrf_state);
     ArpIterator FindUpperBoundArpEntry(const ArpKey &key);
     ArpIterator FindLowerBoundArpEntry(const ArpKey &key);