[sub intf] ecmp hardware convergence acceleration at parent port oper…

… status changes (sonic-net#1492) Take sub port interface into ecmp acceleration account when a next hop object goes through a local sub port interface. Signed-off-by: Wenda Ni <[email protected]>
daall · Dec 7, 2020 · 4948ebf · 4948ebf
1 parent 0ff63a0
commit 4948ebf
Show file tree

Hide file tree

Showing 4 changed files with 403 additions and 35 deletions.
diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp
@@ -133,6 +133,15 @@ bool NeighOrch::addNextHop(const IpAddress &ipAddress, const string &alias)
                         ipAddress.to_string().c_str(), alias.c_str());
         return false;
     }
+    if (p.m_type == Port::SUBPORT)
+    {
+        if (!gPortsOrch->getPort(p.m_parent_port_id, p))
+        {
+            SWSS_LOG_ERROR("Neighbor %s seen on sub interface %s whose parent port doesn't exist",
+                            ipAddress.to_string().c_str(), alias.c_str());
+            return false;
+        }
+    }
 
     NextHopKey nexthop = { ipAddress, alias };
     assert(!hasNextHop(nexthop));
@@ -185,7 +194,7 @@ bool NeighOrch::addNextHop(const IpAddress &ipAddress, const string &alias)
     gFgNhgOrch->validNextHopInNextHopGroup(nexthop);
 
     // For nexthop with incoming port which has down oper status, NHFLAGS_IFDOWN
-    // flag Should be set on it.
+    // flag should be set on it.
     // This scenario may happen under race condition where buffered neighbor event
     // is processed after incoming port is down.
     if (p.m_oper_status == SAI_PORT_OPER_STATUS_DOWN)

diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp
@@ -635,7 +635,7 @@ bool PortsOrch::addSubPort(Port &port, const string &alias, const bool &adminUp,
     }
     if (vlan_id > MAX_VALID_VLAN_ID)
     {
-        SWSS_LOG_ERROR("sub interface %s Port object creation failed: invalid VLAN id %u", alias.c_str(), vlan_id);
+        SWSS_LOG_ERROR("Sub interface %s Port object creation failed: invalid VLAN id %u", alias.c_str(), vlan_id);
         return false;
     }
 
@@ -2815,7 +2815,6 @@ void PortsOrch::doLagTask(Consumer &consumer)
             // Retrieve attributes
             uint32_t mtu = 0;
             string learn_mode;
-            bool operation_status_changed = false;
             string operation_status;
 
             for (auto i : kfvFieldsValues(t))
@@ -2837,16 +2836,6 @@ void PortsOrch::doLagTask(Consumer &consumer)
                         it++;
                         continue;
                     }
-
-                    gNeighOrch->ifChangeInformNextHop(alias,
-                                                 (operation_status == "up"));
-                    Port lag;
-                    if (getPort(alias, lag))
-                    {
-                        operation_status_changed =
-                           (string_oper_status.at(operation_status) !=
-                                                    lag.m_oper_status);
-                    }
                 }
             }
 
@@ -2868,19 +2857,13 @@ void PortsOrch::doLagTask(Consumer &consumer)
             }
             else
             {
-
                 if (!operation_status.empty())
                 {
-                    l.m_oper_status = string_oper_status.at(operation_status);
+                    updatePortOperStatus(l, string_oper_status.at(operation_status));
+
                     m_portList[alias] = l;
                 }
-                if (operation_status_changed)
-                {
-                    PortOperStateUpdate update;
-                    update.port = l;
-                    update.operStatus = string_oper_status.at(operation_status);
-                    notify(SUBJECT_TYPE_PORT_OPER_STATE_CHANGE, static_cast<void *>(&update));
-                }
+
                 if (mtu != 0)
                 {
                     l.m_mtu = mtu;
@@ -4124,22 +4107,35 @@ void PortsOrch::updatePortOperStatus(Port &port, sai_port_oper_status_t status)
             oper_status_strings.at(status).c_str());
     if (status == port.m_oper_status)
     {
-        return ;
+        return;
     }
 
-    updateDbPortOperStatus(port, status);
+    if (port.m_type == Port::PHY)
+    {
+        updateDbPortOperStatus(port, status);
+    }
     port.m_oper_status = status;
 
     bool isUp = status == SAI_PORT_OPER_STATUS_UP;
-    if (!setHostIntfsOperStatus(port, isUp))
+    if (port.m_type == Port::PHY)
     {
-        SWSS_LOG_ERROR("Failed to set host interface %s operational status %s", port.m_alias.c_str(),
-                isUp ? "up" : "down");
+        if (!setHostIntfsOperStatus(port, isUp))
+        {
+            SWSS_LOG_ERROR("Failed to set host interface %s operational status %s", port.m_alias.c_str(),
+                    isUp ? "up" : "down");
+        }
     }
     if (!gNeighOrch->ifChangeInformNextHop(port.m_alias, isUp))
     {
         SWSS_LOG_WARN("Inform nexthop operation failed for interface %s", port.m_alias.c_str());
     }
+    for (const auto &child_port : port.m_child_ports)
+    {
+        if (!gNeighOrch->ifChangeInformNextHop(child_port, isUp))
+        {
+            SWSS_LOG_WARN("Inform nexthop operation failed for sub interface %s", child_port.c_str());
+        }
+    }
 
     PortOperStateUpdate update = {port, status};
     notify(SUBJECT_TYPE_PORT_OPER_STATE_CHANGE, static_cast<void *>(&update));

diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h
@@ -37,10 +37,10 @@ static const map<sai_port_oper_status_t, string> oper_status_strings =
 
 static const unordered_map<string, sai_port_oper_status_t> string_oper_status =
 {
-    { "unknown", SAI_PORT_OPER_STATUS_UNKNOWN },
-    { "up", SAI_PORT_OPER_STATUS_UP },
-    { "down", SAI_PORT_OPER_STATUS_DOWN },
-    { "testing", SAI_PORT_OPER_STATUS_TESTING },
+    { "unknown",     SAI_PORT_OPER_STATUS_UNKNOWN },
+    { "up",          SAI_PORT_OPER_STATUS_UP },
+    { "down",        SAI_PORT_OPER_STATUS_DOWN },
+    { "testing",     SAI_PORT_OPER_STATUS_TESTING },
     { "not present", SAI_PORT_OPER_STATUS_NOT_PRESENT }
 };