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

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> When CONFIG_SMP is not selected, CONFIG_RPS will not be enabled. Causes the
> kernel to not create rx queues in sysfs[1] when creating tun devices.
> Uevent02 will check if the rx queue exists, adjust the uevents dynamically
> in setup() to pass this test.
> 
> [1]: https://github.com/torvalds/linux/blob/3245cb65fd91cd514801bf91f5a3066d562f0ac4/net/core/net-sysfs.c#L1109
> 
> Signed-off-by: Dylan Jhong <dylan@andestech.com>
> ---
>  testcases/kernel/uevents/uevent02.c | 168 ++++++++++++++++------------
>  1 file changed, 96 insertions(+), 72 deletions(-)
> 
> diff --git a/testcases/kernel/uevents/uevent02.c b/testcases/kernel/uevents/uevent02.c
> index ce0cf757d..dcdf393d4 100644
> --- a/testcases/kernel/uevents/uevent02.c
> +++ b/testcases/kernel/uevents/uevent02.c
> @@ -18,11 +18,77 @@
>  #include <linux/if.h>
>  #include <linux/if_tun.h>
>  
> +#include "tst_kconfig.h"
>  #include "tst_test.h"
>  
>  #include "uevent.h"
>  
>  #define TUN_PATH "/dev/net/tun"
> +#define CONFIG_RPS "CONFIG_RPS"
> +#define MAX_UEVENTS 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",
> +	}
> +};
> +const struct uevent_desc *uevents[MAX_UEVENTS];

Now it makes sense to keep them here, but please make them all static.

>  static void generate_tun_uevents(void)
>  {
> @@ -44,78 +110,6 @@ static void verify_uevent(void)
>  {
>  	int pid, fd;
>  
> -	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",
> -		}
> -	};
> -
> -	const struct uevent_desc *const uevents[] = {
> -		&add,
> -		&add_rx,
> -		&add_tx,
> -		&rem_rx,
> -		&rem_tx,
> -		&rem,
> -		NULL
> -	};
> -
>  	pid = SAFE_FORK();
>  	if (!pid) {
>  		fd = open_uevent_netlink();
> @@ -131,7 +125,37 @@ static void verify_uevent(void)
>  	wait_for_pid(pid);
>  }
>  
> +static void setup(void)
> +{
> +	struct tst_kconfig_var kconfig = {
> +		.id = CONFIG_RPS,
> +		.id_len = sizeof(CONFIG_RPS)-1,
> +	};
> +	int i = 0;
> +
> +	tst_kconfig_read(&kconfig, 1);
> +
> +	if (kconfig.choice != 'y') {
> +		tst_res(TINFO, "CONFIG_RPS not supported by kernel");
> +		uevents[i++] = &add;
> +		uevents[i++] = &add_tx;
> +		uevents[i++] = &rem_tx;
> +		uevents[i++] = &rem;
> +		uevents[i++] = NULL;
> +	} else {
> +		tst_res(TINFO, "CONFIG_RPS supported by kernel");
> +		uevents[i++] = &add;
> +		uevents[i++] = &add_rx;
> +		uevents[i++] = &add_tx;
> +		uevents[i++] = &rem_rx;
> +		uevents[i++] = &rem_tx;
> +		uevents[i++] = &rem;
> +		uevents[i++] = NULL;
> +	}

I actually liked the version where we had the two ifs in the middle of
the initialization a bit better, since it was obviously clear what the
difference between the two cases is.

> +}
> +
>  static struct tst_test test = {
> +	.setup = setup,
>  	.test_all = verify_uevent,
>  	.forks_child = 1,
>  	.needs_checkpoints = 1,
> -- 
> 2.34.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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