diff mbox series

[1/2] tst_test: Add support for device discovery

Message ID 20200623112827.10744-2-chrubis@suse.cz
State Superseded
Headers show
Series Device discovery & UART test | expand

Commit Message

Cyril Hrubis June 23, 2020, 11:28 a.m. UTC
Device discovery
----------------

The problem
-----------

Each lab has a different hardware capabilities and configuration. A test
that heavily depends on a hardware needs to get this information in
order to be able to run correctly.

The solution
------------

The test declares which devices it needs for a testing. In the uart test
these are UART_RX and UART_TX which are two UART endpoints which are
connected together.

This information is then passed as a parameter to a device-discovery.sh
script that prints, possibly several lines, of device listrs, which is
then parsed by the test library and the test is executed accordingly.

The data are passed to the test as a environment variables, if these are
set prior to the test start, we do not even attempt to do a device
discovery. If these are unset, we run the device discovery and loop the
test around the lists we got.

Why this solution?
------------------

The device discovery is lab specific and does not belong to the test
itself. This is an attempt to abstract the interface between the test
and the hardware so that we can finally add device drivers tests into
LTP.

Missing pieces
--------------

There are stil a few missing pieces, but these are probably easy to fix
as well.

Device reconfiguration
~~~~~~~~~~~~~~~~~~~~~~

I suppose that we may need to run a command so that the devices are
reconfigured as we need them. I.e. the device-discovery.sh will have to
output a comand that needs to be executed in order to prepare the
physical environment e.g. relays in case of the UART.

Device parameters
~~~~~~~~~~~~~~~~~

We may as well need some extra info about the devices, e.g. is hardware
flow connected in case of UART. So the device-discover.sh will add one
more environment variable e.g. UART_PARS="hwflow" that could be parsed
in the test as well.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 device-discovery.sh |   5 +
 include/tst_test.h  |   3 +
 lib/tst_devices.c   | 228 ++++++++++++++++++++++++++++++++++++++++++++
 lib/tst_devices.h   |  32 +++++++
 lib/tst_test.c      |  60 ++++++++++--
 5 files changed, 321 insertions(+), 7 deletions(-)
 create mode 100755 device-discovery.sh
 create mode 100644 lib/tst_devices.c
 create mode 100644 lib/tst_devices.h

Comments

Michal Simek June 24, 2020, 8:41 a.m. UTC | #1
On 23. 06. 20 13:28, Cyril Hrubis via lists.yoctoproject.org wrote:
> Device discovery
> ----------------
> 
> The problem
> -----------
> 
> Each lab has a different hardware capabilities and configuration. A test
> that heavily depends on a hardware needs to get this information in
> order to be able to run correctly.
> 
> The solution
> ------------
> 
> The test declares which devices it needs for a testing. In the uart test
> these are UART_RX and UART_TX which are two UART endpoints which are
> connected together.
> 
> This information is then passed as a parameter to a device-discovery.sh
> script that prints, possibly several lines, of device listrs, which is
> then parsed by the test library and the test is executed accordingly.
> 
> The data are passed to the test as a environment variables, if these are
> set prior to the test start, we do not even attempt to do a device
> discovery. If these are unset, we run the device discovery and loop the
> test around the lists we got.
> 
> Why this solution?
> ------------------
> 
> The device discovery is lab specific and does not belong to the test
> itself. This is an attempt to abstract the interface between the test
> and the hardware so that we can finally add device drivers tests into
> LTP.
> 
> Missing pieces
> --------------
> 
> There are stil a few missing pieces, but these are probably easy to fix

still

> as well.
> 
> Device reconfiguration
> ~~~~~~~~~~~~~~~~~~~~~~
> 
> I suppose that we may need to run a command so that the devices are
> reconfigured as we need them. I.e. the device-discovery.sh will have to
> output a comand that needs to be executed in order to prepare the

command

> physical environment e.g. relays in case of the UART.
> 
> Device parameters
> ~~~~~~~~~~~~~~~~~
> 
> We may as well need some extra info about the devices, e.g. is hardware
> flow connected in case of UART. So the device-discover.sh will add one

device-discovery.sh

> more environment variable e.g. UART_PARS="hwflow" that could be parsed
> in the test as well.
> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  device-discovery.sh |   5 +
>  include/tst_test.h  |   3 +
>  lib/tst_devices.c   | 228 ++++++++++++++++++++++++++++++++++++++++++++
>  lib/tst_devices.h   |  32 +++++++
>  lib/tst_test.c      |  60 ++++++++++--
>  5 files changed, 321 insertions(+), 7 deletions(-)
>  create mode 100755 device-discovery.sh
>  create mode 100644 lib/tst_devices.c
>  create mode 100644 lib/tst_devices.h
> 
> diff --git a/device-discovery.sh b/device-discovery.sh
> new file mode 100755
> index 000000000..08460c41f
> --- /dev/null
> +++ b/device-discovery.sh
> @@ -0,0 +1,5 @@
> +#!/bin/sh
> +
> +if [ "$1" = "UART_RX-UART_TX" ]; then

I am not getting this condition and what exactly you want to tell by that.
> +	echo "UART_RX=/dev/ttyUSB0 UART_TX=/dev/ttyUSB0"

In fpga world you can connect two uart inside chip and test different
device drivers that's why at the end of day only user knows which uarts
are connected to each other and none will be able to come up with
universal device-deiscovery.sh script to cover all these cases.

Not exactly sure how LTP handles this in general but I think it makes
sense to extend your test (txt_test) parameters to pass TX/RX channel
via parameters directly to test.

Something like this
uart01_115200 uart01 -b 115200 -t /dev/ttyXX0 -r /dev/ttyXX1

IIRC RX and TX device could be the same which can mean that you want to
use internal or external loopbacks.

Thanks,
Michal
Cyril Hrubis June 24, 2020, 9:05 a.m. UTC | #2
Hi!
> > +++ b/device-discovery.sh
> > @@ -0,0 +1,5 @@
> > +#!/bin/sh
> > +
> > +if [ "$1" = "UART_RX-UART_TX" ]; then
> 
> I am not getting this condition and what exactly you want to tell by that.
> > +	echo "UART_RX=/dev/ttyUSB0 UART_TX=/dev/ttyUSB0"
> 
> In fpga world you can connect two uart inside chip and test different
> device drivers that's why at the end of day only user knows which uarts
> are connected to each other and none will be able to come up with
> universal device-deiscovery.sh script to cover all these cases.

That's the whole point of the patchset.

This is a script I've used for testing with a hardware loopback on the
USB-to-serial bridge, it's not supposed to be included in the end
result. I've kept it there so that people will get the idea how it
should look like.

Also script is passed which devices the test requests, so for i2c eeprom
the parameters would be different and the script would output, for each
present eeprom, a line with an address, bus, etc.

The whole point of the script is to do plug into a Lab CI so that the
information about test device, in this case UART loops, is exposed to
the test. In a real world situation it would either do a call to a
whatever is maintaining a lab hardware inventory, or be written down by
a user before these tests are executed.

> Not exactly sure how LTP handles this in general but I think it makes
> sense to extend your test (txt_test) parameters to pass TX/RX channel
> via parameters directly to test.
> 
> Something like this
> uart01_115200 uart01 -b 115200 -t /dev/ttyXX0 -r /dev/ttyXX1

You can pass them in an environment variables. If UART_TX and UART_RX
are set the device discovery is not attempted at all and the test just
uses these.

If they are not the script is executed and the test loops over the
result(s). It would be more complicated if the devices were passed over
command line parameters since we would have to re-execute the binary.

> IIRC RX and TX device could be the same which can mean that you want to
> use internal or external loopbacks.

The test works fine with external loopback. We will have to add a way
how to pass "enable loopback in MCR" to the test if we want to use that,
but that falls under the "Missing pieces" in the patch description.
Michal Simek June 24, 2020, 9:21 a.m. UTC | #3
On 24. 06. 20 11:05, Cyril Hrubis wrote:
> Hi!
>>> +++ b/device-discovery.sh
>>> @@ -0,0 +1,5 @@
>>> +#!/bin/sh
>>> +
>>> +if [ "$1" = "UART_RX-UART_TX" ]; then
>>
>> I am not getting this condition and what exactly you want to tell by that.
>>> +	echo "UART_RX=/dev/ttyUSB0 UART_TX=/dev/ttyUSB0"
>>
>> In fpga world you can connect two uart inside chip and test different
>> device drivers that's why at the end of day only user knows which uarts
>> are connected to each other and none will be able to come up with
>> universal device-deiscovery.sh script to cover all these cases.
> 
> That's the whole point of the patchset.
> 
> This is a script I've used for testing with a hardware loopback on the
> USB-to-serial bridge, it's not supposed to be included in the end
> result. I've kept it there so that people will get the idea how it
> should look like.
> 
> Also script is passed which devices the test requests, so for i2c eeprom
> the parameters would be different and the script would output, for each
> present eeprom, a line with an address, bus, etc.
> 
> The whole point of the script is to do plug into a Lab CI so that the
> information about test device, in this case UART loops, is exposed to
> the test. In a real world situation it would either do a call to a
> whatever is maintaining a lab hardware inventory, or be written down by
> a user before these tests are executed.

I understand that you want to run a script which can be wired with Lab CI.

> 
>> Not exactly sure how LTP handles this in general but I think it makes
>> sense to extend your test (txt_test) parameters to pass TX/RX channel
>> via parameters directly to test.
>>
>> Something like this
>> uart01_115200 uart01 -b 115200 -t /dev/ttyXX0 -r /dev/ttyXX1
> 
> You can pass them in an environment variables. If UART_TX and UART_RX
> are set the device discovery is not attempted at all and the test just
> uses these.
> 
> If they are not the script is executed and the test loops over the
> result(s). It would be more complicated if the devices were passed over
> command line parameters since we would have to re-execute the binary.

I didn't run LTP for quite a long time myself but on xilinx devices you
have 3 different uart instances which you can wire: cadence uart (or
pl011), ns16550 and uartlite.
That means with the same hw design you should be able to to test
cadence<=>ns16550 and ns16550<=>uartlite. It means you need to exchange
variables in the middle of testing.

Not sure if this is supported but I would simply generate runtest
description based on information I got from device discovery.
But I am far from testing at this stage.


> 
>> IIRC RX and TX device could be the same which can mean that you want to
>> use internal or external loopbacks.
> 
> The test works fine with external loopback. We will have to add a way
> how to pass "enable loopback in MCR" to the test if we want to use that,
> but that falls under the "Missing pieces" in the patch description.

I am happy with todo. :-)

Cheers,
Michal
Cyril Hrubis June 24, 2020, 9:29 a.m. UTC | #4
Hi!
> >> Not exactly sure how LTP handles this in general but I think it makes
> >> sense to extend your test (txt_test) parameters to pass TX/RX channel
> >> via parameters directly to test.
> >>
> >> Something like this
> >> uart01_115200 uart01 -b 115200 -t /dev/ttyXX0 -r /dev/ttyXX1
> > 
> > You can pass them in an environment variables. If UART_TX and UART_RX
> > are set the device discovery is not attempted at all and the test just
> > uses these.
> > 
> > If they are not the script is executed and the test loops over the
> > result(s). It would be more complicated if the devices were passed over
> > command line parameters since we would have to re-execute the binary.
> 
> I didn't run LTP for quite a long time myself but on xilinx devices you
> have 3 different uart instances which you can wire: cadence uart (or
> pl011), ns16550 and uartlite.
> That means with the same hw design you should be able to to test
> cadence<=>ns16550 and ns16550<=>uartlite. It means you need to exchange
> variables in the middle of testing.

The whole point of the script is that it returns one configuration per
line and the test then loops over these, which is a bit more flexible
than runtest files.

> Not sure if this is supported but I would simply generate runtest
> description based on information I got from device discovery.
> But I am far from testing at this stage.

The direction I would like to take in the long term is to slowly get rid
of runtest files and replace them with database that would be used by
the test execution framework to execute tests. There are too many
limitations that are imposed by runtest files, which in the end shape
the ways we think about tests. We should have get rid of these long time
ago...
Cixi Geng June 24, 2020, 9:57 a.m. UTC | #5
add Orson in mail loop

Hi Cyril
in the ltp-ddt test script ,it  can auto detect which serial devices are on
the machine, and filter out the serial ports in used,So I am more
concerned about whether it can have this function in  the device-discovery.sh

Cyril Hrubis <chrubis@suse.cz> 于2020年6月24日周三 下午5:29写道:
>
> Hi!
> > >> Not exactly sure how LTP handles this in general but I think it makes
> > >> sense to extend your test (txt_test) parameters to pass TX/RX channel
> > >> via parameters directly to test.
> > >>
> > >> Something like this
> > >> uart01_115200 uart01 -b 115200 -t /dev/ttyXX0 -r /dev/ttyXX1
> > >
> > > You can pass them in an environment variables. If UART_TX and UART_RX
> > > are set the device discovery is not attempted at all and the test just
> > > uses these.
> > >
> > > If they are not the script is executed and the test loops over the
> > > result(s). It would be more complicated if the devices were passed over
> > > command line parameters since we would have to re-execute the binary.
> >
> > I didn't run LTP for quite a long time myself but on xilinx devices you
> > have 3 different uart instances which you can wire: cadence uart (or
> > pl011), ns16550 and uartlite.
> > That means with the same hw design you should be able to to test
> > cadence<=>ns16550 and ns16550<=>uartlite. It means you need to exchange
> > variables in the middle of testing.
>
> The whole point of the script is that it returns one configuration per
> line and the test then loops over these, which is a bit more flexible
> than runtest files.
>
> > Not sure if this is supported but I would simply generate runtest
> > description based on information I got from device discovery.
> > But I am far from testing at this stage.
>
> The direction I would like to take in the long term is to slowly get rid
> of runtest files and replace them with database that would be used by
> the test execution framework to execute tests. There are too many
> limitations that are imposed by runtest files, which in the end shape
> the ways we think about tests. We should have get rid of these long time
> ago...
>
> --
> Cyril Hrubis
> chrubis@suse.cz
Cyril Hrubis June 24, 2020, 10:10 a.m. UTC | #6
Hi!
> in the ltp-ddt test script ,it  can auto detect which serial devices are on
> the machine, and filter out the serial ports in used???So I am more
> concerned about whether it can have this function in  the device-discovery.sh

That's the piece that is lab specific and used in your lab, there will
be no offical script and you are supposed to maintain it yourself.

The approach here is to isolate all the lab specific details into a
single place since that code does not belong to the LTP upstream
codebase.
Michal Simek June 24, 2020, 10:53 a.m. UTC | #7
On 24. 06. 20 11:29, Cyril Hrubis wrote:
> Hi!
>>>> Not exactly sure how LTP handles this in general but I think it makes
>>>> sense to extend your test (txt_test) parameters to pass TX/RX channel
>>>> via parameters directly to test.
>>>>
>>>> Something like this
>>>> uart01_115200 uart01 -b 115200 -t /dev/ttyXX0 -r /dev/ttyXX1
>>>
>>> You can pass them in an environment variables. If UART_TX and UART_RX
>>> are set the device discovery is not attempted at all and the test just
>>> uses these.
>>>
>>> If they are not the script is executed and the test loops over the
>>> result(s). It would be more complicated if the devices were passed over
>>> command line parameters since we would have to re-execute the binary.
>>
>> I didn't run LTP for quite a long time myself but on xilinx devices you
>> have 3 different uart instances which you can wire: cadence uart (or
>> pl011), ns16550 and uartlite.
>> That means with the same hw design you should be able to to test
>> cadence<=>ns16550 and ns16550<=>uartlite. It means you need to exchange
>> variables in the middle of testing.
> 
> The whole point of the script is that it returns one configuration per
> line and the test then loops over these, which is a bit more flexible
> than runtest files.

ok. I expect this will end up that you will share example even with all
lines commented to show how people should use this script.

> 
>> Not sure if this is supported but I would simply generate runtest
>> description based on information I got from device discovery.
>> But I am far from testing at this stage.
> 
> The direction I would like to take in the long term is to slowly get rid
> of runtest files and replace them with database that would be used by
> the test execution framework to execute tests. There are too many
> limitations that are imposed by runtest files, which in the end shape
> the ways we think about tests. We should have get rid of these long time
> ago...

Hopefully that database won't be any complicated one to still have an
option to run LTP on constrained systems.

Definitely I agree with cleaning that runtest part.

Thanks,
Michal
Carlos Hernandez June 24, 2020, 6:16 p.m. UTC | #8
Hi Cyril,

Thanks for looking at this problem...

I think we a need:

1) A common way to define Hardware capabilities, i.e. UART_RX and 
UART_TX in your example.

I suspect for device-driver tests, ltp would be called by a test 
automation framework. It should be the test automation framework 
responsibility to setup the equipment per the HW capabilities requested 
by the test.

So from ltp point of view, the only requirement is to advertise the 
required capabilities. Of course, this implies a common understanding of 
the capabilities' tags.


2) A way to set platform-specific values when required. Ideally the test 
logic can figure out the values to use dynamically but for some test 
cases, it is required to statically defined them based on the platform 
the test is running on.

In ltp-ddt we added this functionality as platform overrides 
http://arago-project.org/git/projects/?p=test-automation/ltp-ddt.git;a=blob;f=README-DDT;h=78b79cd3ca0f66a6ef30b5dc05737188c146a9ca;hb=HEAD#l46, 
borrowing an idea from OE/Yocto world. I think a different approach 
where these info is maintained in a separate file with an API that it is 
called by the test case logic would work. However, I think that this 
information is not lab-specific but board-specific and it should be part 
of ltp.


Regards,

Carlos


On 6/23/20 7:28 AM, Cyril Hrubis wrote:
> Device discovery
> ----------------
>
> The problem
> -----------
>
> Each lab has a different hardware capabilities and configuration. A test
> that heavily depends on a hardware needs to get this information in
> order to be able to run correctly.
>
> The solution
> ------------
>
> The test declares which devices it needs for a testing. In the uart test
> these are UART_RX and UART_TX which are two UART endpoints which are
> connected together.
>
> This information is then passed as a parameter to a device-discovery.sh
> script that prints, possibly several lines, of device listrs, which is
> then parsed by the test library and the test is executed accordingly.
>
> The data are passed to the test as a environment variables, if these are
> set prior to the test start, we do not even attempt to do a device
> discovery. If these are unset, we run the device discovery and loop the
> test around the lists we got.
>
> Why this solution?
> ------------------
>
> The device discovery is lab specific and does not belong to the test
> itself. This is an attempt to abstract the interface between the test
> and the hardware so that we can finally add device drivers tests into
> LTP.
>
> Missing pieces
> --------------
>
> There are stil a few missing pieces, but these are probably easy to fix
> as well.
>
> Device reconfiguration
> ~~~~~~~~~~~~~~~~~~~~~~
>
> I suppose that we may need to run a command so that the devices are
> reconfigured as we need them. I.e. the device-discovery.sh will have to
> output a comand that needs to be executed in order to prepare the
> physical environment e.g. relays in case of the UART.
>
> Device parameters
> ~~~~~~~~~~~~~~~~~
>
> We may as well need some extra info about the devices, e.g. is hardware
> flow connected in case of UART. So the device-discover.sh will add one
> more environment variable e.g. UART_PARS="hwflow" that could be parsed
> in the test as well.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>   device-discovery.sh |   5 +
>   include/tst_test.h  |   3 +
>   lib/tst_devices.c   | 228 ++++++++++++++++++++++++++++++++++++++++++++
>   lib/tst_devices.h   |  32 +++++++
>   lib/tst_test.c      |  60 ++++++++++--
>   5 files changed, 321 insertions(+), 7 deletions(-)
>   create mode 100755 device-discovery.sh
>   create mode 100644 lib/tst_devices.c
>   create mode 100644 lib/tst_devices.h
>
> diff --git a/device-discovery.sh b/device-discovery.sh
> new file mode 100755
> index 000000000..08460c41f
> --- /dev/null
> +++ b/device-discovery.sh
> @@ -0,0 +1,5 @@
> +#!/bin/sh
> +
> +if [ "$1" = "UART_RX-UART_TX" ]; then
> +	echo "UART_RX=/dev/ttyUSB0 UART_TX=/dev/ttyUSB0"
> +fi
> diff --git a/include/tst_test.h b/include/tst_test.h
> index b84f7b9dd..3c3693098 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -215,6 +215,9 @@ struct tst_test {
>   	/* NULL terminated array of needed kernel drivers */
>   	const char * const *needs_drivers;
>   
> +	/* NULL terminated array of devices */
> +	const char *const *needs_devices;
> +
>   	/*
>   	 * NULL terminated array of (/proc, /sys) files to save
>   	 * before setup and restore after cleanup
> diff --git a/lib/tst_devices.c b/lib/tst_devices.c
> new file mode 100644
> index 000000000..087e2021b
> --- /dev/null
> +++ b/lib/tst_devices.c
> @@ -0,0 +1,228 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Cyril Hrubis <chrubis@suse.cz>
> + */
> +
> +/*
> + * Device discovery code.
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#include "tst_devices.h"
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +
> +struct tst_devlist {
> +	struct tst_devlist *next;
> +	unsigned int dev_cnt;
> +	char *devs[];
> +};
> +
> +static char *create_devstr(const char *const needs_devices[])
> +{
> +	unsigned int i;
> +	size_t len = 1;
> +
> +	for (i = 0; needs_devices[i]; i++)
> +		len += strlen(needs_devices[i]) + 1;
> +
> +	char *res = malloc(len);
> +
> +	if (!res) {
> +		tst_res(TWARN | TERRNO, "Malloc failed");
> +		return NULL;
> +	}
> +
> +	char *p = res;
> +
> +	for (i = 0; needs_devices[i]; i++) {
> +		strcpy(p, needs_devices[i]);
> +		p += strlen(needs_devices[i]);
> +		if (needs_devices[i+1]) {
> +			p[0] = '-';
> +			p++;
> +		}
> +	}
> +
> +	tst_res(TINFO, "Device discovery string '%s'", res);
> +
> +	return res;
> +}
> +
> +static int get_index(const char *env, const char *const needs_devices[])
> +{
> +	char *p = strdup(env);
> +	char *save;
> +	char *name = strtok_r(p, "=", &save);
> +	unsigned int i;
> +
> +	for (i = 0; needs_devices[i]; i++)
> +		if (!strcmp(needs_devices[i], name))
> +			return i;
> +
> +	return -1;
> +}
> +
> +static unsigned int count_devices(const char *const needs_devices[])
> +{
> +	unsigned int i = 0;
> +
> +	while (needs_devices[i])
> +		i++;
> +
> +	return i;
> +}
> +
> +static struct tst_devlist *new_devlist(char *devices[], unsigned int dev_cnt,
> +                                       const char *const needs_devices[])
> +{
> +	unsigned int i;
> +	int incomplete = 0;
> +
> +	for (i = 0; i < dev_cnt; i++) {
> +		if (!devices[i]) {
> +			tst_res(TWARN, "Missing env var '%s'", needs_devices[i]);
> +			incomplete = 1;
> +		}
> +	}
> +
> +	if (incomplete)
> +		goto err;
> +
> +	struct tst_devlist *new = malloc(sizeof(struct tst_devlist) + dev_cnt * sizeof(void*));
> +
> +	if (!new) {
> +		tst_res(TWARN, "Malloc failed");
> +		goto err;
> +	}
> +
> +	for (i = 0; i < dev_cnt; i++)
> +		new->devs[i] = devices[i];
> +
> +	new->dev_cnt = dev_cnt;
> +
> +	return new;
> +err:
> +	for (i = 0; needs_devices[i]; i++)
> +		free(devices[i]);
> +
> +	return NULL;
> +}
> +
> +struct tst_devlist *tst_devlist_discover(const char *const needs_devices[])
> +{
> +	const char *ltproot = getenv("LTPROOT");
> +	const char *device_discovery = getenv("DEVICE_DISCOVERY");
> +	char buf[2048];
> +	struct tst_devlist *root = NULL;
> +
> +	if (!device_discovery) {
> +		if (!ltproot) {
> +			tst_res(TCONF, "No LTPROOT nor DEVICE_DISCOVERY set!");
> +			return NULL;
> +		}
> +
> +		snprintf(buf, sizeof(buf), "%s/device-discovery.sh", ltproot);
> +
> +		device_discovery = buf;
> +	}
> +
> +	char *devstr = create_devstr(needs_devices);
> +
> +	if (!devstr)
> +		return NULL;
> +
> +	char *cmdline = malloc(strlen(buf) + strlen(devstr) + 3);
> +
> +	if (!cmdline) {
> +		tst_res(TWARN | TERRNO, "Malloc failed");
> +		return NULL;
> +	}
> +
> +	sprintf(cmdline, "%s \"%s\"", device_discovery, devstr);
> +
> +	free(devstr);
> +
> +	FILE *pipe = popen(cmdline, "r");
> +
> +	unsigned int dev_cnt = count_devices(needs_devices);
> +	char *devices[dev_cnt];
> +	unsigned int devlist_cnt = 0;
> +
> +	memset(devices, 0, sizeof(devices));
> +
> +	while (fgets(buf, sizeof(buf), pipe)) {
> +		char *tok, *save, *str = buf;
> +
> +		while ((tok = strtok_r(str, " \n\t", &save))) {
> +			int i = get_index(tok, needs_devices);
> +
> +			if (i < 0) {
> +				tst_res(TWARN, "Invalid env var '%s'", tok);
> +				break;
> +			}
> +
> +			if (devices[i]) {
> +				tst_res(TWARN,
> +				        "Duplicated env var '%s' and '%s'",
> +				        tok, devices[i]);
> +				break;
> +			}
> +
> +			devices[i] = strdup(tok);
> +
> +			str = NULL;
> +		}
> +
> +		struct tst_devlist *list = new_devlist(devices, dev_cnt, needs_devices);
> +
> +		memset(devices, 0, sizeof(devices));
> +
> +		if (!list)
> +			continue;
> +
> +		list->next = root;
> +		root = list;
> +		devlist_cnt++;
> +	}
> +
> +	free(cmdline);
> +
> +	if (!pipe)
> +		tst_res(TWARN | TERRNO, "Failed to execute device discovery");
> +
> +	pclose(pipe);
> +
> +	tst_res(TINFO, "Loaded %i device list(s)", devlist_cnt);
> +
> +	return root;
> +}
> +
> +unsigned int tst_devlist_cnt(struct tst_devlist *self)
> +{
> +	struct tst_devlist *i;
> +	unsigned int cnt = 0;
> +
> +	for (i = self; i; i = i->next)
> +		cnt++;
> +
> +	return cnt;
> +}
> +
> +void tst_devlist_setup(struct tst_devlist *self, unsigned int i)
> +{
> +	struct tst_devlist *l;
> +	unsigned int j, cnt = 0;
> +
> +	for (l = self; l; l = l->next) {
> +		if (i == cnt)
> +			break;
> +		cnt++;
> +	}
> +
> +	for (j = 0; j < l->dev_cnt; j++)
> +		putenv(l->devs[j]);
> +}
> diff --git a/lib/tst_devices.h b/lib/tst_devices.h
> new file mode 100644
> index 000000000..dd0047716
> --- /dev/null
> +++ b/lib/tst_devices.h
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Cyril Hrubis <chrubis@suse.cz>
> + */
> +
> +/*
> + * Device discovery code.
> + */
> +
> +#ifndef TST_DEVICES_H__
> +#define TST_DEVICES_H__
> +
> +struct tst_devlist;
> +
> +/*
> + * Attempts to run a device discovery scripts for a given needs_devices list.
> + */
> +struct tst_devlist *tst_devlist_discover(const char *const needs_devices[]);
> +
> +/*
> + * Returns the number of device configurations.
> + */
> +unsigned int tst_devlist_cnt(struct tst_devlist *self);
> +
> +/*
> + * Setups the environment for a device list with index i.
> + *
> + * Exports the device list base on the variables.
> + */
> +void tst_devlist_setup(struct tst_devlist *self, unsigned int i);
> +
> +#endif /* TST_DEVICES_H__ */
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index e93c88ba5..4ae6d7d52 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -27,6 +27,7 @@
>   #include "tst_wallclock.h"
>   #include "tst_sys_conf.h"
>   #include "tst_kconfig.h"
> +#include "tst_devices.h"
>   
>   #include "old_resource.h"
>   #include "old_device.h"
> @@ -42,6 +43,8 @@ const char *TCID __attribute__((weak));
>   
>   struct tst_test *tst_test;
>   
> +static struct tst_devlist *devlist;
> +
>   static const char *tid;
>   static int iterations = 1;
>   static float duration = -1;
> @@ -471,6 +474,22 @@ static void print_test_tags(void)
>   	printf("\n");
>   }
>   
> +static void print_test_devices(void)
> +{
> +	const char *const *devices = tst_test->needs_devices;
> +	int i;
> +
> +	if (!devices)
> +		return;
> +
> +	printf("\nNeeded devices\n--------------\n");
> +
> +	for (i = 0; devices[i]; i++)
> +		printf("%s\n", devices[i]);
> +
> +	printf("\n");
> +}
> +
>   static void check_option_collision(void)
>   {
>   	unsigned int i, j;
> @@ -550,6 +569,7 @@ static void parse_opts(int argc, char *argv[])
>   		case 'h':
>   			print_help();
>   			print_test_tags();
> +			print_test_devices();
>   			exit(0);
>   		case 'i':
>   			iterations = atoi(optarg);
> @@ -907,6 +927,24 @@ static void do_setup(int argc, char *argv[])
>   				tst_brk(TCONF, "%s driver not available", name);
>   	}
>   
> +	if (tst_test->needs_devices) {
> +		int i;
> +		const char *name;
> +		int all_set = 1;
> +
> +		for (i = 0; (name = tst_test->needs_devices[i]); i++) {
> +			if (!getenv(name))
> +				all_set = 0;
> +		}
> +
> +		if (!all_set) {
> +			devlist = tst_devlist_discover(tst_test->needs_devices);
> +
> +			if (!devlist)
> +				tst_brk(TCONF, "No devices discovered");
> +		}
> +	}
> +
>   	if (tst_test->format_device)
>   		tst_test->needs_device = 1;
>   
> @@ -1335,6 +1373,7 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
>   {
>   	int ret = 0;
>   	unsigned int test_variants = 1;
> +	unsigned int dev_list, device_lists;
>   
>   	lib_pid = getpid();
>   	tst_test = self;
> @@ -1349,14 +1388,21 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
>   	if (tst_test->test_variants)
>   		test_variants = tst_test->test_variants;
>   
> -	for (tst_variant = 0; tst_variant < test_variants; tst_variant++) {
> -		if (tst_test->all_filesystems)
> -			ret |= run_tcases_per_fs();
> -		else
> -			ret |= fork_testrun();
> +	device_lists = tst_devlist_cnt(devlist);
> +
> +	for (dev_list = 0; dev_list < device_lists; dev_list++) {
>   
> -		if (ret & ~(TCONF))
> -			goto exit;
> +		tst_devlist_setup(devlist, dev_list);
> +
> +		for (tst_variant = 0; tst_variant < test_variants; tst_variant++) {
> +			if (tst_test->all_filesystems)
> +				ret |= run_tcases_per_fs();
> +			else
> +				ret |= fork_testrun();
> +
> +			if (ret & ~(TCONF))
> +				goto exit;
> +		}
>   	}
>   
>   exit:
Cyril Hrubis June 25, 2020, 12:34 p.m. UTC | #9
Hi!
> I think we a need:
> 
> 1) A common way to define Hardware capabilities, i.e. UART_RX and 
> UART_TX in your example.
> 
> I suspect for device-driver tests, ltp would be called by a test 
> automation framework. It should be the test automation framework 
> responsibility to setup the equipment per the HW capabilities requested 
> by the test.
>
> So from ltp point of view, the only requirement is to advertise the 
> required capabilities. Of course, this implies a common understanding of 
> the capabilities' tags.

I do agree here, but this will not work until we have the metadata
export in place. Once we have the infrastructure to generate the
description of the tests the test automation will simply load the JSON
file and then run tests and reconfigure the hardware between loops as
needed.

But that would mean that we will not have an upstream solution until we
get rid of ltp-pan and runtest files. This is fine with me as I actually
want to get the metadata generator in LTP tree soon enough.

> 2) A way to set platform-specific values when required. Ideally the test 
> logic can figure out the values to use dynamically but for some test 
> cases, it is required to statically defined them based on the platform 
> the test is running on.
> 
> In ltp-ddt we added this functionality as platform overrides 
> http://arago-project.org/git/projects/?p=test-automation/ltp-ddt.git;a=blob;f=README-DDT;h=78b79cd3ca0f66a6ef30b5dc05737188c146a9ca;hb=HEAD#l46, 
> borrowing an idea from OE/Yocto world. I think a different approach 
> where these info is maintained in a separate file with an API that it is 
> called by the test case logic would work. However, I think that this 
> information is not lab-specific but board-specific and it should be part 
> of ltp.

I'm not convinced that this belongs to LTP git but we can setup a shared
git repo for platform descriptions if things go well.
diff mbox series

Patch

diff --git a/device-discovery.sh b/device-discovery.sh
new file mode 100755
index 000000000..08460c41f
--- /dev/null
+++ b/device-discovery.sh
@@ -0,0 +1,5 @@ 
+#!/bin/sh
+
+if [ "$1" = "UART_RX-UART_TX" ]; then
+	echo "UART_RX=/dev/ttyUSB0 UART_TX=/dev/ttyUSB0"
+fi
diff --git a/include/tst_test.h b/include/tst_test.h
index b84f7b9dd..3c3693098 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -215,6 +215,9 @@  struct tst_test {
 	/* NULL terminated array of needed kernel drivers */
 	const char * const *needs_drivers;
 
+	/* NULL terminated array of devices */
+	const char *const *needs_devices;
+
 	/*
 	 * NULL terminated array of (/proc, /sys) files to save
 	 * before setup and restore after cleanup
diff --git a/lib/tst_devices.c b/lib/tst_devices.c
new file mode 100644
index 000000000..087e2021b
--- /dev/null
+++ b/lib/tst_devices.c
@@ -0,0 +1,228 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+/*
+ * Device discovery code.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "tst_devices.h"
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+
+struct tst_devlist {
+	struct tst_devlist *next;
+	unsigned int dev_cnt;
+	char *devs[];
+};
+
+static char *create_devstr(const char *const needs_devices[])
+{
+	unsigned int i;
+	size_t len = 1;
+
+	for (i = 0; needs_devices[i]; i++)
+		len += strlen(needs_devices[i]) + 1;
+
+	char *res = malloc(len);
+
+	if (!res) {
+		tst_res(TWARN | TERRNO, "Malloc failed");
+		return NULL;
+	}
+
+	char *p = res;
+
+	for (i = 0; needs_devices[i]; i++) {
+		strcpy(p, needs_devices[i]);
+		p += strlen(needs_devices[i]);
+		if (needs_devices[i+1]) {
+			p[0] = '-';
+			p++;
+		}
+	}
+
+	tst_res(TINFO, "Device discovery string '%s'", res);
+
+	return res;
+}
+
+static int get_index(const char *env, const char *const needs_devices[])
+{
+	char *p = strdup(env);
+	char *save;
+	char *name = strtok_r(p, "=", &save);
+	unsigned int i;
+
+	for (i = 0; needs_devices[i]; i++)
+		if (!strcmp(needs_devices[i], name))
+			return i;
+
+	return -1;
+}
+
+static unsigned int count_devices(const char *const needs_devices[])
+{
+	unsigned int i = 0;
+
+	while (needs_devices[i])
+		i++;
+
+	return i;
+}
+
+static struct tst_devlist *new_devlist(char *devices[], unsigned int dev_cnt,
+                                       const char *const needs_devices[])
+{
+	unsigned int i;
+	int incomplete = 0;
+
+	for (i = 0; i < dev_cnt; i++) {
+		if (!devices[i]) {
+			tst_res(TWARN, "Missing env var '%s'", needs_devices[i]);
+			incomplete = 1;
+		}
+	}
+
+	if (incomplete)
+		goto err;
+
+	struct tst_devlist *new = malloc(sizeof(struct tst_devlist) + dev_cnt * sizeof(void*));
+
+	if (!new) {
+		tst_res(TWARN, "Malloc failed");
+		goto err;
+	}
+
+	for (i = 0; i < dev_cnt; i++)
+		new->devs[i] = devices[i];
+
+	new->dev_cnt = dev_cnt;
+
+	return new;
+err:
+	for (i = 0; needs_devices[i]; i++)
+		free(devices[i]);
+
+	return NULL;
+}
+
+struct tst_devlist *tst_devlist_discover(const char *const needs_devices[])
+{
+	const char *ltproot = getenv("LTPROOT");
+	const char *device_discovery = getenv("DEVICE_DISCOVERY");
+	char buf[2048];
+	struct tst_devlist *root = NULL;
+
+	if (!device_discovery) {
+		if (!ltproot) {
+			tst_res(TCONF, "No LTPROOT nor DEVICE_DISCOVERY set!");
+			return NULL;
+		}
+
+		snprintf(buf, sizeof(buf), "%s/device-discovery.sh", ltproot);
+
+		device_discovery = buf;
+	}
+
+	char *devstr = create_devstr(needs_devices);
+
+	if (!devstr)
+		return NULL;
+
+	char *cmdline = malloc(strlen(buf) + strlen(devstr) + 3);
+
+	if (!cmdline) {
+		tst_res(TWARN | TERRNO, "Malloc failed");
+		return NULL;
+	}
+
+	sprintf(cmdline, "%s \"%s\"", device_discovery, devstr);
+
+	free(devstr);
+
+	FILE *pipe = popen(cmdline, "r");
+
+	unsigned int dev_cnt = count_devices(needs_devices);
+	char *devices[dev_cnt];
+	unsigned int devlist_cnt = 0;
+
+	memset(devices, 0, sizeof(devices));
+
+	while (fgets(buf, sizeof(buf), pipe)) {
+		char *tok, *save, *str = buf;
+
+		while ((tok = strtok_r(str, " \n\t", &save))) {
+			int i = get_index(tok, needs_devices);
+
+			if (i < 0) {
+				tst_res(TWARN, "Invalid env var '%s'", tok);
+				break;
+			}
+
+			if (devices[i]) {
+				tst_res(TWARN,
+				        "Duplicated env var '%s' and '%s'",
+				        tok, devices[i]);
+				break;
+			}
+
+			devices[i] = strdup(tok);
+
+			str = NULL;
+		}
+
+		struct tst_devlist *list = new_devlist(devices, dev_cnt, needs_devices);
+
+		memset(devices, 0, sizeof(devices));
+
+		if (!list)
+			continue;
+
+		list->next = root;
+		root = list;
+		devlist_cnt++;
+	}
+
+	free(cmdline);
+
+	if (!pipe)
+		tst_res(TWARN | TERRNO, "Failed to execute device discovery");
+
+	pclose(pipe);
+
+	tst_res(TINFO, "Loaded %i device list(s)", devlist_cnt);
+
+	return root;
+}
+
+unsigned int tst_devlist_cnt(struct tst_devlist *self)
+{
+	struct tst_devlist *i;
+	unsigned int cnt = 0;
+
+	for (i = self; i; i = i->next)
+		cnt++;
+
+	return cnt;
+}
+
+void tst_devlist_setup(struct tst_devlist *self, unsigned int i)
+{
+	struct tst_devlist *l;
+	unsigned int j, cnt = 0;
+
+	for (l = self; l; l = l->next) {
+		if (i == cnt)
+			break;
+		cnt++;
+	}
+
+	for (j = 0; j < l->dev_cnt; j++)
+		putenv(l->devs[j]);
+}
diff --git a/lib/tst_devices.h b/lib/tst_devices.h
new file mode 100644
index 000000000..dd0047716
--- /dev/null
+++ b/lib/tst_devices.h
@@ -0,0 +1,32 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+/*
+ * Device discovery code.
+ */
+
+#ifndef TST_DEVICES_H__
+#define TST_DEVICES_H__
+
+struct tst_devlist;
+
+/*
+ * Attempts to run a device discovery scripts for a given needs_devices list.
+ */
+struct tst_devlist *tst_devlist_discover(const char *const needs_devices[]);
+
+/*
+ * Returns the number of device configurations.
+ */
+unsigned int tst_devlist_cnt(struct tst_devlist *self);
+
+/*
+ * Setups the environment for a device list with index i.
+ *
+ * Exports the device list base on the variables.
+ */
+void tst_devlist_setup(struct tst_devlist *self, unsigned int i);
+
+#endif /* TST_DEVICES_H__ */
diff --git a/lib/tst_test.c b/lib/tst_test.c
index e93c88ba5..4ae6d7d52 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -27,6 +27,7 @@ 
 #include "tst_wallclock.h"
 #include "tst_sys_conf.h"
 #include "tst_kconfig.h"
+#include "tst_devices.h"
 
 #include "old_resource.h"
 #include "old_device.h"
@@ -42,6 +43,8 @@  const char *TCID __attribute__((weak));
 
 struct tst_test *tst_test;
 
+static struct tst_devlist *devlist;
+
 static const char *tid;
 static int iterations = 1;
 static float duration = -1;
@@ -471,6 +474,22 @@  static void print_test_tags(void)
 	printf("\n");
 }
 
+static void print_test_devices(void)
+{
+	const char *const *devices = tst_test->needs_devices;
+	int i;
+
+	if (!devices)
+		return;
+
+	printf("\nNeeded devices\n--------------\n");
+
+	for (i = 0; devices[i]; i++)
+		printf("%s\n", devices[i]);
+
+	printf("\n");
+}
+
 static void check_option_collision(void)
 {
 	unsigned int i, j;
@@ -550,6 +569,7 @@  static void parse_opts(int argc, char *argv[])
 		case 'h':
 			print_help();
 			print_test_tags();
+			print_test_devices();
 			exit(0);
 		case 'i':
 			iterations = atoi(optarg);
@@ -907,6 +927,24 @@  static void do_setup(int argc, char *argv[])
 				tst_brk(TCONF, "%s driver not available", name);
 	}
 
+	if (tst_test->needs_devices) {
+		int i;
+		const char *name;
+		int all_set = 1;
+
+		for (i = 0; (name = tst_test->needs_devices[i]); i++) {
+			if (!getenv(name))
+				all_set = 0;
+		}
+
+		if (!all_set) {
+			devlist = tst_devlist_discover(tst_test->needs_devices);
+
+			if (!devlist)
+				tst_brk(TCONF, "No devices discovered");
+		}
+	}
+
 	if (tst_test->format_device)
 		tst_test->needs_device = 1;
 
@@ -1335,6 +1373,7 @@  void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
 {
 	int ret = 0;
 	unsigned int test_variants = 1;
+	unsigned int dev_list, device_lists;
 
 	lib_pid = getpid();
 	tst_test = self;
@@ -1349,14 +1388,21 @@  void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
 	if (tst_test->test_variants)
 		test_variants = tst_test->test_variants;
 
-	for (tst_variant = 0; tst_variant < test_variants; tst_variant++) {
-		if (tst_test->all_filesystems)
-			ret |= run_tcases_per_fs();
-		else
-			ret |= fork_testrun();
+	device_lists = tst_devlist_cnt(devlist);
+
+	for (dev_list = 0; dev_list < device_lists; dev_list++) {
 
-		if (ret & ~(TCONF))
-			goto exit;
+		tst_devlist_setup(devlist, dev_list);
+
+		for (tst_variant = 0; tst_variant < test_variants; tst_variant++) {
+			if (tst_test->all_filesystems)
+				ret |= run_tcases_per_fs();
+			else
+				ret |= fork_testrun();
+
+			if (ret & ~(TCONF))
+				goto exit;
+		}
 	}
 
 exit: