lib/tst_test.c: add 'needs_drivers' option with tst_check_drivers cmd

The drivers are checked with tst_check_drivers.

Signed-off-by Alexey Kodanev <[email protected]>

---

 testcases/lib/tst_test.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

Hi Alexey,

testcases/lib/tst_test.sh tst_run() · line 382

Quoted 7 lines

The drivers are checked with tst_check_drivers.

Signed-off-by Alexey Kodanev <[email protected]>

---

testcases/lib/tst_test.sh +1 -0

 	tst_test_cmds $TST_NEEDS_CMDS
+	tst_test_drivers $TST_NEEDS_DRIVERS

I'm sorry here must be some fix:

(quoted diff) +3 -0

+	if [ -n "$TST_NEEDS_DRIVERS" ]; then
+		tst_test_drivers $TST_NEEDS_DRIVERS
+	fi

Otherwise tests without $TST_NEEDS_DRIVERS TCONF:
tcp_ipsec_vti 1 TCONF: 'Please provide kernel driver list' driver not found

It should parse iproute commands output and exit the test with TCONF if there are certain messages returned from iproute/RTNETLINK.

Signed-off-by Alexey Kodanev <[email protected]>

---
 testcases/lib/tst_test.sh |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

Signed-off-by Alexey Kodanev <[email protected]>

---
 testcases/network/stress/ipsec/ipsec_lib.sh |   21 +++------------------
 1 files changed, 3 insertions(+), 18 deletions(-)

Quoted 6 lines

Quoted 6 lines

Hi Petr,

Since the server waits for requests from the client, the timeout value for UDP/DCCP is the same as for the other protocols. Also the server starts earlier than the client, so it should wait some time to get the client requests.

Quoted 6 lines

Quoted 2 lines

I've changed the client side only because either request from the client or reply from the server might be lost.

Thanks for your explanation.
I mean: UDP itself is the only protocol which don't support listen() so netstress server using UDP timeouts after some time. The default is 100ms.

Hmm, for the server the default should be 60 sec. Does it timeout earlier?

Hi Alexey,

Quoted 7 lines

Quoted 6 lines

Quoted 6 lines

Hi Petr,

Since the server waits for requests from the client, the timeout value for UDP/DCCP is the same as for the other protocols. Also the server starts earlier than the client, so it should wait some time to get the client requests.

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

I've changed the client side only because either request from the client or reply from the server might be lost.

Thanks for your explanation.
I mean: UDP itself is the only protocol which don't support listen() so netstress server using UDP timeouts after some time. The default is 100ms.

Hmm, for the server the default should be 60 sec. Does it timeout earlier?

Yes. The timeout is affected by value of -m. Thats' what I meant by my previous:

$ date +"%T.%3N"; testcases/network/netstress/netstress -m 1 -T udp; date +"%T.%3N"
15:52:34.501
tst_test.c:1015: INFO: Timeout per run is 0h 05m 00s
The timeout is actually 1ms. Try it with -m 1000 and it'll be indeed 1s
netstress.c:917: INFO: max requests '3'
netstress.c:944: INFO: using UDP
netstress.c:676: INFO: assigning a name to the server socket...
netstress.c:683: INFO: bind to port 47728
netstress.c:575: FAIL: recv failed, sock '3'
netstress.c:642: BROK: Server closed
...
15:52:34.516

This is caused by udp/udp_lite not using listen(), but maybe is should be at least mentioned in help (if wanted behavior).

...

Quoted 11 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+	tst_netload -T udp -r 1 -a 1 -H $addr

Unfortunately, it won't work this way. tst_netload is using requests/response model for UDP. You have to make sure the server can send responses.

Also, netstress UDP/DCCP client allows to pass 'max_etime_cnt' (12) failed requests after which it will double receive timeout, starting from 100ms up to 3.2s. This is about 30 sec if there is no connection. With a single request or below max_etime_cnt + 1 value it will always pass. I guess we need to set the proper request parameter limit for UDP/DCCP protocol or check whether the number of timeout errors is equal or greater than the number of requests.

Applied the fix for this bug:
4ceb442b56ad ("netstress: handle timeout errors for a small number of requests")

The drivers are checked with modprobe. If modrpobe is not available

on the system, the checks are silently skipped.

Signed-off-by Alexey Kodanev <[email protected]>

---
 include/tst_test.h                |    5 +++++
 lib/tst_test.c                    |   26 ++++++++++++++++++++++++++
 testcases/lib/.gitignore          |    1 +
 testcases/lib/Makefile            |    2 +-
 testcases/lib/tst_check_drivers.c |   24 ++++++++++++++++++++++++
 5 files changed, 57 insertions(+), 1 deletions(-)
 create mode 100644 testcases/lib/tst_check_drivers.c

Hi Alexey,

Quoted 1 line

The drivers are checked with modprobe. If modrpobe is not available

^ typo modrpobe>> on the system, the checks are silently skipped.

Quoted 1 line

Signed-off-by Alexey Kodanev <[email protected]>

Acked-by Petr Vorel <[email protected]>

Quoted 8 lines

---
 include/tst_test.h                |    5 +++++
 lib/tst_test.c                    |   26 ++++++++++++++++++++++++++
 testcases/lib/.gitignore          |    1 +
 testcases/lib/Makefile            |    2 +-
 testcases/lib/tst_check_drivers.c |   24 ++++++++++++++++++++++++
 5 files changed, 57 insertions(+), 1 deletions(-)
 create mode 100644 testcases/lib/tst_check_drivers.c

It'd be nice to add some docs into doc/test-writing-guidelines.txt.

include/tst_test.h +5 -0

index 98dacf3..221796d 100644
@@ -170,6 +170,9 @@ struct tst_test {
 
 	/* NULL terminated array of resource file names */
 	const char *const *resource_files;
+
+	/* NULL terminated array of needed kernel drivers */
+	const char * const *needs_drivers;
 };
 
 /*

@@ -219,6 +222,8 @@ const char *tst_strstatus(int status);
 
 void tst_set_timeout(int timeout);
 
+int tst_check_drivers(void);
+
 #ifndef TST_NO_DEFAULT_MAIN
 
 static struct tst_test test;

lib/tst_test.c +12 -0

index 2f3d357..6cb74cf 100644
@@ -645,6 +645,29 @@ static int needs_tmpdir(void)
 	       tst_test->needs_checkpoints;
 }
 
+int tst_check_drivers(void)
+{
+	const char *name;
+	int i, res;
+
+	for (i = 0; (name = tst_test->needs_drivers[i]); ++i) {
+		const char * const argv[] = { "modprobe", name, NULL };
+
+		res = tst_run_cmd_(NULL, argv, "/dev/null", "/dev/null", 1);
+		if (res == 255)
+			return res; /* it looks like modprobe not available */
+		if (res) {

Hi...
I do not like much that we change the behavir based on tst_test content. Maybe it would be cleaner to define a tst_check_driver(const char *name), then we can do the loop over tst_test->needs_drivers in the tst_test.c library and loop over argv in the shell helper.

Quoted 5 lines

Hi...
I do not like much that we change the behavir based on tst_test content. Maybe it would be cleaner to define a tst_check_driver(const char *name), then we can do the loop over tst_test->needs_drivers in the tst_test.c library and loop over argv in the shell helper.

Hi Cyril,

OK, will send the 2nd version.

Thanks,
Alexey

Hi,

Quoted 6 lines · Cyril Hrubis

Quoted 5 lines

Hi...
I do not like much that we change the behavir based on tst_test content. Maybe it would be cleaner to define a tst_check_driver(const char *name), then we can do the loop over tst_test->needs_drivers in the tst_test.c library and loop over argv in the shell helper.

+1 it makes sense.

Quoted 3 lines

Hi Cyril,

OK, will send the 2nd version.

BTW it'd be nice to have support to load module with parameters.