Re: [LTP] [PATCH] kernel/uevent: Adjust the number of uevents dynamically in uevent02

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> Signed-off-by: Dylan Jhong <dylan@andestech.com>
> ---
>  testcases/kernel/uevents/uevent02.c | 146 ++++++++++++++--------------
>  1 file changed, 73 insertions(+), 73 deletions(-)
> 
> diff --git a/testcases/kernel/uevents/uevent02.c b/testcases/kernel/uevents/uevent02.c
> index ce0cf757d..059320f1c 100644
> --- a/testcases/kernel/uevents/uevent02.c
> +++ b/testcases/kernel/uevents/uevent02.c
> @@ -19,11 +19,71 @@
>  #include <linux/if_tun.h>
>  
>  #include "tst_test.h"
> +#include "tst_private.h"

This header is called private for a reason, the tst_kconfig_get() is not
meant to be used from tests, you are supposed to call tst_config_read()
as it's done in shmget02.c for example.

>  #include "uevent.h"
>  
>  #define TUN_PATH "/dev/net/tun"
>  
> +#define MAX_UEVENT 7
> +
> +struct uevent_desc add = {
> +	.msg = "add@/devices/virtual/net/ltp-tun0",
> +	.value_cnt = 4,
> +	.values = (const char*[]) {
> +		"ACTION=add",
> +		"DEVPATH=/devices/virtual/net/ltp-tun0",
> +		"SUBSYSTEM=net",
> +		"INTERFACE=ltp-tun0",
> +	}
> +};
> +struct uevent_desc add_rx = {
> +	.msg = "add@/devices/virtual/net/ltp-tun0/queues/rx-0",
> +	.value_cnt = 3,
> +	.values = (const char*[]) {
> +		"ACTION=add",
> +		"DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
> +		"SUBSYSTEM=queues",
> +	}
> +};
> +struct uevent_desc add_tx = {
> +	.msg = "add@/devices/virtual/net/ltp-tun0/queues/tx-0",
> +	.value_cnt = 3,
> +	.values = (const char*[]) {
> +		"ACTION=add",
> +		"DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
> +		"SUBSYSTEM=queues",
> +	}
> +};
> +struct uevent_desc rem_rx = {
> +	.msg = "remove@/devices/virtual/net/ltp-tun0/queues/rx-0",
> +	.value_cnt = 3,
> +	.values = (const char*[]) {
> +		"ACTION=remove",
> +		"DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
> +		"SUBSYSTEM=queues",
> +	}
> +};
> +struct uevent_desc rem_tx = {
> +	.msg = "remove@/devices/virtual/net/ltp-tun0/queues/tx-0",
> +	.value_cnt = 3,
> +	.values = (const char*[]) {
> +		"ACTION=remove",
> +		"DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
> +		"SUBSYSTEM=queues",
> +	}
> +};
> +struct uevent_desc rem = {
> +	.msg = "remove@/devices/virtual/net/ltp-tun0",
> +	.value_cnt = 4,
> +	.values = (const char*[]) {
> +		"ACTION=remove",
> +		"DEVPATH=/devices/virtual/net/ltp-tun0",
> +		"SUBSYSTEM=net",
> +		"INTERFACE=ltp-tun0",
> +	}
> +};

Why do we have to move these outside of the function? I do not see a
single reason to do so.

> +	const struct uevent_desc *uevents[MAX_UEVENT];
> +	int pid, fd, i = 0;
> +	int has_RPS = tst_kconfig_get("CONFIG_RPS");

Getting the flag should be done once in the test setup, otherwise kernel
config will be parsed in each iteration of the test.

> +	uevents[i++] = &add;
> +	if (has_RPS)
> +		uevents[i++] = &add_rx;
> +	uevents[i++] = &add_tx;
> +	if (has_RPS)
> +		uevents[i++] = &rem_rx;
> +	uevents[i++] = &rem_tx;
> +	uevents[i++] = &rem;
> +	uevents[i++] = NULL;



-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp