Revert "[portsorch] use ingress/egress disable for LAG member enable/disable (sonic-net#1166)"

This reverts commit 4edcc4d.

Author: abdosi

orchagent/portsorch.cpp

@@ -2494,51 +2494,39 @@ void PortsOrch::doLagMemberTask(Consumer &consumer)
                     status = fvValue(i);
             }
 
-            if (lag.m_members.find(port_alias) == lag.m_members.end())
-            {
-                /* Assert the port doesn't belong to any LAG already */
-                assert(!port.m_lag_id && !port.m_lag_member_id);
-
-                if (!addLagMember(lag, port))
-                {
-                    it++;
-                    continue;
-                }
-            }
-
             /* Sync an enabled member */
             if (status == "enabled")
             {
-                /* enable collection first, distribution-only mode
-                 * is not supported on Mellanox platform
-                 */
-                if (setCollectionOnLagMember(port, true) &&
-                    setDistributionOnLagMember(port, true))
+                /* Duplicate entry */
+                if (lag.m_members.find(port_alias) != lag.m_members.end())
                 {
                     it = consumer.m_toSync.erase(it);
+                    continue;
                 }
+
+                /* Assert the port doesn't belong to any LAG */
+                assert(!port.m_lag_id && !port.m_lag_member_id);
+
+                if (addLagMember(lag, port))
+                    it = consumer.m_toSync.erase(it);
                 else
-                {
                     it++;
-                    continue;
-                }
             }
             /* Sync an disabled member */
             else /* status == "disabled" */
             {
-                /* disable distribution first, distribution-only mode
-                 * is not supported on Mellanox platform
-                 */
-                if (setDistributionOnLagMember(port, false) &&
-                    setCollectionOnLagMember(port, false))
+                /* "status" is "disabled" at start when m_lag_id and
+                 * m_lag_member_id are absent */
+                if (!port.m_lag_id || !port.m_lag_member_id)
                 {
                     it = consumer.m_toSync.erase(it);
+                    continue;
                 }
+
+                if (removeLagMember(lag, port))
+                    it = consumer.m_toSync.erase(it);
                 else
-                {
                     it++;
-                    continue;
-                }
             }
         }
         /* Remove a LAG member */
@@ -2556,13 +2544,9 @@ void PortsOrch::doLagMemberTask(Consumer &consumer)
             }
 
             if (removeLagMember(lag, port))
-            {
                 it = consumer.m_toSync.erase(it);
-            }
             else
-            {
                 it++;
-            }
         }
         else
         {
@@ -3356,52 +3340,6 @@ bool PortsOrch::removeLagMember(Port &lag, Port &port)
     return true;
 }
 
-bool PortsOrch::setCollectionOnLagMember(Port &lagMember, bool enableCollection)
-{
-    /* Port must be LAG member */
-    assert(port.m_lag_member_id);
-
-    sai_status_t status = SAI_STATUS_FAILURE;
-    sai_attribute_t attr {};
-
-    attr.id = SAI_LAG_MEMBER_ATTR_INGRESS_DISABLE;
-    attr.value.booldata = !enableCollection;
-
-    status = sai_lag_api->set_lag_member_attribute(lagMember.m_lag_member_id, &attr);
-    if (status != SAI_STATUS_SUCCESS)
-    {
-        SWSS_LOG_ERROR("Failed to %s collection on LAG member %s",
-            enableCollection ? "enable" : "disable",
-            lagMember.m_alias.c_str());
-        return false;
-    }
-
-    return true;
-}
-
-bool PortsOrch::setDistributionOnLagMember(Port &lagMember, bool enableDistribution)
-{
-    /* Port must be LAG member */
-    assert(port.m_lag_member_id);
-
-    sai_status_t status = SAI_STATUS_FAILURE;
-    sai_attribute_t attr {};
-
-    attr.id = SAI_LAG_MEMBER_ATTR_EGRESS_DISABLE;
-    attr.value.booldata = !enableDistribution;
-
-    status = sai_lag_api->set_lag_member_attribute(lagMember.m_lag_member_id, &attr);
-    if (status != SAI_STATUS_SUCCESS)
-    {
-        SWSS_LOG_ERROR("Failed to %s distribution on LAG member %s",
-            enableDistribution ? "enable" : "disable",
-            lagMember.m_alias.c_str());
-        return false;
-    }
-
-    return true;
-}
-
 void PortsOrch::generateQueueMap()
 {
     if (m_isQueueMapGenerated)

orchagent/portsorch.h

@@ -170,8 +170,6 @@ class PortsOrch : public Orch, public Subject
     bool removeLag(Port lag);
     bool addLagMember(Port &lag, Port &port);
     bool removeLagMember(Port &lag, Port &port);
-    bool setCollectionOnLagMember(Port &lagMember, bool enableCollection);
-    bool setDistributionOnLagMember(Port &lagMember, bool enableDistribution);
     void getLagMember(Port &lag, vector<Port> &portv);
 
     bool addPort(const set<int> &lane_set, uint32_t speed, int an=0, string fec="");

tests/test_portchannel.py

@@ -33,41 +33,13 @@ def test_Portchannel(self, dvs, testlog):
         assert len(lagms) == 1
 
         (status, fvs) = lagmtbl.get(lagms[0])
-        fvs = dict(fvs)
-        assert status
-        assert "SAI_LAG_MEMBER_ATTR_LAG_ID" in fvs
-        assert fvs.pop("SAI_LAG_MEMBER_ATTR_LAG_ID") == lags[0]
-        assert "SAI_LAG_MEMBER_ATTR_PORT_ID" in fvs
-        assert dvs.asicdb.portoidmap[fvs.pop("SAI_LAG_MEMBER_ATTR_PORT_ID")] == "Ethernet0"
-        assert "SAI_LAG_MEMBER_ATTR_INGRESS_DISABLE" in fvs
-        assert fvs.pop("SAI_LAG_MEMBER_ATTR_INGRESS_DISABLE") == "false"
-        assert "SAI_LAG_MEMBER_ATTR_EGRESS_DISABLE" in fvs
-        assert fvs.pop("SAI_LAG_MEMBER_ATTR_EGRESS_DISABLE") == "false"
-        assert not fvs
-
-        ps = swsscommon.ProducerStateTable(db, "LAG_MEMBER_TABLE")
-        fvs = swsscommon.FieldValuePairs([("status", "disabled")])
-
-        ps.set("PortChannel0001:Ethernet0", fvs)
-
-        time.sleep(1)
-
-        lagmtbl = swsscommon.Table(asicdb, "ASIC_STATE:SAI_OBJECT_TYPE_LAG_MEMBER")
-        lagms = lagmtbl.getKeys()
-        assert len(lagms) == 1
-
-        (status, fvs) = lagmtbl.get(lagms[0])
-        fvs = dict(fvs)
-        assert status
-        assert "SAI_LAG_MEMBER_ATTR_LAG_ID" in fvs
-        assert fvs.pop("SAI_LAG_MEMBER_ATTR_LAG_ID") == lags[0]
-        assert "SAI_LAG_MEMBER_ATTR_PORT_ID" in fvs
-        assert dvs.asicdb.portoidmap[fvs.pop("SAI_LAG_MEMBER_ATTR_PORT_ID")] == "Ethernet0"
-        assert "SAI_LAG_MEMBER_ATTR_INGRESS_DISABLE" in fvs
-        assert fvs.pop("SAI_LAG_MEMBER_ATTR_INGRESS_DISABLE") == "true"
-        assert "SAI_LAG_MEMBER_ATTR_EGRESS_DISABLE" in fvs
-        assert fvs.pop("SAI_LAG_MEMBER_ATTR_EGRESS_DISABLE") == "true"
-        assert not fvs
+        for fv in fvs:
+            if fv[0] == "SAI_LAG_MEMBER_ATTR_LAG_ID":
+                assert fv[1] == lags[0]
+            elif fv[0] == "SAI_LAG_MEMBER_ATTR_PORT_ID":
+                assert dvs.asicdb.portoidmap[fv[1]] == "Ethernet0"
+            else:
+                assert False
 
         # remove port channel member
         ps = swsscommon.ProducerStateTable(db, "LAG_MEMBER_TABLE")