Re: [dts] [PATCH V1] test: add vhost/virtio pvp performance test based on qemu

["Yao, Lei A" <lei.a.yao@intel.com>]

Thanks. Some update in the comment.

BRs
Lei

> -----Original Message-----
> From: Liu, Yong
> Sent: Wednesday, December 21, 2016 10:11 AM
> To: Yao, Lei A <lei.a.yao@intel.com>; dts@dpdk.org
> Cc: Yao, Lei A <lei.a.yao@intel.com>
> Subject: RE: [dts] [PATCH V1] test: add vhost/virtio pvp performance test
> based on qemu
> 
> Thanks Lei, some comments below.
> 
> > -----Original Message-----
> > From: dts [mailto:dts-bounces@dpdk.org] On Behalf Of lei,yao
> > Sent: Tuesday, December 20, 2016 2:08 PM
> > To: dts@dpdk.org
> > Cc: Yao, Lei A
> > Subject: [dts] [PATCH V1] test: add vhost/virtio pvp performance test
> > based on qemu
> >
> > From: lei yao <lei.a.yao@intel.com>
> >
> > The test case include 3 part:
> > 1. virtio 0.95 performance test
> > virtio0.95 is the default version for qemu. Vector, normal, mergeable path
> > will be tested on this version
> > 2. Test vector performance on differnt qemu version. For exapmle:
> qemu2.5,
> > qemu2.6, qemu2.7
> > 3. Test vector performance on virtio 1.0.
> >
> > Signed-off-by: lei yao <lei.a.yao@intel.com>
> > ---
> >  tests/TestSuite_vhost_qemu_pvp_performance.py | 471
> > ++++++++++++++++++++++++++
> >  1 file changed, 471 insertions(+)
> >  create mode 100644 tests/TestSuite_vhost_qemu_pvp_performance.py
> >
> > diff --git a/tests/TestSuite_vhost_qemu_pvp_performance.py
> > b/tests/TestSuite_vhost_qemu_pvp_performance.py
> > new file mode 100644
> > index 0000000..ced777f
> > --- /dev/null
> > +++ b/tests/TestSuite_vhost_qemu_pvp_performance.py
> > @@ -0,0 +1,471 @@
> > +# BSD LICENSE
> > +#
> > +# Copyright(c) 2010-2015 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.
> > +
> > +Vhost PVP performance using Qemu test suite.
> > +"""
> > +import os
> > +import dts
> 
> Please remove dependency on dts module, we tried to make test suite more
> independent.
> 
I will remove this
> > +import string
> > +import re
> > +import time
> > +import utils
> > +from scapy.utils import wrpcap, rdpcap
> > +from test_case import TestCase
> > +from exception import VerifyFailure
> > +from settings import HEADER_SIZE
> > +from etgen import IxiaPacketGenerator
> > +from qemu_kvm import QEMUKvm
> > +
> > +
> > +class TestVhostUserOneCopyOneVm(TestCase, IxiaPacketGenerator):
> > +
> If you do not want to modify internal function of IxiaPacketGenerator,
> there's no need to inherit this module.
> 
Removed
> > +    def set_up_all(self):
> > +        # To Extend IXIA packet generator method, call the tester's
> > method.
> > +
> >
> self.tester.extend_external_packet_generator(TestVhostUserOneCopyOne
> Vm,
> > self)
> 
> Same comment as previous.
Removed
> > +
> > +
> > +        # Get and verify the ports
> > +        self.dut_ports = self.dut.get_ports()
> > +        self.verify(len(self.dut_ports) >= 1, "Insufficient ports for
> > testing")
> > +
> > +        # Get the port's socket
> > +        self.pf = self.dut_ports[0]
> > +        netdev = self.dut.ports_info[self.pf]['port']
> > +        self.socket = netdev.get_nic_socket()
> > +        self.cores = self.dut.get_core_list("1S/3C/1T",
> > socket=self.socket)
> > +
> > +        # Set the params of vhost sample
> > +        self.vhost_app = "./examples/vhost/build/vhost-switch"
> > +        self.vm2vm = 0
> > +        # This parameter is used to define mergeable on/off
> > +        self.jumbo = 0
> > +
> > +        # Using file to save the vhost sample output since in jumboframe
> > case,
> > +        # there will be lots of output
> > +        self.vhost_test = self.vhost_app + \
> > +            " -c %s -n %d --socket-mem 2048,2048 -- -p 0x1 --
> > mergeable %d" + \
> > +            " --vm2vm %d --socket-file ./vhost-net  > ./vhost.out &"
> 
> I'm not sure whether each platform can support two sockets. If there's no
> special requirement, please just remove socket-mem option.
> 
For this PVP test, don't use socket-mem will bring performance instability for the result. 
Because this is performance test, I can't change it to general " -m " , I need achieve
best and stable performance data. 
> > +        # build the vhost sample in vhost-user mode.
> > +        if self.nic in ['niantic']:
> > +            self.dut.send_expect(
> > +                "sed -i -e 's/#define MAX_QUEUES.*$/#define MAX_QUEUES
> > 128/' "
> > +                "./examples/vhost/main.c",
> > +                "#", 10)
> > +        else:
> > +            self.dut.send_expect(
> > +                "sed -i -e 's/#define MAX_QUEUES.*$/#define MAX_QUEUES
> > 512/' "
> > +                "./examples/vhost/main.c",
> > +                "#", 10)
> 
> Queue numbers are different between different kinds of NICS. If not sure
> with the number, please left it as default.
> I guess the logic should be like below:
> 
> if self.nic in ['niantic']:
>     ...
> else if self.nic.startwith('fortville')
>     ...
> 
Changed as your suggestion.
> 
> > +        out = self.dut.send_expect("make -C examples/vhost", "#")
> > +        self.verify("Error" not in out, "compilation error")
> > +        self.verify("No such file" not in out, "Not found file error")
> > +
> > +        self.virtio1 = "eth1"
> > +        self.virtio1_mac = "52:54:00:00:00:01"
> > +        self.src1 = "192.168.4.1"
> > +        self.dst1 = "192.168.3.1"
> > +        self.vm_dut = None
> > +
> > +        self.number_of_ports = 1
> > +        self.header_row = ["FrameSize(B)", "Throughput(Mpps)",
> > "LineRate(%)","Cycle"]
> > +        self.memory_channel = 4
> > +	# Specify the path of Qemu binary for different Qemu versions, user
> > can modify this as necessary
> > +        self.qemu_path=["/home/lei/qemu_install/qemu_2.5/bin/qemu-
> system-
> > x86_64",\
> > +                    "/home/lei/qemu_install/qemu_2.6/bin/qemu-system-
> > x86_64",\
> > +                    "/home/lei/qemu_install/qemu_2.7/bin/qemu-system-
> > x86_64"]
> > +
> It's not good to add your local path into the code. You can use configuration
> file to work around it.
> Also has question about why we need to check different versions of qemu.
> It's one time task or every time we need to do that.
> If just for one time, you can change configuration for qemu path and rerun
> those cases.
> 
Yes, the qemu version performance test is not required for regression test, only tested for
release cycle. I will remove this test and let user just change the qemu path 
in config file. 

> > +
> > +    def set_up(self):
> > +        #
> > +        # Run before each test case.
> > +        #
> > +        # Launch vhost sample using default params
> > +        #
> > +	self.dut.send_expect("rm -rf ./vhost.out", "#")
> > +	self.dut.send_expect("rm -rf ./vhost-net*", "#")
> > +	self.dut.send_expect("killall -s INT vhost-switch", "#")
> > +	self.dut.send_expect("killall -s INT qemu-system-x86_64", "#")
> > +
> Please make sure code aligned, you can use pep8 check to help on that.
> 
> > +        self.frame_sizes = [64, 128, 256, 512, 1024, 1500]
> > +        self.vm_testpmd_vector = "./x86_64-native-linuxapp-
> > gcc/app/testpmd -c 0x3 -n 3" \
> > +                + " -- -i --txqflags=0xf01 --disable-hw-vlan-filter"
> > +        self.vm_testpmd_normal = "./x86_64-native-linuxapp-
> > gcc/app/testpmd -c 0x3 -n 3" \
> > +                + " -- -i --txqflags=0xf00 --disable-hw-vlan-filter"
> > +        self.dut.send_expect("rm -rf ./vhost.out", "#")
> > +
> > +
> > +    def launch_vhost_sample(self):
> > +        #
> > +        # Launch the vhost sample with different parameters
> > +        #
> > +        self.coremask = utils.create_mask(self.cores)
> > +        self.vhostapp_testcmd = self.vhost_test % (
> > +            self.coremask, self.memory_channel, self.jumbo, self.vm2vm)
> > +        self.dut.send_expect(self.vhostapp_testcmd, "# ", 40)
> > +        time.sleep(30)
> > +        try:
> > +            print "Launch vhost sample:"
> It's better to replace print with self.logger.info function.
> 
Changed.
> > +            self.dut.session.copy_file_from("/root/dpdk/vhost.out")
> 
> Dpdk folder may be changed by command line, you can use self.dut.base_dir
> + "vhost.out".
> 
Changed.
> > +            fp = open('./vhost.out', 'r')
> > +            out = fp.read()
> > +            fp.close()
> > +            if "Error" in out:
> > +                raise Exception("Launch vhost sample failed")
> > +            else:
> > +                print "Launch vhost sample finished"
> > +        except Exception as e:
> > +            print utils.RED("Failed to launch vhost sample: %s" % str(e))
> > +
> It's better to replace print with self.logger.error function.
Changed 
> > +
> > +
> > +    def start_onevm(self, path="", modem=0):
> > +        #
> > +        # Start One VM with one virtio device
> > +        #
> > +
> > +        self.vm = QEMUKvm(self.dut, 'vm0', 'vhost_sample')
> > +        if(path!=""):
> > +            self.vm.set_qemu_emulator(path)
> > +        vm_params = {}
> > +        vm_params['driver'] = 'vhost-user'
> > +        vm_params['opt_path'] = './vhost-net'
> > +        vm_params['opt_mac'] = self.virtio1_mac
> > +        if(modem == 1):
> > +            vm_params['opt_settings'] = 'disable-modern=false'
> > +        self.vm.set_vm_device(**vm_params)
> > +
> > +        try:
> > +            self.vm_dut = self.vm.start()
> > +            if self.vm_dut is None:
> > +                raise Exception("Set up VM ENV failed")
> > +        except Exception as e:
> > +            print utils.RED("Failure for %s" % str(e))
> > +
> > +        return True
> > +
> > +
> > +
> > +    def vm_testpmd_start(self):
> > +        #
> > +        # Start testpmd in vm
> > +        #
> > +        if self.vm_dut is not None:
> > +            self.vm_dut.send_expect(self.vm_testpmd_vector, "testpmd>",
> > 20)
> > +            self.vm_dut.send_expect("start tx_first", "testpmd>")
> > +
> > +    def vm_tx_first_start(self):
> > +        if self.vm_dut is not None:
> > +            # Start tx_first
> > +            self.vm_dut.send_expect("start tx_first", "testpmd>")
> > +
> Look like this function is redundant, vm_testpmd_start has already start
> traffic.
> 
Removed.
> 
> > +
> > +    def send_verify(self, case, frame_sizes, mac_address, vlan_id1=0,
> > tag="Performance"):
> > +        self.result_table_create(self.header_row)
> > +        self.virtio_mac=mac_address
> 
> This function is internal used, it's better not change class variable in this local
> function.
> You can do that in upper layer.
> 
Changed.
> > +        for frame_size in frame_sizes:
> > +            info = "Running test %s, and %d frame size." % (case,
> > frame_size)
> > +            self.logger.info(info)
> > +            payload = frame_size - HEADER_SIZE['eth'] - HEADER_SIZE['ip']
> > +            flow1 =
> > '[Ether(dst="%s")/Dot1Q(vlan=%s)/IP(src="%s",dst="%s")/("X"*%d)]' % (
> > +                self.virtio_mac, vlan_id1, self.src1, self.dst1, payload)
> > +            self.tester.scapy_append('wrpcap("flow1.pcap", %s)' % flow1)
> > +            self.tester.scapy_execute()
> > +
> > +            tgenInput = []
> > +            port = self.tester.get_local_port(self.pf)
> > +            tgenInput.append((port, port, "flow1.pcap"))
> > +
> > +            _, pps = self.tester.traffic_generator_throughput(tgenInput,
> > delay=30)
> > +            Mpps = pps / 1000000.0
> > +            pct  = Mpps * 100 / float(self.wirespeed(self.nic, frame_size,
> > +                                      self.number_of_ports))
> > +            data_row = [frame_size, str(Mpps), str(pct),tag]
> 
> Should add space after ',', please check with pep8 tool.
> 
> > +	    self.result_table_add(data_row)
> > +        self.result_table_print()
> > +
> Look like code is not aligned.
> 
> > +
> > +    def test_perf_qemu_vector_pmdfwd(self):
> > +        #
> > +        # Test the pvp performance for vector path
> > +        #
> > +        # start testpmd on VM
> > +        self.jumbo = 0
> > +        self.launch_vhost_sample()
> > +        self.start_onevm()
> > +        if self.vm_dut is not None:
> 
> When self.vm_dut is None, should we take that as error?
Yes. I will handle error handler.
> 
> > +            # Start testpmd with user
> > +            self.vm_dut.send_expect(self.vm_testpmd_vector, "testpmd>",
> > 20)
> > +            self.vm_dut.send_expect("start tx_first", "testpmd>")
> > +
> > +
> > +        time.sleep(5)
> > +        # Clean the output to ensure the commands cat ./vhost.out can be
> > sent
> > +        # and got answered correctly.
> > +        self.dut.get_session_output(timeout=2)
> > +        self.dut.session.copy_file_from("/root/dpdk/vhost.out")
> 
> Copy vhost.out and strip some information is used in several cases. It's better
> to abstract it to separated function and will make case code more clear.
> 
> > +        fp = open('./vhost.out', 'r')
> > +        out = fp.read()
> > +        fp.close()
> > +        # Get the VLAN ID for virtio
> > +        print "Check the vlan info: "
> > +        l1 = re.findall(
> > +            'MAC_ADDRESS.*?%s.*?and.*?VLAN_TAG.*?(\d+).*?registered' %
> > +            (str(self.virtio1_mac)), out)
> > +        if len(l1) > 0:
> > +            vlan_id1 = l1[0]
> > +            print vlan_id1
> 
> Please remove debug code here. If you need some special output, please
> use function like self.logger.info("Found vlan id %d for virtio device" %
> vlan_id1).
> > +        else:
> > +            vlan_id1 = 1000
> Without vlan information, should we consider it as error?
> 

Actually, I want remove this vlan related code. Using vhost_sample,
The vlanid is always 1000. 
> > +        self.send_verify(self.running_case, self.frame_sizes,
> > self.virtio1_mac, vlan_id1 ,"Virtio 0.95 Vector Performance")
> > +
> > +
> > +    def test_perf_qemu_normal_pmdfwd(self):
> > +        #
> > +        # Test the performance for normal path
> > +        #
> > +        # start testpmd on VM
> > +        self.jumbo = 0
> > +        self.launch_vhost_sample()
> > +        self.start_onevm()
> > +        if self.vm_dut is not None:
> > +            # Start testpmd with user
> > +            self.vm_dut.send_expect(self.vm_testpmd_normal, "testpmd>",
> > 20)
> > +            self.vm_dut.send_expect("start tx_first", "testpmd>")
> > +
> > +	time.sleep(5)
> > +        # Clean the output to ensure the commands cat ./vhost.out can be
> > sent
> > +        # and got answered correctly.
> > +        self.dut.get_session_output(timeout=2)
> > +        self.dut.session.copy_file_from("/root/dpdk/vhost.out")
> > +        fp = open('./vhost.out', 'r')
> > +        out = fp.read()
> > +        fp.close()
> > +        # Get the VLAN ID for virtio
> > +        print "Check the vlan info: "
> > +        l1 = re.findall(
> > +            'MAC_ADDRESS.*?%s.*?and.*?VLAN_TAG.*?(\d+).*?registered' %
> > +            (str(self.virtio1_mac)), out)
> > +        if len(l1) > 0:
> > +            vlan_id1 = l1[0]
> > +            print vlan_id1
> > +        else:
> > +            vlan_id1 = 1000
> > +        self.send_verify(self.running_case, self.frame_sizes,
> > self.virtio1_mac, vlan_id1, "Virtio 0.95 Normal Performance")
> > +
> > +
> > +    def test_perf_qemu_mergeable_pmdfwd(self):
> > +        #
> > +        # Test the performance for mergeable path
> > +        #
> > +        # start testpmd on VM
> > +        self.jumbo = 1
> > +        self.launch_vhost_sample()
> > +        self.start_onevm()
> > +        if self.vm_dut is not None:
> > +            # Start testpmd with user
> > +            self.vm_dut.send_expect(self.vm_testpmd_vector, "testpmd>",
> > 20)
> > +            self.vm_dut.send_expect("start tx_first", "testpmd>")
> > +
> > +        time.sleep(5)
> > +        # Clean the output to ensure the commands cat ./vhost.out can be
> > sent
> > +        # and got answered correctly.
> > +        self.dut.get_session_output(timeout=2)
> > +        self.dut.session.copy_file_from("/root/dpdk/vhost.out")
> > +        fp = open('./vhost.out', 'r')
> > +        out = fp.read()
> > +        fp.close()
> > +        # Get the VLAN ID for virtio
> > +        print "Check the vlan info: "
> > +        l1 = re.findall(
> > +            'MAC_ADDRESS.*?%s.*?and.*?VLAN_TAG.*?(\d+).*?registered' %
> > +            (str(self.virtio1_mac)), out)
> > +        if len(l1) > 0:
> > +            vlan_id1 = l1[0]
> > +            print vlan_id1
> > +        else:
> > +            vlan_id1 = 1000
> > +        self.send_verify(self.running_case, self.frame_sizes,
> > self.virtio1_mac, vlan_id1 , "Virtio 0.95 Mergeable Performance")
> > +
> > +    def test_perf_different_qemuversion_vector_pmdfwd(self):
> > +        #
> > +        # Test the performance for different qemu versions, for example:
> > qemu2.5,
> > +        # qemu2.6, qemu2.7
> > +        #
> > +        # start testpmd on VM
> > +        for path in self.qemu_path:
> > +            self.jumbo = 0
> > +            self.launch_vhost_sample()
> > +            self.start_onevm(path)
> > +            if self.vm_dut is not None:
> > +            # Start testpmd in VM
> > +                self.vm_dut.send_expect(self.vm_testpmd_vector,
> > "testpmd>", 20)
> > +                self.vm_dut.send_expect("start tx_first", "testpmd>")
> > +            time.sleep(5)
> > +
> > +            self.dut.get_session_output(timeout=2)
> > +            self.dut.session.copy_file_from("/root/dpdk/vhost.out")
> > +            fp = open('./vhost.out', 'r')
> > +            out = fp.read()
> > +            fp.close()
> > +            # Get the VLAN ID for virtio
> > +            print "Check the vlan info: "
> > +            l1 = re.findall(
> > +                'MAC_ADDRESS.*?%s.*?and.*?VLAN_TAG.*?(\d+).*?registered' %
> > +                (str(self.virtio1_mac)), out)
> > +            if len(l1) > 0:
> > +                vlan_id1 = l1[0]
> > +                print vlan_id1
> > +            else:
> > +                vlan_id1 = 1000
> > +            self.send_verify(self.running_case, self.frame_sizes,
> > self.virtio1_mac, vlan_id1, path)
> > +            print "This is the vector result for qemu path %s" % path
> > +	    self.dut.send_expect("rm -rf ./vhost.out", "#")
> > +            self.dut.send_expect("rm -rf ./vhost-net*", "#")
> > +            self.dut.send_expect("killall -s INT vhost-switch", "#")
> > +            self.dut.send_expect("killall -s INT qemu-system-x86_64", "#")
> > +
> > +    def test_perf_virtio_modern_qemu_vector_pmdfwd(self):
> > +        #
> > +        # Test the vhost/virtio pvp performance for virtio1.0
> > +        #
> > +        #
> > +        # start testpmd on VM
> > +        self.jumbo = 0
> > +        self.launch_vhost_sample()
> > +        self.start_onevm("",1)
> > +        if self.vm_dut is not None:
> > +            # Start testpmd with user
> > +            self.vm_dut.send_expect(self.vm_testpmd_vector, "testpmd>",
> > 20)
> > +            self.vm_dut.send_expect("start tx_first", "testpmd>")
> > +
> > +        time.sleep(5)
> > +        self.dut.get_session_output(timeout=2)
> > +        self.dut.session.copy_file_from("/root/dpdk/vhost.out")
> > +        fp = open('./vhost.out', 'r')
> > +        out = fp.read()
> > +        fp.close()
> > +        # Get the VLAN ID for virtio
> > +        print "Check the vlan info: "
> > +        l1 = re.findall(
> > +            'MAC_ADDRESS.*?%s.*?and.*?VLAN_TAG.*?(\d+).*?registered' %
> > +            (str(self.virtio1_mac)), out)
> > +        if len(l1) > 0:
> > +            vlan_id1 = l1[0]
> > +            print vlan_id1
> > +        else:
> > +            vlan_id1 = 1000
> > +        self.send_verify(self.running_case, self.frame_sizes,
> > self.virtio1_mac, vlan_id1, "virtio1.0, Vector")
> > +
> > +
> > +    def test_perf_virtio_modern_qemu_normal_pmdfwd(self):
> > +        #
> > +        # Test the performance of one vm with 2virtio devices in legacy
> > fwd
> > +        #
> > +        # start testpmd on VM
> > +        self.jumbo = 0
> > +        self.launch_vhost_sample()
> > +        self.start_onevm("",1)
> > +        if self.vm_dut is not None:
> > +            # Start testpmd with user
> > +            self.vm_dut.send_expect(self.vm_testpmd_normal, "testpmd>",
> > 20)
> > +            self.vm_dut.send_expect("start tx_first", "testpmd>")
> > +
> > +        time.sleep(5)
> > +        self.dut.get_session_output(timeout=2)
> > +        self.dut.session.copy_file_from("/root/dpdk/vhost.out")
> > +        fp = open('./vhost.out', 'r')
> > +        out = fp.read()
> > +        fp.close()
> > +        # Get the VLAN ID for virtio
> > +        print "Check the vlan info: "
> > +        l1 = re.findall(
> > +            'MAC_ADDRESS.*?%s.*?and.*?VLAN_TAG.*?(\d+).*?registered' %
> > +            (str(self.virtio1_mac)), out)
> > +        if len(l1) > 0:
> > +            vlan_id1 = l1[0]
> > +            print vlan_id1
> > +        else:
> > +            vlan_id1 = 1000
> > +        self.send_verify(self.running_case, self.frame_sizes,
> > self.virtio1_mac, vlan_id1, "virtio1.0, Normal")
> > +
> > +
> > +    def test_perf_virtio_modern_qemu_mergeable_pmdfwd(self):
> > +        #
> > +        # Test the performance of one vm with 2virtio devices in legacy
> > fwd
> > +        #
> > +        # start testpmd on VM
> > +        self.jumbo = 1
> > +        self.launch_vhost_sample()
> > +        self.start_onevm("",1)
> > +        if self.vm_dut is not None:
> > +            # Start testpmd with user
> > +            self.vm_dut.send_expect(self.vm_testpmd_vector, "testpmd>",
> > 20)
> > +            self.vm_dut.send_expect("start tx_first", "testpmd>")
> > +
> > +        time.sleep(5)
> > +        self.dut.get_session_output(timeout=2)
> > +        self.dut.session.copy_file_from("/root/dpdk/vhost.out")
> > +        fp = open('./vhost.out', 'r')
> > +        out = fp.read()
> > +        fp.close()
> > +        # Get the VLAN ID for virtio
> > +        print "Check the vlan info: "
> > +        l1 = re.findall(
> > +            'MAC_ADDRESS.*?%s.*?and.*?VLAN_TAG.*?(\d+).*?registered' %
> > +            (str(self.virtio1_mac)), out)
> > +        if len(l1) > 0:
> > +            vlan_id1 = l1[0]
> > +            print vlan_id1
> > +        else:
> > +            vlan_id1 = 1000
> > +        self.send_verify(self.running_case, self.frame_sizes,
> > self.virtio1_mac, vlan_id1, "virtio1.0, Mergeable")
> > +
> > +
> > +
> > +
> > +
> 
> Too much blank line, it should be only one between each method in class.
> 
> > +    def tear_down(self):
> > +        #
> > +        # Run after each test case.
> > +        # Clear vhost-switch and qemu to avoid blocking the following TCs
> > +        #
> > +        self.dut.send_expect("killall -s INT vhost-switch", "#")
> > +        self.dut.send_expect("killall -s INT qemu-system-x86_64", "#")
> 
> Please use self.vm.stop to close virtual machine, there're some resources
> need to cleaned up by stop function.
> 
Changed.
> > +        time.sleep(2)
> > +
> > +    def tear_down_all(self):
> > +        """
> > +        Run after each test suite.
> > +        """
> > +        pass
> > --
> > 2.7.4