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

[fdb & vstest/mirror] Populate FdbEntry port_name in APPL_DB SET path #1827

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

wendani
Copy link
Contributor

@wendani wendani commented Jul 20, 2021

Why I did it

In setting fdb entry via APPL_DB, which is used by warm-reboot and vs test, restore populating FdbEntry port_name, the operation of which was introduced in #1242, but removed (by mistake) in #1275.

Due to the absence of port_name, the corresponding entry is not notified at port oper status change for (synchronous) neighbor flush (to NeighborOrch) nor for (asynchronous) FDB_CHANGE update (to MirrorOrch), the latter of which matters if the port serves as a mirror monitor port.

Missing port_name for FdbEntry created from APPL_DB SET path

Jul 19 05:41:31.403733 9faa323d761d NOTICE #orchagent: :- update: Fdb entry vlan oid: 0x260000000005f6, mac: 36:51:05:aa:c8:1d, dst port: Ethernet4
Jul 19 05:41:31.403736 9faa323d761d NOTICE #orchagent: :- update: Fdb entry vlan oid: 0x260000000005f6, mac: 66:66:66:66:66:66, dst port:
Jul 19 05:41:31.403738 9faa323d761d NOTICE #orchagent: :- update: Fdb entry vlan oid: 0x260000000005f6, mac: d6:c9:fe:7a:94:29, dst port: Ethernet4

What I did

  1. Extend vs test coverage for mirror monitor port in directly connected vlan subnet:

    • vlan member port oper status change
    • fdb flush all from APPL_DB
    • fdb flush bridge port from APPL_DB
  2. Fix space in syslog message

Jul 19 00:12:33.216334 9faa323d761d INFO #orchagent: :- update: FDB event:3, MAC: 00:00:00:00:00:00 , BVID: 0x0 ,                    bridge port ID: 0x3a0000000005f7.
Jul 19 00:12:33.216345 9faa323d761d INFO #orchagent: :- update: FDB Flush event received: [ 00:00:00:00:00:00 , 0x0 ],                        bridge port ID: 0x3a0000000005f7.


Jul 19 01:30:45.186820 9faa323d761d INFO #orchagent: :- notifyObserversFDBFlush: Adding MAC learnt on [ port:Ethernet4 , bvid:0x260000000005f6]                           to ARP flush
Jul 19 01:30:45.186827 9faa323d761d INFO #orchagent: :- notifyObserversFDBFlush: Adding MAC learnt on [ port:Ethernet4 , bvid:0x260000000005f6]                           to ARP flush

How I verified it

vs test

Details if related

  • In [DPB/VLAN] Add test VS cases and fix FDB flush issues  #1242, port oper status change on vlan member port triggers fdb flush on that port, which further triggers neighbor flush. An asynchronous fdb event (FLUSHED event in the vs case) will be notified from sai/asic, and triggers update to observers. If the port serves as a mirror monitor port, we expect mirror session to be removed from sai/asic.

  • FdbEntry port_name does not participate in key equivalence comparison as dicated by FdbEntry::operator==()

  • vs test failure log before the change

=========================================================================== FAILURES ============================================================================
_____________________________________________________________ TestMirror.test_MirrorToVlanAddRemove _____________________________________________________________

self = <test_mirror.TestMirror object at 0x7f74777169b0>, dvs = <conftest.DockerVirtualSwitch object at 0x7f747b9737f0>
testlog = <function testlog at 0x7f7477747378>

    @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"
    
        dvs.runcmd("swssloglevel -l INFO -c orchagent")
        # 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
    
        # test port oper status down that triggers fdb flush on port, which further triggers neighbor flush
        self.set_port_oper_status(dvs, "Ethernet4", "down")
>       assert self.get_mirror_session_state(session)["status"] == "inactive"
E       AssertionError: assert 'active' == 'inactive'
E         - inactive
E         ? --
E         + active

test_mirror.py:350: AssertionError
==================================================================== short test summary info ====================================================================
FAILED test_mirror.py::TestMirror::test_MirrorToVlanAddRemove - AssertionError: assert 'active' == 'inactive'
================================================================= 1 failed in 72.40s (0:01:12) ==================================================================

@wendani wendani requested a review from prsunny as a code owner July 20, 2021 01:58
@wendani wendani changed the title [fdb & vs test/mirror] Populate FdbEntry port_name in APPL_DB SET path [fdb & vstest/mirror] Populate FdbEntry port_name in APPL_DB SET path Jul 20, 2021
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.

1 participant