[config] Add vlan member only after checking RIF is not present

config/config_mgmt.py

@@ -14,7 +14,7 @@
 
     # SONiC specific imports
     import sonic_yang
-    from swsssdk import port_util
+    from utilities_common import port_util
     from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector
 
     # Using load_source to 'import /usr/local/bin/sonic-cfggen as sonic_cfggen'

config/vlan.py

@@ -3,6 +3,7 @@
 
 from time import sleep
 from .utils import log
+from utilities_common import port_util
 
 #
 # 'vlan' group ('config vlan ...')
@@ -134,16 +135,21 @@ def add_vlan_member(db, vid, port, untagged):
     if clicommon.is_port_vlan_member(db.cfgdb, port, vlan):
         ctx.fail("{} is already a member of {}".format(port, vlan))
 
-    if clicommon.is_valid_port(db.cfgdb, port):
-        is_port = True
-    elif clicommon.is_valid_portchannel(db.cfgdb, port):
-        is_port = False
-    else:
+    if not clicommon.is_valid_port(db.cfgdb, port) and \
+       not clicommon.is_valid_portchannel(db.cfgdb, port):
         ctx.fail("{} does not exist".format(port))
 
-    if (is_port and clicommon.is_port_router_interface(db.cfgdb, port)) or \
-       (not is_port and clicommon.is_pc_router_interface(db.cfgdb, port)):
-        ctx.fail("{} is a router interface!".format(port))
+    if_name_map, \
+    if_oid_map = port_util.get_interface_oid_map(db.db)
+    rif_port_oid_map = port_util.get_rif_port_map(db.db)
+
+    if port not in if_name_map:
+        ctx.fail("Can not find {} in OID map".format(port))
+
+    port_oid = if_name_map[port]
+    for rif in rif_port_oid_map.keys():
+        if rif_port_oid_map[rif] == port_oid:
+            ctx.fail("{} is a router interface! Check assigned L3 addresses, a related static routes etc.".format(port))
 
     if (clicommon.interface_is_untagged_member(db.cfgdb, port) and untagged):
         ctx.fail("{} is already untagged member!".format(port))

scripts/fdbshow

@@ -31,7 +31,7 @@ import json
 import sys
 
 from natsort import natsorted
-from swsssdk import port_util
+from utilities_common import port_util
 from swsscommon.swsscommon import SonicV2Connector
 from tabulate import tabulate
 

scripts/nbrshow

@@ -33,7 +33,7 @@ import subprocess
 import re
 
 from natsort import natsorted
-from swsssdk import port_util
+from utilities_common import port_util
 from swsscommon.swsscommon import SonicV2Connector
 from tabulate import tabulate
 

tests/vlan_input/asic_db.json

@@ -0,0 +1,16 @@
+{
+    "ASIC_STATE:SAI_OBJECT_TYPE_ROUTER_INTERFACE:oid:0x6000000000595": {
+        "SAI_ROUTER_INTERFACE_ATTR_VIRTUAL_ROUTER_ID": "oid:0x3000000000002",
+        "SAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS": "DE:AD:BE:EF:CA:FE",
+        "SAI_ROUTER_INTERFACE_ATTR_TYPE": "SAI_ROUTER_INTERFACE_TYPE_PORT",
+        "SAI_ROUTER_INTERFACE_ATTR_PORT_ID": "oid:0x60000000005a1",
+        "SAI_ROUTER_INTERFACE_ATTR_MTU": "9100"
+    },
+    "ASIC_STATE:SAI_OBJECT_TYPE_ROUTER_INTERFACE:oid:0x60000000006a6": {
+        "SAI_ROUTER_INTERFACE_ATTR_VIRTUAL_ROUTER_ID": "oid:0x3000000000003",
+        "SAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS": "DE:AD:BE:EF:CA:FE",
+        "SAI_ROUTER_INTERFACE_ATTR_TYPE": "SAI_ROUTER_INTERFACE_TYPE_PORT",
+        "SAI_ROUTER_INTERFACE_ATTR_PORT_ID": "oid:0x1000000000019",
+        "SAI_ROUTER_INTERFACE_ATTR_MTU": "9100"
+    }
+}

tests/vlan_input/counters_db.json

@@ -0,0 +1,16 @@
+{
+    "COUNTERS_PORT_NAME_MAP": {
+        "Ethernet4": "oid:0x1000000000012",
+        "Ethernet8": "oid:0x1000000000013",
+        "Ethernet12": "oid:0x1000000000014",
+        "Ethernet16": "oid:0x1000000000015",
+        "Ethernet20": "oid:0x1000000000016",
+        "Ethernet24": "oid:0x1000000000017",
+        "Ethernet28": "oid:0x1000000000018",
+        "Ethernet32": "oid:0x1000000000019"
+    },
+    "COUNTERS_LAG_NAME_MAP": {
+        "PortChannel0001": "oid:0x60000000005a1",
+        "PortChannel1001": "oid:0x60000000007c9"
+    }
+}

tests/vlan_test.py

@@ -7,6 +7,10 @@
 import config.main as config
 import show.main as show
 from utilities_common.db import Db
+from .mock_tables import dbconnector
+
+test_path = os.path.dirname(os.path.abspath(__file__))
+mock_db_path = os.path.join(test_path, "vlan_input")
 
 show_vlan_brief_output="""\
 +-----------+-----------------+------------+----------------+-----------------------+-------------+
@@ -170,6 +174,10 @@ class TestVlan(object):
     @classmethod
     def setup_class(cls):
         os.environ['UTILITIES_UNIT_TESTING'] = "1"
+        jsonfile = os.path.join(mock_db_path, 'counters_db')
+        dbconnector.dedicated_dbs['COUNTERS_DB'] = jsonfile
+        jsonfile = os.path.join(mock_db_path, 'asic_db')
+        dbconnector.dedicated_dbs['ASIC_DB'] = jsonfile
         print("SETUP")
 
     def test_show_vlan(self):
@@ -319,6 +327,28 @@ def test_config_vlan_add_portchannel_member(self):
         assert result.exit_code == 0
         assert result.output == show_vlan_brief_with_portchannel_output
 
+    def test_config_vlan_add_port_member_not_in_cntr_map(self):
+        runner = CliRunner()
+        db = Db()
+
+        result = runner.invoke(config.config.commands["vlan"].commands["member"].commands["add"], \
+				["1000", "Ethernet36", "--untagged"], obj=db)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Error: Can not find Ethernet36 in OID map" in result.output
+
+    def test_config_vlan_add_rif_port_member(self):
+        runner = CliRunner()
+        db = Db()
+
+        result = runner.invoke(config.config.commands["vlan"].commands["member"].commands["add"], \
+				["1000", "Ethernet32", "--untagged"], obj=db)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Error: Ethernet32 is a router interface! Check assigned L3 addresses, a related static routes etc." in result.output
+
     def test_config_vlan_add_rif_portchannel_member(self):
         runner = CliRunner()
         db = Db()
@@ -328,7 +358,7 @@ def test_config_vlan_add_rif_portchannel_member(self):
         print(result.exit_code)
         print(result.output)
         assert result.exit_code != 0
-        assert "Error: PortChannel0001 is a router interface!" in result.output
+        assert "Error: PortChannel0001 is a router interface! Check assigned L3 addresses, a related static routes etc." in result.output
 
     def test_config_vlan_del_vlan(self):
         runner = CliRunner()
@@ -676,4 +706,6 @@ def test_config_set_router_port_on_member_interface(self):
     @classmethod
     def teardown_class(cls):
         os.environ['UTILITIES_UNIT_TESTING'] = "0"
+        dbconnector.dedicated_dbs['COUNTERS_DB'] = None
+        dbconnector.dedicated_dbs['ASIC_DB'] = None
         print("TEARDOWN")

utilities_common/port_util.py

@@ -0,0 +1,140 @@
+"""
+Bridge/Port mapping utility library.
+"""
+import re
+
+
+SONIC_ETHERNET_RE_PATTERN = "^Ethernet(\d+)$"
+"""
+Ethernet-BP refers to BackPlane interfaces
+in multi-asic platform.
+"""
+SONIC_ETHERNET_BP_RE_PATTERN = "^Ethernet-BP(\d+)$"
+SONIC_VLAN_RE_PATTERN = "^Vlan(\d+)$"
+SONIC_PORTCHANNEL_RE_PATTERN = "^PortChannel(\d+)$"
+SONIC_MGMT_PORT_RE_PATTERN = "^eth(\d+)$"
+
+
+class BaseIdx:
+    ethernet_base_idx = 1
+    vlan_interface_base_idx = 2000
+    ethernet_bp_base_idx = 9000
+    portchannel_base_idx = 1000
+    mgmt_port_base_idx = 10000
+
+
+def get_index_from_str(if_name):
+    """
+    OIDs are 1-based, interfaces are 0-based, return the 1-based index
+    Ethernet N = N + 1
+    Vlan N = N + 2000
+    Ethernet_BP N = N + 9000
+    PortChannel N = N + 1000
+    eth N = N + 10000
+    """
+    patterns = {
+        SONIC_ETHERNET_RE_PATTERN: BaseIdx.ethernet_base_idx,
+        SONIC_ETHERNET_BP_RE_PATTERN: BaseIdx.ethernet_bp_base_idx,
+        SONIC_VLAN_RE_PATTERN: BaseIdx.vlan_interface_base_idx,
+        SONIC_PORTCHANNEL_RE_PATTERN: BaseIdx.portchannel_base_idx,
+        SONIC_MGMT_PORT_RE_PATTERN: BaseIdx.mgmt_port_base_idx
+    }
+
+    for pattern, baseidx in patterns.items():
+        match = re.match(pattern, if_name)
+        if match:
+            return int(match.group(1)) + baseidx
+
+def get_interface_oid_map(db):
+    """
+        Get the Interface names from Counters DB
+    """
+    db.connect('COUNTERS_DB')
+    if_name_map = db.get_all('COUNTERS_DB', 'COUNTERS_PORT_NAME_MAP', blocking=True)
+    if_lag_name_map = db.get_all('COUNTERS_DB', 'COUNTERS_LAG_NAME_MAP', blocking=True)
+    if_name_map.update(if_lag_name_map)
+
+    oid_pfx = len("oid:0x")
+    if_name_map = {if_name: sai_oid[oid_pfx:] for if_name, sai_oid in if_name_map.items()}
+    if_id_map = {sai_oid: if_name for if_name, sai_oid in if_name_map.items()
+                 # only map the interface if it's a style understood to be a SONiC interface.
+                 if get_index_from_str(if_name) is not None}
+
+    return if_name_map, if_id_map
+
+def get_bridge_port_map(db):
+    """
+        Get the Bridge port mapping from ASIC DB
+    """
+    db.connect('ASIC_DB')
+    br_port_str = db.keys('ASIC_DB', "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT:*")
+    if not br_port_str:
+        return {}
+
+    if_br_oid_map = {}
+    offset = len("ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT:")
+    oid_pfx = len("oid:0x")
+    for br_s in br_port_str:
+        # Example output: ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT:oid:0x3a000000000616
+        br_port_id = br_s[(offset + oid_pfx):]
+        ent = db.get_all('ASIC_DB', br_s, blocking=True)
+        if "SAI_BRIDGE_PORT_ATTR_PORT_ID" in ent:
+            port_id = ent["SAI_BRIDGE_PORT_ATTR_PORT_ID"][oid_pfx:]
+            if_br_oid_map[br_port_id] = port_id
+
+    return if_br_oid_map
+
+def get_vlan_id_from_bvid(db, bvid):
+    """
+        Get the Vlan Id from Bridge Vlan Object
+    """
+    db.connect('ASIC_DB')
+    vlan_obj = db.keys('ASIC_DB', str("ASIC_STATE:SAI_OBJECT_TYPE_VLAN:" + bvid))
+    vlan_entry = db.get_all('ASIC_DB', vlan_obj[0], blocking=True)
+    vlan_id = None
+    if "SAI_VLAN_ATTR_VLAN_ID" in vlan_entry:
+        vlan_id = vlan_entry["SAI_VLAN_ATTR_VLAN_ID"]
+
+    return vlan_id
+
+def get_rif_port_map(db):
+    """
+        Get the RIF port mapping from ASIC DB
+    """
+    db.connect('ASIC_DB')
+    rif_keys_str = db.keys('ASIC_DB', "ASIC_STATE:SAI_OBJECT_TYPE_ROUTER_INTERFACE:*")
+    if not rif_keys_str:
+        return {}
+
+    rif_port_oid_map = {}
+    for rif_s in rif_keys_str:
+        rif_id = rif_s[len("ASIC_STATE:SAI_OBJECT_TYPE_ROUTER_INTERFACE:oid:0x"):]
+        ent = db.get_all('ASIC_DB', rif_s, blocking=True)
+        if "SAI_ROUTER_INTERFACE_ATTR_PORT_ID" in ent:
+            port_id = ent["SAI_ROUTER_INTERFACE_ATTR_PORT_ID"].lstrip("oid:0x")
+            rif_port_oid_map[rif_id] = port_id
+
+    return rif_port_oid_map
+
+def get_vlan_interface_oid_map(db):
+    """
+        Get Vlan Interface names and sai oids
+    """
+    db.connect('COUNTERS_DB')
+    rif_name_map = db.get_all('COUNTERS_DB', 'COUNTERS_RIF_NAME_MAP', blocking=True)
+    rif_type_name_map = db.get_all('COUNTERS_DB', 'COUNTERS_RIF_TYPE_MAP', blocking=True)
+
+    if not rif_name_map or not rif_type_name_map:
+        return {}
+
+    oid_pfx = len("oid:0x")
+    vlan_if_name_map = {}
+
+    for if_name, sai_oid in rif_name_map.items():
+        # Check if RIF is l3 vlan interface
+        if rif_type_name_map[sai_oid] in 'SAI_ROUTER_INTERFACE_TYPE_VLAN':
+            # Check if interface name is in style understood to be a SONiC interface
+            if get_index_from_str(if_name):
+                vlan_if_name_map[sai_oid[oid_pfx:]] = if_name
+
+    return vlan_if_name_map