diff mbox series

[net-next] selftest: epoll_busy_poll: epoll busy poll tests

Message ID 20240502212013.274758-1-jdamato@fastly.com (mailing list archive)
State New
Headers show
Series [net-next] selftest: epoll_busy_poll: epoll busy poll tests | expand

Commit Message

Joe Damato May 2, 2024, 9:20 p.m. UTC
Add a simple test for the epoll busy poll ioctls.

This test ensures that the ioctls have the expected return codes and
that the kernel properly gets and sets epoll busy poll parameters.

The test can be expanded in the future to do real busy polling (provided
another machine to act as the client is available).

To run the test (use -s for "simple" test):

./epoll_busy_poll -s

On success, nothing is written to stdout/stderr and the exit code is 0.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 tools/testing/selftests/net/.gitignore        |   1 +
 tools/testing/selftests/net/Makefile          |   1 +
 tools/testing/selftests/net/epoll_busy_poll.c | 279 ++++++++++++++++++
 3 files changed, 281 insertions(+)
 create mode 100644 tools/testing/selftests/net/epoll_busy_poll.c

Comments

Jakub Kicinski May 3, 2024, 10:49 p.m. UTC | #1
On Thu,  2 May 2024 21:20:11 +0000 Joe Damato wrote:
> --- a/tools/testing/selftests/net/Makefile
> +++ b/tools/testing/selftests/net/Makefile
> @@ -84,6 +84,7 @@ TEST_GEN_FILES += sctp_hello
>  TEST_GEN_FILES += csum
>  TEST_GEN_FILES += ip_local_port_range
>  TEST_GEN_FILES += bind_wildcard
> +TEST_GEN_FILES += epoll_busy_poll

"GEN" is for files which are built for other tests to use.
IOW unless there's also a wrapper script under TEST_PROGS
(or the C code is itself under TEST_PROGS) this test won't
be executed by most CIs.

FWIW here's how we run the tests in our CI upstream CI:
https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style

>  TEST_PROGS += test_vxlan_mdb.sh
>  TEST_PROGS += test_bridge_neigh_suppress.sh
>  TEST_PROGS += test_vxlan_nolocalbypass.sh

> +static void do_simple_test(void)
> +{
> +	int fd;
> +
> +	fd = epoll_create1(0);
> +	if (fd == -1)
> +		error(1, errno, "epoll_create");
> +
> +	do_simple_test_invalid_fd();
> +	do_simple_test_invalid_ioctl(fd);
> +	do_simple_test_get_params(fd);
> +	do_simple_test_set_invalid(fd);
> +	do_simple_test_set_and_get_valid(fd);

You don't want to use the kselftest_harness for this?
No strong preference here, but seems like you could
pop the epoll_create1 into a FIXTURE() and then the
test cases into TEST_F() and we'd get the KTAP output
formatting, ability to run the tests selectively etc.
for free.

tools/testing/selftests/net/tap.c is probably a good example 
to take a look at
Joe Damato May 3, 2024, 11:09 p.m. UTC | #2
On Fri, May 03, 2024 at 03:49:39PM -0700, Jakub Kicinski wrote:
> On Thu,  2 May 2024 21:20:11 +0000 Joe Damato wrote:
> > --- a/tools/testing/selftests/net/Makefile
> > +++ b/tools/testing/selftests/net/Makefile
> > @@ -84,6 +84,7 @@ TEST_GEN_FILES += sctp_hello
> >  TEST_GEN_FILES += csum
> >  TEST_GEN_FILES += ip_local_port_range
> >  TEST_GEN_FILES += bind_wildcard
> > +TEST_GEN_FILES += epoll_busy_poll
> 
> "GEN" is for files which are built for other tests to use.
> IOW unless there's also a wrapper script under TEST_PROGS
> (or the C code is itself under TEST_PROGS) this test won't
> be executed by most CIs.

Ah, I see. OK.

If I decided to go with the kselftest_harness as mentioned below, I'd need
to include a wrapper script to run the binary with the right cmd line
arg(s) and put that in TEST_PROGS?

> FWIW here's how we run the tests in our CI upstream CI:
> https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style

Thanks for the link, I'll give this a close read.

> >  TEST_PROGS += test_vxlan_mdb.sh
> >  TEST_PROGS += test_bridge_neigh_suppress.sh
> >  TEST_PROGS += test_vxlan_nolocalbypass.sh
> 
> > +static void do_simple_test(void)
> > +{
> > +	int fd;
> > +
> > +	fd = epoll_create1(0);
> > +	if (fd == -1)
> > +		error(1, errno, "epoll_create");
> > +
> > +	do_simple_test_invalid_fd();
> > +	do_simple_test_invalid_ioctl(fd);
> > +	do_simple_test_get_params(fd);
> > +	do_simple_test_set_invalid(fd);
> > +	do_simple_test_set_and_get_valid(fd);
> 
> You don't want to use the kselftest_harness for this?
> No strong preference here, but seems like you could
> pop the epoll_create1 into a FIXTURE() and then the
> test cases into TEST_F() and we'd get the KTAP output
> formatting, ability to run the tests selectively etc.
> for free.

I have no preference. I looked at some random .c file test in the directory
and it wasn't using the kselftest_harness stuff so I just went with that.

The advantages of kselftest_harness make sense, so I can give it a rewrite
to use kselftest_harness in v2.

> tools/testing/selftests/net/tap.c is probably a good example 
> to take a look at

Thanks, I'll look at that one. I had previously just kinda scanned
reuseaddr_conflict.c and rxtimestamp.c and some other ones. Seemed like a
bunch were just regular C programs so I went that route, but the advantages
you list make a lot of sense.
Jakub Kicinski May 4, 2024, 12:25 a.m. UTC | #3
On Fri, 3 May 2024 16:09:45 -0700 Joe Damato wrote:
> > "GEN" is for files which are built for other tests to use.
> > IOW unless there's also a wrapper script under TEST_PROGS
> > (or the C code is itself under TEST_PROGS) this test won't
> > be executed by most CIs.  
> 
> Ah, I see. OK.
> 
> If I decided to go with the kselftest_harness as mentioned below, I'd need
> to include a wrapper script to run the binary with the right cmd line
> arg(s) and put that in TEST_PROGS?

harness or not, the only two real requirements for including in
TEST_PROGS directly is to:
 - return non-zero exit code on failure; and
 - not require any command line arguments.
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index d996a0ab0765..777cfd027076 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -5,6 +5,7 @@  bind_wildcard
 csum
 cmsg_sender
 diag_uid
+epoll_busy_poll
 fin_ack_lat
 gro
 hwtstamp_config
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 5befca249452..c83c5d9c1ad9 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -84,6 +84,7 @@  TEST_GEN_FILES += sctp_hello
 TEST_GEN_FILES += csum
 TEST_GEN_FILES += ip_local_port_range
 TEST_GEN_FILES += bind_wildcard
+TEST_GEN_FILES += epoll_busy_poll
 TEST_PROGS += test_vxlan_mdb.sh
 TEST_PROGS += test_bridge_neigh_suppress.sh
 TEST_PROGS += test_vxlan_nolocalbypass.sh
diff --git a/tools/testing/selftests/net/epoll_busy_poll.c b/tools/testing/selftests/net/epoll_busy_poll.c
new file mode 100644
index 000000000000..3066a41a2acb
--- /dev/null
+++ b/tools/testing/selftests/net/epoll_busy_poll.c
@@ -0,0 +1,279 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Basic per-epoll context busy poll test.
+ *
+ * Only tests the ioctls, but should be expanded to test two connected hosts in
+ * the future
+ */
+
+#define _GNU_SOURCE
+
+#include <error.h>
+#include <errno.h>
+#include <inttypes.h>
+#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <sys/epoll.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+
+/* if the headers haven't been updated, we need to define some things */
+#if !defined(EPOLL_IOC_TYPE)
+struct epoll_params {
+	uint32_t busy_poll_usecs;
+	uint16_t busy_poll_budget;
+	uint8_t prefer_busy_poll;
+
+	/* pad the struct to a multiple of 64bits */
+	uint8_t __pad;
+};
+
+#define EPOLL_IOC_TYPE 0x8A
+#define EPIOCSPARAMS _IOW(EPOLL_IOC_TYPE, 0x01, struct epoll_params)
+#define EPIOCGPARAMS _IOR(EPOLL_IOC_TYPE, 0x02, struct epoll_params)
+#endif
+
+enum epoll_test_types {
+	TEST_UNDEFINED = -1,
+	TEST_SIMPLE = 0,
+};
+
+static enum epoll_test_types test_type = TEST_UNDEFINED;
+
+static void usage(const char *filepath)
+{
+	error(1, 0, "Usage: %s [options]", filepath);
+}
+
+static void parse_opts(int argc, char **argv)
+{
+	int c;
+
+	while ((c = getopt(argc, argv, "s")) != -1) {
+		switch (c) {
+		case 's':
+			test_type = TEST_SIMPLE;
+			break;
+		}
+	}
+
+	if (optind != argc)
+		usage(argv[0]);
+}
+
+static void do_simple_test_get_params(int fd)
+{
+	/* begin by getting the epoll params from the kernel
+	 *
+	 * the default should be default and all fields should be zero'd by the
+	 * kernel, so set params fields to garbage to test this.
+	 */
+	struct epoll_params *invalid_params;
+	struct epoll_params params;
+	int ret = 0;
+
+	params.busy_poll_usecs = 0xff;
+	params.busy_poll_budget = 0xff;
+	params.prefer_busy_poll = 1;
+	params.__pad = 0xf;
+
+	if (ioctl(fd, EPIOCGPARAMS, &params) != 0)
+		error(1, errno, "ioctl EPIOCGPARAMS");
+
+	if (params.busy_poll_usecs != 0)
+		error(1, 0, "EPIOCGPARAMS busy_poll_usecs should have been 0");
+
+	if (params.busy_poll_budget != 0)
+		error(1, 0, "EPIOCGPARAMS busy_poll_budget should have been 0");
+
+	if (params.prefer_busy_poll != 0)
+		error(1, 0, "EPIOCGPARAMS prefer_busy_poll should have been 0");
+
+	if (params.__pad != 0)
+		error(1, 0, "EPIOCGPARAMS __pad should have been 0");
+
+	invalid_params = (struct epoll_params *)0xdeadbeef;
+	ret = ioctl(fd, EPIOCGPARAMS, invalid_params);
+	if (ret != -1)
+		error(1, 0, "EPIOCGPARAMS should error with invalid params");
+
+	if (errno != EFAULT)
+		error(1, 0,
+		      "EPIOCGPARAMS with invalid params should set errno to EFAULT");
+}
+
+static void do_simple_test_set_invalid(int fd)
+{
+	/* Set some unacceptable values and check for error */
+	struct epoll_params *invalid_params;
+	struct epoll_params params;
+	int ret;
+
+	memset(&params, 0, sizeof(struct epoll_params));
+
+	params.__pad = 1;
+
+	ret = ioctl(fd, EPIOCSPARAMS, &params);
+
+	if (ret != -1)
+		error(1, 0, "EPIOCSPARAMS with non-zero __pad should error");
+
+	if (errno != EINVAL)
+		error(1, 0, "EPIOCSPARAMS with non-zero __pad errno should be EINVAL");
+
+	params.__pad = 0;
+	params.busy_poll_usecs = (unsigned int)INT_MAX + 1;
+
+	ret = ioctl(fd, EPIOCSPARAMS, &params);
+
+	if (ret != -1)
+		error(1, 0, "EPIOCSPARAMS should error busy_poll_usecs > S32_MAX");
+
+	if (errno != EINVAL)
+		error(1, 0, "EPIOCSPARAMS with busy_poll_usecs > S32_MAX, errno should be EINVAL");
+
+	params.__pad = 0;
+	params.busy_poll_usecs = 32;
+	params.prefer_busy_poll = 2;
+
+	ret = ioctl(fd, EPIOCSPARAMS, &params);
+
+	if (ret != -1)
+		error(1, 0, "EPIOCSPARAMS should error prefer_busy_poll > 1");
+
+	if (errno != EINVAL)
+		error(1, 0, "EPIOCSPARAMS with prefer_busy_poll > 1 errno should be EINVAL");
+
+	params.__pad = 0;
+	params.busy_poll_usecs = 32;
+	params.prefer_busy_poll = 1;
+	params.busy_poll_budget = 65535;
+
+	ret = ioctl(fd, EPIOCSPARAMS, &params);
+
+	if (ret != -1)
+		error(1, 0, "EPIOCSPARAMS should error busy_poll_budget > NAPI_POLL_WEIGHT");
+
+	if (errno != EPERM)
+		error(1, 0,
+		      "EPIOCSPARAMS with busy_poll_budget > NAPI_POLL_WEIGHT (without CAP_NET_ADMIN) errno should be EPERM");
+
+	invalid_params = (struct epoll_params *)0xdeadbeef;
+	ret = ioctl(fd, EPIOCSPARAMS, invalid_params);
+
+	if (ret != -1)
+		error(1, 0, "EPIOCSPARAMS should error when epoll_params is invalid");
+
+	if (errno != EFAULT)
+		error(1, 0, "EPIOCSPARAMS should set errno to EFAULT when epoll_params is invalid");
+}
+
+static void do_simple_test_set_and_get_valid(int fd)
+{
+	struct epoll_params params;
+	int ret;
+
+	memset(&params, 0, sizeof(struct epoll_params));
+
+	params.busy_poll_usecs = 25;
+	params.busy_poll_budget = 16;
+	params.prefer_busy_poll = 1;
+
+	ret = ioctl(fd, EPIOCSPARAMS, &params);
+
+	if (ret != 0)
+		error(1, errno, "EPIOCSPARAMS with valid params should not error");
+
+	/* check that the kernel returns the same values back */
+
+	memset(&params, 0, sizeof(struct epoll_params));
+
+	ret = ioctl(fd, EPIOCGPARAMS, &params);
+
+	if (ret != 0)
+		error(1, errno, "EPIOCGPARAMS should not error");
+
+	if (params.busy_poll_usecs != 25 ||
+	    params.busy_poll_budget != 16 ||
+	    params.prefer_busy_poll != 1 ||
+	    params.__pad != 0)
+		error(1, 0, "EPIOCGPARAMS returned incorrect values");
+}
+
+static void do_simple_test_invalid_fd(void)
+{
+	struct epoll_params params;
+	int ret;
+	int fd;
+
+	fd = socket(AF_UNIX, SOCK_DGRAM, 0);
+
+	if (fd == -1)
+		error(1, errno, "creating unix socket");
+
+	ret = ioctl(fd, EPIOCGPARAMS, &params);
+
+	if (ret != -1)
+		error(1, 0, "EPIOCGPARAMS on invalid epoll FD should error");
+
+	if (errno != ENOTTY)
+		error(1, 0, "EPIOCGPARAMS on invalid epoll FD should set errno to ENOTTY");
+
+	memset(&params, 0, sizeof(struct epoll_params));
+
+	ret = ioctl(fd, EPIOCSPARAMS, &params);
+
+	if (ret != -1)
+		error(1, 0, "EPIOCSPARAMS on invalid epoll FD should error");
+
+	if (errno != ENOTTY)
+		error(1, 0, "EPIOCSPARAMS on invalid epoll FD should set errno to ENOTTY");
+}
+
+static void do_simple_test_invalid_ioctl(int fd)
+{
+	struct epoll_params params;
+	int invalid_ioctl = EPIOCGPARAMS + 10;
+	int ret;
+
+	ret = ioctl(fd, invalid_ioctl, &params);
+
+	if (ret != -1)
+		error(1, 0, "invalid ioctl should return error");
+
+	if (errno != EINVAL)
+		error(1, 0, "invalid ioctl should set errno to EINVAL");
+}
+
+static void do_simple_test(void)
+{
+	int fd;
+
+	fd = epoll_create1(0);
+	if (fd == -1)
+		error(1, errno, "epoll_create");
+
+	do_simple_test_invalid_fd();
+	do_simple_test_invalid_ioctl(fd);
+	do_simple_test_get_params(fd);
+	do_simple_test_set_invalid(fd);
+	do_simple_test_set_and_get_valid(fd);
+
+	if (close(fd))
+		error(1, errno, "close");
+}
+
+int main(int argc, char **argv)
+{
+	parse_opts(argc, argv);
+
+	if (test_type == TEST_SIMPLE)
+		do_simple_test();
+	else
+		error(1, 0, "unknown test type: %d", test_type);
+
+	return 0;
+}