Re: [dts] [PATCH] tests: add TestSuite_veb_switch.py

["Liu, Yong" <yong.liu@intel.com>]

Yuan, some comments below.

> -----Original Message-----
> From: dts [mailto:dts-bounces@dpdk.org] On Behalf Of peng,yuan
> Sent: Tuesday, December 06, 2016 3:36 PM
> To: dts@dpdk.org
> Cc: Peng, Yuan
> Subject: [dts] [PATCH] tests: add TestSuite_veb_switch.py
> 
> From: Peng Yuan <yuan.peng@intel.com>
> 
> Signed-off-by: Peng Yuan <yuan.peng@intel.com>
> 
> diff --git a/tests/TestSuite_veb_switch.py b/tests/TestSuite_veb_switch.py
> new file mode 100644
> index 0000000..3492579
> --- /dev/null
> +++ b/tests/TestSuite_veb_switch.py
> @@ -0,0 +1,523 @@
> +# BSD LICENSE
> +#
> +# Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> +# All rights reserved.
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions
> +# are met:
> +#
> +#   * Redistributions of source code must retain the above copyright
> +#     notice, this list of conditions and the following disclaimer.
> +#   * Redistributions in binary form must reproduce the above copyright
> +#     notice, this list of conditions and the following disclaimer in
> +#     the documentation and/or other materials provided with the
> +#     distribution.
> +#   * Neither the name of Intel Corporation nor the names of its
> +#     contributors may be used to endorse or promote products derived
> +#     from this software without specific prior written permission.
> +#
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +"""
> +DPDK Test suite.
> +
> +Test VEB Switch and floating VEB Features by Poll Mode Drivers.
> +"""
> +
> +import re
> +import time
> +
> +from virt_dut import VirtDut
> +from project_dpdk import DPDKdut
> +from dut import Dut
> +from net_device import GetNicObj
> +from net_device import RemoveNicObj

There's no need to import those two functions.

> +from test_case import TestCase
> +from pmd_output import PmdOutput
> +from settings import HEADER_SIZE
> +from packet import Packet
> +from utils import RED
> +
> +
> +
> +    def send_packet(self, vf_mac, itf, tran_type=""):
> +        """
> +        Send 1 packet
> +        """
> +        self.tester.scapy_foreground()
> +        self.tester.scapy_append('sys.path.append("./")')
> +        self.dut.send_expect("start", "testpmd>")
> +        mac = self.dut.get_mac_address(0)
> +
> +        if tran_type == "vlan":
> +            packet = r'sendp([Ether(dst="%s",
> src=get_if_hwaddr("%s"))/Dot1Q(vlan=1)/IP(src="192.168.0.1",
> dst="192.168.0.2")], iface="%s")' % (vf_mac, itf, itf)
> +            self.tester.scapy_append(packet)
> +            self.tester.scapy_execute()
> +            time.sleep(.5)
> +        else:
> +            packet = r'sendp([Ether(dst="%s",
> src=get_if_hwaddr("%s"))/IP(src="192.168.0.1", dst="192.168.0.2")],
> iface="%s")' % (vf_mac, itf, itf)
> +            self.tester.scapy_append(packet)
> +            self.tester.scapy_execute()
> +            time.sleep(.5)
> +
Recommend to use packet module function replace of scapy command. It will be helpful in later efficiency improvement.

> +    def set_up(self):
> +        """
> +        This is to clear up environment before the case run.
> +        """
> +        self.dut.kill_all()
> +
> +    def setup_1pf_ddriver_2vf_env(self, driver='default'):
> +        self.dut.generate_sriov_vfs_by_port(self.used_dut_port, 2,
> driver='igb_uio')
> +        self.sriov_vfs_port =
> self.dut.ports_info[self.used_dut_port]['vfs_port']
> +        self.dut.send_expect("./tools/dpdk-devbind.py -s", "# ", 3)

Remove useless code here, it only waste the time and can't help on anything. 

> +        try:
> +
> +            for port in self.sriov_vfs_port:
> +                self.dut.send_expect("./tools/dpdk-devbind.py -b
> igb_uio %s" % (port.pci), "# ", 3)

Use self.drivername replace of igb_uio, igb_uio may not be the driver required to test. There're other choices like vfio-pci.

> +            self.dut.send_expect("./tools/dpdk-devbind.py -s", "# ", 3)

Remove useless code here, it only waste the time and can't help on anything.

> +            time.sleep(1)
> +            for port_id in self.dut_ports:
> +                if port_id == self.used_dut_port:
> +                    continue
> +                port = self.dut.ports_info[port_id]['port']
> +                port.bind_driver()
> +

Why need to bind other PF device to default kernel driver?
If only want to use some devices, you can use whitelist parameter. 

> +            self.setup_1pf_ddriver_2vf_env_flag = 1
> +        except Exception as e:
> +            self.destroy_1pf_ddriver_2vf_env()
> +            raise Exception(e)
> +
> +    def destroy_1pf_ddriver_2vf_env(self):
> +        self.session_third.send_expect("quit", "# ")
> +        time.sleep(2)
> +        self.session_secondary.send_expect("quit", "# ")
> +        time.sleep(2)
> +        self.dut.send_expect("quit", "# ")
> +        time.sleep(2)
> +        self.dut.destroy_sriov_vfs_by_port(self.used_dut_port)
> +        port = self.dut.ports_info[self.used_dut_port]['port']
> +        port.bind_driver()
> +        self.setup_1pf_ddriver_2vf_env_flag = 0
> +
> +    def setup_1pf_ddriver_1vf_env(self, driver='default'):

Most part of this function look like function setup_1pf_kdriver_2vf_env, please combine these functions and just use parameter to distinguish the difference.

> +
> +    def test_VEB_switching_inter_vfs(self):
> +        """
> +        Kernel PF, then create 2VFs. VFs running dpdk testpmd, send
> traffic to VF1, and set the packet's DEST MAC to VF2, check if VF2 can
> receive the packets. Check Inter          VF-VF MAC switch.
> +    """
> +        self.setup_1pf_kdriver_2vf_env(driver='')
> +        self.dut.send_expect("./x86_64-native-linuxapp-gcc/app/testpmd -c
> 0xf -n 4 --socket-mem 1024,1024 -w %s --file-prefix=test1 -- -i --crc-
> strip --eth-peer=0,%s" % (self.sriov_vfs_port[0].pci, self.vf1_mac),
> "testpmd>", 120)

Please not hard code DPDK target, in test suite you can use self.target.

> +
> +    def tear_down_all(self):
> +        """
> +        Run after each test suite.
> +        """
> +        self.dut.kill_all()
> +        self.dut.close_session(self.session_secondary)
> +        self.dut.close_session(self.session_third)
> +        # Marvin recommended that all the dut ports should be bound to
> igb_uio.
> +        for port_id in self.dut_ports:
> +            port = self.dut.ports_info[port_id]['port']
> +            port.bind_driver(driver='igb_uio')
> +


Only need to used port back.

> +        pass
> --
> 2.5.0