Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mirror] Detach session dst ip from route orch LPM calculation regardless of session status at session CONFIG DB removal #1800

Merged
merged 4 commits into from
Jul 2, 2021

Conversation

wendani
Copy link
Contributor

@wendani wendani commented Jun 23, 2021

Why I did it
Mirror session dst ip is attached to route orch LPM calculation at mirror session CONFIG DB creation to find a mirror monitor port and update it dynamically at run time.

Accordingly, at mirror session CONFIG DB removal, dst ip should be detached unconditionally from route orch, regardless of current session status, i,e., regardless of whether mirror session is created in sai/asic or not.

Note in current processing logic that dst ip is detached only if current session status is active, i.e., mirror session is created in sai/asic.

What I did
Fix route orch detachment in the removal path described above.

How I verified it

Vs test extension to test_MirrorAddRemove, test_MirrorToVlanAddRemove, and test_MirrorToLagAddRemove, to check route orch attach and detach message in syslog at mirror session creation and removal, respectively.

Extended vs test fails before the fix.

Details if related

vs test failure message

Mirror monitor port via RIF over physical port
=========================================================================== FAILURES ============================================================================
________________________________________________________________ TestMirror.test_MirrorAddRemove ________________________________________________________________

self = <test_mirror.TestMirror object at 0x7faae61eb710>, dvs = <conftest.DockerVirtualSwitch object at 0x7faae61eb438>
testlog = <function testlog at 0x7faae653b378>

    def test_MirrorAddRemove(self, dvs, testlog):
        """
        This test covers the basic mirror session creation and removal operations
        Operation flow:
        1. Create mirror session
           The session remains inactive because no nexthop/neighbor exists
        2. Bring up port; assign IP; create neighbor; create route
           The session remains inactive until the route is created
        3. Remove route; remove neighbor; remove IP; bring down port
           The session becomes inactive again till the end
        4. Remove miror session
        """
        self.setup_db(dvs)
    
        session = "TEST_SESSION"
    
        marker = dvs.add_log_marker()
        # create mirror session
        self.create_mirror_session(session, "1.1.1.1", "2.2.2.2", "0x6558", "8", "100", "0")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
        self.check_syslog(dvs, marker, "Attached next hop observer .* for destination IP 2.2.2.2", 1)
    
        # bring up Ethernet16
        self.set_interface_status(dvs, "Ethernet16", "up")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # add IP address to Ethernet16
        self.add_ip_address("Ethernet16", "10.0.0.0/31")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # add neighbor to Ethernet16
        self.add_neighbor("Ethernet16", "10.0.0.1", "02:04:06:08:10:12")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # add route to mirror destination via 10.0.0.1
        self.add_route(dvs, "2.2.2.2", "10.0.0.1")
        assert self.get_mirror_session_state(session)["status"] == "active"
        assert self.get_mirror_session_state(session)["monitor_port"] == "Ethernet16"
        assert self.get_mirror_session_state(session)["dst_mac"] == "02:04:06:08:10:12"
        assert self.get_mirror_session_state(session)["route_prefix"] == "2.2.2.2/32"
    
        # check asic database
        tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_MIRROR_SESSION")
        mirror_entries = tbl.getKeys()
        assert len(mirror_entries) == 1
    
        (status, fvs) = tbl.get(mirror_entries[0])
        assert status == True
        assert len(fvs) == 11
        for fv in fvs:
            if fv[0] == "SAI_MIRROR_SESSION_ATTR_MONITOR_PORT":
                assert dvs.asicdb.portoidmap[fv[1]] == "Ethernet16"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_TYPE":
                assert fv[1] == "SAI_MIRROR_SESSION_TYPE_ENHANCED_REMOTE"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_ERSPAN_ENCAPSULATION_TYPE":
                assert fv[1] == "SAI_ERSPAN_ENCAPSULATION_TYPE_MIRROR_L3_GRE_TUNNEL"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION":
                assert fv[1] == "4"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_TOS":
                assert fv[1] == "32"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_TTL":
                assert fv[1] == "100"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_SRC_IP_ADDRESS":
                assert fv[1] == "1.1.1.1"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_IP_ADDRESS":
                assert fv[1] == "2.2.2.2"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_SRC_MAC_ADDRESS":
                assert fv[1] == dvs.runcmd("bash -c \"ip link show eth0 | grep ether | awk '{print $2}'\"")[1].strip().upper()
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS":
                assert fv[1] == "02:04:06:08:10:12"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_GRE_PROTOCOL_TYPE":
                assert fv[1] == "25944" # 0x6558
            else:
                assert False
    
        # remove route
        self.remove_route(dvs, "2.2.2.2")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # remove neighbor
        self.remove_neighbor("Ethernet16", "10.0.0.1")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # remove IP address
        self.remove_ip_address("Ethernet16", "10.0.0.0/31")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # bring down Ethernet16
        self.set_interface_status(dvs, "Ethernet16", "down")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        marker = dvs.add_log_marker()
        # remove mirror session
        self.remove_mirror_session(session)
>       self.check_syslog(dvs, marker, "Detached next hop observer for destination IP 2.2.2.2", 1)

test_mirror.py:204: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <test_mirror.TestMirror object at 0x7faae61eb710>, dvs = <conftest.DockerVirtualSwitch object at 0x7faae61eb438>
marker = '=== start marker 2021-06-23T09:28:26.424399 ===', log = 'Detached next hop observer for destination IP 2.2.2.2', expected_cnt = 1

    def check_syslog(self, dvs, marker, log, expected_cnt):
        (ec, out) = dvs.runcmd(['sh', '-c', "awk \'/%s/,ENDFILE {print;}\' /var/log/syslog | grep \'%s\' | wc -l" % (marker, log)])
>       assert out.strip() == str(expected_cnt)
E       AssertionError: assert '0' == '1'
E         - 1
E         + 0

test_mirror.py:108: AssertionError
==================================================================== short test summary info ====================================================================
FAILED test_mirror.py::TestMirror::test_MirrorAddRemove - AssertionError: assert '0' == '1'
================================================================= 1 failed in 71.03s (0:01:11) ==================================================================
Mirror monitor port via vlan RIF
=========================================================================== FAILURES ============================================================================
_____________________________________________________________ TestMirror.test_MirrorToVlanAddRemove _____________________________________________________________

self = <test_mirror.TestMirror object at 0x7ff400017ba8>, dvs = <conftest.DockerVirtualSwitch object at 0x7ff40001b588>
testlog = <function testlog at 0x7ff400367378>

    @pytest.mark.skipif(StrictVersion(distro.linux_distribution()[1]) <= StrictVersion('8.9'), reason="Debian 8.9 or before has no support")
    def test_MirrorToVlanAddRemove(self, dvs, testlog):
        """
        This test covers basic mirror session creation and removal operation
        with destination port sits in a VLAN
        Opeartion flow:
        1. Create mirror session
        2. Create VLAN; assign IP; create neighbor; create FDB
           The session should be up only at this time.
        3. Remove FDB; remove neighbor; remove IP; remove VLAN
        4. Remove mirror session
        """
        self.setup_db(dvs)
    
        session = "TEST_SESSION"
    
        marker = dvs.add_log_marker()
        # create mirror session
        self.create_mirror_session(session, "5.5.5.5", "6.6.6.6", "0x6558", "8", "100", "0")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
        self.check_syslog(dvs, marker, "Attached next hop observer .* for destination IP 6.6.6.6", 1)
    
        # create vlan; create vlan member
        self.create_vlan(dvs, "6")
        self.create_vlan_member("6", "Ethernet4")
    
        # bring up vlan and member
        self.set_interface_status(dvs, "Vlan6", "up")
        self.set_interface_status(dvs, "Ethernet4", "up")
    
        # add ip address to vlan 6
        self.add_ip_address("Vlan6", "6.6.6.0/24")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # create neighbor to vlan 6
        self.add_neighbor("Vlan6", "6.6.6.6", "66:66:66:66:66:66")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # create fdb entry to ethernet4
        self.create_fdb("6", "66-66-66-66-66-66", "Ethernet4")
        assert self.get_mirror_session_state(session)["status"] == "active"
    
        # check asic database
        tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_MIRROR_SESSION")
        mirror_entries = tbl.getKeys()
        assert len(mirror_entries) == 1
    
        (status, fvs) = tbl.get(mirror_entries[0])
        assert status == True
        assert len(fvs) == 16
        for fv in fvs:
            if fv[0] == "SAI_MIRROR_SESSION_ATTR_MONITOR_PORT":
                assert dvs.asicdb.portoidmap[fv[1]] == "Ethernet4"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_TYPE":
                assert fv[1] == "SAI_MIRROR_SESSION_TYPE_ENHANCED_REMOTE"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_ERSPAN_ENCAPSULATION_TYPE":
                assert fv[1] == "SAI_ERSPAN_ENCAPSULATION_TYPE_MIRROR_L3_GRE_TUNNEL"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION":
                assert fv[1] == "4"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_TOS":
                assert fv[1] == "32"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_TTL":
                assert fv[1] == "100"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_SRC_IP_ADDRESS":
                assert fv[1] == "5.5.5.5"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_IP_ADDRESS":
                assert fv[1] == "6.6.6.6"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_SRC_MAC_ADDRESS":
                assert fv[1] == dvs.runcmd("bash -c \"ip link show eth0 | grep ether | awk '{print $2}'\"")[1].strip().upper()
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS":
                assert fv[1] == "66:66:66:66:66:66"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_GRE_PROTOCOL_TYPE":
                assert fv[1] == "25944" # 0x6558
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_VLAN_HEADER_VALID":
                assert fv[1] == "true"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_VLAN_TPID":
                assert fv[1] == "33024"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_VLAN_ID":
                assert fv[1] == "6"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_VLAN_PRI":
                assert fv[1] == "0"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_VLAN_CFI":
                assert fv[1] == "0"
            else:
                assert False
    
        # remove fdb entry
        self.remove_fdb("6", "66-66-66-66-66-66")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # remove neighbor
        self.remove_neighbor("Vlan6", "6.6.6.6")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # remove ip address
        self.remove_ip_address("Vlan6", "6.6.6.0/24")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # bring down vlan and member
        self.set_interface_status(dvs, "Ethernet4", "down")
        self.set_interface_status(dvs, "Vlan6", "down")
    
        # remove vlan member; remove vlan
        self.remove_vlan_member("6", "Ethernet4")
        self.remove_vlan("6")
    
        marker = dvs.add_log_marker()
        # remove mirror session
        self.remove_mirror_session(session)
>       self.check_syslog(dvs, marker, "Detached next hop observer for destination IP 6.6.6.6", 1)

test_mirror.py:354: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <test_mirror.TestMirror object at 0x7ff400017ba8>, dvs = <conftest.DockerVirtualSwitch object at 0x7ff40001b588>
marker = '=== start marker 2021-06-23T19:15:33.267526 ===', log = 'Detached next hop observer for destination IP 6.6.6.6', expected_cnt = 1

    def check_syslog(self, dvs, marker, log, expected_cnt):
        (ec, out) = dvs.runcmd(['sh', '-c', "awk \'/%s/,ENDFILE {print;}\' /var/log/syslog | grep \'%s\' | wc -l" % (marker, log)])
>       assert out.strip() == str(expected_cnt)
E       AssertionError: assert '0' == '1'
E         - 1
E         + 0

test_mirror.py:108: AssertionError
==================================================================== short test summary info ====================================================================
FAILED test_mirror.py::TestMirror::test_MirrorToVlanAddRemove - AssertionError: assert '0' == '1'
================================================================= 1 failed in 76.70s (0:01:16) ==================================================================
Mirror monitor port via RIF over LAG
=========================================================================== FAILURES ============================================================================
_____________________________________________________________ TestMirror.test_MirrorToLagAddRemove ______________________________________________________________

self = <test_mirror.TestMirror object at 0x7fc1f8d08d68>, dvs = <conftest.DockerVirtualSwitch object at 0x7fc1f8d0c438>
testlog = <function testlog at 0x7fc1f9058378>

    def test_MirrorToLagAddRemove(self, dvs, testlog):
        """
        This test covers basic mirror session creation and removal operations
        with destination port sits in a LAG
        Operation flow:
        1. Create mirror sesion
        2. Create LAG; assign IP; create directly connected neighbor
           The session shoudl be up only at this time.
        3. Remove neighbor; remove IP; remove LAG
        4. Remove mirror session
    
        """
        self.setup_db(dvs)
    
        session = "TEST_SESSION"
    
        marker = dvs.add_log_marker()
        # create mirror session
        self.create_mirror_session(session, "10.10.10.10", "11.11.11.11", "0x6558", "8", "100", "0")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
        self.check_syslog(dvs, marker, "Attached next hop observer .* for destination IP 11.11.11.11", 1)
    
        # create port channel; create port channel member
        self.create_port_channel(dvs, "008")
        self.create_port_channel_member("008", "Ethernet88")
    
        # bring up port channel and port channel member
        self.set_interface_status(dvs, "PortChannel008", "up")
        self.set_interface_status(dvs, "Ethernet88", "up")
    
        # add ip address to port channel 008
        self.add_ip_address("PortChannel008", "11.11.11.0/24")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # create neighbor to port channel 008
        self.add_neighbor("PortChannel008", "11.11.11.11", "88:88:88:88:88:88")
        assert self.get_mirror_session_state(session)["status"] == "active"
    
        # check asic database
        tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_MIRROR_SESSION")
        assert len(tbl.getKeys()) == 1
    
        (status, fvs) = tbl.get(tbl.getKeys()[0])
        assert status == True
        for fv in fvs:
            if fv[0] == "SAI_MIRROR_SESSION_ATTR_MONITOR_PORT":
                assert dvs.asicdb.portoidmap[fv[1]] == "Ethernet88"
            elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS":
                assert fv[1] == "88:88:88:88:88:88"
    
        # remove neighbor
        self.remove_neighbor("PortChannel008", "11.11.11.11")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # remove ip address
        self.remove_ip_address("PortChannel008", "11.11.11.0/24")
        assert self.get_mirror_session_state(session)["status"] == "inactive"
    
        # bring down port channel and port channel member
        self.set_interface_status(dvs, "PortChannel008", "down")
        self.set_interface_status(dvs, "Ethernet88", "down")
    
        # remove port channel member; remove port channel
        self.remove_port_channel_member("008", "Ethernet88")
        self.remove_port_channel(dvs, "008")
    
        marker = dvs.add_log_marker()
        # remove mirror session
        self.remove_mirror_session(session)
>       self.check_syslog(dvs, marker, "Detached next hop observer for destination IP 11.11.11.11", 1)

test_mirror.py:455: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <test_mirror.TestMirror object at 0x7fc1f8d08d68>, dvs = <conftest.DockerVirtualSwitch object at 0x7fc1f8d0c438>
marker = '=== start marker 2021-06-23T19:22:26.840753 ===', log = 'Detached next hop observer for destination IP 11.11.11.11', expected_cnt = 1

    def check_syslog(self, dvs, marker, log, expected_cnt):
        (ec, out) = dvs.runcmd(['sh', '-c', "awk \'/%s/,ENDFILE {print;}\' /var/log/syslog | grep \'%s\' | wc -l" % (marker, log)])
>       assert out.strip() == str(expected_cnt)
E       AssertionError: assert '0' == '1'
E         - 1
E         + 0

test_mirror.py:108: AssertionError
==================================================================== short test summary info ====================================================================
FAILED test_mirror.py::TestMirror::test_MirrorToLagAddRemove - AssertionError: assert '0' == '1'
================================================================= 1 failed in 75.17s (0:01:15) ==================================================================

session status

Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
@wendani wendani requested a review from prsunny as a code owner June 23, 2021 21:50
Copy link
Collaborator

@prsunny prsunny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@prsunny prsunny merged commit 1367646 into sonic-net:master Jul 2, 2021
qiluo-msft pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Jul 5, 2021
[flex-counters] Delay flex counters stats init for faster boot time (sonic-net/sonic-swss#1803)
[mirror] Detach session dst ip from route orch LPM calculation regardless of session status at session CONFIG DB removal (sonic-net/sonic-swss#1800)
[Dynamic Buffer Calc] Support dynamic buffer calculation on top of port auto negotiation (sonic-net/sonic-swss#1762)
[neighorch] VOQ encap index change handling (sonic-net/sonic-swss#1729)
[neighorch] Mac for voq neighbors in VS platforms (sonic-net/sonic-swss#1724)
[acl mirror action] Mirror session ref count fix at acl rule attachment (sonic-net/sonic-swss#1761)
judyjoseph pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Aug 7, 2021
[flex-counters] Delay flex counters stats init for faster boot time (sonic-net/sonic-swss#1803)
[mirror] Detach session dst ip from route orch LPM calculation regardless of session status at session CONFIG DB removal (sonic-net/sonic-swss#1800)
[Dynamic Buffer Calc] Support dynamic buffer calculation on top of port auto negotiation (sonic-net/sonic-swss#1762)
[neighorch] VOQ encap index change handling (sonic-net/sonic-swss#1729)
[neighorch] Mac for voq neighbors in VS platforms (sonic-net/sonic-swss#1724)
[acl mirror action] Mirror session ref count fix at acl rule attachment (sonic-net/sonic-swss#1761)
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
[flex-counters] Delay flex counters stats init for faster boot time (sonic-net/sonic-swss#1803)
[mirror] Detach session dst ip from route orch LPM calculation regardless of session status at session CONFIG DB removal (sonic-net/sonic-swss#1800)
[Dynamic Buffer Calc] Support dynamic buffer calculation on top of port auto negotiation (sonic-net/sonic-swss#1762)
[neighorch] VOQ encap index change handling (sonic-net/sonic-swss#1729)
[neighorch] Mac for voq neighbors in VS platforms (sonic-net/sonic-swss#1724)
[acl mirror action] Mirror session ref count fix at acl rule attachment (sonic-net/sonic-swss#1761)
judyjoseph pushed a commit that referenced this pull request Aug 10, 2021
…less of session status at session CONFIG DB removal (#1800)

* Detach dst ip from routeorch lpm calculation regardless of mirror session status
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
…less of session status at session CONFIG DB removal (sonic-net#1800)

* Detach dst ip from routeorch lpm calculation regardless of mirror session status
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants