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. Change-Id: I6561835406eb0157c5d5543c3733fb01abf5375c closes-bug: 1572520
Juniper · May 5, 2016 · e09183e · e09183e
1 parent f0f83de
commit e09183e
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 15 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
@@ -147,7 +147,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;
             }
@@ -176,7 +179,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;
@@ -228,7 +234,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
@@ -57,7 +57,7 @@ void ArpProto::VrfNotify(DBTablePartBase *part, DBEntryBase *entry) {
     state = static_cast<ArpVrfState *>(entry->GetState(part->parent(),
                                                    vrf_table_listener_id_));
     if (entry->IsDeleted()) {
-        if (state) {
+        if (state && !state->deleted) {
             for (ArpProto::ArpIterator it = arp_cache_.begin();
                  it != arp_cache_.end();) {
                 ArpEntry *arp_entry = it->second;
@@ -307,7 +307,9 @@ void ArpVrfState::Delete() {
 }
 
 void ArpVrfState::WalkDone(DBTableBase *partition, ArpVrfState *state) {
-    arp_proto->ValidateAndClearVrfState(vrf);
+    if (arp_proto->ValidateAndClearVrfState(vrf, state) == false)
+        return;
+
     state->rt_table->Unregister(route_table_listener_id);
     state->table_delete_ref.Reset(NULL);
     delete state;
@@ -463,6 +465,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);
@@ -508,14 +518,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 *>
@@ -525,6 +533,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);