[mirrororch]: Add retry logic when deleting referenced mirror session (…

…sonic-net#1104) When a mirror session is referenced by an ACL rule, we cannot remove the mirror session. However, once the ACL rule gets removed, a retry would help clean up the to-be-removed mirror session. Signed-off-by: Shu0T1an ChenG <[email protected]>
madhanmellanox · Oct 26, 2019 · 22402e5 · 22402e5
1 parent 731a8f5
commit 22402e5
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 8 deletions.
diff --git a/orchagent/mirrororch.cpp b/orchagent/mirrororch.cpp
@@ -352,29 +352,35 @@ void MirrorOrch::createEntry(const string& key, const vector<FieldValueTuple>& d
     m_routeOrch->attach(this, entry.dstIp);
 }
 
-void MirrorOrch::deleteEntry(const string& name)
+task_process_status MirrorOrch::deleteEntry(const string& name)
 {
     SWSS_LOG_ENTER();
 
     auto sessionIter = m_syncdMirrors.find(name);
     if (sessionIter == m_syncdMirrors.end())
     {
-        SWSS_LOG_ERROR("Failed to delete session. Session %s doesn't exist.\n", name.c_str());
-        return;
+        SWSS_LOG_ERROR("Failed to remove non-existent mirror session %s",
+                name.c_str());
+        return task_process_status::task_invalid_entry;
     }
 
     auto& session = sessionIter->second;
 
     if (session.refCount)
     {
-        SWSS_LOG_ERROR("Failed to delete session. Session %s in use.\n", name.c_str());
-        return;
+        SWSS_LOG_WARN("Failed to remove still referenced mirror session %s, retry...",
+                name.c_str());
+        return task_process_status::task_need_retry;
     }
 
     if (session.status)
     {
         m_routeOrch->detach(this, session.dstIp);
-        deactivateSession(name, session);
+        if (!deactivateSession(name, session))
+        {
+            SWSS_LOG_ERROR("Failed to remove mirror session %s", name.c_str());
+            return task_process_status::task_failed;
+        }
     }
 
     if (!session.policer.empty())
@@ -387,6 +393,8 @@ void MirrorOrch::deleteEntry(const string& name)
     m_syncdMirrors.erase(sessionIter);
 
     SWSS_LOG_NOTICE("Removed mirror session %s", name.c_str());
+
+    return task_process_status::task_success;
 }
 
 void MirrorOrch::setSessionState(const string& name, const MirrorEntry& session, const string& attr)
@@ -1144,7 +1152,13 @@ void MirrorOrch::doTask(Consumer& consumer)
         }
         else if (op == DEL_COMMAND)
         {
-            deleteEntry(key);
+            auto task_status = deleteEntry(key);
+            // Specifically retry the task when asked
+            if (task_status == task_process_status::task_need_retry)
+            {
+                it++;
+                continue;
+            }
         }
         else
         {

diff --git a/orchagent/mirrororch.h b/orchagent/mirrororch.h
@@ -94,7 +94,7 @@ class MirrorOrch : public Orch, public Observer, public Subject
     bool m_freeze = false;
 
     void createEntry(const string&, const vector<FieldValueTuple>&);
-    void deleteEntry(const string&);
+    task_process_status deleteEntry(const string&);
 
     bool activateSession(const string&, MirrorEntry&);
     bool deactivateSession(const string&, MirrorEntry&);