[Everflow/ERSPAN] Set correct destination port and mac address when the nexthop is updated for ERSPAN mirror destination (sonic-net#2392)

* [Everflow/ERSPAN] Set correct destination port and mac address when the nexthop is updated for ERSPAN mirror destination
* [Everflow] Fixed show mirror-session, Acl rule remove failure, orchagent crash

Signed-off-by: Sakthivadivu Saravanaraj <[email protected]>

Author: saksarav-nokia

orchagent/aclorch.cpp

@@ -1477,6 +1477,11 @@ const vector<AclRangeConfig>& AclRule::getRangeConfig() const
     return m_rangeConfig;
 }
 
+bool AclRule::getCreateCounter() const
+{
+    return m_createCounter;
+}
+
 shared_ptr<AclRule> AclRule::makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple& data)
 {
     shared_ptr<AclRule> aclRule;
@@ -1624,6 +1629,13 @@ bool AclRule::createCounter()
 bool AclRule::removeRanges()
 {
     SWSS_LOG_ENTER();
+    if (!m_ranges.size())
+    {
+       //The Acl Rules which have mirror action will not have ranges created till the mirror becomes active
+       SWSS_LOG_INFO("No Acl Range created for ACL Rule %s in table %s", m_id.c_str(), m_pTable->getId().c_str());
+       return true;
+    }
+
     for (const auto& rangeConfig: m_rangeConfig)
     {
         if (!AclRange::remove(rangeConfig.rangeType, rangeConfig.min, rangeConfig.max))
@@ -1924,6 +1936,16 @@ bool AclRuleMirror::activate()
         setAction(it.first, attr.value.aclaction);
     }
 
+    // If the rule with mirror action is removed and then mirror is activated, create the counter before rule is created
+    if (!hasCounter())
+    {
+        if (getCreateCounter() && !createCounter())
+        {
+            SWSS_LOG_ERROR("createCounter failed for Rule %s session %s", m_id.c_str(), m_sessionName.c_str());
+            return false;
+       }
+    }
+
     if (!AclRule::createRule())
     {
         return false;

orchagent/aclorch.h

@@ -263,6 +263,7 @@ class AclRule
     sai_object_id_t getCounterOid() const;
     bool hasCounter() const;
     vector<sai_object_id_t> getInPorts() const;
+    bool getCreateCounter() const;
 
     const vector<AclRangeConfig>& getRangeConfig() const;
     static shared_ptr<AclRule> makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple&);

orchagent/mirrororch.cpp

@@ -583,13 +583,30 @@ void MirrorOrch::setSessionState(const string& name, const MirrorEntry& session,
     if (attr.empty() || attr == MIRROR_SESSION_MONITOR_PORT)
     {
         Port port;
-        m_portsOrch->getPort(session.neighborInfo.portId, port);
+        if ((gMySwitchType == "voq") && (session.type == MIRROR_SESSION_ERSPAN))
+        {
+             if (!m_portsOrch->getRecircPort(port, "Rec"))
+             {
+                 SWSS_LOG_ERROR("Failed to get recirc port for mirror session %s", name.c_str());
+                 return;
+             }
+	}
+	else
+	{
+           m_portsOrch->getPort(session.neighborInfo.portId, port);
+	}
         fvVector.emplace_back(MIRROR_SESSION_MONITOR_PORT, port.m_alias);
     }
 
     if (attr.empty() || attr == MIRROR_SESSION_DST_MAC_ADDRESS)
     {
-        value = session.neighborInfo.mac.to_string();
+        if ((gMySwitchType == "voq") && (session.type == MIRROR_SESSION_ERSPAN))
+        {
+             value = gMacAddress.to_string();
+        } else
+        {
+             value = session.neighborInfo.mac.to_string();
+        }
         fvVector.emplace_back(MIRROR_SESSION_DST_MAC_ADDRESS, value);
     }
 
@@ -928,7 +945,7 @@ bool MirrorOrch::activateSession(const string& name, MirrorEntry& session)
             Port recirc_port;
             if (!m_portsOrch->getRecircPort(recirc_port, "Rec"))
             {
-                SWSS_LOG_ERROR("Failed to get recirc prot");
+                SWSS_LOG_ERROR("Failed to get recirc port");
                 return false;
             }
             attr.value.oid = recirc_port.m_port_id;
@@ -999,9 +1016,9 @@ bool MirrorOrch::activateSession(const string& name, MirrorEntry& session)
 
         attr.id = SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS;
         // Use router mac as mirror dst mac in voq switch.
-        if (gMySwitchType == "voq")
+        if ((gMySwitchType == "voq") && (session.type == MIRROR_SESSION_ERSPAN))
         {
-            memcpy(attr.value.mac, gMacAddress.getMac(), sizeof(sai_mac_t));
+             memcpy(attr.value.mac, gMacAddress.getMac(), sizeof(sai_mac_t));
         }
         else
         {
@@ -1115,13 +1132,19 @@ bool MirrorOrch::updateSessionDstMac(const string& name, MirrorEntry& session)
 
     sai_attribute_t attr;
     attr.id = SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS;
-    memcpy(attr.value.mac, session.neighborInfo.mac.getMac(), sizeof(sai_mac_t));
+    if ((gMySwitchType == "voq") && (session.type == MIRROR_SESSION_ERSPAN))
+    {
+         memcpy(attr.value.mac, gMacAddress.getMac(), sizeof(sai_mac_t));
+    } else
+    {
+         memcpy(attr.value.mac, session.neighborInfo.mac.getMac(), sizeof(sai_mac_t));
+    }
 
     sai_status_t status = sai_mirror_api->set_mirror_session_attribute(session.sessionId, &attr);
     if (status != SAI_STATUS_SUCCESS)
     {
         SWSS_LOG_ERROR("Failed to update mirror session %s destination MAC to %s, rv:%d",
-                name.c_str(), session.neighborInfo.mac.to_string().c_str(), status);
+                name.c_str(), sai_serialize_mac(attr.value.mac).c_str(), status);
         task_process_status handle_status =  handleSaiSetStatus(SAI_API_MIRROR, status);
         if (handle_status != task_success)
         {
@@ -1130,7 +1153,7 @@ bool MirrorOrch::updateSessionDstMac(const string& name, MirrorEntry& session)
     }
 
     SWSS_LOG_NOTICE("Update mirror session %s destination MAC to %s",
-            name.c_str(), session.neighborInfo.mac.to_string().c_str());
+            name.c_str(), sai_serialize_mac(attr.value.mac).c_str());
 
     setSessionState(name, session, MIRROR_SESSION_DST_MAC_ADDRESS);
 
@@ -1148,7 +1171,20 @@ bool MirrorOrch::updateSessionDstPort(const string& name, MirrorEntry& session)
 
     sai_attribute_t attr;
     attr.id = SAI_MIRROR_SESSION_ATTR_MONITOR_PORT;
-    attr.value.oid = session.neighborInfo.portId;
+    // Set monitor port to recirc port in voq switch.
+    if ((gMySwitchType == "voq") && (session.type == MIRROR_SESSION_ERSPAN))
+    {
+         if (!m_portsOrch->getRecircPort(port, "Rec"))
+         {
+             SWSS_LOG_ERROR("Failed to get recirc port for mirror session %s", name.c_str());
+             return false;
+         }
+         attr.value.oid = port.m_port_id;
+    }
+    else
+    {
+         attr.value.oid = session.neighborInfo.portId;
+    }
 
     sai_status_t status = sai_mirror_api->
         set_mirror_session_attribute(session.sessionId, &attr);

orchagent/neighorch.cpp

@@ -597,18 +597,37 @@ void NeighOrch::decreaseNextHopRefCount(const NextHopKey &nexthop, uint32_t coun
 
 bool NeighOrch::getNeighborEntry(const NextHopKey &nexthop, NeighborEntry &neighborEntry, MacAddress &macAddress)
 {
+    Port inbp;
+    string nbr_alias;
     if (!hasNextHop(nexthop))
     {
         return false;
     }
+    if (gMySwitchType == "voq")
+    {
+        gPortsOrch->getInbandPort(inbp);
+        assert(inbp.m_alias.length());
+    }
 
     for (const auto &entry : m_syncdNeighbors)
     {
-        if (entry.first.ip_address == nexthop.ip_address && entry.first.alias == nexthop.alias)
+        if (entry.first.ip_address == nexthop.ip_address)
         {
-            neighborEntry = entry.first;
-            macAddress = entry.second.mac;
-            return true;
+           if (m_intfsOrch->isRemoteSystemPortIntf(entry.first.alias))
+           {
+               //For remote system ports, nexthops are always on inband.
+               nbr_alias = inbp.m_alias;
+           }
+           else
+           {
+               nbr_alias = entry.first.alias;
+           }
+           if (nbr_alias == nexthop.alias)
+           {
+              neighborEntry = entry.first;
+              macAddress = entry.second.mac;
+              return true;
+           }
         }
     }