[RFC] tests/avocado: re-factor igb test to avoid timeouts

The core of the test was utilising "ethtool -t eth1 offline" to run
through a test sequence. For reasons unknown the test hangs under some
configurations of the build on centos8-stream. Fundamentally running
the old fedora-31 cloud-init is just too much for something that is
directed at testing one device. So we:

  - replace fedora with a custom kernel + buildroot rootfs
  - rename the test from IGB to NetDevEthtool
  - re-factor the common code, add (currently skipped) tests for other
     devices which support ethtool
  - remove the KVM limitation as its fast enough to run in KVM or TCG

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 tests/avocado/igb.py            | 38 --------------
 tests/avocado/netdev-ethtool.py | 93 +++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 38 deletions(-)
 delete mode 100644 tests/avocado/igb.py
 create mode 100644 tests/avocado/netdev-ethtool.py

On 21/3/23 19:17, Alex Bennée wrote:
> The core of the test was utilising "ethtool -t eth1 offline" to run
> through a test sequence. For reasons unknown the test hangs under some
> configurations of the build on centos8-stream. Fundamentally running
> the old fedora-31 cloud-init is just too much for something that is
> directed at testing one device. So we:
> 
>    - replace fedora with a custom kernel + buildroot rootfs
>    - rename the test from IGB to NetDevEthtool
>    - re-factor the common code, add (currently skipped) tests for other
>       devices which support ethtool
>    - remove the KVM limitation as its fast enough to run in KVM or TCG
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   tests/avocado/igb.py            | 38 --------------
>   tests/avocado/netdev-ethtool.py | 93 +++++++++++++++++++++++++++++++++
>   2 files changed, 93 insertions(+), 38 deletions(-)
>   delete mode 100644 tests/avocado/igb.py
>   create mode 100644 tests/avocado/netdev-ethtool.py
> 
> diff --git a/tests/avocado/igb.py b/tests/avocado/igb.py
> deleted file mode 100644
> index abf5dfa07f..0000000000
> --- a/tests/avocado/igb.py
> +++ /dev/null
> @@ -1,38 +0,0 @@
> -# SPDX-License-Identifier: GPL-2.0-or-later
> -# ethtool tests for igb registers, interrupts, etc
> -
> -from avocado_qemu import LinuxTest
> -
> -class IGB(LinuxTest):
> -    """
> -    :avocado: tags=accel:kvm
> -    :avocado: tags=arch:x86_64
> -    :avocado: tags=distro:fedora
> -    :avocado: tags=distro_version:31
> -    :avocado: tags=machine:q35
> -    """
> -
> -    timeout = 180
> -
> -    def test(self):
> -        self.require_accelerator('kvm')
> -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
> -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
> -        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
> -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
> -        initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
> -
> -        # Ideally we want to test MSI as well, but it is blocked by a bug
> -        # fixed with:
> -        # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=28e96556baca7056d11d9fb3cdd0aba4483e00d8
> -        kernel_params = self.distro.default_kernel_params + ' pci=nomsi'
> -
> -        self.vm.add_args('-kernel', kernel_path,
> -                         '-initrd', initrd_path,
> -                         '-append', kernel_params,
> -                         '-accel', 'kvm',
> -                         '-device', 'igb')
> -        self.launch_and_wait()
> -        self.ssh_command('dnf -y install ethtool')
> -        self.ssh_command('ethtool -t eth1 offline')
> diff --git a/tests/avocado/netdev-ethtool.py b/tests/avocado/netdev-ethtool.py
> new file mode 100644
> index 0000000000..da0a22d51c
> --- /dev/null
> +++ b/tests/avocado/netdev-ethtool.py
> @@ -0,0 +1,93 @@
> +# ethtool tests for emulated network devices
> +#
> +# This test leverages ethtool's --test sequence to validate network
> +# device behaviour.
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +import time
> +
> +from avocado import skip
> +
> +from avocado_qemu import QemuSystemTest
> +from avocado_qemu import exec_command, exec_command_and_wait_for_pattern
> +from avocado_qemu import wait_for_console_pattern
> +
> +class NetDevEthtool(QemuSystemTest):
> +    """
> +    :avocado: tags=arch:x86_64
> +    :avocado: tags=machine:q35
> +    """
> +
> +    KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 root=/dev/sda console=ttyS0 '
> +    # Runs in about 20s under KVM, 26s under TCG, 37s under GCOV
> +    timeout = 45
> +
> +    def common_test_code(self, netdev, extra_args=None):
> +        base_url = ('https://fileserver.linaro.org/s/'
> +                    'kE4nCFLdQcoBF9t/download?'
> +                    'path=%2Figb-net-test&files=' )
> +
> +        # This custom kernel has drivers for all the supported network
> +        # devices we can emulate in QEMU
> +        kernel_url = base_url + 'bzImage'
> +        kernel_hash = '784daede6dab993597f36efbf01f69f184c55152'
> +        kernel_path = self.fetch_asset(name="bzImage",
> +                                       locations=(kernel_url), asset_hash=kernel_hash)
> +
> +        rootfs_url = base_url + 'rootfs.ext4'
> +        rootfs_hash = '7d28c1bf429de3b441a63756a51f163442ea574b'
> +        rootfs_path = self.fetch_asset(name="rootfs.ext4",
> +                                       locations=(rootfs_url),
> +                                       asset_hash=rootfs_hash)
> +
> +        kernel_params = self.KERNEL_COMMON_COMMAND_LINE
> +        if extra_args:
> +            kernel_params += extra_args
> +
> +        self.vm.add_args('-kernel', kernel_path,
> +                         '-append', kernel_params,
> +                         '-blockdev',
> +                         f"driver=raw,file.driver=file,file.filename={rootfs_path},node-name=hd0",
> +                         '-device', 'driver=ide-hd,bus=ide.0,unit=0,drive=hd0',
> +                         '-device', netdev)
> +
> +        self.vm.set_console(console_index=0)
> +        self.vm.launch()
> +
> +        wait_for_console_pattern(self, "Welcome to Buildroot", vm=None)
> +        time.sleep(0.2)
> +        exec_command(self, 'root')
> +        time.sleep(0.2)
> +        exec_command_and_wait_for_pattern(self,
> +                                          "ethtool -t eth1 offline",
> +                                          "The test result is PASS",
> +                                          "The test result is FAIL")
> +        time.sleep(0.2)
> +        exec_command_and_wait_for_pattern(self, 'halt', "reboot: System halted")
> +
> +    # Skip testing for MSI for now. Allegedly it was fixed by:
> +    #   28e96556ba (igb: Allocate MSI-X vector when testing)
> +    # but I'm seeing oops in the kernel
> +    @skip("Kernel bug with MSI enabled")
> +    def test_igb(self):

            """
            :avocado: tags=device:igb
            """


> +        self.common_test_code("igb")
> +
> +    def test_igb_nomsi(self):

            """
            :avocado: tags=device:igb
            """

> +        self.common_test_code("igb", "pci=nomsi")
> +
> +
> +    # It seems the other popular cards we model in QEMU currently fail
> +    # the pattern test with:
> +    #
> +    #   pattern test failed (reg 0x00178): got 0x00000000 expected 0x00005A5A
> +    #
> +    # So for now we skip them.
> +
> +    @skip("Incomplete reg 0x00178 support")
> +    def test_e1000(self):

            """
            :avocado: tags=device:e1000
            """

> +        self.common_test_code("e1000")
> +
> +    @skip("Incomplete reg 0x00178 support")
> +    def test_i82550(self):

            """
            :avocado: tags=device:i82550
            """

> +        self.common_test_code("i82550")

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

On 2023/03/22 3:17, Alex Bennée wrote:
> The core of the test was utilising "ethtool -t eth1 offline" to run
> through a test sequence. For reasons unknown the test hangs under some
> configurations of the build on centos8-stream. Fundamentally running
> the old fedora-31 cloud-init is just too much for something that is
> directed at testing one device. So we:
> 
>    - replace fedora with a custom kernel + buildroot rootfs
>    - rename the test from IGB to NetDevEthtool
>    - re-factor the common code, add (currently skipped) tests for other
>       devices which support ethtool
>    - remove the KVM limitation as its fast enough to run in KVM or TCG

I tried this but it seems the rootfs is corrupted:
2023-03-22 13:53:06,728 __init__         L0153 DEBUG| EXT4-fs (sda): 
INFO: recovery required on readonly filesystem
2023-03-22 13:53:06,728 __init__         L0153 DEBUG| EXT4-fs (sda): 
write access will be enabled during recovery
(snip)
2023-03-22 13:54:24,534 __init__         L0153 DEBUG| EXT4-fs (sda): I/O 
error while writing superblock
2023-03-22 13:54:24,535 __init__         L0153 DEBUG| EXT4-fs (sda): 
error loading journal
2023-03-22 13:54:24,542 __init__         L0153 DEBUG| VFS: Cannot open 
root device "sda" or unknown-block(8,0): error -5

I have a few more comments:

- It may be possible to use microvm to trim it down further.

- I'm worried that having a rootfs for a single test is too costly to 
maintain. If you just want to avoid cloud-init, you can just specify:
init=/bin/sh

- "time.sleep(0.2)" for waiting commands may be too fragile. Instead, 
you may write a fully-automated shell script which does not need any 
synchronization. For example, you may specify something like the 
following as the kernel parameter and run self.vm.kill() after the tests 
finish:
init=/bin/ethtool -- -t eth1 offline

- If I remember correctly, e1000 and e1000e tests include some checks 
contradicting with the datasheet. I suspect it is because their tests 
are written for some other devices in the same product family, but I 
haven't investigated further.

Regards,
Akihiko Odaki

> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   tests/avocado/igb.py            | 38 --------------
>   tests/avocado/netdev-ethtool.py | 93 +++++++++++++++++++++++++++++++++
>   2 files changed, 93 insertions(+), 38 deletions(-)
>   delete mode 100644 tests/avocado/igb.py
>   create mode 100644 tests/avocado/netdev-ethtool.py
> 
> diff --git a/tests/avocado/igb.py b/tests/avocado/igb.py
> deleted file mode 100644
> index abf5dfa07f..0000000000
> --- a/tests/avocado/igb.py
> +++ /dev/null
> @@ -1,38 +0,0 @@
> -# SPDX-License-Identifier: GPL-2.0-or-later
> -# ethtool tests for igb registers, interrupts, etc
> -
> -from avocado_qemu import LinuxTest
> -
> -class IGB(LinuxTest):
> -    """
> -    :avocado: tags=accel:kvm
> -    :avocado: tags=arch:x86_64
> -    :avocado: tags=distro:fedora
> -    :avocado: tags=distro_version:31
> -    :avocado: tags=machine:q35
> -    """
> -
> -    timeout = 180
> -
> -    def test(self):
> -        self.require_accelerator('kvm')
> -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
> -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
> -        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
> -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
> -        initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
> -
> -        # Ideally we want to test MSI as well, but it is blocked by a bug
> -        # fixed with:
> -        # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=28e96556baca7056d11d9fb3cdd0aba4483e00d8
> -        kernel_params = self.distro.default_kernel_params + ' pci=nomsi'
> -
> -        self.vm.add_args('-kernel', kernel_path,
> -                         '-initrd', initrd_path,
> -                         '-append', kernel_params,
> -                         '-accel', 'kvm',
> -                         '-device', 'igb')
> -        self.launch_and_wait()
> -        self.ssh_command('dnf -y install ethtool')
> -        self.ssh_command('ethtool -t eth1 offline')
> diff --git a/tests/avocado/netdev-ethtool.py b/tests/avocado/netdev-ethtool.py
> new file mode 100644
> index 0000000000..da0a22d51c
> --- /dev/null
> +++ b/tests/avocado/netdev-ethtool.py
> @@ -0,0 +1,93 @@
> +# ethtool tests for emulated network devices
> +#
> +# This test leverages ethtool's --test sequence to validate network
> +# device behaviour.
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +import time
> +
> +from avocado import skip
> +
> +from avocado_qemu import QemuSystemTest
> +from avocado_qemu import exec_command, exec_command_and_wait_for_pattern
> +from avocado_qemu import wait_for_console_pattern
> +
> +class NetDevEthtool(QemuSystemTest):
> +    """
> +    :avocado: tags=arch:x86_64
> +    :avocado: tags=machine:q35
> +    """
> +
> +    KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 root=/dev/sda console=ttyS0 '
> +    # Runs in about 20s under KVM, 26s under TCG, 37s under GCOV
> +    timeout = 45
> +
> +    def common_test_code(self, netdev, extra_args=None):
> +        base_url = ('https://fileserver.linaro.org/s/'
> +                    'kE4nCFLdQcoBF9t/download?'
> +                    'path=%2Figb-net-test&files=' )
> +
> +        # This custom kernel has drivers for all the supported network
> +        # devices we can emulate in QEMU
> +        kernel_url = base_url + 'bzImage'
> +        kernel_hash = '784daede6dab993597f36efbf01f69f184c55152'
> +        kernel_path = self.fetch_asset(name="bzImage",
> +                                       locations=(kernel_url), asset_hash=kernel_hash)
> +
> +        rootfs_url = base_url + 'rootfs.ext4'
> +        rootfs_hash = '7d28c1bf429de3b441a63756a51f163442ea574b'
> +        rootfs_path = self.fetch_asset(name="rootfs.ext4",
> +                                       locations=(rootfs_url),
> +                                       asset_hash=rootfs_hash)
> +
> +        kernel_params = self.KERNEL_COMMON_COMMAND_LINE
> +        if extra_args:
> +            kernel_params += extra_args
> +
> +        self.vm.add_args('-kernel', kernel_path,
> +                         '-append', kernel_params,
> +                         '-blockdev',
> +                         f"driver=raw,file.driver=file,file.filename={rootfs_path},node-name=hd0",
> +                         '-device', 'driver=ide-hd,bus=ide.0,unit=0,drive=hd0',
> +                         '-device', netdev)
> +
> +        self.vm.set_console(console_index=0)
> +        self.vm.launch()
> +
> +        wait_for_console_pattern(self, "Welcome to Buildroot", vm=None)
> +        time.sleep(0.2)
> +        exec_command(self, 'root')
> +        time.sleep(0.2)
> +        exec_command_and_wait_for_pattern(self,
> +                                          "ethtool -t eth1 offline",
> +                                          "The test result is PASS",
> +                                          "The test result is FAIL")
> +        time.sleep(0.2)
> +        exec_command_and_wait_for_pattern(self, 'halt', "reboot: System halted")
> +
> +    # Skip testing for MSI for now. Allegedly it was fixed by:
> +    #   28e96556ba (igb: Allocate MSI-X vector when testing)
> +    # but I'm seeing oops in the kernel
> +    @skip("Kernel bug with MSI enabled")
> +    def test_igb(self):
> +        self.common_test_code("igb")
> +
> +    def test_igb_nomsi(self):
> +        self.common_test_code("igb", "pci=nomsi")
> +
> +
> +    # It seems the other popular cards we model in QEMU currently fail
> +    # the pattern test with:
> +    #
> +    #   pattern test failed (reg 0x00178): got 0x00000000 expected 0x00005A5A
> +    #
> +    # So for now we skip them.
> +
> +    @skip("Incomplete reg 0x00178 support")
> +    def test_e1000(self):
> +        self.common_test_code("e1000")
> +
> +    @skip("Incomplete reg 0x00178 support")
> +    def test_i82550(self):
> +        self.common_test_code("i82550")

Akihiko Odaki <akihiko.odaki@daynix.com> writes:

> On 2023/03/22 3:17, Alex Bennée wrote:
>> The core of the test was utilising "ethtool -t eth1 offline" to run
>> through a test sequence. For reasons unknown the test hangs under some
>> configurations of the build on centos8-stream. Fundamentally running
>> the old fedora-31 cloud-init is just too much for something that is
>> directed at testing one device. So we:
>>    - replace fedora with a custom kernel + buildroot rootfs
>>    - rename the test from IGB to NetDevEthtool
>>    - re-factor the common code, add (currently skipped) tests for other
>>       devices which support ethtool
>>    - remove the KVM limitation as its fast enough to run in KVM or TCG
>
> I tried this but it seems the rootfs is corrupted:
> 2023-03-22 13:53:06,728 __init__         L0153 DEBUG| EXT4-fs (sda):
> INFO: recovery required on readonly filesystem
> 2023-03-22 13:53:06,728 __init__         L0153 DEBUG| EXT4-fs (sda):
> write access will be enabled during recovery
> (snip)
> 2023-03-22 13:54:24,534 __init__         L0153 DEBUG| EXT4-fs (sda):
> I/O error while writing superblock
> 2023-03-22 13:54:24,535 __init__         L0153 DEBUG| EXT4-fs (sda):
> error loading journal
> 2023-03-22 13:54:24,542 __init__         L0153 DEBUG| VFS: Cannot open
> root device "sda" or unknown-block(8,0): error -5

That's weird. I'm not seeing it when running here. However I can
regenerate the whole thing and re-upload. Are there any other network
tools worth adding?

> I have a few more comments:
>
> - It may be possible to use microvm to trim it down further.

Does microvm have PCI now? Most of the saving comes down to having a
much lighter rootfs than the full cloud init of fedora. I think there is
only really a syslogd and a klogd running at the start.

> - I'm worried that having a rootfs for a single test is too costly to
>   maintain. If you just want to avoid cloud-init, you can just
>  specify:
> init=/bin/sh

Not really too bad. Buildroot makes it pretty easy. The config can be
found here:

  https://fileserver.linaro.org/s/Lk8z7kN3s3ds7kd

>
> - "time.sleep(0.2)" for waiting commands may be too fragile. Instead,
>   you may write a fully-automated shell script which does not need any
>   synchronization.

Yeah this a hangover of the avocado system and how it deals with the
console output that it can't handle login prompts without newlines.

> For example, you may specify something like the
>   following as the kernel parameter and run self.vm.kill() after the
>   tests finish:
> init=/bin/ethtool -- -t eth1 offline

I can do that. It does cause the kernel to panic once ethtool is done
but we've passed the test by that point.

>
> - If I remember correctly, e1000 and e1000e tests include some checks
>   contradicting with the datasheet. I suspect it is because their
>   tests are written for some other devices in the same product family,
>   but I haven't investigated further.
>
> Regards,
> Akihiko Odaki
>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   tests/avocado/igb.py            | 38 --------------
>>   tests/avocado/netdev-ethtool.py | 93 +++++++++++++++++++++++++++++++++
>>   2 files changed, 93 insertions(+), 38 deletions(-)
>>   delete mode 100644 tests/avocado/igb.py
>>   create mode 100644 tests/avocado/netdev-ethtool.py
>> diff --git a/tests/avocado/igb.py b/tests/avocado/igb.py
>> deleted file mode 100644
>> index abf5dfa07f..0000000000
>> --- a/tests/avocado/igb.py
>> +++ /dev/null
>> @@ -1,38 +0,0 @@
>> -# SPDX-License-Identifier: GPL-2.0-or-later
>> -# ethtool tests for igb registers, interrupts, etc
>> -
>> -from avocado_qemu import LinuxTest
>> -
>> -class IGB(LinuxTest):
>> -    """
>> -    :avocado: tags=accel:kvm
>> -    :avocado: tags=arch:x86_64
>> -    :avocado: tags=distro:fedora
>> -    :avocado: tags=distro_version:31
>> -    :avocado: tags=machine:q35
>> -    """
>> -
>> -    timeout = 180
>> -
>> -    def test(self):
>> -        self.require_accelerator('kvm')
>> -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
>> -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
>> -        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>> -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
>> -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
>> -        initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
>> -
>> -        # Ideally we want to test MSI as well, but it is blocked by a bug
>> -        # fixed with:
>> - #
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=28e96556baca7056d11d9fb3cdd0aba4483e00d8
>> -        kernel_params = self.distro.default_kernel_params + ' pci=nomsi'
>> -
>> -        self.vm.add_args('-kernel', kernel_path,
>> -                         '-initrd', initrd_path,
>> -                         '-append', kernel_params,
>> -                         '-accel', 'kvm',
>> -                         '-device', 'igb')
>> -        self.launch_and_wait()
>> -        self.ssh_command('dnf -y install ethtool')
>> -        self.ssh_command('ethtool -t eth1 offline')
>> diff --git a/tests/avocado/netdev-ethtool.py b/tests/avocado/netdev-ethtool.py
>> new file mode 100644
>> index 0000000000..da0a22d51c
>> --- /dev/null
>> +++ b/tests/avocado/netdev-ethtool.py
>> @@ -0,0 +1,93 @@
>> +# ethtool tests for emulated network devices
>> +#
>> +# This test leverages ethtool's --test sequence to validate network
>> +# device behaviour.
>> +#
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +import time
>> +
>> +from avocado import skip
>> +
>> +from avocado_qemu import QemuSystemTest
>> +from avocado_qemu import exec_command, exec_command_and_wait_for_pattern
>> +from avocado_qemu import wait_for_console_pattern
>> +
>> +class NetDevEthtool(QemuSystemTest):
>> +    """
>> +    :avocado: tags=arch:x86_64
>> +    :avocado: tags=machine:q35
>> +    """
>> +
>> +    KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 root=/dev/sda console=ttyS0 '
>> +    # Runs in about 20s under KVM, 26s under TCG, 37s under GCOV
>> +    timeout = 45
>> +
>> +    def common_test_code(self, netdev, extra_args=None):
>> +        base_url = ('https://fileserver.linaro.org/s/'
>> +                    'kE4nCFLdQcoBF9t/download?'
>> +                    'path=%2Figb-net-test&files=' )
>> +
>> +        # This custom kernel has drivers for all the supported network
>> +        # devices we can emulate in QEMU
>> +        kernel_url = base_url + 'bzImage'
>> +        kernel_hash = '784daede6dab993597f36efbf01f69f184c55152'
>> +        kernel_path = self.fetch_asset(name="bzImage",
>> +                                       locations=(kernel_url), asset_hash=kernel_hash)
>> +
>> +        rootfs_url = base_url + 'rootfs.ext4'
>> +        rootfs_hash = '7d28c1bf429de3b441a63756a51f163442ea574b'
>> +        rootfs_path = self.fetch_asset(name="rootfs.ext4",
>> +                                       locations=(rootfs_url),
>> +                                       asset_hash=rootfs_hash)
>> +
>> +        kernel_params = self.KERNEL_COMMON_COMMAND_LINE
>> +        if extra_args:
>> +            kernel_params += extra_args
>> +
>> +        self.vm.add_args('-kernel', kernel_path,
>> +                         '-append', kernel_params,
>> +                         '-blockdev',
>> +                         f"driver=raw,file.driver=file,file.filename={rootfs_path},node-name=hd0",
>> +                         '-device', 'driver=ide-hd,bus=ide.0,unit=0,drive=hd0',
>> +                         '-device', netdev)
>> +
>> +        self.vm.set_console(console_index=0)
>> +        self.vm.launch()
>> +
>> +        wait_for_console_pattern(self, "Welcome to Buildroot", vm=None)
>> +        time.sleep(0.2)
>> +        exec_command(self, 'root')
>> +        time.sleep(0.2)
>> +        exec_command_and_wait_for_pattern(self,
>> +                                          "ethtool -t eth1 offline",
>> +                                          "The test result is PASS",
>> +                                          "The test result is FAIL")
>> +        time.sleep(0.2)
>> +        exec_command_and_wait_for_pattern(self, 'halt', "reboot: System halted")
>> +
>> +    # Skip testing for MSI for now. Allegedly it was fixed by:
>> +    #   28e96556ba (igb: Allocate MSI-X vector when testing)
>> +    # but I'm seeing oops in the kernel
>> +    @skip("Kernel bug with MSI enabled")
>> +    def test_igb(self):
>> +        self.common_test_code("igb")
>> +
>> +    def test_igb_nomsi(self):
>> +        self.common_test_code("igb", "pci=nomsi")
>> +
>> +
>> +    # It seems the other popular cards we model in QEMU currently fail
>> +    # the pattern test with:
>> +    #
>> +    #   pattern test failed (reg 0x00178): got 0x00000000 expected 0x00005A5A
>> +    #
>> +    # So for now we skip them.
>> +
>> +    @skip("Incomplete reg 0x00178 support")
>> +    def test_e1000(self):
>> +        self.common_test_code("e1000")
>> +
>> +    @skip("Incomplete reg 0x00178 support")
>> +    def test_i82550(self):
>> +        self.common_test_code("i82550")

On 2023/03/22 19:04, Alex Bennée wrote:
> 
> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
> 
>> On 2023/03/22 3:17, Alex Bennée wrote:
>>> The core of the test was utilising "ethtool -t eth1 offline" to run
>>> through a test sequence. For reasons unknown the test hangs under some
>>> configurations of the build on centos8-stream. Fundamentally running
>>> the old fedora-31 cloud-init is just too much for something that is
>>> directed at testing one device. So we:
>>>     - replace fedora with a custom kernel + buildroot rootfs
>>>     - rename the test from IGB to NetDevEthtool
>>>     - re-factor the common code, add (currently skipped) tests for other
>>>        devices which support ethtool
>>>     - remove the KVM limitation as its fast enough to run in KVM or TCG
>>
>> I tried this but it seems the rootfs is corrupted:
>> 2023-03-22 13:53:06,728 __init__         L0153 DEBUG| EXT4-fs (sda):
>> INFO: recovery required on readonly filesystem
>> 2023-03-22 13:53:06,728 __init__         L0153 DEBUG| EXT4-fs (sda):
>> write access will be enabled during recovery
>> (snip)
>> 2023-03-22 13:54:24,534 __init__         L0153 DEBUG| EXT4-fs (sda):
>> I/O error while writing superblock
>> 2023-03-22 13:54:24,535 __init__         L0153 DEBUG| EXT4-fs (sda):
>> error loading journal
>> 2023-03-22 13:54:24,542 __init__         L0153 DEBUG| VFS: Cannot open
>> root device "sda" or unknown-block(8,0): error -5
> 
> That's weird. I'm not seeing it when running here. However I can
> regenerate the whole thing and re-upload. Are there any other network
> tools worth adding?

Only ethtool is needed for testing Intel NICs.

> 
>> I have a few more comments:
>>
>> - It may be possible to use microvm to trim it down further.
> 
> Does microvm have PCI now? Most of the saving comes down to having a
> much lighter rootfs than the full cloud init of fedora. I think there is
> only really a syslogd and a klogd running at the start.

microvm supports PCIe. You can enable it by specifying e.g., -M 
microvm,pcie=on

> 
>> - I'm worried that having a rootfs for a single test is too costly to
>>    maintain. If you just want to avoid cloud-init, you can just
>>   specify:
>> init=/bin/sh
> 
> Not really too bad. Buildroot makes it pretty easy. The config can be
> found here:
> 
>    https://fileserver.linaro.org/s/Lk8z7kN3s3ds7kd

Buildroot indeed automates everything to build rootfs, but it still 
takes lots of time to build because it needs to build everything. It 
also fetches sources from the origins of the packages if I understand it 
correctly, and I'm worried that may harm the reproducibility of the builds.

These problems are not present with Fedora: you can add or replace a 
particular component with a package (in this case ethtool is added), and 
Fedora mirrors everything to build the binary.

Regards,
Akihiko Odaki

> 
>>
>> - "time.sleep(0.2)" for waiting commands may be too fragile. Instead,
>>    you may write a fully-automated shell script which does not need any
>>    synchronization.
> 
> Yeah this a hangover of the avocado system and how it deals with the
> console output that it can't handle login prompts without newlines.
> 
>> For example, you may specify something like the
>>    following as the kernel parameter and run self.vm.kill() after the
>>    tests finish:
>> init=/bin/ethtool -- -t eth1 offline
> 
> I can do that. It does cause the kernel to panic once ethtool is done
> but we've passed the test by that point.
> 
>>
>> - If I remember correctly, e1000 and e1000e tests include some checks
>>    contradicting with the datasheet. I suspect it is because their
>>    tests are written for some other devices in the same product family,
>>    but I haven't investigated further.
>>
>> Regards,
>> Akihiko Odaki
>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> ---
>>>    tests/avocado/igb.py            | 38 --------------
>>>    tests/avocado/netdev-ethtool.py | 93 +++++++++++++++++++++++++++++++++
>>>    2 files changed, 93 insertions(+), 38 deletions(-)
>>>    delete mode 100644 tests/avocado/igb.py
>>>    create mode 100644 tests/avocado/netdev-ethtool.py
>>> diff --git a/tests/avocado/igb.py b/tests/avocado/igb.py
>>> deleted file mode 100644
>>> index abf5dfa07f..0000000000
>>> --- a/tests/avocado/igb.py
>>> +++ /dev/null
>>> @@ -1,38 +0,0 @@
>>> -# SPDX-License-Identifier: GPL-2.0-or-later
>>> -# ethtool tests for igb registers, interrupts, etc
>>> -
>>> -from avocado_qemu import LinuxTest
>>> -
>>> -class IGB(LinuxTest):
>>> -    """
>>> -    :avocado: tags=accel:kvm
>>> -    :avocado: tags=arch:x86_64
>>> -    :avocado: tags=distro:fedora
>>> -    :avocado: tags=distro_version:31
>>> -    :avocado: tags=machine:q35
>>> -    """
>>> -
>>> -    timeout = 180
>>> -
>>> -    def test(self):
>>> -        self.require_accelerator('kvm')
>>> -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
>>> -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
>>> -        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>>> -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
>>> -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
>>> -        initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
>>> -
>>> -        # Ideally we want to test MSI as well, but it is blocked by a bug
>>> -        # fixed with:
>>> - #
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=28e96556baca7056d11d9fb3cdd0aba4483e00d8
>>> -        kernel_params = self.distro.default_kernel_params + ' pci=nomsi'
>>> -
>>> -        self.vm.add_args('-kernel', kernel_path,
>>> -                         '-initrd', initrd_path,
>>> -                         '-append', kernel_params,
>>> -                         '-accel', 'kvm',
>>> -                         '-device', 'igb')
>>> -        self.launch_and_wait()
>>> -        self.ssh_command('dnf -y install ethtool')
>>> -        self.ssh_command('ethtool -t eth1 offline')
>>> diff --git a/tests/avocado/netdev-ethtool.py b/tests/avocado/netdev-ethtool.py
>>> new file mode 100644
>>> index 0000000000..da0a22d51c
>>> --- /dev/null
>>> +++ b/tests/avocado/netdev-ethtool.py
>>> @@ -0,0 +1,93 @@
>>> +# ethtool tests for emulated network devices
>>> +#
>>> +# This test leverages ethtool's --test sequence to validate network
>>> +# device behaviour.
>>> +#
>>> +# SPDX-License-Identifier: GPL-2.0-or-later
>>> +
>>> +import time
>>> +
>>> +from avocado import skip
>>> +
>>> +from avocado_qemu import QemuSystemTest
>>> +from avocado_qemu import exec_command, exec_command_and_wait_for_pattern
>>> +from avocado_qemu import wait_for_console_pattern
>>> +
>>> +class NetDevEthtool(QemuSystemTest):
>>> +    """
>>> +    :avocado: tags=arch:x86_64
>>> +    :avocado: tags=machine:q35
>>> +    """
>>> +
>>> +    KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 root=/dev/sda console=ttyS0 '
>>> +    # Runs in about 20s under KVM, 26s under TCG, 37s under GCOV
>>> +    timeout = 45
>>> +
>>> +    def common_test_code(self, netdev, extra_args=None):
>>> +        base_url = ('https://fileserver.linaro.org/s/'
>>> +                    'kE4nCFLdQcoBF9t/download?'
>>> +                    'path=%2Figb-net-test&files=' )
>>> +
>>> +        # This custom kernel has drivers for all the supported network
>>> +        # devices we can emulate in QEMU
>>> +        kernel_url = base_url + 'bzImage'
>>> +        kernel_hash = '784daede6dab993597f36efbf01f69f184c55152'
>>> +        kernel_path = self.fetch_asset(name="bzImage",
>>> +                                       locations=(kernel_url), asset_hash=kernel_hash)
>>> +
>>> +        rootfs_url = base_url + 'rootfs.ext4'
>>> +        rootfs_hash = '7d28c1bf429de3b441a63756a51f163442ea574b'
>>> +        rootfs_path = self.fetch_asset(name="rootfs.ext4",
>>> +                                       locations=(rootfs_url),
>>> +                                       asset_hash=rootfs_hash)
>>> +
>>> +        kernel_params = self.KERNEL_COMMON_COMMAND_LINE
>>> +        if extra_args:
>>> +            kernel_params += extra_args
>>> +
>>> +        self.vm.add_args('-kernel', kernel_path,
>>> +                         '-append', kernel_params,
>>> +                         '-blockdev',
>>> +                         f"driver=raw,file.driver=file,file.filename={rootfs_path},node-name=hd0",
>>> +                         '-device', 'driver=ide-hd,bus=ide.0,unit=0,drive=hd0',
>>> +                         '-device', netdev)
>>> +
>>> +        self.vm.set_console(console_index=0)
>>> +        self.vm.launch()
>>> +
>>> +        wait_for_console_pattern(self, "Welcome to Buildroot", vm=None)
>>> +        time.sleep(0.2)
>>> +        exec_command(self, 'root')
>>> +        time.sleep(0.2)
>>> +        exec_command_and_wait_for_pattern(self,
>>> +                                          "ethtool -t eth1 offline",
>>> +                                          "The test result is PASS",
>>> +                                          "The test result is FAIL")
>>> +        time.sleep(0.2)
>>> +        exec_command_and_wait_for_pattern(self, 'halt', "reboot: System halted")
>>> +
>>> +    # Skip testing for MSI for now. Allegedly it was fixed by:
>>> +    #   28e96556ba (igb: Allocate MSI-X vector when testing)
>>> +    # but I'm seeing oops in the kernel
>>> +    @skip("Kernel bug with MSI enabled")
>>> +    def test_igb(self):
>>> +        self.common_test_code("igb")
>>> +
>>> +    def test_igb_nomsi(self):
>>> +        self.common_test_code("igb", "pci=nomsi")
>>> +
>>> +
>>> +    # It seems the other popular cards we model in QEMU currently fail
>>> +    # the pattern test with:
>>> +    #
>>> +    #   pattern test failed (reg 0x00178): got 0x00000000 expected 0x00005A5A
>>> +    #
>>> +    # So for now we skip them.
>>> +
>>> +    @skip("Incomplete reg 0x00178 support")
>>> +    def test_e1000(self):
>>> +        self.common_test_code("e1000")
>>> +
>>> +    @skip("Incomplete reg 0x00178 support")
>>> +    def test_i82550(self):
>>> +        self.common_test_code("i82550")
> 
>

Akihiko Odaki <akihiko.odaki@daynix.com> writes:

> On 2023/03/22 19:04, Alex Bennée wrote:
>> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
>> 
>>> On 2023/03/22 3:17, Alex Bennée wrote:
>>>> The core of the test was utilising "ethtool -t eth1 offline" to run
>>>> through a test sequence. For reasons unknown the test hangs under some
>>>> configurations of the build on centos8-stream. Fundamentally running
>>>> the old fedora-31 cloud-init is just too much for something that is
>>>> directed at testing one device. So we:
>>>>     - replace fedora with a custom kernel + buildroot rootfs
>>>>     - rename the test from IGB to NetDevEthtool
>>>>     - re-factor the common code, add (currently skipped) tests for other
>>>>        devices which support ethtool
>>>>     - remove the KVM limitation as its fast enough to run in KVM or TCG
>>>
>>> I tried this but it seems the rootfs is corrupted:
>>> 2023-03-22 13:53:06,728 __init__         L0153 DEBUG| EXT4-fs (sda):
>>> INFO: recovery required on readonly filesystem
>>> 2023-03-22 13:53:06,728 __init__         L0153 DEBUG| EXT4-fs (sda):
>>> write access will be enabled during recovery
>>> (snip)
>>> 2023-03-22 13:54:24,534 __init__         L0153 DEBUG| EXT4-fs (sda):
>>> I/O error while writing superblock
>>> 2023-03-22 13:54:24,535 __init__         L0153 DEBUG| EXT4-fs (sda):
>>> error loading journal
>>> 2023-03-22 13:54:24,542 __init__         L0153 DEBUG| VFS: Cannot open
>>> root device "sda" or unknown-block(8,0): error -5
>> That's weird. I'm not seeing it when running here. However I can
>> regenerate the whole thing and re-upload. Are there any other network
>> tools worth adding?
>
> Only ethtool is needed for testing Intel NICs.
>
>> 
>>> I have a few more comments:
>>>
>>> - It may be possible to use microvm to trim it down further.
>> Does microvm have PCI now? Most of the saving comes down to having a
>> much lighter rootfs than the full cloud init of fedora. I think there is
>> only really a syslogd and a klogd running at the start.
>
> microvm supports PCIe. You can enable it by specifying e.g., -M
> microvm,pcie=on
>
>> 
>>> - I'm worried that having a rootfs for a single test is too costly to
>>>    maintain. If you just want to avoid cloud-init, you can just
>>>   specify:
>>> init=/bin/sh
>> Not really too bad. Buildroot makes it pretty easy. The config can
>> be
>> found here:
>>    https://fileserver.linaro.org/s/Lk8z7kN3s3ds7kd
>
> Buildroot indeed automates everything to build rootfs, but it still
> takes lots of time to build because it needs to build everything. It
> also fetches sources from the origins of the packages if I understand
> it correctly, and I'm worried that may harm the reproducibility of the
> builds.
>
> These problems are not present with Fedora: you can add or replace a
> particular component with a package (in this case ethtool is added),
> and Fedora mirrors everything to build the binary.

It's certainly preferable to lean on the distros and their
infrastructure although:

  - Fedora is a poor choice given the support lifetime
  - The various "full-fat" distros we run avocado tests for seem to be
    very bloated (esp compared to my local Debian setup which boots very
    quickly)
  - It's hard to argue with the time saving and stability improvements,
    especially as we are limited on CI time these days

On 2023/03/22 20:22, Alex Bennée wrote:
> 
> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
> 
>> On 2023/03/22 19:04, Alex Bennée wrote:
>>> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
>>>
>>>> On 2023/03/22 3:17, Alex Bennée wrote:
>>>>> The core of the test was utilising "ethtool -t eth1 offline" to run
>>>>> through a test sequence. For reasons unknown the test hangs under some
>>>>> configurations of the build on centos8-stream. Fundamentally running
>>>>> the old fedora-31 cloud-init is just too much for something that is
>>>>> directed at testing one device. So we:
>>>>>      - replace fedora with a custom kernel + buildroot rootfs
>>>>>      - rename the test from IGB to NetDevEthtool
>>>>>      - re-factor the common code, add (currently skipped) tests for other
>>>>>         devices which support ethtool
>>>>>      - remove the KVM limitation as its fast enough to run in KVM or TCG
>>>>
>>>> I tried this but it seems the rootfs is corrupted:
>>>> 2023-03-22 13:53:06,728 __init__         L0153 DEBUG| EXT4-fs (sda):
>>>> INFO: recovery required on readonly filesystem
>>>> 2023-03-22 13:53:06,728 __init__         L0153 DEBUG| EXT4-fs (sda):
>>>> write access will be enabled during recovery
>>>> (snip)
>>>> 2023-03-22 13:54:24,534 __init__         L0153 DEBUG| EXT4-fs (sda):
>>>> I/O error while writing superblock
>>>> 2023-03-22 13:54:24,535 __init__         L0153 DEBUG| EXT4-fs (sda):
>>>> error loading journal
>>>> 2023-03-22 13:54:24,542 __init__         L0153 DEBUG| VFS: Cannot open
>>>> root device "sda" or unknown-block(8,0): error -5
>>> That's weird. I'm not seeing it when running here. However I can
>>> regenerate the whole thing and re-upload. Are there any other network
>>> tools worth adding?
>>
>> Only ethtool is needed for testing Intel NICs.
>>
>>>
>>>> I have a few more comments:
>>>>
>>>> - It may be possible to use microvm to trim it down further.
>>> Does microvm have PCI now? Most of the saving comes down to having a
>>> much lighter rootfs than the full cloud init of fedora. I think there is
>>> only really a syslogd and a klogd running at the start.
>>
>> microvm supports PCIe. You can enable it by specifying e.g., -M
>> microvm,pcie=on
>>
>>>
>>>> - I'm worried that having a rootfs for a single test is too costly to
>>>>     maintain. If you just want to avoid cloud-init, you can just
>>>>    specify:
>>>> init=/bin/sh
>>> Not really too bad. Buildroot makes it pretty easy. The config can
>>> be
>>> found here:
>>>     https://fileserver.linaro.org/s/Lk8z7kN3s3ds7kd
>>
>> Buildroot indeed automates everything to build rootfs, but it still
>> takes lots of time to build because it needs to build everything. It
>> also fetches sources from the origins of the packages if I understand
>> it correctly, and I'm worried that may harm the reproducibility of the
>> builds.
>>
>> These problems are not present with Fedora: you can add or replace a
>> particular component with a package (in this case ethtool is added),
>> and Fedora mirrors everything to build the binary.
> 
> It's certainly preferable to lean on the distros and their
> infrastructure although:
> 
>    - Fedora is a poor choice given the support lifetime

I don't think the lifetime matters that much as we do not care about the 
security of the VMs created during the tests.

>    - The various "full-fat" distros we run avocado tests for seem to be
>      very bloated (esp compared to my local Debian setup which boots very
>      quickly) >    - It's hard to argue with the time saving and stability improvements,
>      especially as we are limited on CI time these days
> 

You don't have to perform the normal userspace initialization the 
distribution comes with for this workload. We can just add something 
like the following to the kernel command line:
init=/bin/sh -- -c "rpm -i /path/to/ethtool.rpm && ethtool -t eth1 offline"

This is very similar to what Docker containers do. In Docker, you do not 
run daemons like systemd but directly run the application or use very 
small init.

The reason why I didn't write the tests in this way is because the 
current testing infrastructure does not have a means to provide a file 
to the guest. I think adding such functionality is much simpler than 
introducing Buildroot.

Regards,
Akihiko Odaki

Akihiko Odaki <akihiko.odaki@daynix.com> writes:

<snip>
>>    - The various "full-fat" distros we run avocado tests for seem to be
>>      very bloated (esp compared to my local Debian setup which boots very
>>      quickly) >    - It's hard to argue with the time saving and stability improvements,
>>      especially as we are limited on CI time these days
>> 
>
> You don't have to perform the normal userspace initialization the
> distribution comes with for this workload. We can just add something
> like the following to the kernel command line:
> init=/bin/sh -- -c "rpm -i /path/to/ethtool.rpm && ethtool -t eth1 offline"
>
> This is very similar to what Docker containers do. In Docker, you do
> not run daemons like systemd but directly run the application or use
> very small init.
>
> The reason why I didn't write the tests in this way is because the
> current testing infrastructure does not have a means to provide a file
> to the guest. I think adding such functionality is much simpler than
> introducing Buildroot.

I don't really have time to add a new feature to avocado to get this PR
dealt with (the original reason I was looking at this test was trying to
figure out why it was stalling on the centos-8-stream custom runner).

I've sent out a v2 which incorporates your other feedback and speeds the
test up some more with the direct call from init. I'll include this
version in the re-roll of the testing PR. We can always look at
reverting it to using fedora after v8.0 is out. In the meantime its just
one more random test binary I'm serving amongst the many I already
support.