Date: Mon, 4 Dec 2023 12:44:44 +0000
From: "Michalik, Michal" <michal.michalik@...el.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"vadim.fedorenko@...ux.dev" <vadim.fedorenko@...ux.dev>, "Kubalewski,
 Arkadiusz" <arkadiusz.kubalewski@...el.com>, "jonathan.lemon@...il.com"
	<jonathan.lemon@...il.com>, "pabeni@...hat.com" <pabeni@...hat.com>, poros
	<poros@...hat.com>, "Olech, Milena" <milena.olech@...el.com>, mschmidt
	<mschmidt@...hat.com>, "linux-clk@...r.kernel.org"
	<linux-clk@...r.kernel.org>, "bvanassche@....org" <bvanassche@....org>,
	"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>
Subject: RE: [PATCH RFC net-next v4 2/2] selftests/dpll: add DPLL system
 integration selftests

Merging two threads.

On 1 December 2023 8:53 PM CET, Jakub Kicinski wrote:
> 
> On Fri, 1 Dec 2023 18:33:11 +0000 Michalik, Michal wrote:
>> That looks really promising - great idea. I tried only vmtest today, and my tests
>> work kind of flawless with my own built kernel (nested VMs):
>>   $ vmtest -k /home/net-next/vmlinux "modprobe netdevsim && KSRC=/home/net-next/ pytest"
>>   => vmlinux
>>   ===> Booting
>>   ===> Setting up VM
>>   ===> Running command  
>>   ============================= test session starts ==============================
>>   platform linux -- Python 3.9.16, pytest-7.4.3, pluggy-1.3.0
>>   rootdir: /home/net-next/tools/testing/selftests/drivers/net/netdevsim/dpll
>>   collected 91 items  
>> 
>>   test_dpll.py ........................................................... [ 64%]
>>   ................................                                         [100%]
>> 
>>   ============================= 91 passed in 10.54s ==============================
>> 
>> I will try to take a look at virtme-ng next week, but to be frank I already like
>> the vmtest.
> 
> Hm, FWIW I manged to get virtme-ng to work (I was pointing it at a
> vmlinux not bzImage which it expects). But vmtest is still unhappy.
> 
> $ vmtest -k build/vmlinux "echo Running!"
> => vmlinux
> ===> Booting
> Failed to connect QGA
> 
> Caused by:
>     Timed out waiting for QGA connection
> 

I have seen this before I got the proper qemu version, actually I compiled it from scratch:
  $ qemu-system-x86_64 --version
  QEMU emulator version 8.1.3

Which version of qemu are you using?

Btw. I agree that logs for vmtest are not very helpful, the .vmtest.log file is basically empty
for me every time.

> 
> Are you on Ubuntu? I'm on Fedora. Maybe it has some distro deps :(
> 

I'm using Rocky, so kind of similar to Fedora.
  $ cat /etc/rocky-release
  Rocky Linux release 9.2 (Blue Onyx)

Also, installed qemu-guest-agent and edk2-ovmf packages according to vmtest instructions.
Have you installed those?

>> >> To be honest I did not see that. I agree that this is a simpler solution, but I am
>> >> not sure if that is not "too simple". What I mean, I'm not sure who wrote the sdsi
>> >> tests, but maybe they were not aware about the Python best practices? Python used
>> >> to be my first language, and I would vote for using the venvs if you asked me.
>> >> I understand that it haven't been done before, but we are here to try to improve
>> >> the things, yes?   
>> > 
>> > I think I already asked how long the setup takes but my only concern 
>> > is that the setup will be slower, and less useful during development.
>> 
>> I wanted for "run_dpll_test.sh" to be userfriendly even for people who does not
>> have a clue how python/pytest works. If somebody is developing tests, I assume
>> he/she knows what she is doing and is using own environment either way, like
>> venvs with additional Python debug tools and direct pytest in tests directory:
>>   KSRC=<KERNEL SRC> pytest
> 
> Fair point.
> 
>> I don't feel like it is slowing anybody down. But since vmtest looks promising,
>> maybe I can prepare a reverse logic. What I mean is I will prepare script which
>> helps prepare the environment, but the default will be to use "locally installed
>> stuff" when people just run "make -C tools/testing/selftests".
> 
> Let's keep it as is. 10sec for automated run is fine.

OK

On 1 December 2023 9:03 PM CET, Jakub Kicinski wrote:
> 
> On Thu, 23 Nov 2023 05:52:43 -0500 Michal Michalik wrote:
>> +++ b/tools/testing/selftests/drivers/net/netdevsim/dpll/ynlfamilyhandler.py
>> @@ -0,0 +1,49 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# Wrapper for the YNL library used to interact with the netlink interface.
>> +#
>> +# Copyright (c) 2023, Intel Corporation.
>> +# Author: Michal Michalik <michal.michalik@...el.com>
>> +
>> +import sys
>> +from pathlib import Path
>> +from dataclasses import dataclass
>> +
>> +from .consts import KSRC, YNLSPEC, YNLPATH
>> +
>> +
>> +try:
>> +    ynl_full_path = Path(KSRC) / YNLPATH
>> +    sys.path.append(ynl_full_path.as_posix())
>> +    from lib import YnlFamily
>> +except ModuleNotFoundError:
>> +    print("Failed importing `ynl` library from kernel sources, please set KSRC")
>> +    sys.exit(1)
> 
> Do you have any suggestions on how we could build up a common Python
> library for selftests? Can we create a directory for "library" code
> somewhere under tools/testing/ ? Adding a wrapper like this for every
> test is going to hurt.
> 

Agree, my approach is not very elegant but I could not figure out anything more
useful at that time. Having a common Python libraries might be a good idea - let
me think a bit how to handle it. 

> Calling out to YNL, manipulating network namespaces, manipulating
> netdevsim instances, etc - will be fairly common for a lot of networking
> tests.
> 
> There's already some code in tools/testing/selftests/bpf/test_offload.py
> which is likely Python-incompetent cause I wrote it. But much like YNL
> it'd be nice if it was available for new tests for reuse.
> 

I will familiarize myself with that - thanks for pointing that out.

> Can we somehow "add to python's library search path" or some such?

Yeah, we might consider using PYTHONPATH in this "new common lib place":
https://docs.python.org/3/using/cmdline.html#envvar-PYTHONPATH

Powered by blists - more mailing lists