Code-review comments addressed

sonic-net · May 14, 2020 · ed7a3a5 · ed7a3a5
1 parent aa72b67
commit ed7a3a5
Show file tree

Hide file tree

Showing 5 changed files with 117 additions and 82 deletions.
diff --git a/orchagent/fdborch.cpp b/orchagent/fdborch.cpp
@@ -159,8 +159,7 @@ void FdbOrch::update(sai_fdb_event_t type, const sai_fdb_entry_t* entry, sai_obj
         break;
 
     case SAI_FDB_EVENT_FLUSHED:
-        if ((bridge_port_id == SAI_NULL_OBJECT_ID && entry->bv_id == SAI_NULL_OBJECT_ID)
-            || (bridge_port_id && entry->bv_id == SAI_NULL_OBJECT_ID))
+        if (entry->bv_id == SAI_NULL_OBJECT_ID)
         {
             for (auto itr = m_entries.begin(); itr != m_entries.end();)
             {
@@ -175,21 +174,32 @@ void FdbOrch::update(sai_fdb_event_t type, const sai_fdb_entry_t* entry, sai_obj
                 update.add = false;
                 itr++;
 
-                storeFdbEntryState(update);
+                if (SAI_NULL_OBJECT_ID != bridge_port_id)
+                {
+                    if (!m_portsOrch->getPortByBridgePortId(bridge_port_id, update.port))
+                    {
+                        SWSS_LOG_ERROR("Failed to get port by bridge port ID 0x%" PRIx64, bridge_port_id);
+                        return;
+                    }
+                    SWSS_LOG_DEBUG("FDB flush: mac: %s  port: %s bridge port OID: 0x%" PRIx64 ".",
+                                   update.entry.mac.to_string().c_str(),
+                                   update.port.m_alias.c_str(),
+                                   bridge_port_id);
+                }
+                else
+                {
+                    SWSS_LOG_DEBUG("FDB flush: mac: %s",
+                                   update.entry.mac.to_string().c_str());
+                }
 
-                SWSS_LOG_DEBUG("FdbOrch notification: mac %s was removed", update.entry.mac.to_string().c_str());
+                storeFdbEntryState(update);
 
                 for (auto observer: m_observers)
                 {
                     observer->update(SUBJECT_TYPE_FDB_CHANGE, &update);
                 }
             }
         }
-        else if (bridge_port_id && entry->bv_id == SAI_NULL_OBJECT_ID)
-        {
-            /*this is a placeholder for flush port fdb case, not supported yet.*/
-            SWSS_LOG_ERROR("FdbOrch notification: not supported flush port fdb action, port_id = 0x%" PRIx64 ", bv_id = 0x%" PRIx64 ".", bridge_port_id, entry->bv_id);
-        }
         else if (bridge_port_id == SAI_NULL_OBJECT_ID && entry->bv_id != SAI_NULL_OBJECT_ID)
         {
             /*this is a placeholder for flush vlan fdb case, not supported yet.*/

diff --git a/tests/conftest.py b/tests/conftest.py
@@ -16,6 +16,7 @@
 from dvslib import dvs_database as dvs_db
 from dvslib import dvs_acl
 from dvslib import dvs_vlan
+from dvslib import dvs_lag
 
 def ensure_system(cmd):
     (rc, output) = commands.getstatusoutput(cmd)
@@ -982,14 +983,6 @@ def get_state_db(self):
 
         return self.state_db
 
-    def get_dvs_acl(self):
-        if not self.dvs_acl:
-            self.dvs_acl = dvs_acl.DVSAcl(self.get_asic_db(),
-                                          self.get_config_db(),
-                                          self.get_state_db(),
-                                          self.get_counters_db())
-        return self.dvs_acl
-
 @pytest.yield_fixture(scope="module")
 def dvs(request):
     name = request.config.getoption("--dvsname")
@@ -1010,21 +1003,26 @@ def testlog(request, dvs):
     yield testlog
     dvs.runcmd("logger === finish test %s ===" % request.node.name)
 
-@pytest.yield_fixture(scope="class")
-def dvs_vlan_manager(request, dvs):
-    request.cls.dvs_vlan = dvs_vlan.DVSVlan(dvs.get_asic_db(),
-                                            dvs.get_config_db(),
-                                            dvs.get_state_db(),
-                                            dvs.get_counters_db(),
-                                            dvs.get_app_db())
 
+################# DVSLIB module manager fixtures #############################
 @pytest.yield_fixture(scope="class")
 def dvs_acl_manager(request, dvs):
     request.cls.dvs_acl = dvs_acl.DVSAcl(dvs.get_asic_db(),
                                          dvs.get_config_db(),
                                          dvs.get_state_db(),
                                          dvs.get_counters_db())
 
+@pytest.yield_fixture(scope="class")
+def dvs_lag_manager(request, dvs):
+    request.cls.dvs_lag = dvs_lag.DVSLag(dvs.get_config_db())
+
+@pytest.yield_fixture(scope="class")
+def dvs_vlan_manager(request, dvs):
+    request.cls.dvs_vlan = dvs_vlan.DVSVlan(dvs.get_asic_db(),
+                                            dvs.get_config_db(),
+                                            dvs.get_state_db(),
+                                            dvs.get_counters_db(),
+                                            dvs.get_app_db())
 ##################### DPB fixtures ###########################################
 @pytest.yield_fixture(scope="module")
 def create_dpb_config_file(dvs):

diff --git a/tests/dvslib/dvs_lag.py b/tests/dvslib/dvs_lag.py
@@ -0,0 +1,21 @@
+class DVSLag(object):
+    def __init__(self, cdb):
+        self.config_db = cdb
+
+    def create_port_channel(self, lag_id, admin_status="up", mtu="1500"):
+        lag = "PortChannel{}".format(lag_id)
+        lag_entry = {"admin_status": admin_status, "mtu": mtu}
+        self.config_db.create_entry("PORTCHANNEL", lag, lag_entry)
+
+    def remove_port_channel(self, lag_id):
+        lag = "PortChannel{}".format(lag_id)
+        self.config_db.delete_entry("PORTCHANNEL", lag)
+
+    def create_port_channel_member(self, lag_id, interface):
+        member = "PortChannel{}|{}".format(lag_id, interface)
+        member_entry = {"NULL": "NULL"}
+        self.config_db.create_entry("PORTCHANNEL_MEMBER", member, member_entry)
+
+    def remove_port_channel_member(self, lag_id, interface):
+        member = "PortChannel{}|{}".format(lag_id, interface)
+        self.config_db.delete_entry("PORTCHANNEL_MEMBER", member)