[202111] [portchannel] Added ACL/PBH binding checks to the port before getting added to portchannel (#2186)

- What I did
Backport #2151 to 202111
The change to handle it in oA is already added.

When this check is not performed when adding the config, the portchannel configuration will be inconsistent b/w Kernel and ASIC

- How I did it
Utilize the match infra to implement methods to check for ACL/PBH bindings to a port

- How to verify it
Unit tests

Signed-off-by: Vivek Reddy Karri <[email protected]>

Author: vivekrnv

config/main.py

@@ -28,6 +28,7 @@
 from utilities_common import bgp_util
 import utilities_common.cli as clicommon
 from utilities_common.general import load_db_config
+from utilities_common.helper import get_port_pbh_binding, get_port_acl_binding
 
 from .utils import log
 
@@ -1733,14 +1734,15 @@ def synchronous_mode(sync_mode):
 @click.option('-n', '--namespace', help='Namespace name',
              required=True if multi_asic.is_multi_asic() else False, type=click.Choice(multi_asic.get_namespace_list()))
 @click.pass_context
-def portchannel(ctx, namespace):
+@clicommon.pass_db
+def portchannel(db, ctx, namespace):
     # Set namespace to default_namespace if it is None.
     if namespace is None:
         namespace = DEFAULT_NAMESPACE
 
     config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=str(namespace))
     config_db.connect()
-    ctx.obj = {'db': config_db, 'namespace': str(namespace)}
+    ctx.obj = {'db': config_db, 'namespace': str(namespace), 'db_wrap': db}
 
 @portchannel.command('add')
 @click.argument('portchannel_name', metavar='<portchannel_name>', required=True)
@@ -1870,6 +1872,22 @@ def add_portchannel_member(ctx, portchannel_name, port_name):
         if port_tpid != DEFAULT_TPID:
             ctx.fail("Port TPID of {}: {} is not at default 0x8100".format(port_name, port_tpid))
 
+    # Don't allow a port to be a member of portchannel if already has ACL bindings
+    try:
+        acl_bindings = get_port_acl_binding(ctx.obj['db_wrap'], port_name, ctx.obj['namespace'])
+        if acl_bindings:
+            ctx.fail("Port {} is already bound to following ACL_TABLES: {}".format(port_name, acl_bindings))
+    except Exception as e:
+        ctx.fail(str(e))
+
+    # Don't allow a port to be a member of portchannel if already has PBH bindings
+    try:
+        pbh_bindings = get_port_pbh_binding(ctx.obj['db_wrap'], port_name, DEFAULT_NAMESPACE)
+        if pbh_bindings:
+            ctx.fail("Port {} is already bound to following PBH_TABLES: {}".format(port_name, pbh_bindings))
+    except Exception as e:
+        ctx.fail(str(e))
+
     db.set_entry('PORTCHANNEL_MEMBER', (portchannel_name, port_name),
             {'NULL': 'NULL'})
 

dump/match_helper.py

@@ -1,6 +1,23 @@
 from dump.match_infra import MatchRequest
 from dump.helper import handle_multiple_keys_matched_error
 
+# Return dict helper methods
+
+def check_error(ret):
+    """ Check if the match request failed """
+    if ret["error"]:
+        return True, ret["error"]
+    else:
+        return False, ""
+
+def get_matched_keys(ret):
+    """ Return Matched Keys """
+    failed, err_str = check_error(ret)
+    if not failed:
+        return ret["keys"], ""
+    else:
+        return [], err_str
+
 # Port Helper Methods
 
 def fetch_port_oid(match_engine, port_name, ns):

dump/match_infra.py

@@ -250,6 +250,10 @@ def clear(self, namespace=None):
         elif namespace in self.cache:
             del self.cache[namespace]
 
+    def fill(self, ns, conn, connected_to):
+        """ Update internal cache """
+        self.cache[ns] = {'conn': conn, 'connected_to': set(connected_to)}
+
 
 class MatchEngine:
     """

tests/mock_tables/config_db.json

@@ -532,6 +532,10 @@
         "services@": "SSH",
         "type": "CTRLPLANE"
     },
+    "PBH_TABLE|pbh_table1": {
+        "description": "NVGRE",
+        "interface_list@": "Ethernet8,Ethernet60"
+    },
     "VLAN|Vlan1000": {
         "dhcp_servers@": "192.0.0.1,192.0.0.2,192.0.0.3,192.0.0.4",
         "vlanid": "1000"

tests/portchannel_test.py

@@ -157,6 +157,28 @@ def test_delete_portchannel_member_which_is_member_of_another_po(self):
         assert result.exit_code != 0
         assert "Error: Ethernet116 is not a member of portchannel PortChannel1001" in result.output
 
+    def test_add_portchannel_member_with_acl_bindngs(self):
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb, 'db_wrap':db, 'namespace':''}
+
+        result = runner.invoke(config.config.commands["portchannel"].commands["member"].commands["add"], ["PortChannel0002", "Ethernet100"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Error: Port Ethernet100 is already bound to following ACL_TABLES:" in result.output
+
+    def test_add_portchannel_member_with_pbh_bindngs(self):
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb, 'db_wrap':db, 'namespace':''}
+
+        result = runner.invoke(config.config.commands["portchannel"].commands["member"].commands["add"], ["PortChannel0002", "Ethernet60"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Error: Port Ethernet60 is already bound to following PBH_TABLES:" in result.output
+
     @classmethod
     def teardown_class(cls):
         os.environ['UTILITIES_UNIT_TESTING'] = "0"

utilities_common/db.py

@@ -15,15 +15,15 @@ def __init__(self):
         self.db = SonicV2Connector(host="127.0.0.1")
 
         # Skip connecting to chassis databases in line cards
-        db_list = list(self.db.get_db_list())
+        self.db_list = list(self.db.get_db_list())
         if not device_info.is_supervisor():
             try:
-                db_list.remove('CHASSIS_APP_DB')
-                db_list.remove('CHASSIS_STATE_DB')
+                self.db_list.remove('CHASSIS_APP_DB')
+                self.db_list.remove('CHASSIS_STATE_DB')
             except Exception:
                 pass
 
-        for db_id in db_list:
+        for db_id in self.db_list:
             self.db.connect(db_id)
 
         self.cfgdb_clients[constants.DEFAULT_NAMESPACE] = self.cfgdb

utilities_common/helper.py

@@ -0,0 +1,66 @@
+from dump.match_infra import MatchEngine, MatchRequest, ConnectionPool
+from dump.match_helper import get_matched_keys
+from .db import Db
+
+def get_port_acl_binding(db_wrap, port, ns):
+    """
+    Verify if the port is not bound to any ACL Table
+    
+    Args:
+        db_wrap: utilities_common.Db() object
+        port: Iface name
+        ns: namespace
+
+    Returns:
+        list: ACL_TABLE names if found, 
+                otherwise empty
+    """ 
+    ACL = "ACL_TABLE" # Table to look for port bindings
+    if not isinstance(db_wrap, Db):
+        raise Exception("db_wrap object is not of type utilities_common.Db")
+
+    conn_pool = ConnectionPool()
+    conn_pool.fill(ns, db_wrap.db_clients[ns], db_wrap.db_list)
+    m_engine = MatchEngine(conn_pool)
+    req = MatchRequest(db="CONFIG_DB",
+                      table=ACL,
+                      key_pattern="*",
+                      field="ports@",
+                      value=port,
+                      ns=ns,
+                      match_entire_list=False)
+    ret = m_engine.fetch(req)
+    acl_tables, _ = get_matched_keys(ret)
+    return acl_tables
+
+
+def get_port_pbh_binding(db_wrap, port, ns):
+    """
+    Verify if the port is not bound to any PBH Table
+    
+    Args:
+        db_wrap: Db() object
+        port: Iface name
+        ns: namespace
+
+    Returns:
+        list: PBH_TABLE names if found, 
+                otherwise empty
+    """ 
+    PBH = "PBH_TABLE" # Table to look for port bindings
+    if not isinstance(db_wrap, Db):
+        raise Exception("db_wrap object is not of type utilities_common.Db")
+
+    conn_pool = ConnectionPool()
+    conn_pool.fill(ns, db_wrap.db_clients[ns], db_wrap.db_list)
+    m_engine = MatchEngine(conn_pool)
+    req = MatchRequest(db="CONFIG_DB",
+                      table=PBH,
+                      key_pattern="*",
+                      field="interface_list@",
+                      value=port,
+                      ns=ns,
+                      match_entire_list=False)
+    ret = m_engine.fetch(req)
+    pbh_tables, _ = get_matched_keys(ret)
+    return pbh_tables