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

[sub intf] Validate parent port role #1784

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

Conversation

wendani
Copy link
Contributor

@wendani wendani commented Jun 13, 2021

Why I did it
Fix #1616

What I did
Check the role of parent port, and drop further processing if the parent port setting does not qualify a valid sub port interface use case. Specifically, we will not instantiate a Port object for sub port, which is the stepping stone for sub port router interface creation, if parent port role check hits the following cases:

  1. parent physical port is a member of vlan, issue reported in [Sub-ports] SONiC allows adding of sub-ports, if parent port of sub-port is added to VLAN #1616

RIF type sub port -- physical port -- Vlan

  1. parent lag is a member of vlan

RIF type sub port -- LAG -- Vlan

  1. parent physical port is a member of lag

RIF type sub port -- physical port -- LAG

How I verified it
New vs test test_sub_port_intf_parent_misconfig, crafted around the steps in #1616

In the case setting above, load sub port interface profile into DB (CONFIG_DB or APPL_DB), and verify that sub port router interface object is not created in ASIC DB.

Before the change, vs test fails.

Details if related
May need a separate PR to prevent the same incorrect config case from getting into Linux kernel.

vs test failure message

parent physical port being a member of vlan
=========================================================================== FAILURES ============================================================================
______________________________________________________ TestSubPortIntf.test_sub_port_intf_parent_misconfig ______________________________________________________

self = <test_sub_port_intf.TestSubPortIntf object at 0x7f9a60159a58>, dvs = <conftest.DockerVirtualSwitch object at 0x7f9a603766a0>

    def test_sub_port_intf_parent_misconfig(self, dvs):
        self.connect_dbs(dvs)
    
        #self._test_sub_port_intf_parent_misconfig(dvs, self.SUB_PORT_INTERFACE_UNDER_TEST, self.LAG_UNDER_TEST)
>       self._test_sub_port_intf_parent_misconfig(dvs, self.SUB_PORT_INTERFACE_UNDER_TEST, self.VLAN_UNDER_TEST)

test_sub_port_intf.py:548: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <test_sub_port_intf.TestSubPortIntf object at 0x7f9a60159a58>, dvs = <conftest.DockerVirtualSwitch object at 0x7f9a603766a0>
sub_port_intf_name = 'Ethernet64.10', grand_port = 'Vlan1000', vrf_name = None

    def _test_sub_port_intf_parent_misconfig(self, dvs, sub_port_intf_name, grand_port, vrf_name=None):
        substrs = sub_port_intf_name.split(VLAN_SUB_INTERFACE_SEPARATOR)
        parent_port = substrs[0]
    
        vrf_oid = self.default_vrf_oid
        rif_cnt = len(self.get_oids(ASIC_RIF_TABLE))
    
        self.set_parent_port_admin_status(dvs, parent_port, UP)
        if vrf_name:
            self.create_vrf(vrf_name)
            vrf_oid = self.get_newly_created_oid(ASIC_VIRTUAL_ROUTER_TABLE, [vrf_oid])
    
        if grand_port.startswith(LAG_PREFIX):
            assert parent_port.startswith(ETHERNET_PREFIX)
            phy_ports = [parent_port]
            # Create lag
            self.set_parent_port_admin_status(dvs, grand_port, UP)
            # Add parent physical port to lag
            self.add_lag_members(grand_port, phy_ports)
            self.asic_db.wait_for_n_keys(ASIC_LAG_MEMBER_TABLE, 1)
        else:
            assert grand_port.startswith(VLAN_PREFIX)
            # Create vlan
            vlan_cnt = len(self.asic_db.get_keys(ASIC_VLAN_TABLE))
            vlan_id = int(grand_port[len(VLAN_PREFIX):])
            dvs.create_vlan("{}".format(vlan_id))
            vlan_cnt += 1
            self.asic_db.wait_for_n_keys(ASIC_VLAN_TABLE, vlan_cnt)
            # Add parent port to vlan
            vlan_member_cnt = len(self.asic_db.get_keys(ASIC_VLAN_MEMBER_TABLE))
            dvs.create_vlan_member("{}".format(vlan_id), parent_port)
            vlan_member_cnt += 1
            self.asic_db.wait_for_n_keys(ASIC_VLAN_MEMBER_TABLE, vlan_member_cnt)
    
        # Test injecting sub port interface profile directly to APPL_DB
        self.create_sub_port_intf_profile_appl_db(sub_port_intf_name, UP, vrf_name)
        time.sleep(2)
        # Verify no sub port router interface created in ASIC_DB
>       assert rif_cnt == len(self.get_oids(ASIC_RIF_TABLE))
E       assert 1 == 2
E         +1
E         -2

test_sub_port_intf.py:502: AssertionError
==================================================================== short test summary info ====================================================================
FAILED test_sub_port_intf.py::TestSubPortIntf::test_sub_port_intf_parent_misconfig - assert 1 == 2
================================================================= 1 failed in 67.07s (0:01:07) ==================================================================
parent lag being a member of vlan
=========================================================================== FAILURES ============================================================================
______________________________________________________ TestSubPortIntf.test_sub_port_intf_parent_misconfig ______________________________________________________

self = <test_sub_port_intf.TestSubPortIntf object at 0x7f04dd480828>, dvs = <conftest.DockerVirtualSwitch object at 0x7f04dd65a080>

    def test_sub_port_intf_parent_misconfig(self, dvs):
        self.connect_dbs(dvs)
    
        #self._test_sub_port_intf_parent_misconfig(dvs, self.SUB_PORT_INTERFACE_UNDER_TEST, self.LAG_UNDER_TEST)
        #self._test_sub_port_intf_parent_misconfig(dvs, self.SUB_PORT_INTERFACE_UNDER_TEST, self.VLAN_UNDER_TEST)
>       self._test_sub_port_intf_parent_misconfig(dvs, self.LAG_SUB_PORT_INTERFACE_UNDER_TEST, self.VLAN_UNDER_TEST)

test_sub_port_intf.py:549: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <test_sub_port_intf.TestSubPortIntf object at 0x7f04dd480828>, dvs = <conftest.DockerVirtualSwitch object at 0x7f04dd65a080>
sub_port_intf_name = 'PortChannel1.20', grand_port = 'Vlan1000', vrf_name = None

    def _test_sub_port_intf_parent_misconfig(self, dvs, sub_port_intf_name, grand_port, vrf_name=None):
        substrs = sub_port_intf_name.split(VLAN_SUB_INTERFACE_SEPARATOR)
        parent_port = substrs[0]
    
        vrf_oid = self.default_vrf_oid
        rif_cnt = len(self.get_oids(ASIC_RIF_TABLE))
    
        self.set_parent_port_admin_status(dvs, parent_port, UP)
        if vrf_name:
            self.create_vrf(vrf_name)
            vrf_oid = self.get_newly_created_oid(ASIC_VIRTUAL_ROUTER_TABLE, [vrf_oid])
    
        if grand_port.startswith(LAG_PREFIX):
            assert parent_port.startswith(ETHERNET_PREFIX)
            phy_ports = [parent_port]
            # Create lag
            self.set_parent_port_admin_status(dvs, grand_port, UP)
            # Add parent physical port to lag
            self.add_lag_members(grand_port, phy_ports)
            self.asic_db.wait_for_n_keys(ASIC_LAG_MEMBER_TABLE, 1)
        else:
            assert grand_port.startswith(VLAN_PREFIX)
            # Create vlan
            vlan_cnt = len(self.asic_db.get_keys(ASIC_VLAN_TABLE))
            vlan_id = int(grand_port[len(VLAN_PREFIX):])
            dvs.create_vlan("{}".format(vlan_id))
            vlan_cnt += 1
            self.asic_db.wait_for_n_keys(ASIC_VLAN_TABLE, vlan_cnt)
            # Add parent port to vlan
            vlan_member_cnt = len(self.asic_db.get_keys(ASIC_VLAN_MEMBER_TABLE))
            dvs.create_vlan_member("{}".format(vlan_id), parent_port)
            vlan_member_cnt += 1
            self.asic_db.wait_for_n_keys(ASIC_VLAN_MEMBER_TABLE, vlan_member_cnt)
    
        # Test injecting sub port interface profile directly to APPL_DB
        self.create_sub_port_intf_profile_appl_db(sub_port_intf_name, UP, vrf_name)
        time.sleep(2)
        # Verify no sub port router interface created in ASIC_DB
>       assert rif_cnt == len(self.get_oids(ASIC_RIF_TABLE))
E       assert 1 == 2
E         +1
E         -2

test_sub_port_intf.py:502: AssertionError
==================================================================== short test summary info ====================================================================
FAILED test_sub_port_intf.py::TestSubPortIntf::test_sub_port_intf_parent_misconfig - assert 1 == 2
================================================================= 1 failed in 65.57s (0:01:05) ==================================================================
parent physical port being a member of lag
=========================================================================== FAILURES ============================================================================
______________________________________________________ TestSubPortIntf.test_sub_port_intf_parent_misconfig ______________________________________________________

self = <test_sub_port_intf.TestSubPortIntf object at 0x7f6ae7335cc0>, dvs = <conftest.DockerVirtualSwitch object at 0x7f6ae731ed30>

    def test_sub_port_intf_parent_misconfig(self, dvs):
        self.connect_dbs(dvs)
    
>       self._test_sub_port_intf_parent_misconfig(dvs, self.SUB_PORT_INTERFACE_UNDER_TEST, self.LAG_UNDER_TEST)

test_sub_port_intf.py:547: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <test_sub_port_intf.TestSubPortIntf object at 0x7f6ae7335cc0>, dvs = <conftest.DockerVirtualSwitch object at 0x7f6ae731ed30>
sub_port_intf_name = 'Ethernet64.10', grand_port = 'PortChannel1', vrf_name = None

    def _test_sub_port_intf_parent_misconfig(self, dvs, sub_port_intf_name, grand_port, vrf_name=None):
        substrs = sub_port_intf_name.split(VLAN_SUB_INTERFACE_SEPARATOR)
        parent_port = substrs[0]
    
        vrf_oid = self.default_vrf_oid
        rif_cnt = len(self.get_oids(ASIC_RIF_TABLE))
    
        self.set_parent_port_admin_status(dvs, parent_port, UP)
        if vrf_name:
            self.create_vrf(vrf_name)
            vrf_oid = self.get_newly_created_oid(ASIC_VIRTUAL_ROUTER_TABLE, [vrf_oid])
    
        if grand_port.startswith(LAG_PREFIX):
            assert parent_port.startswith(ETHERNET_PREFIX)
            phy_ports = [parent_port]
            # Create lag
            self.set_parent_port_admin_status(dvs, grand_port, UP)
            # Add parent physical port to lag
            self.add_lag_members(grand_port, phy_ports)
            self.asic_db.wait_for_n_keys(ASIC_LAG_MEMBER_TABLE, 1)
        else:
            assert grand_port.startswith(VLAN_PREFIX)
            # Create vlan
            vlan_cnt = len(self.asic_db.get_keys(ASIC_VLAN_TABLE))
            vlan_id = int(grand_port[len(VLAN_PREFIX):])
            dvs.create_vlan("{}".format(vlan_id))
            vlan_cnt += 1
            self.asic_db.wait_for_n_keys(ASIC_VLAN_TABLE, vlan_cnt)
            # Add parent port to vlan
            vlan_member_cnt = len(self.asic_db.get_keys(ASIC_VLAN_MEMBER_TABLE))
            dvs.create_vlan_member("{}".format(vlan_id), parent_port)
            vlan_member_cnt += 1
            self.asic_db.wait_for_n_keys(ASIC_VLAN_MEMBER_TABLE, vlan_member_cnt)
    
        # Test injecting sub port interface profile directly to APPL_DB
        self.create_sub_port_intf_profile_appl_db(sub_port_intf_name, UP, vrf_name)
        time.sleep(2)
        # Verify no sub port router interface created in ASIC_DB
>       assert rif_cnt == len(self.get_oids(ASIC_RIF_TABLE))
E       assert 1 == 2
E         +1
E         -2

test_sub_port_intf.py:502: AssertionError
==================================================================== short test summary info ====================================================================
FAILED test_sub_port_intf.py::TestSubPortIntf::test_sub_port_intf_parent_misconfig - assert 1 == 2
================================================================= 1 failed in 66.89s (0:01:06) ==================================================================

wendani added 9 commits June 10, 2021 02:35
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>
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>
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
@wendani wendani changed the title [sub intf] Check parent port role [sub intf] Validate parent port role Jun 14, 2021
@prsunny prsunny requested a review from neethajohn June 14, 2021 16:09
@wendani wendani requested a review from prsunny as a code owner July 3, 2021 18:26
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
…ic-net#1784)

Fixes sonic-net/sonic-buildimage#8323 and added unit tests for all ecn queue configs. This regression was caused because the current ecn unit tests didn't cover the queue configs.

Signed-off-by: Neetha John <nejo@microsoft.com>

How to verify it
Ran tests/ecn_test.py and all cases passed
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.

[Sub-ports] SONiC allows adding of sub-ports, if parent port of sub-port is added to VLAN
1 participant