[sub intf] Port object reference count update (#1712)

Signed-off-by: Wenda Ni <wonda.ni@gmail.com>

There are two parts of changes in this pr:
The motivation is that for each Port object instantiated, it should be initialized in PortsOrch::m_port_ref_count. For each Port object to be removed, its m_port_ref_count check should be zero to proceed further. These operations are generic and therefore should also apply to sub port Port object.

Currently, Port object reference count increments
- When a router interface is created on top.
- When an ACL table is bound to Port (i.e., becoming a member of the ACL table group associated with the Port object).

Port object reference count decrements
- When a router interface on top is removed.
- When an ACL table is unbound from Port (i.e, removing membership from the ACL table group associated with the Port object).
Item 1, router interface creation and removal, is of direct relevance to sub port Port object.

The other part of change is the reference count update to parent port Port object when a sub port Port object is added/removed on top. This part is motivated by change in Add reference counting to ports for ACL bindings.

What I did
At sub port Port object instantiation
  - Init sub port Port object reference count
  - Increase parent port Port object reference count
At sub port Port object removal
  - Check sub port Port object reference count drops to zero
  - Decrease parent port Port object reference count
At physical port removal
  - Check physical port Port object reference count drops to zero

How I verified it
vs test extension
Issue parent port removal at APPL_DB level before removing sub port interface. Verify that parent port persists in ASIC_DB until sub interface is removed.
Without the change, extended vs test fails:

orchagent/portsorch.cpp

@@ -649,6 +649,8 @@ bool PortsOrch::getPortByBridgePortId(sai_object_id_t bridge_port_id, Port &port
 
 bool PortsOrch::addSubPort(Port &port, const string &alias, const bool &adminUp, const uint32_t &mtu)
 {
+    SWSS_LOG_ENTER();
+
     size_t found = alias.find(VLAN_SUB_INTERFACE_SEPARATOR);
     if (found == string::npos)
     {
@@ -726,15 +728,19 @@ bool PortsOrch::addSubPort(Port &port, const string &alias, const bool &adminUp,
         }
     }
 
-    parentPort.m_child_ports.insert(p.m_alias);
+    parentPort.m_child_ports.insert(alias);
+    increasePortRefCount(parentPort.m_alias);
 
     m_portList[alias] = p;
+    m_port_ref_count[alias] = 0;
     port = p;
     return true;
 }
 
 bool PortsOrch::removeSubPort(const string &alias)
 {
+    SWSS_LOG_ENTER();
+
     auto it = m_portList.find(alias);
     if (it == m_portList.end())
     {
@@ -749,6 +755,12 @@ bool PortsOrch::removeSubPort(const string &alias)
         return false;
     }
 
+    if (m_port_ref_count[alias] > 0)
+    {
+        SWSS_LOG_ERROR("Unable to remove sub interface %s: ref count %u", alias.c_str(), m_port_ref_count[alias]);
+        return false;
+    }
+
     Port parentPort;
     if (!getPort(port.m_parent_port_id, parentPort))
     {
@@ -759,6 +771,10 @@ bool PortsOrch::removeSubPort(const string &alias)
     {
         SWSS_LOG_WARN("Sub interface %s not associated to parent port %s", alias.c_str(), parentPort.m_alias.c_str());
     }
+    else
+    {
+        decreasePortRefCount(parentPort.m_alias);
+    }
     m_portList[parentPort.m_alias] = parentPort;
 
     m_portList.erase(it);
@@ -2696,6 +2712,13 @@ void PortsOrch::doPortTask(Consumer &consumer)
         }
         else if (op == DEL_COMMAND)
         {
+            if (m_port_ref_count[alias] > 0)
+            {
+                SWSS_LOG_WARN("Unable to remove port %s: ref count %u", alias.c_str(), m_port_ref_count[alias]);
+                it++;
+                continue;
+            }
+
             SWSS_LOG_NOTICE("Deleting Port %s", alias.c_str());
             auto port_id = m_portList[alias].m_port_id;
             auto hif_id = m_portList[alias].m_hif_id;
@@ -3743,6 +3766,7 @@ bool PortsOrch::addVlan(string vlan_alias)
 bool PortsOrch::removeVlan(Port vlan)
 {
     SWSS_LOG_ENTER();
+
     if (m_port_ref_count[vlan.m_alias] > 0)
     {
         SWSS_LOG_ERROR("Failed to remove ref count %d VLAN %s",

tests/test_sub_port_intf.py

@@ -29,6 +29,7 @@
 ASIC_LAG_MEMBER_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_LAG_MEMBER"
 ASIC_HOSTIF_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF"
 ASIC_VIRTUAL_ROUTER_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER"
+ASIC_PORT_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_PORT"
 ASIC_LAG_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_LAG"
 
 ADMIN_STATUS = "admin_status"
@@ -65,6 +66,21 @@ def connect_dbs(self, dvs):
 
         self.default_vrf_oid = self.get_default_vrf_oid()
 
+        phy_port = self.SUB_PORT_INTERFACE_UNDER_TEST.split(VLAN_SUB_INTERFACE_SEPARATOR)[0]
+
+        self.port_fvs = {}
+        self.port_fvs[phy_port] = self.app_db.get_entry(APP_PORT_TABLE_NAME, phy_port)
+
+        self.buf_pg_fvs = {}
+        for key in self.config_db.get_keys("BUFFER_PG"):
+            if phy_port in key:
+                self.buf_pg_fvs[key] = self.config_db.get_entry("BUFFER_PG", key)
+
+        self.buf_q_fvs = {}
+        for key in self.config_db.get_keys("BUFFER_QUEUE"):
+            if phy_port in key:
+                self.buf_q_fvs[key] = self.config_db.get_entry("BUFFER_QUEUE", key)
+
     def get_parent_port_index(self, port_name):
         if port_name.startswith(ETHERNET_PREFIX):
             idx = int(port_name[len(ETHERNET_PREFIX):])
@@ -110,6 +126,26 @@ def create_sub_port_intf_profile(self, sub_port_intf_name, vrf_name=None):
 
         self.config_db.create_entry(CFG_VLAN_SUB_INTF_TABLE_NAME, sub_port_intf_name, fvs)
 
+    def create_phy_port_appl_db(self, dvs, port_name):
+        assert port_name in self.port_fvs
+
+        hostif_cnt = len(self.asic_db.get_keys(ASIC_HOSTIF_TABLE))
+
+        fvs = swsscommon.FieldValuePairs(list(self.port_fvs[port_name].items()))
+        tbl = swsscommon.ProducerStateTable(self.app_db.db_connection, APP_PORT_TABLE_NAME)
+        tbl.set(port_name, fvs)
+
+        self.asic_db.wait_for_n_keys(ASIC_HOSTIF_TABLE, hostif_cnt + 1)
+        dvs.asicdb._generate_oid_to_interface_mapping()
+
+        for key, fvs in self.buf_pg_fvs.items():
+            if port_name in key:
+                self.config_db.create_entry("BUFFER_PG", key, fvs)
+
+        for key, fvs in self.buf_q_fvs.items():
+            if port_name in key:
+                self.config_db.create_entry("BUFFER_QUEUE", key, fvs)
+
     def add_lag_members(self, lag, members):
         fvs = {"NULL": "NULL"}
 
@@ -162,6 +198,23 @@ def remove_vrf(self, vrf_name):
     def check_vrf_removal(self, vrf_oid):
         self.asic_db.wait_for_deleted_keys(ASIC_VIRTUAL_ROUTER_TABLE, [vrf_oid])
 
+    def remove_parent_port_appl_db(self, port_name):
+        if port_name.startswith(ETHERNET_PREFIX):
+            tbl_name = APP_PORT_TABLE_NAME
+
+            for key in self.buf_pg_fvs:
+                if port_name in key:
+                    self.config_db.delete_entry("BUFFER_PG", key)
+
+            for key in self.buf_q_fvs:
+                if port_name in key:
+                    self.config_db.delete_entry("BUFFER_QUEUE", key)
+        else:
+            assert port_name.startswith(LAG_PREFIX)
+            tbl_name = APP_LAG_TABLE_NAME
+        tbl = swsscommon.ProducerStateTable(self.app_db.db_connection, tbl_name)
+        tbl._del(port_name)
+
     def remove_lag(self, lag):
         self.config_db.delete_entry(CFG_LAG_TABLE_NAME, lag)
 
@@ -734,21 +787,24 @@ def _test_sub_port_intf_removal(self, dvs, sub_port_intf_name, removal_seq_test=
             state_tbl_name = STATE_PORT_TABLE_NAME
             phy_ports = [parent_port]
             parent_port_oid = dvs.asicdb.portnamemap[parent_port]
+            asic_tbl_name = ASIC_PORT_TABLE
         else:
             assert parent_port.startswith(LAG_PREFIX)
             state_tbl_name = STATE_LAG_TABLE_NAME
             phy_ports = self.LAG_MEMBERS_UNDER_TEST
             old_lag_oids = self.get_oids(ASIC_LAG_TABLE)
+            asic_tbl_name = ASIC_LAG_TABLE
 
         vrf_oid = self.default_vrf_oid
         old_rif_oids = self.get_oids(ASIC_RIF_TABLE)
 
         self.set_parent_port_admin_status(dvs, parent_port, "up")
         if parent_port.startswith(LAG_PREFIX):
             parent_port_oid = self.get_newly_created_oid(ASIC_LAG_TABLE, old_lag_oids)
-            # Add lag members to test physical port host interface vlan tag attribute
-            self.add_lag_members(parent_port, self.LAG_MEMBERS_UNDER_TEST)
-            self.asic_db.wait_for_n_keys(ASIC_LAG_MEMBER_TABLE, len(self.LAG_MEMBERS_UNDER_TEST))
+            if removal_seq_test == False:
+                # Add lag members to test physical port host interface vlan tag attribute
+                self.add_lag_members(parent_port, self.LAG_MEMBERS_UNDER_TEST)
+                self.asic_db.wait_for_n_keys(ASIC_LAG_MEMBER_TABLE, len(self.LAG_MEMBERS_UNDER_TEST))
         if vrf_name:
             self.create_vrf(vrf_name)
             vrf_oid = self.get_newly_created_oid(ASIC_VIRTUAL_ROUTER_TABLE, [vrf_oid])
@@ -779,6 +835,14 @@ def _test_sub_port_intf_removal(self, dvs, sub_port_intf_name, removal_seq_test=
         self.check_sub_port_intf_fvs(self.app_db, APP_INTF_TABLE_NAME, sub_port_intf_name, fv_dict)
 
         if removal_seq_test == True:
+            obj_cnt = len(self.asic_db.get_keys(asic_tbl_name))
+            # Remove parent port before removing sub port interface
+            self.remove_parent_port_appl_db(parent_port)
+            time.sleep(2)
+
+            # Verify parent port persists in ASIC_DB
+            self.asic_db.wait_for_n_keys(asic_tbl_name, obj_cnt)
+
             # Remove a sub port interface before removing sub port interface IP addresses
             self.remove_sub_port_intf_profile(sub_port_intf_name)
             time.sleep(2)
@@ -851,23 +915,32 @@ def _test_sub_port_intf_removal(self, dvs, sub_port_intf_name, removal_seq_test=
         # Verify that sub port router interface entry is removed from ASIC_DB
         self.check_sub_port_intf_key_removal(self.asic_db, ASIC_RIF_TABLE, rif_oid)
 
-        # Verify physical port host interface vlan tag attribute
-        fv_dict = {
-            "SAI_HOSTIF_ATTR_VLAN_TAG": "SAI_HOSTIF_VLAN_TAG_STRIP",
-        }
-        for phy_port in phy_ports:
-            hostif_oid = dvs.asicdb.hostifnamemap[phy_port]
-            self.check_sub_port_intf_fvs(self.asic_db, ASIC_HOSTIF_TABLE, hostif_oid, fv_dict)
+        if removal_seq_test == True:
+            # Verify that parent port is removed from ASIC_DB
+            self.asic_db.wait_for_n_keys(asic_tbl_name, obj_cnt - 1)
+        else:
+            # Verify physical port host interface vlan tag attribute
+            fv_dict = {
+                "SAI_HOSTIF_ATTR_VLAN_TAG": "SAI_HOSTIF_VLAN_TAG_STRIP",
+            }
+            for phy_port in phy_ports:
+                hostif_oid = dvs.asicdb.hostifnamemap[phy_port]
+                self.check_sub_port_intf_fvs(self.asic_db, ASIC_HOSTIF_TABLE, hostif_oid, fv_dict)
 
         # Remove vrf if created
         if vrf_name:
             self.remove_vrf(vrf_name)
             self.asic_db.wait_for_n_keys(ASIC_VIRTUAL_ROUTER_TABLE, 1)
 
-        if parent_port.startswith(LAG_PREFIX):
-            # Remove lag members from lag parent port
-            self.remove_lag_members(parent_port, self.LAG_MEMBERS_UNDER_TEST)
-            self.asic_db.wait_for_n_keys(ASIC_LAG_MEMBER_TABLE, 0)
+        if parent_port.startswith(ETHERNET_PREFIX):
+            if removal_seq_test == True:
+                self.create_phy_port_appl_db(dvs, parent_port)
+                self.asic_db.wait_for_n_keys(asic_tbl_name, obj_cnt)
+        else:
+            if removal_seq_test == False:
+                # Remove lag members from lag parent port
+                self.remove_lag_members(parent_port, self.LAG_MEMBERS_UNDER_TEST)
+                self.asic_db.wait_for_n_keys(ASIC_LAG_MEMBER_TABLE, 0)
 
             # Remove lag
             self.remove_lag(parent_port)