Re: [RFC PATCH 08/14] KVM-test: Add a subtest of nic promisc

[Lucas Meneghel Rodrigues <lmr@redhat.com>]

On Tue, 2010-07-20 at 09:35 +0800, Amos Kong wrote:
> This test mainly covers TCP sent from host to guest and from guest to host
> with repeatedly turn on/off NIC promiscuous mode.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  0 files changed, 0 insertions(+), 0 deletions(-)
> 
> diff --git a/client/tests/kvm/tests/nic_promisc.py b/client/tests/kvm/tests/nic_promisc.py
> new file mode 100644
> index 0000000..9a0c979
> --- /dev/null
> +++ b/client/tests/kvm/tests/nic_promisc.py
> @@ -0,0 +1,87 @@
> +import logging, commands
> +from autotest_lib.client.common_lib import error
> +import kvm_utils, kvm_test_utils, kvm_net_utils
> +
> +def run_nic_promisc(test, params, env):
> +    """
> +    Test nic driver in promisc mode:
> +
> +    1) Boot up a guest
> +    2) Repeatedly enable/disable promiscuous mode in guest
> +    3) TCP data transmission from host to guest, and from guest to host,
> +       with 1/1460/65000/100000000 bytes payloads
> +    4) Clean temporary files
> +    5) Stop enable/disable promiscuous mode change
> +
> +    @param test: kvm test object
> +    @param params: Dictionary with the test parameters
> +    @param env: Dictionary with test environment
> +    """
> +    timeout = int(params.get("login_timeout", 360))
> +    vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> +    session = kvm_test_utils.wait_for_login(vm, timeout=timeout)
> +    logging.info("Trying to log into guest '%s' by serial", vm.name)
> +    session2 = kvm_utils.wait_for(lambda: vm.serial_login(),
> +                                  timeout, 0, step=2)
> +    if not session2:
> +        raise error.TestFail("Could not log into guest '%s'" % vm.name)
> +
> +    def compare(filename):
> +        cmd = "md5sum %s" % filename
> +        s1, ret_host = commands.getstatusoutput(cmd)
> +        s2, ret_guest = session.get_command_status_output(cmd)
> +        if s1 != 0 or s2 != 0:
> +            logging.debug("ret_host:%s, ret_guest:%s" % (ret_host, ret_guest))
> +            logging.error("Could not get md5, cmd:%s" % cmd)
> +            return False
> +        if ret_host.strip() != ret_guest.strip():
> +            logging.debug("ret_host :%s, ret_guest:%s" % (ret_host, ret_guest))
> +            logging.error("Files' md5sum mismatch" % (receiver))
> +            return False

^ The above debug messages will be confusing when looked by someone who
is not familiar with the test code, so we should make their lives
easier:

    def compare(filename):
        cmd = "md5sum %s" % filename
        rc_host, md5_host = commands.getstatusoutput(cmd)
        rc_guest, md5_guest = session.get_command_status_output(cmd)
        if rc_host:
            logging.debug('Could not get MD5 hash for file %s on host, output: %s', filename, md5_host)
            return False
        if rc_guest:
            logging.debug('Could not get MD5 hash for file %s on guest, output: %s', filename, md5_guest)
            return False
        md5host = md5host.strip()
        md5guest = md5guest.strip()
        if md5host != md5guest:
            logging.error('MD5 hash mismatch between file %s present on guest and on host', filename)
            logging.error('MD5 hash for file on guest: %s, MD5 hash for file on host: %s', md5_host, md5_guest)
            return False
        return True


> +        return True
> +
> +    ethname = kvm_net_utils.get_linux_ifname(session, vm.get_macaddr(0))
> +    set_promisc_cmd = "ip link set %s promisc on; sleep 0.01;" % ethname
> +    set_promisc_cmd += "ip link set %s promisc off; sleep 0.01" % ethname

^ You could do the above on a single attribution, see comment on patch 7
of the patchseries.

> +    logging.info("Set promisc change repeatedly in guest")
> +    session2.sendline("while true; do %s; done" % set_promisc_cmd)
> +
> +    dd_cmd = "dd if=/dev/urandom of=%s bs=%d count=1"
> +    filename = "/tmp/nic_promisc_file"
> +    file_size = params.get("file_size", "1, 1460, 65000, 100000000").split(",")
> +    try:
> +        for size in file_size:
> +            logging.info("Create %s bytes file on host" % size)
> +            s, o = commands.getstatusoutput(dd_cmd % (filename, int(size)))
> +            if s != 0:
> +                logging.debug("Output: %s"% o)
> +                raise error.TestFail("Create file on host failed")
> +
> +            logging.info("Transfer file from host to guest")
> +            if not vm.copy_files_to(filename, filename):
> +                raise error.TestFail("File transfer failed")
> +            if not compare(filename):
> +                raise error.TestFail("Compare file failed")

^ It'd be better if we don't abruptly fail the whole test if we get a
failure for a single size, what about having a global failure counter,
and increment it if we have failures, making sure we log errors
appropriately?

> +            logging.info("Create %s bytes file on guest" % size)
> +            if session.get_command_status(dd_cmd % (filename, int(size)),
> +                                                    timeout=100) != 0:
> +                raise error.TestFail("Create file on guest failed")
> +
> +            logging.info("Transfer file from guest to host")
> +            if not vm.copy_files_from(filename, filename):
> +                raise error.TestFail("File transfer failed")
> +            if not compare(filename):
> +                raise error.TestFail("Compare file failed")

^ Same comment as above.

> +            logging.info("Clean temporal files")

^ Typo, temporary

> +            cmd = "rm -f %s" % filename
> +            s1, o = commands.getstatusoutput(cmd)
> +            s2 = session.get_command_status(cmd)
> +            if s1 != 0 or s2 != 0:
> +                raise error.TestError("Fail to clean temporal files")

^ Same as above, with the exception that maybe a failure during cleanup
is not something so bad we'd need to fail the entire test because of it.

> +    finally:
> +        logging.info("Restore the %s to the nonpromisc mode" % ethname)
> +        session2.close()
> +        session.get_command_status("ip link set %s promisc off" % ethname)
> +        session.close()
> diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample
> index 03d15c0..9e2b9a0 100644
> --- a/client/tests/kvm/tests_base.cfg.sample
> +++ b/client/tests/kvm/tests_base.cfg.sample
> @@ -370,6 +370,10 @@ variants:
>          scp_timeout = 300
>          thread_num = 10
>  
> +    - nic_promisc:  install setup unattended_install.cdrom
> +        type = nic_promisc
> +        file_size = 1, 1460, 65000, 100000000
> +
>      - physical_resources_check: install setup unattended_install.cdrom
>          type = physical_resources_check
>          catch_uuid_cmd = dmidecode | awk -F: '/UUID/ {print $2}'
> @@ -1046,7 +1050,7 @@ variants:
>  
>      # Windows section
>      - @Windows:
> -        no autotest linux_s3 vlan_tag ioquit unattended_install.(url|nfs|remote_ks) jumbo file_transfer nicdriver_unload
> +        no autotest linux_s3 vlan_tag ioquit unattended_install.(url|nfs|remote_ks) jumbo file_transfer nicdriver_unload nic_promisc
>          shutdown_command = shutdown /s /f /t 0
>          reboot_command = shutdown /r /f /t 0
>          status_test_command = echo %errorlevel%
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html