2023-04-01 09:35:16

by Ivan Orlov

[permalink] [raw]
Subject: [PATCH] selftests: media_tests: Add new subtest to video_device_test

Add new subtest to video_device_test to cover the VIDIOC_G_PRIORITY
and VIDIOC_S_PRIORITY ioctl calls. This test tries to set the priority
associated with the file descriptior via ioctl VIDIOC_S_PRIORITY
command from V4L2 API. After that, the test tries to get the new
priority via VIDIOC_G_PRIORITY ioctl command and compares the result
with the v4l2_priority it set before. At the end, the test restores the
old priority.

This test will increase the code coverage for video_device_test, so
I think it might be useful. Additionally, this patch will refactor the
video_device_test a little bit, according to the new functionality.

Signed-off-by: Ivan Orlov <[email protected]>
---
.../selftests/media_tests/video_device_test.c | 131 +++++++++++++-----
1 file changed, 93 insertions(+), 38 deletions(-)

diff --git a/tools/testing/selftests/media_tests/video_device_test.c b/tools/testing/selftests/media_tests/video_device_test.c
index 0f6aef2e2593..5e6f65ad2ca3 100644
--- a/tools/testing/selftests/media_tests/video_device_test.c
+++ b/tools/testing/selftests/media_tests/video_device_test.c
@@ -13,18 +13,9 @@
* in the Kselftest run. This test should be run when hardware and driver
* that makes use of V4L2 API is present.
*
- * This test opens user specified Video Device and calls video ioctls in a
- * loop once every 10 seconds.
- *
* Usage:
* sudo ./video_device_test -d /dev/videoX
- *
- * While test is running, remove the device or unbind the driver and
- * ensure there are no use after free errors and other Oops in the
- * dmesg.
- * When possible, enable KaSan kernel config option for use-after-free
- * error detection.
-*/
+ */

#include <stdio.h>
#include <unistd.h>
@@ -37,45 +28,67 @@
#include <time.h>
#include <linux/videodev2.h>

-int main(int argc, char **argv)
+#define PRIORITY_MAX 4
+
+int priority_test(int fd)
{
- int opt;
- char video_dev[256];
- int count;
- struct v4l2_tuner vtuner;
- struct v4l2_capability vcap;
+ /* This test will try to update the priority associated with a file descriptor */
+
+ enum v4l2_priority old_priority, new_priority, priority_to_compare;
int ret;
- int fd;
+ int result = 0;

- if (argc < 2) {
- printf("Usage: %s [-d </dev/videoX>]\n", argv[0]);
- exit(-1);
+ ret = ioctl(fd, VIDIOC_G_PRIORITY, &old_priority);
+ if (ret < 0) {
+ printf("Failed to get priority: %s\n", strerror(errno));
+ return -1;
+ }
+ new_priority = (old_priority + 1) % PRIORITY_MAX;
+ ret = ioctl(fd, VIDIOC_S_PRIORITY, &new_priority);
+ if (ret < 0) {
+ printf("Failed to set priority: %s\n", strerror(errno));
+ return -1;
+ }
+ ret = ioctl(fd, VIDIOC_G_PRIORITY, &priority_to_compare);
+ if (ret < 0) {
+ printf("Failed to get new priority: %s\n", strerror(errno));
+ result = -1;
+ goto cleanup;
+ }
+ if (priority_to_compare != new_priority) {
+ printf("Priority wasn't set - test failed\n");
+ result = -1;
}

- /* Process arguments */
- while ((opt = getopt(argc, argv, "d:")) != -1) {
- switch (opt) {
- case 'd':
- strncpy(video_dev, optarg, sizeof(video_dev) - 1);
- video_dev[sizeof(video_dev)-1] = '\0';
- break;
- default:
- printf("Usage: %s [-d </dev/videoX>]\n", argv[0]);
- exit(-1);
- }
+cleanup:
+ ret = ioctl(fd, VIDIOC_S_PRIORITY, &old_priority);
+ if (ret < 0) {
+ printf("Failed to restore priority: %s\n", strerror(errno));
+ return -1;
}
+ return result;
+}
+
+int loop_test(int fd)
+{
+ /*
+ * This test opens user specified Video Device and calls video ioctls in a
+ * loop once every 10 seconds.
+ * While test is running, remove the device or unbind the driver and
+ * ensure there are no use after free errors and other Oops in the
+ * dmesg.
+ * When possible, enable KaSan kernel config option for use-after-free
+ * error detection.
+ */
+ int count;
+ struct v4l2_tuner vtuner;
+ struct v4l2_capability vcap;
+ int ret;

/* Generate random number of interations */
srand((unsigned int) time(NULL));
count = rand();

- /* Open Video device and keep it open */
- fd = open(video_dev, O_RDWR);
- if (fd == -1) {
- printf("Video Device open errno %s\n", strerror(errno));
- exit(-1);
- }
-
printf("\nNote:\n"
"While test is running, remove the device or unbind\n"
"driver and ensure there are no use after free errors\n"
@@ -98,4 +111,46 @@ int main(int argc, char **argv)
sleep(10);
count--;
}
+ return 0;
+}
+
+int main(int argc, char **argv)
+{
+ int opt;
+ char video_dev[256];
+ int fd;
+ int test_result;
+
+ if (argc < 2) {
+ printf("Usage: %s [-d </dev/videoX>]\n", argv[0]);
+ exit(-1);
+ }
+
+ /* Process arguments */
+ while ((opt = getopt(argc, argv, "d:")) != -1) {
+ switch (opt) {
+ case 'd':
+ strncpy(video_dev, optarg, sizeof(video_dev) - 1);
+ video_dev[sizeof(video_dev)-1] = '\0';
+ break;
+ default:
+ printf("Usage: %s [-d </dev/videoX>]\n", argv[0]);
+ exit(-1);
+ }
+ }
+
+ /* Open Video device and keep it open */
+ fd = open(video_dev, O_RDWR);
+ if (fd == -1) {
+ printf("Video Device open errno %s\n", strerror(errno));
+ exit(-1);
+ }
+
+ test_result = priority_test(fd);
+ if (!test_result)
+ printf("Priority test - PASSED\n");
+ else
+ printf("Priority test - FAILED\n");
+
+ loop_test(fd);
}
--
2.34.1


2023-04-05 19:13:53

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH] selftests: media_tests: Add new subtest to video_device_test

On 4/1/23 03:22, Ivan Orlov wrote:
> Add new subtest to video_device_test to cover the VIDIOC_G_PRIORITY
> and VIDIOC_S_PRIORITY ioctl calls. This test tries to set the priority
> associated with the file descriptior via ioctl VIDIOC_S_PRIORITY
> command from V4L2 API. After that, the test tries to get the new
> priority via VIDIOC_G_PRIORITY ioctl command and compares the result
> with the v4l2_priority it set before. At the end, the test restores the
> old priority.
>
> This test will increase the code coverage for video_device_test, so
> I think it might be useful. Additionally, this patch will refactor the
> video_device_test a little bit, according to the new functionality.
>
> Signed-off-by: Ivan Orlov <[email protected]>
> ---
> .../selftests/media_tests/video_device_test.c | 131 +++++++++++++-----
> 1 file changed, 93 insertions(+), 38 deletions(-)
>
> diff --git a/tools/testing/selftests/media_tests/video_device_test.c b/tools/testing/selftests/media_tests/video_device_test.c
> index 0f6aef2e2593..5e6f65ad2ca3 100644
> --- a/tools/testing/selftests/media_tests/video_device_test.c
> +++ b/tools/testing/selftests/media_tests/video_device_test.c
> @@ -13,18 +13,9 @@
> * in the Kselftest run. This test should be run when hardware and driver
> * that makes use of V4L2 API is present.
> *
> - * This test opens user specified Video Device and calls video ioctls in a
> - * loop once every 10 seconds.
> - *
> * Usage:
> * sudo ./video_device_test -d /dev/videoX
> - *
> - * While test is running, remove the device or unbind the driver and
> - * ensure there are no use after free errors and other Oops in the
> - * dmesg.
> - * When possible, enable KaSan kernel config option for use-after-free
> - * error detection.
> -*/
> + */

Leave this text here so it is easy to spot. There is no need to move this
down.

>
> #include <stdio.h>
> #include <unistd.h>
> @@ -37,45 +28,67 @@
> #include <time.h>
> #include <linux/videodev2.h>
>
> -int main(int argc, char **argv)
> +#define PRIORITY_MAX 4
> +
> +int priority_test(int fd)
> {
> - int opt;
> - char video_dev[256];
> - int count;
> - struct v4l2_tuner vtuner;
> - struct v4l2_capability vcap;
> + /* This test will try to update the priority associated with a file descriptor */
> +
> + enum v4l2_priority old_priority, new_priority, priority_to_compare;
> int ret;
> - int fd;
> + int result = 0;
>
> - if (argc < 2) {
> - printf("Usage: %s [-d </dev/videoX>]\n", argv[0]);
> - exit(-1);
> + ret = ioctl(fd, VIDIOC_G_PRIORITY, &old_priority);
> + if (ret < 0) {
> + printf("Failed to get priority: %s\n", strerror(errno));
> + return -1;
> + }
> + new_priority = (old_priority + 1) % PRIORITY_MAX;
> + ret = ioctl(fd, VIDIOC_S_PRIORITY, &new_priority);
> + if (ret < 0) {
> + printf("Failed to set priority: %s\n", strerror(errno));
> + return -1;
> + }
> + ret = ioctl(fd, VIDIOC_G_PRIORITY, &priority_to_compare);
> + if (ret < 0) {
> + printf("Failed to get new priority: %s\n", strerror(errno));
> + result = -1;
> + goto cleanup;
> + }
> + if (priority_to_compare != new_priority) {
> + printf("Priority wasn't set - test failed\n");
> + result = -1;
> }
>
> - /* Process arguments */
> - while ((opt = getopt(argc, argv, "d:")) != -1) {
> - switch (opt) {
> - case 'd':
> - strncpy(video_dev, optarg, sizeof(video_dev) - 1);
> - video_dev[sizeof(video_dev)-1] = '\0';
> - break;
> - default:
> - printf("Usage: %s [-d </dev/videoX>]\n", argv[0]);
> - exit(-1);
> - }
> +cleanup:
> + ret = ioctl(fd, VIDIOC_S_PRIORITY, &old_priority);
> + if (ret < 0) {
> + printf("Failed to restore priority: %s\n", strerror(errno));
> + return -1;
> }
> + return result;
> +}
> +
> +int loop_test(int fd)
> +{
> + /*
> + * This test opens user specified Video Device and calls video ioctls in a
> + * loop once every 10 seconds.
> + * While test is running, remove the device or unbind the driver and
> + * ensure there are no use after free errors and other Oops in the
> + * dmesg.
> + * When possible, enable KaSan kernel config option for use-after-free
> + * error detection.
> + */
> + int count;
> + struct v4l2_tuner vtuner;
> + struct v4l2_capability vcap;
> + int ret;
>
> /* Generate random number of interations */
> srand((unsigned int) time(NULL));
> count = rand();
>
> - /* Open Video device and keep it open */
> - fd = open(video_dev, O_RDWR);
> - if (fd == -1) {
> - printf("Video Device open errno %s\n", strerror(errno));
> - exit(-1);
> - }
> -
> printf("\nNote:\n"
> "While test is running, remove the device or unbind\n"
> "driver and ensure there are no use after free errors\n"
> @@ -98,4 +111,46 @@ int main(int argc, char **argv)
> sleep(10);
> count--;
> }
> + return 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> + int opt;
> + char video_dev[256];
> + int fd;
> + int test_result;
> +
> + if (argc < 2) {
> + printf("Usage: %s [-d </dev/videoX>]\n", argv[0]);
> + exit(-1);
> + }
> +
> + /* Process arguments */
> + while ((opt = getopt(argc, argv, "d:")) != -1) {
> + switch (opt) {
> + case 'd':
> + strncpy(video_dev, optarg, sizeof(video_dev) - 1);
> + video_dev[sizeof(video_dev)-1] = '\0';
> + break;
> + default:
> + printf("Usage: %s [-d </dev/videoX>]\n", argv[0]);
> + exit(-1);
> + }
> + }
> +
> + /* Open Video device and keep it open */
> + fd = open(video_dev, O_RDWR);
> + if (fd == -1) {
> + printf("Video Device open errno %s\n", strerror(errno));
> + exit(-1);
> + }
> +
> + test_result = priority_test(fd);
> + if (!test_result)
> + printf("Priority test - PASSED\n");
> + else
> + printf("Priority test - FAILED\n");
> +
> + loop_test(fd);
> }

Otherwise looks good to me.

thanks,
-- Shuah

2023-04-05 20:05:15

by Ivan Orlov

[permalink] [raw]
Subject: Re: [PATCH] selftests: media_tests: Add new subtest to video_device_test

On 4/5/23 23:03, Shuah Khan wrote:
> Leave this text here so it is easy to spot. There is no need to move this
> down.
>

> Otherwise looks good to me.
>
> thanks,
> -- Shuah

Thank you for the review! I'll send the v2 with the mentioned changes.