Validate destination port is not LAG (sonic-net#2053)

* Validate destination port is not LAG

* Enhance to support mirroring of source port rx/tx to different destination ports

Author: rupesh-k

config/main.py

@@ -829,18 +829,36 @@ def interface_is_in_portchannel(portchannel_member_table, interface_name):
 
     return False
 
-def interface_has_mirror_config(mirror_table, interface_name):
-    """ Check if port is already configured with mirror config """
-    for _, v in mirror_table.items():
-        if 'src_port' in v and v['src_port'] == interface_name:
+def check_mirror_direction_config(v, direction):
+    """ Check if port is already configured for mirror in same direction """
+    if direction:
+        direction=direction.upper()
+        if ('direction' in v and v['direction'] == 'BOTH') or (direction == 'BOTH'):
             return True
-        if 'dst_port' in v and v['dst_port'] == interface_name:
+        if 'direction' in v and v['direction'] == direction:
             return True
+    else:
+        return True
+
+def interface_has_mirror_config(ctx, mirror_table, dst_port, src_port, direction):
+    """ Check if dst/src port is already configured with mirroring in same direction """
+    for _, v in mirror_table.items():
+        if src_port:
+            for port in src_port.split(","):
+                if 'dst_port' in v and v['dst_port'] == port:
+                    ctx.fail("Error: Source Interface {} already has mirror config".format(port))
+                if 'src_port' in v and re.search(port,v['src_port']):
+                    if check_mirror_direction_config(v, direction):
+                        ctx.fail("Error: Source Interface {} already has mirror config in same direction".format(port))
+        if dst_port:
+            if ('dst_port' in v and v['dst_port'] == dst_port) or ('src_port' in v and re.search(dst_port,v['src_port'])):
+                ctx.fail("Error: Destination Interface {} already has mirror config".format(dst_port))
 
     return False
 
 def validate_mirror_session_config(config_db, session_name, dst_port, src_port, direction):
     """ Check if SPAN mirror-session config is valid """
+    ctx = click.get_current_context()
     if len(config_db.get_entry('MIRROR_SESSION', session_name)) != 0:
         click.echo("Error: {} already exists".format(session_name))
         return False
@@ -851,41 +869,34 @@ def validate_mirror_session_config(config_db, session_name, dst_port, src_port,
 
     if dst_port:
         if not interface_name_is_valid(config_db, dst_port):
-            click.echo("Error: Destination Interface {} is invalid".format(dst_port))
-            return False
+            ctx.fail("Error: Destination Interface {} is invalid".format(dst_port))
+
+        if is_portchannel_present_in_db(config_db, dst_port):
+            ctx.fail("Error: Destination Interface {} is not supported".format(dst_port))
 
         if interface_is_in_vlan(vlan_member_table, dst_port):
-            click.echo("Error: Destination Interface {} has vlan config".format(dst_port))
-            return False
+            ctx.fail("Error: Destination Interface {} has vlan config".format(dst_port))
 
-        if interface_has_mirror_config(mirror_table, dst_port):
-            click.echo("Error: Destination Interface {} already has mirror config".format(dst_port))
-            return False
 
         if interface_is_in_portchannel(portchannel_member_table, dst_port):
-            click.echo("Error: Destination Interface {} has portchannel config".format(dst_port))
-            return False
+            ctx.fail("Error: Destination Interface {} has portchannel config".format(dst_port))
 
         if clicommon.is_port_router_interface(config_db, dst_port):
-            click.echo("Error: Destination Interface {} is a L3 interface".format(dst_port))
-            return False
+            ctx.fail("Error: Destination Interface {} is a L3 interface".format(dst_port))
 
     if src_port:
         for port in src_port.split(","):
             if not interface_name_is_valid(config_db, port):
-                click.echo("Error: Source Interface {} is invalid".format(port))
-                return False
+                ctx.fail("Error: Source Interface {} is invalid".format(port))
             if dst_port and dst_port == port:
-                click.echo("Error: Destination Interface cant be same as Source Interface")
-                return False
-            if interface_has_mirror_config(mirror_table, port):
-                click.echo("Error: Source Interface {} already has mirror config".format(port))
-                return False
+                ctx.fail("Error: Destination Interface cant be same as Source Interface")
+
+    if interface_has_mirror_config(ctx, mirror_table, dst_port, src_port, direction):
+        return False
 
     if direction:
         if direction not in ['rx', 'tx', 'both']:
-            click.echo("Error: Direction {} is invalid".format(direction))
-            return False
+            ctx.fail("Error: Direction {} is invalid".format(direction))
 
     return True
 
@@ -2086,7 +2097,7 @@ def add_span(session_name, dst_port, src_port, direction, queue, policer):
         dst_port = interface_alias_to_name(None, dst_port)
         if dst_port is None:
             click.echo("Error: Destination Interface {} is invalid".format(dst_port))
-            return
+            return False
 
     session_info = {
             "type" : "SPAN",

tests/config_mirror_session_test.py

@@ -164,8 +164,88 @@ def test_mirror_session_span_add():
     assert result.exit_code != 0
     assert ERR_MSG_VALUE_FAILURE in result.stdout
 
+    # Verify invalid dst port
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "Ethern", "Ethernet4", "rx", "100"])
+    assert result.exit_code != 0
+    assert "Error: Destination Interface Ethern is invalid" in result.stdout
+
+    # Verify destination port not have vlan config
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "Ethernet24", "Ethernet4", "rx", "100"])
+    assert result.exit_code != 0
+    assert "Error: Destination Interface Ethernet24 has vlan config" in result.stdout
+
+    # Verify destination port is not part of portchannel
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "Ethernet116", "Ethernet4", "rx", "100"])
+    assert result.exit_code != 0
+    assert "Error: Destination Interface Ethernet116 has portchannel config" in result.stdout
+
+    # Verify destination port not router interface
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "Ethernet0", "Ethernet4", "rx", "100"])
+    assert result.exit_code != 0
+    assert "Error: Destination Interface Ethernet0 is a L3 interface" in result.stdout
+
+    # Verify destination port not Portchannel
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "PortChannel1001"])
+    assert result.exit_code != 0
+    assert "Error: Destination Interface PortChannel1001 is not supported" in result.output
+
+    # Verify source interface is invalid
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "Ethernet52", "Ethern", "rx", "100"])
+    assert result.exit_code != 0
+    assert "Error: Source Interface Ethern is invalid" in result.stdout
+
+    # Verify source interface is not same as destination
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "Ethernet52", "Ethernet52", "rx", "100"])
+    assert result.exit_code != 0
+    assert "Error: Destination Interface cant be same as Source Interface" in result.stdout
+
+    # Verify destination port not have mirror config
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "Ethernet44", "Ethernet56", "rx", "100"])
+    assert result.exit_code != 0
+    assert "Error: Destination Interface Ethernet44 already has mirror config" in result.output
+
+    # Verify source port is not configured as dstport in other session
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "Ethernet52", "Ethernet44", "rx", "100"])
+    assert result.exit_code != 0
+    assert "Error: Source Interface Ethernet44 already has mirror config" in result.output
+
+    # Verify source port is not configured in same direction
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "Ethernet52", "Ethernet8,Ethernet40", "rx", "100"])
+    assert result.exit_code != 0
+    assert "Error: Source Interface Ethernet40 already has mirror config in same direction" in result.output
+
+    # Verify direction is invalid
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "Ethernet52", "Ethernet56", "px", "100"])
+    assert result.exit_code != 0
+    assert "Error: Direction px is invalid" in result.stdout
+
     # Positive case
     with mock.patch('config.main.add_span') as mocked:
+        result = runner.invoke(
+                config.config.commands["mirror_session"].commands["span"].commands["add"],
+                ["test_session", "Ethernet8", "Ethernet4", "tx", "100"])
         result = runner.invoke(
                 config.config.commands["mirror_session"].commands["span"].commands["add"],
                 ["test_session", "Ethernet0", "Ethernet4", "rx", "100"])

tests/mock_tables/config_db.json

@@ -2553,5 +2553,10 @@
     },
     "QUEUE|Ethernet96|6": {
         "scheduler": "[SCHEDULER|scheduler.0]"
+    },
+    "MIRROR_SESSION|test_session_db1": {
+        "dst_port": "Ethernet44",
+        "src_port": "Ethernet40,Ethernet48",
+        "direction": "RX"
     }
 }