All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs
@ 2021-10-06 18:56 Yucong Sun
  2021-10-06 18:56 ` [PATCH bpf-next v6 01/14] " Yucong Sun
                   ` (14 more replies)
  0 siblings, 15 replies; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong, Yucong Sun

This patch series adds "-j" parelell execution to test_progs, with "--debug" to
display server/worker communications. Also, some Tests that often fails in
parallel are marked as serial test, and it will run in sequence after parallel
execution is done.

This patch series also adds a error summary after all tests execution finished.

V6 -> V5:
  * adding error summary logic for non parallel mode too.
  * changed how serial tests are implemented, use main process instead of worker 0.
  * fixed a dozen broken test when running in parallel.

V5 -> V4:
  * change to SOCK_SEQPACKET for close notification.
  * move all debug output to "--debug" mode
  * output log as test finish, and all error logs again after summary line.
  * variable naming / style changes
  * adds serial_test_name() to replace serial test lists.


Yucong Sun (14):
  selftests/bpf: Add parallelism to test_progs
  selftests/bpf: Allow some tests to be executed in sequence
  selftests/bpf: disable perf rate limiting when running tests.
  selftests/bpf: add per worker cgroup suffix
  selftests/bpf: adding read_perf_max_sample_freq() helper
  selftests/bpf: fix race condition in enable_stats
  selftests/bpf: make cgroup_v1v2 use its own port
  selftests/bpf: adding a namespace reset for tc_redirect
  selftests/bpf: Make uprobe tests use different attach functions.
  selftests/bpf: adding pid filtering for atomics test
  selftests/bpf: adding random delay for send_signal test
  selftests/bpf: Fix pid check in fexit_sleep test
  selftests/bpf: increase loop count for perf_branches
  selfetest/bpf: make some tests serial

 tools/testing/selftests/bpf/cgroup_helpers.c  |   6 +-
 tools/testing/selftests/bpf/cgroup_helpers.h  |   2 +-
 .../selftests/bpf/prog_tests/atomics.c        |   1 +
 .../selftests/bpf/prog_tests/attach_probe.c   |   8 +-
 .../selftests/bpf/prog_tests/bpf_cookie.c     |  10 +-
 .../bpf/prog_tests/bpf_iter_setsockopt.c      |   2 +-
 .../selftests/bpf/prog_tests/bpf_obj_id.c     |   2 +-
 .../bpf/prog_tests/cg_storage_multi.c         |   2 +-
 .../bpf/prog_tests/cgroup_attach_autodetach.c |   2 +-
 .../bpf/prog_tests/cgroup_attach_multi.c      |   2 +-
 .../bpf/prog_tests/cgroup_attach_override.c   |   2 +-
 .../selftests/bpf/prog_tests/cgroup_link.c    |   2 +-
 .../selftests/bpf/prog_tests/cgroup_v1v2.c    |   2 +-
 .../selftests/bpf/prog_tests/check_mtu.c      |   2 +-
 .../selftests/bpf/prog_tests/fexit_bpf2bpf.c  |   3 +-
 .../prog_tests/flow_dissector_load_bytes.c    |   2 +-
 .../bpf/prog_tests/flow_dissector_reattach.c  |   2 +-
 .../bpf/prog_tests/get_branch_snapshot.c      |   2 +-
 .../selftests/bpf/prog_tests/kfree_skb.c      |   3 +-
 .../bpf/prog_tests/migrate_reuseport.c        |   2 +-
 .../selftests/bpf/prog_tests/modify_return.c  |   3 +-
 .../bpf/prog_tests/ns_current_pid_tgid.c      |   3 +-
 .../selftests/bpf/prog_tests/perf_branches.c  |  10 +-
 .../selftests/bpf/prog_tests/perf_buffer.c    |   2 +-
 .../selftests/bpf/prog_tests/perf_link.c      |   5 +-
 .../selftests/bpf/prog_tests/probe_user.c     |   3 +-
 .../bpf/prog_tests/raw_tp_writable_test_run.c |   3 +-
 .../bpf/prog_tests/select_reuseport.c         |   2 +-
 .../selftests/bpf/prog_tests/send_signal.c    |   6 +-
 .../bpf/prog_tests/send_signal_sched_switch.c |   3 +-
 .../bpf/prog_tests/sk_storage_tracing.c       |   2 +-
 .../selftests/bpf/prog_tests/snprintf_btf.c   |   2 +-
 .../selftests/bpf/prog_tests/sock_fields.c    |   2 +-
 .../selftests/bpf/prog_tests/sockmap_listen.c |   2 +-
 .../bpf/prog_tests/stacktrace_build_id_nmi.c  |  19 +-
 .../selftests/bpf/prog_tests/task_pt_regs.c   |   8 +-
 .../selftests/bpf/prog_tests/tc_redirect.c    |  14 +
 .../testing/selftests/bpf/prog_tests/timer.c  |   3 +-
 .../selftests/bpf/prog_tests/timer_mim.c      |   2 +-
 .../bpf/prog_tests/tp_attach_query.c          |   2 +-
 .../selftests/bpf/prog_tests/trace_printk.c   |   2 +-
 .../selftests/bpf/prog_tests/trace_vprintk.c  |   2 +-
 .../bpf/prog_tests/trampoline_count.c         |   3 +-
 .../selftests/bpf/prog_tests/xdp_attach.c     |   2 +-
 .../selftests/bpf/prog_tests/xdp_bonding.c    |   2 +-
 .../bpf/prog_tests/xdp_cpumap_attach.c        |   2 +-
 .../bpf/prog_tests/xdp_devmap_attach.c        |   2 +-
 .../selftests/bpf/prog_tests/xdp_info.c       |   2 +-
 .../selftests/bpf/prog_tests/xdp_link.c       |   2 +-
 tools/testing/selftests/bpf/progs/atomics.c   |  16 +
 .../selftests/bpf/progs/connect4_dropper.c    |   2 +-
 .../testing/selftests/bpf/progs/fexit_sleep.c |   4 +-
 .../selftests/bpf/progs/test_enable_stats.c   |   2 +-
 tools/testing/selftests/bpf/test_progs.c      | 671 +++++++++++++++++-
 tools/testing/selftests/bpf/test_progs.h      |  37 +-
 55 files changed, 790 insertions(+), 116 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 01/14] selftests/bpf: Add parallelism to test_progs
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-08 22:26   ` Andrii Nakryiko
  2021-10-06 18:56 ` [PATCH bpf-next v6 02/14] selftests/bpf: Allow some tests to be executed in sequence Yucong Sun
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

This patch adds "-j" mode to test_progs, executing tests in multiple
process.  "-j" mode is optional, and works with all existing test
selection mechanism, as well as "-v", "-l" etc.

In "-j" mode, main process use UDS/DGRAM to communicate to each forked
worker, commanding it to run tests and collect logs. After all tests are
finished, a summary is printed. main process use multiple competing
threads to dispatch work to worker, trying to keep them all busy.

The test status will be printed as soon as it is finished, if there are
error logs, it will be printed after the final summary line.

By specifying "--debug", additional debug information on server/worker
communication will be printed.

Example output:
  > ./test_progs -n 15-20 -j
  [   12.801730] bpf_testmod: loading out-of-tree module taints kernel.
  Launching 8 workers.
  #20 btf_split:OK
  #16 btf_endian:OK
  #18 btf_module:OK
  #17 btf_map_in_map:OK
  #19 btf_skc_cls_ingress:OK
  #15 btf_dump:OK
  Summary: 6/20 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 tools/testing/selftests/bpf/test_progs.c | 598 +++++++++++++++++++++--
 tools/testing/selftests/bpf/test_progs.h |  36 +-
 2 files changed, 600 insertions(+), 34 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 2ed01f615d20..51e18d8df7f2 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -12,6 +12,11 @@
 #include <string.h>
 #include <execinfo.h> /* backtrace */
 #include <linux/membarrier.h>
+#include <sys/sysinfo.h> /* get_nprocs */
+#include <netinet/in.h>
+#include <sys/select.h>
+#include <sys/socket.h>
+#include <sys/un.h>
 
 /* Adapted from perf/util/string.c */
 static bool glob_match(const char *str, const char *pat)
@@ -48,6 +53,8 @@ struct prog_test_def {
 	bool force_log;
 	int error_cnt;
 	int skip_cnt;
+	int sub_succ_cnt;
+	bool should_run;
 	bool tested;
 	bool need_cgroup_cleanup;
 
@@ -97,6 +104,10 @@ static void dump_test_log(const struct prog_test_def *test, bool failed)
 	if (stdout == env.stdout)
 		return;
 
+	/* worker always holds log */
+	if (env.worker_id != -1)
+		return;
+
 	fflush(stdout); /* exports env.log_buf & env.log_cnt */
 
 	if (env.verbosity > VERBOSE_NONE || test->force_log || failed) {
@@ -107,8 +118,6 @@ static void dump_test_log(const struct prog_test_def *test, bool failed)
 				fprintf(env.stdout, "\n");
 		}
 	}
-
-	fseeko(stdout, 0, SEEK_SET); /* rewind */
 }
 
 static void skip_account(void)
@@ -172,14 +181,14 @@ void test__end_subtest()
 
 	dump_test_log(test, sub_error_cnt);
 
-	fprintf(env.stdout, "#%d/%d %s/%s:%s\n",
+	fprintf(stdout, "#%d/%d %s/%s:%s\n",
 	       test->test_num, test->subtest_num, test->test_name, test->subtest_name,
 	       sub_error_cnt ? "FAIL" : (test->skip_cnt ? "SKIP" : "OK"));
 
 	if (sub_error_cnt)
-		env.fail_cnt++;
+		test->error_cnt++;
 	else if (test->skip_cnt == 0)
-		env.sub_succ_cnt++;
+		test->sub_succ_cnt++;
 	skip_account();
 
 	free(test->subtest_name);
@@ -474,6 +483,8 @@ enum ARG_KEYS {
 	ARG_LIST_TEST_NAMES = 'l',
 	ARG_TEST_NAME_GLOB_ALLOWLIST = 'a',
 	ARG_TEST_NAME_GLOB_DENYLIST = 'd',
+	ARG_NUM_WORKERS = 'j',
+	ARG_DEBUG = -1,
 };
 
 static const struct argp_option opts[] = {
@@ -495,6 +506,10 @@ static const struct argp_option opts[] = {
 	  "Run tests with name matching the pattern (supports '*' wildcard)." },
 	{ "deny", ARG_TEST_NAME_GLOB_DENYLIST, "NAMES", 0,
 	  "Don't run tests with name matching the pattern (supports '*' wildcard)." },
+	{ "workers", ARG_NUM_WORKERS, "WORKERS", OPTION_ARG_OPTIONAL,
+	  "Number of workers to run in parallel, default to number of cpus." },
+	{ "debug", ARG_DEBUG, NULL, 0,
+	  "print extra debug information for test_progs." },
 	{},
 };
 
@@ -650,7 +665,7 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
 				fprintf(stderr,
 					"Unable to setenv SELFTESTS_VERBOSE=1 (errno=%d)",
 					errno);
-				return -1;
+				return -EINVAL;
 			}
 		}
 
@@ -661,6 +676,20 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
 	case ARG_LIST_TEST_NAMES:
 		env->list_test_names = true;
 		break;
+	case ARG_NUM_WORKERS:
+		if (arg) {
+			env->workers = atoi(arg);
+			if (!env->workers) {
+				fprintf(stderr, "Invalid number of worker: %s.", arg);
+				return -EINVAL;
+			}
+		} else {
+			env->workers = get_nprocs();
+		}
+		break;
+	case ARG_DEBUG:
+		env->debug = true;
+		break;
 	case ARGP_KEY_ARG:
 		argp_usage(state);
 		break;
@@ -678,7 +707,7 @@ static void stdio_hijack(void)
 	env.stdout = stdout;
 	env.stderr = stderr;
 
-	if (env.verbosity > VERBOSE_NONE) {
+	if (env.verbosity > VERBOSE_NONE && env.worker_id == -1) {
 		/* nothing to do, output to stdout by default */
 		return;
 	}
@@ -704,10 +733,6 @@ static void stdio_restore(void)
 		return;
 
 	fclose(stdout);
-	free(env.log_buf);
-
-	env.log_buf = NULL;
-	env.log_cnt = 0;
 
 	stdout = env.stdout;
 	stderr = env.stderr;
@@ -794,11 +819,456 @@ void crash_handler(int signum)
 		dump_test_log(env.test, true);
 	if (env.stdout)
 		stdio_restore();
-
+	if (env.worker_id != -1)
+		fprintf(stderr, "[%d]: ", env.worker_id);
 	fprintf(stderr, "Caught signal #%d!\nStack trace:\n", signum);
 	backtrace_symbols_fd(bt, sz, STDERR_FILENO);
 }
 
+void sigint_handler(int signum) {
+	for (int i = 0; i < env.workers; i++)
+		if (env.worker_socks[i] > 0)
+			close(env.worker_socks[i]);
+}
+
+static int current_test_idx;
+static pthread_mutex_t current_test_lock;
+static pthread_mutex_t stdout_output_lock;
+
+struct test_result {
+	int error_cnt;
+	int skip_cnt;
+	int sub_succ_cnt;
+
+	size_t log_cnt;
+	char *log_buf;
+};
+
+static struct test_result test_results[ARRAY_SIZE(prog_test_defs)];
+
+static inline const char *str_msg(const struct msg *msg, char *buf)
+{
+	switch (msg->type) {
+	case MSG_DO_TEST:
+		sprintf(buf, "MSG_DO_TEST %d", msg->do_test.test_num);
+		break;
+	case MSG_TEST_DONE:
+		sprintf(buf, "MSG_TEST_DONE %d (log: %d)",
+			msg->test_done.test_num,
+			msg->test_done.have_log);
+		break;
+	case MSG_TEST_LOG:
+		sprintf(buf, "MSG_TEST_LOG (cnt: %ld, last: %d)",
+			strlen(msg->test_log.log_buf),
+			msg->test_log.is_last);
+		break;
+	case MSG_EXIT:
+		sprintf(buf, "MSG_EXIT");
+		break;
+	default:
+		sprintf(buf, "UNKNOWN");
+		break;
+	}
+
+	return buf;
+}
+
+static int send_message(int sock, const struct msg *msg)
+{
+	char buf[256];
+
+	if (env.debug)
+		fprintf(stderr, "Sending msg: %s\n", str_msg(msg, buf));
+	return send(sock, msg, sizeof(*msg), 0);
+}
+
+static int recv_message(int sock, struct msg *msg)
+{
+	int ret;
+	char buf[256];
+
+	memset(msg, 0, sizeof(*msg));
+	ret = recv(sock, msg, sizeof(*msg), 0);
+	if (ret >= 0) {
+		if (env.debug)
+			fprintf(stderr, "Received msg: %s\n", str_msg(msg, buf));
+	}
+	return ret;
+}
+
+static void run_one_test(int test_num)
+{
+	struct prog_test_def *test = &prog_test_defs[test_num];
+
+	env.test = test;
+
+	test->run_test();
+
+	/* ensure last sub-test is finalized properly */
+	if (test->subtest_name)
+		test__end_subtest();
+
+	test->tested = true;
+
+	dump_test_log(test, test->error_cnt);
+
+	reset_affinity();
+	restore_netns();
+	if (test->need_cgroup_cleanup)
+		cleanup_cgroup_environment();
+}
+
+struct dispatch_data {
+	int worker_id;
+	int sock_fd;
+};
+
+void *dispatch_thread(void *ctx)
+{
+	struct dispatch_data *data = ctx;
+	int sock_fd;
+	FILE *log_fd = NULL;
+
+	sock_fd = data->sock_fd;
+
+	while (true) {
+		int test_to_run = -1;
+		struct prog_test_def *test;
+		struct test_result *result;
+
+		/* grab a test */
+		{
+			pthread_mutex_lock(&current_test_lock);
+
+			if (current_test_idx >= prog_test_cnt) {
+				pthread_mutex_unlock(&current_test_lock);
+				goto done;
+			}
+
+			test = &prog_test_defs[current_test_idx];
+			test_to_run = current_test_idx;
+			current_test_idx++;
+
+			pthread_mutex_unlock(&current_test_lock);
+		}
+
+		if (!test->should_run)
+			continue;
+
+		/* run test through worker */
+		{
+			struct msg msg_do_test;
+
+			msg_do_test.type = MSG_DO_TEST;
+			msg_do_test.do_test.test_num = test_to_run;
+			if (send_message(sock_fd, &msg_do_test) < 0) {
+				perror("Fail to send command");
+				goto done;
+			}
+			env.worker_current_test[data->worker_id] = test_to_run;
+		}
+
+		/* wait for test done */
+		{
+			int err;
+			struct msg msg_test_done;
+
+			err = recv_message(sock_fd, &msg_test_done);
+			if (err < 0)
+				goto error;
+			if (msg_test_done.type != MSG_TEST_DONE)
+				goto error;
+			if (test_to_run != msg_test_done.test_done.test_num)
+				goto error;
+
+			test->tested = true;
+			result = &test_results[test_to_run];
+
+			result->error_cnt = msg_test_done.test_done.error_cnt;
+			result->skip_cnt = msg_test_done.test_done.skip_cnt;
+			result->sub_succ_cnt = msg_test_done.test_done.sub_succ_cnt;
+
+			/* collect all logs */
+			if (msg_test_done.test_done.have_log) {
+				log_fd = open_memstream(&result->log_buf, &result->log_cnt);
+				if (!log_fd)
+					goto error;
+
+				while (true) {
+					struct msg msg_log;
+
+					if (recv_message(sock_fd, &msg_log) < 0)
+						goto error;
+					if (msg_log.type != MSG_TEST_LOG)
+						goto error;
+
+					fprintf(log_fd, "%s", msg_log.test_log.log_buf);
+					if (msg_log.test_log.is_last)
+						break;
+				}
+				fclose(log_fd);
+				log_fd = NULL;
+			}
+			/* output log */
+			{
+				pthread_mutex_lock(&stdout_output_lock);
+
+				if (result->log_cnt) {
+					result->log_buf[result->log_cnt] = '\0';
+					fprintf(stdout, "%s", result->log_buf);
+					if (result->log_buf[result->log_cnt - 1] != '\n')
+						fprintf(stdout, "\n");
+				}
+
+				fprintf(stdout, "#%d %s:%s\n",
+					test->test_num, test->test_name,
+					result->error_cnt ? "FAIL" : (result->skip_cnt ? "SKIP" : "OK"));
+
+				pthread_mutex_unlock(&stdout_output_lock);
+			}
+
+		} /* wait for test done */
+	} /* while (true) */
+error:
+	if (env.debug)
+		fprintf(stderr, "[%d]: Protocol/IO error: %s.\n", data->worker_id, strerror(errno));
+
+	if (log_fd)
+		fclose(log_fd);
+done:
+	{
+		struct msg msg_exit;
+
+		msg_exit.type = MSG_EXIT;
+		if (send_message(sock_fd, &msg_exit) < 0) {
+			if (env.debug)
+				fprintf(stderr, "[%d]: send_message msg_exit: %s.\n",
+					data->worker_id, strerror(errno));
+		}
+	}
+	return NULL;
+}
+
+static void print_all_error_logs(void)
+{
+	if (env.fail_cnt)
+		fprintf(stdout, "\nAll error logs:\n");
+
+	/* print error logs again */
+
+	for (int i = 0; i < prog_test_cnt; i++) {
+		struct prog_test_def *test;
+		struct test_result *result;
+
+		test = &prog_test_defs[i];
+		result = &test_results[i];
+
+		if (!test->tested || !result->error_cnt)
+			continue;
+
+		fprintf(stdout, "\n#%d %s:%s\n",
+			test->test_num, test->test_name,
+			result->error_cnt ? "FAIL" : (result->skip_cnt ? "SKIP" : "OK"));
+
+		if (result->log_cnt) {
+			result->log_buf[result->log_cnt] = '\0';
+			fprintf(stdout, "%s", result->log_buf);
+			if (result->log_buf[result->log_cnt - 1] != '\n')
+				fprintf(stdout, "\n");
+		}
+	}
+}
+
+static int server_main(void)
+{
+	pthread_t *dispatcher_threads;
+	struct dispatch_data *data;
+	struct sigaction sigact_int = {
+		.sa_handler = sigint_handler,
+		.sa_flags = SA_RESETHAND,
+	};
+
+	sigaction(SIGINT, &sigact_int, NULL);
+
+	dispatcher_threads = calloc(sizeof(pthread_t), env.workers);
+	data = calloc(sizeof(struct dispatch_data), env.workers);
+
+	env.worker_current_test = calloc(sizeof(int), env.workers);
+	for (int i = 0; i < env.workers; i++) {
+		int rc;
+
+		data[i].worker_id = i;
+		data[i].sock_fd = env.worker_socks[i];
+		rc = pthread_create(&dispatcher_threads[i], NULL, dispatch_thread, &data[i]);
+		if (rc < 0) {
+			perror("Failed to launch dispatcher thread");
+			exit(EXIT_ERR_SETUP_INFRA);
+		}
+	}
+
+	/* wait for all dispatcher to finish */
+	for (int i = 0; i < env.workers; i++) {
+		while (true) {
+			int ret = pthread_tryjoin_np(dispatcher_threads[i], NULL);
+
+			if (!ret) {
+				break;
+			} else if (ret == EBUSY) {
+				if (env.debug)
+					fprintf(stderr, "Still waiting for thread %d (test %d).\n",
+						i,  env.worker_current_test[i] + 1);
+				usleep(1000 * 1000);
+				continue;
+			} else {
+				fprintf(stderr, "Unexpected error joining dispatcher thread: %d", ret);
+				break;
+			}
+		}
+	}
+	free(dispatcher_threads);
+	free(env.worker_current_test);
+	free(data);
+
+	/* generate summary */
+	fflush(stderr);
+	fflush(stdout);
+
+	for (int i = 0; i < prog_test_cnt; i++) {
+		struct prog_test_def *current_test;
+		struct test_result *result;
+
+		current_test = &prog_test_defs[i];
+		result = &test_results[i];
+
+		if (!current_test->tested)
+			continue;
+
+		env.succ_cnt += result->error_cnt ? 0 : 1;
+		env.skip_cnt += result->skip_cnt;
+		if (result->error_cnt)
+			env.fail_cnt++;
+		env.sub_succ_cnt += result->sub_succ_cnt;
+	}
+
+	fprintf(stdout, "Summary: %d/%d PASSED, %d SKIPPED, %d FAILED\n",
+		env.succ_cnt, env.sub_succ_cnt, env.skip_cnt, env.fail_cnt);
+
+	print_all_error_logs();
+
+	/* reap all workers */
+	for (int i = 0; i < env.workers; i++) {
+		int wstatus, pid;
+
+		pid = waitpid(env.worker_pids[i], &wstatus, 0);
+		if (pid != env.worker_pids[i])
+			perror("Unable to reap worker");
+	}
+
+	return 0;
+}
+
+static int worker_main(int sock)
+{
+	save_netns();
+
+	while (true) {
+		/* receive command */
+		struct msg msg;
+
+		if (recv_message(sock, &msg) < 0)
+			goto out;
+
+		switch (msg.type) {
+		case MSG_EXIT:
+			if (env.debug)
+				fprintf(stderr, "[%d]: worker exit.\n",
+					env.worker_id);
+			goto out;
+		case MSG_DO_TEST: {
+			int test_to_run;
+			struct prog_test_def *test;
+			struct msg msg_done;
+
+			test_to_run = msg.do_test.test_num;
+			test = &prog_test_defs[test_to_run];
+
+			if (env.debug)
+				fprintf(stderr, "[%d]: #%d:%s running.\n",
+					env.worker_id,
+					test_to_run + 1,
+					test->test_name);
+
+			stdio_hijack();
+
+			run_one_test(test_to_run);
+
+			stdio_restore();
+
+			memset(&msg_done, 0, sizeof(msg_done));
+			msg_done.type = MSG_TEST_DONE;
+			msg_done.test_done.test_num = test_to_run;
+			msg_done.test_done.error_cnt = test->error_cnt;
+			msg_done.test_done.skip_cnt = test->skip_cnt;
+			msg_done.test_done.sub_succ_cnt = test->sub_succ_cnt;
+			msg_done.test_done.have_log = false;
+
+			if (env.verbosity > VERBOSE_NONE || test->force_log || test->error_cnt) {
+				if (env.log_cnt)
+					msg_done.test_done.have_log = true;
+			}
+			if (send_message(sock, &msg_done) < 0) {
+				perror("Fail to send message done");
+				goto out;
+			}
+
+			/* send logs */
+			if (msg_done.test_done.have_log) {
+				char *src;
+				size_t slen;
+
+				src = env.log_buf;
+				slen = env.log_cnt;
+				while (slen) {
+					struct msg msg_log;
+					char *dest;
+					size_t len;
+
+					memset(&msg_log, 0, sizeof(msg_log));
+					msg_log.type = MSG_TEST_LOG;
+					dest = msg_log.test_log.log_buf;
+					len = slen >= MAX_LOG_TRUNK_SIZE ? MAX_LOG_TRUNK_SIZE : slen;
+					memcpy(dest, src, len);
+
+					src += len;
+					slen -= len;
+					if (!slen)
+						msg_log.test_log.is_last = true;
+
+					assert(send_message(sock, &msg_log) >= 0);
+				}
+			}
+			if (env.log_buf) {
+				free(env.log_buf);
+				env.log_buf = NULL;
+				env.log_cnt = 0;
+			}
+			if (env.debug)
+				fprintf(stderr, "[%d]: #%d:%s done.\n",
+					env.worker_id,
+					test_to_run + 1,
+					test->test_name);
+			break;
+		} /* case MSG_DO_TEST */
+		default:
+			if (env.debug)
+				fprintf(stderr, "[%d]: unknown message.\n",  env.worker_id);
+			return -1;
+		}
+	}
+out:
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	static const struct argp argp = {
@@ -809,7 +1279,7 @@ int main(int argc, char **argv)
 	struct sigaction sigact = {
 		.sa_handler = crash_handler,
 		.sa_flags = SA_RESETHAND,
-	};
+		};
 	int err, i;
 
 	sigaction(SIGSEGV, &sigact, NULL);
@@ -837,21 +1307,77 @@ int main(int argc, char **argv)
 		return -1;
 	}
 
-	save_netns();
-	stdio_hijack();
+	env.stdout = stdout;
+	env.stderr = stderr;
+
 	env.has_testmod = true;
 	if (!env.list_test_names && load_bpf_testmod()) {
 		fprintf(env.stderr, "WARNING! Selftests relying on bpf_testmod.ko will be skipped.\n");
 		env.has_testmod = false;
 	}
+
+	/* initializing tests */
 	for (i = 0; i < prog_test_cnt; i++) {
 		struct prog_test_def *test = &prog_test_defs[i];
 
-		env.test = test;
 		test->test_num = i + 1;
-
-		if (!should_run(&env.test_selector,
+		if (should_run(&env.test_selector,
 				test->test_num, test->test_name))
+			test->should_run = true;
+		else
+			test->should_run = false;
+	}
+
+	/* ignore workers if we are just listing */
+	if (env.get_test_cnt || env.list_test_names)
+		env.workers = 0;
+
+	/* launch workers if requested */
+	env.worker_id = -1; /* main process */
+	if (env.workers) {
+		env.worker_pids = calloc(sizeof(__pid_t), env.workers);
+		env.worker_socks = calloc(sizeof(int), env.workers);
+		if (env.debug)
+			fprintf(stdout, "Launching %d workers.\n", env.workers);
+		for (int i = 0; i < env.workers; i++) {
+			int sv[2];
+			pid_t pid;
+
+			if (socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv) < 0) {
+				perror("Fail to create worker socket");
+				return -1;
+			}
+			pid = fork();
+			if (pid < 0) {
+				perror("Failed to fork worker");
+				return -1;
+			} else if (pid != 0) { /* main process */
+				close(sv[1]);
+				env.worker_pids[i] = pid;
+				env.worker_socks[i] = sv[0];
+			} else { /* inside each worker process */
+				close(sv[0]);
+				env.worker_id = i;
+				return worker_main(sv[1]);
+			}
+		}
+
+		if (env.worker_id == -1) {
+			server_main();
+			goto out;
+		}
+	}
+
+	/* The rest of the main process */
+
+	/* on single mode */
+	save_netns();
+
+	for (i = 0; i < prog_test_cnt; i++) {
+		struct prog_test_def *test = &prog_test_defs[i];
+		struct test_result *result;
+
+		if (!test->should_run)
 			continue;
 
 		if (env.get_test_cnt) {
@@ -865,33 +1391,35 @@ int main(int argc, char **argv)
 			continue;
 		}
 
-		test->run_test();
-		/* ensure last sub-test is finalized properly */
-		if (test->subtest_name)
-			test__end_subtest();
+		stdio_hijack();
 
-		test->tested = true;
+		run_one_test(i);
 
-		dump_test_log(test, test->error_cnt);
+		stdio_restore();
 
 		fprintf(env.stdout, "#%d %s:%s\n",
 			test->test_num, test->test_name,
 			test->error_cnt ? "FAIL" : (test->skip_cnt ? "SKIP" : "OK"));
 
+		result = &test_results[i];
+		result->error_cnt = test->error_cnt;
+		if (env.log_buf) {
+			result->log_buf = strdup(env.log_buf);
+			result->log_cnt = env.log_cnt;
+
+			free(env.log_buf);
+			env.log_buf = NULL;
+			env.log_cnt = 0;
+		}
+
 		if (test->error_cnt)
 			env.fail_cnt++;
 		else
 			env.succ_cnt++;
-		skip_account();
 
-		reset_affinity();
-		restore_netns();
-		if (test->need_cgroup_cleanup)
-			cleanup_cgroup_environment();
+		skip_account();
+		env.sub_succ_cnt += test->sub_succ_cnt;
 	}
-	if (!env.list_test_names && env.has_testmod)
-		unload_bpf_testmod();
-	stdio_restore();
 
 	if (env.get_test_cnt) {
 		printf("%d\n", env.succ_cnt);
@@ -904,14 +1432,18 @@ int main(int argc, char **argv)
 	fprintf(stdout, "Summary: %d/%d PASSED, %d SKIPPED, %d FAILED\n",
 		env.succ_cnt, env.sub_succ_cnt, env.skip_cnt, env.fail_cnt);
 
+	print_all_error_logs();
+
+	close(env.saved_netns_fd);
 out:
+	if (!env.list_test_names && env.has_testmod)
+		unload_bpf_testmod();
 	free_str_set(&env.test_selector.blacklist);
 	free_str_set(&env.test_selector.whitelist);
 	free(env.test_selector.num_set);
 	free_str_set(&env.subtest_selector.blacklist);
 	free_str_set(&env.subtest_selector.whitelist);
 	free(env.subtest_selector.num_set);
-	close(env.saved_netns_fd);
 
 	if (env.succ_cnt + env.fail_cnt + env.skip_cnt == 0)
 		return EXIT_NO_TEST;
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 94bef0aa74cf..b239dc9fcef0 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -62,6 +62,7 @@ struct test_env {
 	struct test_selector test_selector;
 	struct test_selector subtest_selector;
 	bool verifier_stats;
+	bool debug;
 	enum verbosity verbosity;
 
 	bool jit_enabled;
@@ -69,7 +70,8 @@ struct test_env {
 	bool get_test_cnt;
 	bool list_test_names;
 
-	struct prog_test_def *test;
+	struct prog_test_def *test; /* current running tests */
+
 	FILE *stdout;
 	FILE *stderr;
 	char *log_buf;
@@ -82,6 +84,38 @@ struct test_env {
 	int skip_cnt; /* skipped tests */
 
 	int saved_netns_fd;
+	int workers; /* number of worker process */
+	int worker_id; /* id number of current worker, main process is -1 */
+	pid_t *worker_pids; /* array of worker pids */
+	int *worker_socks; /* array of worker socks */
+	int *worker_current_test; /* array of current running test for each worker */
+};
+
+#define MAX_LOG_TRUNK_SIZE 8192
+enum msg_type {
+	MSG_DO_TEST = 0,
+	MSG_TEST_DONE = 1,
+	MSG_TEST_LOG = 2,
+	MSG_EXIT = 255,
+};
+struct msg {
+	enum msg_type type;
+	union {
+		struct {
+			int test_num;
+		} do_test;
+		struct {
+			int test_num;
+			int sub_succ_cnt;
+			int error_cnt;
+			int skip_cnt;
+			bool have_log;
+		} test_done;
+		struct {
+			char log_buf[MAX_LOG_TRUNK_SIZE + 1];
+			bool is_last;
+		} test_log;
+	};
 };
 
 extern struct test_env env;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 02/14] selftests/bpf: Allow some tests to be executed in sequence
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
  2021-10-06 18:56 ` [PATCH bpf-next v6 01/14] " Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-08 22:26   ` Andrii Nakryiko
  2021-10-06 18:56 ` [PATCH bpf-next v6 03/14] selftests/bpf: disable perf rate limiting when running tests Yucong Sun
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

This patch allows tests to define serial_test_name() instead of
test_name(), and this will make test_progs execute those in sequence
after all other tests finished executing concurrently.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 tools/testing/selftests/bpf/test_progs.c | 60 +++++++++++++++++++++---
 1 file changed, 54 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 51e18d8df7f2..4e2028189471 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -50,6 +50,7 @@ struct prog_test_def {
 	const char *test_name;
 	int test_num;
 	void (*run_test)(void);
+	void (*run_serial_test)(void);
 	bool force_log;
 	int error_cnt;
 	int skip_cnt;
@@ -455,14 +456,17 @@ static int load_bpf_testmod(void)
 }
 
 /* extern declarations for test funcs */
-#define DEFINE_TEST(name) extern void test_##name(void);
+#define DEFINE_TEST(name)				\
+	extern void test_##name(void) __weak;		\
+	extern void serial_test_##name(void) __weak;
 #include <prog_tests/tests.h>
 #undef DEFINE_TEST
 
 static struct prog_test_def prog_test_defs[] = {
-#define DEFINE_TEST(name) {		\
-	.test_name = #name,		\
-	.run_test = &test_##name,	\
+#define DEFINE_TEST(name) {			\
+	.test_name = #name,			\
+	.run_test = &test_##name,		\
+	.run_serial_test = &serial_test_##name,	\
 },
 #include <prog_tests/tests.h>
 #undef DEFINE_TEST
@@ -902,7 +906,10 @@ static void run_one_test(int test_num)
 
 	env.test = test;
 
-	test->run_test();
+	if (test->run_test)
+		test->run_test();
+	else if (test->run_serial_test)
+		test->run_serial_test();
 
 	/* ensure last sub-test is finalized properly */
 	if (test->subtest_name)
@@ -952,7 +959,7 @@ void *dispatch_thread(void *ctx)
 			pthread_mutex_unlock(&current_test_lock);
 		}
 
-		if (!test->should_run)
+		if (!test->should_run || test->run_serial_test)
 			continue;
 
 		/* run test through worker */
@@ -1129,6 +1136,40 @@ static int server_main(void)
 	free(env.worker_current_test);
 	free(data);
 
+	/* run serial tests */
+	save_netns();
+
+	for (int i = 0; i < prog_test_cnt; i++) {
+		struct prog_test_def *test = &prog_test_defs[i];
+		struct test_result *result = &test_results[i];
+
+		if (!test->should_run || !test->run_serial_test)
+			continue;
+
+		stdio_hijack();
+
+		run_one_test(i);
+
+		stdio_restore();
+		if (env.log_buf) {
+			result->log_cnt = env.log_cnt;
+			result->log_buf = strdup(env.log_buf);
+
+			free(env.log_buf);
+			env.log_buf = NULL;
+			env.log_cnt = 0;
+		}
+		restore_netns();
+
+		fprintf(stdout, "#%d %s:%s\n",
+			test->test_num, test->test_name,
+			test->error_cnt ? "FAIL" : (test->skip_cnt ? "SKIP" : "OK"));
+
+		result->error_cnt = test->error_cnt;
+		result->skip_cnt = test->skip_cnt;
+		result->sub_succ_cnt = test->sub_succ_cnt;
+	}
+
 	/* generate summary */
 	fflush(stderr);
 	fflush(stdout);
@@ -1326,6 +1367,13 @@ int main(int argc, char **argv)
 			test->should_run = true;
 		else
 			test->should_run = false;
+
+		if ((test->run_test == NULL && test->run_serial_test == NULL) ||
+		    (test->run_test != NULL && test->run_serial_test != NULL)) {
+			fprintf(stderr, "Test %d:%s must have either test_%s() or serial_test_%sl() defined.\n",
+				test->test_num, test->test_name, test->test_name, test->test_name);
+			exit(EXIT_ERR_SETUP_INFRA);
+		}
 	}
 
 	/* ignore workers if we are just listing */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 03/14] selftests/bpf: disable perf rate limiting when running tests.
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
  2021-10-06 18:56 ` [PATCH bpf-next v6 01/14] " Yucong Sun
  2021-10-06 18:56 ` [PATCH bpf-next v6 02/14] selftests/bpf: Allow some tests to be executed in sequence Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-08 22:26   ` Andrii Nakryiko
  2021-10-06 18:56 ` [PATCH bpf-next v6 04/14] selftests/bpf: add per worker cgroup suffix Yucong Sun
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong, Yucong Sun

When running tests concurrently, perf rate limiting will often cause
some events to be skipped and make some tests flaky, disabling it making
running tests more robust.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 tools/testing/selftests/bpf/test_progs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 4e2028189471..2ac922f8aa2c 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1376,6 +1376,8 @@ int main(int argc, char **argv)
 		}
 	}
 
+	system("echo 0 > /proc/sys/kernel/perf_cpu_time_max_percent");
+
 	/* ignore workers if we are just listing */
 	if (env.get_test_cnt || env.list_test_names)
 		env.workers = 0;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 04/14] selftests/bpf: add per worker cgroup suffix
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
                   ` (2 preceding siblings ...)
  2021-10-06 18:56 ` [PATCH bpf-next v6 03/14] selftests/bpf: disable perf rate limiting when running tests Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-06 18:56 ` [PATCH bpf-next v6 05/14] selftests/bpf: adding read_perf_max_sample_freq() helper Yucong Sun
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

This patch make each worker use a unique cgroup base directory, thus
allowing tests that uses cgroups to run concurrently.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 tools/testing/selftests/bpf/cgroup_helpers.c | 6 +++---
 tools/testing/selftests/bpf/cgroup_helpers.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/cgroup_helpers.c b/tools/testing/selftests/bpf/cgroup_helpers.c
index f3daa44a8266..8fcd44841bb2 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.c
+++ b/tools/testing/selftests/bpf/cgroup_helpers.c
@@ -11,6 +11,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <ftw.h>
+#include <unistd.h>
 
 #include "cgroup_helpers.h"
 
@@ -33,10 +34,9 @@
 #define CGROUP_MOUNT_DFLT		"/sys/fs/cgroup"
 #define NETCLS_MOUNT_PATH		CGROUP_MOUNT_DFLT "/net_cls"
 #define CGROUP_WORK_DIR			"/cgroup-test-work-dir"
-
 #define format_cgroup_path(buf, path) \
-	snprintf(buf, sizeof(buf), "%s%s%s", CGROUP_MOUNT_PATH, \
-		 CGROUP_WORK_DIR, path)
+	snprintf(buf, sizeof(buf), "%s%s%d%s", CGROUP_MOUNT_PATH, \
+	CGROUP_WORK_DIR, getpid(), path)
 
 #define format_classid_path(buf)				\
 	snprintf(buf, sizeof(buf), "%s%s", NETCLS_MOUNT_PATH,	\
diff --git a/tools/testing/selftests/bpf/cgroup_helpers.h b/tools/testing/selftests/bpf/cgroup_helpers.h
index 629da3854b3e..fcc9cb91b211 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.h
+++ b/tools/testing/selftests/bpf/cgroup_helpers.h
@@ -26,4 +26,4 @@ int join_classid(void);
 int setup_classid_environment(void);
 void cleanup_classid_environment(void);
 
-#endif /* __CGROUP_HELPERS_H */
+#endif /* __CGROUP_HELPERS_H */
\ No newline at end of file
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 05/14] selftests/bpf: adding read_perf_max_sample_freq() helper
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
                   ` (3 preceding siblings ...)
  2021-10-06 18:56 ` [PATCH bpf-next v6 04/14] selftests/bpf: add per worker cgroup suffix Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-08 22:27   ` Andrii Nakryiko
  2021-10-06 18:56 ` [PATCH bpf-next v6 06/14] selftests/bpf: fix race condition in enable_stats Yucong Sun
                   ` (9 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

This patch moved a helper function to test_progs and make all tests
setting sampling frequency use it to read current perf_max_sample_freq,
this will avoid triggering EINVAL error.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 .../selftests/bpf/prog_tests/bpf_cookie.c     |  2 +-
 .../selftests/bpf/prog_tests/perf_branches.c  |  4 ++--
 .../selftests/bpf/prog_tests/perf_link.c      |  2 +-
 .../bpf/prog_tests/stacktrace_build_id_nmi.c  | 19 ++-----------------
 tools/testing/selftests/bpf/test_progs.c      | 15 +++++++++++++++
 tools/testing/selftests/bpf/test_progs.h      |  1 +
 6 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
index 5eea3c3a40fe..19c9f7b53cfa 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
@@ -193,7 +193,7 @@ static void pe_subtest(struct test_bpf_cookie *skel)
 	attr.type = PERF_TYPE_SOFTWARE;
 	attr.config = PERF_COUNT_SW_CPU_CLOCK;
 	attr.freq = 1;
-	attr.sample_freq = 4000;
+	attr.sample_freq = read_perf_max_sample_freq();
 	pfd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, PERF_FLAG_FD_CLOEXEC);
 	if (!ASSERT_GE(pfd, 0, "perf_fd"))
 		goto cleanup;
diff --git a/tools/testing/selftests/bpf/prog_tests/perf_branches.c b/tools/testing/selftests/bpf/prog_tests/perf_branches.c
index 12c4f45cee1a..6b2e3dced619 100644
--- a/tools/testing/selftests/bpf/prog_tests/perf_branches.c
+++ b/tools/testing/selftests/bpf/prog_tests/perf_branches.c
@@ -110,7 +110,7 @@ static void test_perf_branches_hw(void)
 	attr.type = PERF_TYPE_HARDWARE;
 	attr.config = PERF_COUNT_HW_CPU_CYCLES;
 	attr.freq = 1;
-	attr.sample_freq = 4000;
+	attr.sample_freq = read_perf_max_sample_freq();
 	attr.sample_type = PERF_SAMPLE_BRANCH_STACK;
 	attr.branch_sample_type = PERF_SAMPLE_BRANCH_USER | PERF_SAMPLE_BRANCH_ANY;
 	pfd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, PERF_FLAG_FD_CLOEXEC);
@@ -151,7 +151,7 @@ static void test_perf_branches_no_hw(void)
 	attr.type = PERF_TYPE_SOFTWARE;
 	attr.config = PERF_COUNT_SW_CPU_CLOCK;
 	attr.freq = 1;
-	attr.sample_freq = 4000;
+	attr.sample_freq = read_perf_max_sample_freq();
 	pfd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, PERF_FLAG_FD_CLOEXEC);
 	if (CHECK(pfd < 0, "perf_event_open", "err %d\n", pfd))
 		return;
diff --git a/tools/testing/selftests/bpf/prog_tests/perf_link.c b/tools/testing/selftests/bpf/prog_tests/perf_link.c
index b1abd0c46607..74e5bd5f1c19 100644
--- a/tools/testing/selftests/bpf/prog_tests/perf_link.c
+++ b/tools/testing/selftests/bpf/prog_tests/perf_link.c
@@ -38,7 +38,7 @@ void test_perf_link(void)
 	attr.type = PERF_TYPE_SOFTWARE;
 	attr.config = PERF_COUNT_SW_CPU_CLOCK;
 	attr.freq = 1;
-	attr.sample_freq = 4000;
+	attr.sample_freq = read_perf_max_sample_freq();
 	pfd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, PERF_FLAG_FD_CLOEXEC);
 	if (!ASSERT_GE(pfd, 0, "perf_fd"))
 		goto cleanup;
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
index 0a91d8d9954b..150536f01442 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
@@ -2,21 +2,6 @@
 #include <test_progs.h>
 #include "test_stacktrace_build_id.skel.h"
 
-static __u64 read_perf_max_sample_freq(void)
-{
-	__u64 sample_freq = 5000; /* fallback to 5000 on error */
-	FILE *f;
-	__u32 duration = 0;
-
-	f = fopen("/proc/sys/kernel/perf_event_max_sample_rate", "r");
-	if (f == NULL)
-		return sample_freq;
-	CHECK(fscanf(f, "%llu", &sample_freq) != 1, "Get max sample rate",
-		  "return default value: 5000,err %d\n", -errno);
-	fclose(f);
-	return sample_freq;
-}
-
 void test_stacktrace_build_id_nmi(void)
 {
 	int control_map_fd, stackid_hmap_fd, stackmap_fd;
@@ -34,8 +19,6 @@ void test_stacktrace_build_id_nmi(void)
 	int build_id_matches = 0;
 	int retry = 1;
 
-	attr.sample_freq = read_perf_max_sample_freq();
-
 retry:
 	skel = test_stacktrace_build_id__open();
 	if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
@@ -48,6 +31,8 @@ void test_stacktrace_build_id_nmi(void)
 	if (CHECK(err, "skel_load", "skeleton load failed: %d\n", err))
 		goto cleanup;
 
+	attr.sample_freq = read_perf_max_sample_freq();
+
 	pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
 			 0 /* cpu 0 */, -1 /* group id */,
 			 0 /* flags */);
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 2ac922f8aa2c..66825313414b 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1500,3 +1500,18 @@ int main(int argc, char **argv)
 
 	return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
 }
+
+__u64 read_perf_max_sample_freq(void)
+{
+	__u64 sample_freq = 1000; /* fallback to 1000 on error */
+	FILE *f;
+	__u32 duration = 0;
+
+	f = fopen("/proc/sys/kernel/perf_event_max_sample_rate", "r");
+	if (f == NULL)
+		return sample_freq;
+	CHECK(fscanf(f, "%llu", &sample_freq) != 1, "Get max sample rate",
+	      "return default value: 5000,err %d\n", -errno);
+	fclose(f);
+	return sample_freq;
+}
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index b239dc9fcef0..d5ca0d36cc96 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -327,6 +327,7 @@ int extract_build_id(char *build_id, size_t size);
 int kern_sync_rcu(void);
 int trigger_module_test_read(int read_sz);
 int trigger_module_test_write(int write_sz);
+__u64 read_perf_max_sample_freq(void);
 
 #ifdef __x86_64__
 #define SYS_NANOSLEEP_KPROBE_NAME "__x64_sys_nanosleep"
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 06/14] selftests/bpf: fix race condition in enable_stats
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
                   ` (4 preceding siblings ...)
  2021-10-06 18:56 ` [PATCH bpf-next v6 05/14] selftests/bpf: adding read_perf_max_sample_freq() helper Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-06 18:56 ` [PATCH bpf-next v6 07/14] selftests/bpf: make cgroup_v1v2 use its own port Yucong Sun
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

In parallel execution mode, this test now need to use atomic operation
to avoid race condition.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 tools/testing/selftests/bpf/progs/test_enable_stats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/progs/test_enable_stats.c b/tools/testing/selftests/bpf/progs/test_enable_stats.c
index 01a002ade529..1705097d01d7 100644
--- a/tools/testing/selftests/bpf/progs/test_enable_stats.c
+++ b/tools/testing/selftests/bpf/progs/test_enable_stats.c
@@ -13,6 +13,6 @@ __u64 count = 0;
 SEC("raw_tracepoint/sys_enter")
 int test_enable_stats(void *ctx)
 {
-	count += 1;
+	__sync_fetch_and_add(&count, 1);
 	return 0;
 }
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 07/14] selftests/bpf: make cgroup_v1v2 use its own port
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
                   ` (5 preceding siblings ...)
  2021-10-06 18:56 ` [PATCH bpf-next v6 06/14] selftests/bpf: fix race condition in enable_stats Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-06 18:56 ` [PATCH bpf-next v6 08/14] selftests/bpf: adding a namespace reset for tc_redirect Yucong Sun
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

This patch change cgroup_v1v2 use a different port, avoid conflict with
other tests.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c | 2 +-
 tools/testing/selftests/bpf/progs/connect4_dropper.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c b/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c
index ab3b9bc5e6d1..9026b42914d3 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c
@@ -46,7 +46,7 @@ void test_cgroup_v1v2(void)
 {
 	struct network_helper_opts opts = {};
 	int server_fd, client_fd, cgroup_fd;
-	static const int port = 60123;
+	static const int port = 60120;
 
 	/* Step 1: Check base connectivity works without any BPF. */
 	server_fd = start_server(AF_INET, SOCK_STREAM, NULL, port, 0);
diff --git a/tools/testing/selftests/bpf/progs/connect4_dropper.c b/tools/testing/selftests/bpf/progs/connect4_dropper.c
index b565d997810a..d3f4c5e4fb69 100644
--- a/tools/testing/selftests/bpf/progs/connect4_dropper.c
+++ b/tools/testing/selftests/bpf/progs/connect4_dropper.c
@@ -18,7 +18,7 @@ int connect_v4_dropper(struct bpf_sock_addr *ctx)
 {
 	if (ctx->type != SOCK_STREAM)
 		return VERDICT_PROCEED;
-	if (ctx->user_port == bpf_htons(60123))
+	if (ctx->user_port == bpf_htons(60120))
 		return VERDICT_REJECT;
 	return VERDICT_PROCEED;
 }
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 08/14] selftests/bpf: adding a namespace reset for tc_redirect
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
                   ` (6 preceding siblings ...)
  2021-10-06 18:56 ` [PATCH bpf-next v6 07/14] selftests/bpf: make cgroup_v1v2 use its own port Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-08 22:27   ` Andrii Nakryiko
  2021-10-06 18:56 ` [PATCH bpf-next v6 09/14] selftests/bpf: Make uprobe tests use different attach functions Yucong Sun
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

This patch delete ns_src/ns_dst/ns_redir namespaces before recreating
them, making the test more robust.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 .../testing/selftests/bpf/prog_tests/tc_redirect.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
index e87bc4466d9a..25744136e131 100644
--- a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
+++ b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
@@ -176,6 +176,18 @@ static int netns_setup_namespaces(const char *verb)
 	return 0;
 }
 
+static void netns_setup_namespaces_nofail(const char *verb)
+{
+	const char * const *ns = namespaces;
+	char cmd[128];
+
+	while (*ns) {
+		snprintf(cmd, sizeof(cmd), "ip netns %s %s", verb, *ns);
+		system(cmd);
+		ns++;
+	}
+}
+
 struct netns_setup_result {
 	int ifindex_veth_src_fwd;
 	int ifindex_veth_dst_fwd;
@@ -762,6 +774,8 @@ static void test_tc_redirect_peer_l3(struct netns_setup_result *setup_result)
 
 static void *test_tc_redirect_run_tests(void *arg)
 {
+	netns_setup_namespaces_nofail("delete");
+
 	RUN_TEST(tc_redirect_peer);
 	RUN_TEST(tc_redirect_peer_l3);
 	RUN_TEST(tc_redirect_neigh);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 09/14] selftests/bpf: Make uprobe tests use different attach functions.
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
                   ` (7 preceding siblings ...)
  2021-10-06 18:56 ` [PATCH bpf-next v6 08/14] selftests/bpf: adding a namespace reset for tc_redirect Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-08 22:27   ` Andrii Nakryiko
  2021-10-06 18:56 ` [PATCH bpf-next v6 10/14] selftests/bpf: adding pid filtering for atomics test Yucong Sun
                   ` (5 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

Using same address on different processes of the same binary often fail
with EINVAL, this patch make these tests use distinct methods, so they
can run in parallel.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/attach_probe.c | 8 ++++++--
 tools/testing/selftests/bpf/prog_tests/bpf_cookie.c   | 8 ++++++--
 tools/testing/selftests/bpf/prog_tests/task_pt_regs.c | 8 ++++++--
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
index 6c511dcd1465..eff36ba9c148 100644
--- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -5,6 +5,10 @@
 /* this is how USDT semaphore is actually defined, except volatile modifier */
 volatile unsigned short uprobe_ref_ctr __attribute__((unused)) __attribute((section(".probes")));
 
+static int method() {
+	return get_base_addr();
+}
+
 void test_attach_probe(void)
 {
 	DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, uprobe_opts);
@@ -33,7 +37,7 @@ void test_attach_probe(void)
 	if (CHECK(base_addr < 0, "get_base_addr",
 		  "failed to find base addr: %zd", base_addr))
 		return;
-	uprobe_offset = get_uprobe_offset(&get_base_addr, base_addr);
+	uprobe_offset = get_uprobe_offset(&method, base_addr);
 
 	ref_ctr_offset = get_rel_offset((uintptr_t)&uprobe_ref_ctr);
 	if (!ASSERT_GE(ref_ctr_offset, 0, "ref_ctr_offset"))
@@ -98,7 +102,7 @@ void test_attach_probe(void)
 		goto cleanup;
 
 	/* trigger & validate uprobe & uretprobe */
-	get_base_addr();
+	method();
 
 	if (CHECK(skel->bss->uprobe_res != 3, "check_uprobe_res",
 		  "wrong uprobe res: %d\n", skel->bss->uprobe_res))
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
index 19c9f7b53cfa..5ebd8ba988e2 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
@@ -8,6 +8,10 @@
 #include <test_progs.h>
 #include "test_bpf_cookie.skel.h"
 
+static int method() {
+	return get_base_addr();
+}
+
 static void kprobe_subtest(struct test_bpf_cookie *skel)
 {
 	DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts);
@@ -66,7 +70,7 @@ static void uprobe_subtest(struct test_bpf_cookie *skel)
 	ssize_t base_addr;
 
 	base_addr = get_base_addr();
-	uprobe_offset = get_uprobe_offset(&get_base_addr, base_addr);
+	uprobe_offset = get_uprobe_offset(&method, base_addr);
 
 	/* attach two uprobes */
 	opts.bpf_cookie = 0x100;
@@ -99,7 +103,7 @@ static void uprobe_subtest(struct test_bpf_cookie *skel)
 		goto cleanup;
 
 	/* trigger uprobe && uretprobe */
-	get_base_addr();
+	method();
 
 	ASSERT_EQ(skel->bss->uprobe_res, 0x100 | 0x200, "uprobe_res");
 	ASSERT_EQ(skel->bss->uretprobe_res, 0x1000 | 0x2000, "uretprobe_res");
diff --git a/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c b/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
index 37c20b5ffa70..4d2f1435be90 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
+++ b/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
@@ -3,6 +3,10 @@
 #include <test_progs.h>
 #include "test_task_pt_regs.skel.h"
 
+static int method() {
+	return get_base_addr();
+}
+
 void test_task_pt_regs(void)
 {
 	struct test_task_pt_regs *skel;
@@ -14,7 +18,7 @@ void test_task_pt_regs(void)
 	base_addr = get_base_addr();
 	if (!ASSERT_GT(base_addr, 0, "get_base_addr"))
 		return;
-	uprobe_offset = get_uprobe_offset(&get_base_addr, base_addr);
+	uprobe_offset = get_uprobe_offset(&method, base_addr);
 
 	skel = test_task_pt_regs__open_and_load();
 	if (!ASSERT_OK_PTR(skel, "skel_open"))
@@ -32,7 +36,7 @@ void test_task_pt_regs(void)
 	skel->links.handle_uprobe = uprobe_link;
 
 	/* trigger & validate uprobe */
-	get_base_addr();
+	method();
 
 	if (!ASSERT_EQ(skel->bss->uprobe_res, 1, "check_uprobe_res"))
 		goto cleanup;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 10/14] selftests/bpf: adding pid filtering for atomics test
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
                   ` (8 preceding siblings ...)
  2021-10-06 18:56 ` [PATCH bpf-next v6 09/14] selftests/bpf: Make uprobe tests use different attach functions Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-06 18:56 ` [PATCH bpf-next v6 11/14] selftests/bpf: adding random delay for send_signal test Yucong Sun
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

This make atomics test able to run in parallel with other tests.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/atomics.c |  1 +
 tools/testing/selftests/bpf/progs/atomics.c      | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/atomics.c b/tools/testing/selftests/bpf/prog_tests/atomics.c
index ba0e1efe5a45..1486be5d3209 100644
--- a/tools/testing/selftests/bpf/prog_tests/atomics.c
+++ b/tools/testing/selftests/bpf/prog_tests/atomics.c
@@ -225,6 +225,7 @@ void test_atomics(void)
 		test__skip();
 		goto cleanup;
 	}
+	skel->bss->pid = getpid();
 
 	if (test__start_subtest("add"))
 		test_add(skel);
diff --git a/tools/testing/selftests/bpf/progs/atomics.c b/tools/testing/selftests/bpf/progs/atomics.c
index c245345e41ca..16e57313204a 100644
--- a/tools/testing/selftests/bpf/progs/atomics.c
+++ b/tools/testing/selftests/bpf/progs/atomics.c
@@ -10,6 +10,8 @@ bool skip_tests __attribute((__section__(".data"))) = false;
 bool skip_tests = true;
 #endif
 
+__u32 pid = 0;
+
 __u64 add64_value = 1;
 __u64 add64_result = 0;
 __u32 add32_value = 1;
@@ -21,6 +23,8 @@ __u64 add_noreturn_value = 1;
 SEC("fentry/bpf_fentry_test1")
 int BPF_PROG(add, int a)
 {
+	if (pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
 #ifdef ENABLE_ATOMICS_TESTS
 	__u64 add_stack_value = 1;
 
@@ -45,6 +49,8 @@ __s64 sub_noreturn_value = 1;
 SEC("fentry/bpf_fentry_test1")
 int BPF_PROG(sub, int a)
 {
+	if (pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
 #ifdef ENABLE_ATOMICS_TESTS
 	__u64 sub_stack_value = 1;
 
@@ -67,6 +73,8 @@ __u64 and_noreturn_value = (0x110ull << 32);
 SEC("fentry/bpf_fentry_test1")
 int BPF_PROG(and, int a)
 {
+	if (pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
 #ifdef ENABLE_ATOMICS_TESTS
 
 	and64_result = __sync_fetch_and_and(&and64_value, 0x011ull << 32);
@@ -86,6 +94,8 @@ __u64 or_noreturn_value = (0x110ull << 32);
 SEC("fentry/bpf_fentry_test1")
 int BPF_PROG(or, int a)
 {
+	if (pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
 #ifdef ENABLE_ATOMICS_TESTS
 	or64_result = __sync_fetch_and_or(&or64_value, 0x011ull << 32);
 	or32_result = __sync_fetch_and_or(&or32_value, 0x011);
@@ -104,6 +114,8 @@ __u64 xor_noreturn_value = (0x110ull << 32);
 SEC("fentry/bpf_fentry_test1")
 int BPF_PROG(xor, int a)
 {
+	if (pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
 #ifdef ENABLE_ATOMICS_TESTS
 	xor64_result = __sync_fetch_and_xor(&xor64_value, 0x011ull << 32);
 	xor32_result = __sync_fetch_and_xor(&xor32_value, 0x011);
@@ -123,6 +135,8 @@ __u32 cmpxchg32_result_succeed = 0;
 SEC("fentry/bpf_fentry_test1")
 int BPF_PROG(cmpxchg, int a)
 {
+	if (pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
 #ifdef ENABLE_ATOMICS_TESTS
 	cmpxchg64_result_fail = __sync_val_compare_and_swap(&cmpxchg64_value, 0, 3);
 	cmpxchg64_result_succeed = __sync_val_compare_and_swap(&cmpxchg64_value, 1, 2);
@@ -142,6 +156,8 @@ __u32 xchg32_result = 0;
 SEC("fentry/bpf_fentry_test1")
 int BPF_PROG(xchg, int a)
 {
+	if (pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
 #ifdef ENABLE_ATOMICS_TESTS
 	__u64 val64 = 2;
 	__u32 val32 = 2;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 11/14] selftests/bpf: adding random delay for send_signal test
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
                   ` (9 preceding siblings ...)
  2021-10-06 18:56 ` [PATCH bpf-next v6 10/14] selftests/bpf: adding pid filtering for atomics test Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-08 22:27   ` Andrii Nakryiko
  2021-10-06 18:56 ` [PATCH bpf-next v6 12/14] selftests/bpf: Fix pid check in fexit_sleep test Yucong Sun
                   ` (3 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

This patch adds random delay on waiting for the signal to arrive,
making the test more robust.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/send_signal.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
index 776916b61c40..6200256243f2 100644
--- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
+++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
@@ -19,6 +19,7 @@ static void test_send_signal_common(struct perf_event_attr *attr,
 	int err = -1, pmu_fd = -1;
 	char buf[256];
 	pid_t pid;
+	int attempts = 100;
 
 	if (!ASSERT_OK(pipe(pipe_c2p), "pipe_c2p"))
 		return;
@@ -63,7 +64,10 @@ static void test_send_signal_common(struct perf_event_attr *attr,
 		ASSERT_EQ(read(pipe_p2c[0], buf, 1), 1, "pipe_read");
 
 		/* wait a little for signal handler */
-		sleep(1);
+		while (attempts > 0 && !sigusr1_received) {
+			attempts--;
+			usleep(500 + rand() % 500);
+		}
 
 		buf[0] = sigusr1_received ? '2' : '0';
 		ASSERT_EQ(write(pipe_c2p[1], buf, 1), 1, "pipe_write");
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 12/14] selftests/bpf: Fix pid check in fexit_sleep test
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
                   ` (10 preceding siblings ...)
  2021-10-06 18:56 ` [PATCH bpf-next v6 11/14] selftests/bpf: adding random delay for send_signal test Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-06 18:56 ` [PATCH bpf-next v6 13/14] selftests/bpf: increase loop count for perf_branches Yucong Sun
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

bpf_get_current_pid_tgid() returns u64, whose upper 32 bits are the same
as userspace getpid() return value.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 tools/testing/selftests/bpf/progs/fexit_sleep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/fexit_sleep.c b/tools/testing/selftests/bpf/progs/fexit_sleep.c
index 03a672d76353..bca92c9bd29a 100644
--- a/tools/testing/selftests/bpf/progs/fexit_sleep.c
+++ b/tools/testing/selftests/bpf/progs/fexit_sleep.c
@@ -13,7 +13,7 @@ int fexit_cnt = 0;
 SEC("fentry/__x64_sys_nanosleep")
 int BPF_PROG(nanosleep_fentry, const struct pt_regs *regs)
 {
-	if ((int)bpf_get_current_pid_tgid() != pid)
+	if (bpf_get_current_pid_tgid() >> 32 != pid)
 		return 0;
 
 	fentry_cnt++;
@@ -23,7 +23,7 @@ int BPF_PROG(nanosleep_fentry, const struct pt_regs *regs)
 SEC("fexit/__x64_sys_nanosleep")
 int BPF_PROG(nanosleep_fexit, const struct pt_regs *regs, int ret)
 {
-	if ((int)bpf_get_current_pid_tgid() != pid)
+	if (bpf_get_current_pid_tgid() >> 32 != pid)
 		return 0;
 
 	fexit_cnt++;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 13/14] selftests/bpf: increase loop count for perf_branches
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
                   ` (11 preceding siblings ...)
  2021-10-06 18:56 ` [PATCH bpf-next v6 12/14] selftests/bpf: Fix pid check in fexit_sleep test Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-08 22:27   ` Andrii Nakryiko
  2021-10-06 18:56 ` [PATCH bpf-next v6 14/14] selfetest/bpf: make some tests serial Yucong Sun
  2021-10-08 22:26 ` [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Andrii Nakryiko
  14 siblings, 1 reply; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

This make this test more likely to succeed.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/perf_branches.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/perf_branches.c b/tools/testing/selftests/bpf/prog_tests/perf_branches.c
index 6b2e3dced619..d7e88b2c5f36 100644
--- a/tools/testing/selftests/bpf/prog_tests/perf_branches.c
+++ b/tools/testing/selftests/bpf/prog_tests/perf_branches.c
@@ -16,7 +16,7 @@ static void check_good_sample(struct test_perf_branches *skel)
 	int duration = 0;
 
 	if (CHECK(!skel->bss->valid, "output not valid",
-		 "no valid sample from prog"))
+		 "no valid sample from prog\n"))
 		return;
 
 	/*
@@ -46,7 +46,7 @@ static void check_bad_sample(struct test_perf_branches *skel)
 	int duration = 0;
 
 	if (CHECK(!skel->bss->valid, "output not valid",
-		 "no valid sample from prog"))
+		 "no valid sample from prog\n"))
 		return;
 
 	CHECK((required_size != -EINVAL && required_size != -ENOENT),
@@ -84,7 +84,7 @@ static void test_perf_branches_common(int perf_fd,
 	if (CHECK(err, "set_affinity", "cpu #0, err %d\n", err))
 		goto out_destroy;
 	/* spin the loop for a while (random high number) */
-	for (i = 0; i < 1000000; ++i)
+	for (i = 0; i < 100000000; ++i)
 		++j;
 
 	test_perf_branches__detach(skel);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH bpf-next v6 14/14] selfetest/bpf: make some tests serial
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
                   ` (12 preceding siblings ...)
  2021-10-06 18:56 ` [PATCH bpf-next v6 13/14] selftests/bpf: increase loop count for perf_branches Yucong Sun
@ 2021-10-06 18:56 ` Yucong Sun
  2021-10-08 22:27   ` Andrii Nakryiko
  2021-10-08 22:26 ` [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Andrii Nakryiko
  14 siblings, 1 reply; 39+ messages in thread
From: Yucong Sun @ 2021-10-06 18:56 UTC (permalink / raw)
  To: bpf; +Cc: andrii, sunyucong

From: Yucong Sun <sunyucong@gmail.com>

Change tests that often fails in parallel execution mode to serial.

Signed-off-by: Yucong Sun <sunyucong@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c   | 2 +-
 tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c            | 2 +-
 tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c      | 2 +-
 .../selftests/bpf/prog_tests/cgroup_attach_autodetach.c        | 2 +-
 tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c   | 2 +-
 .../testing/selftests/bpf/prog_tests/cgroup_attach_override.c  | 2 +-
 tools/testing/selftests/bpf/prog_tests/cgroup_link.c           | 2 +-
 tools/testing/selftests/bpf/prog_tests/check_mtu.c             | 2 +-
 tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c         | 3 ++-
 .../selftests/bpf/prog_tests/flow_dissector_load_bytes.c       | 2 +-
 .../testing/selftests/bpf/prog_tests/flow_dissector_reattach.c | 2 +-
 tools/testing/selftests/bpf/prog_tests/get_branch_snapshot.c   | 2 +-
 tools/testing/selftests/bpf/prog_tests/kfree_skb.c             | 3 ++-
 tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c     | 2 +-
 tools/testing/selftests/bpf/prog_tests/modify_return.c         | 3 ++-
 tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c   | 3 ++-
 tools/testing/selftests/bpf/prog_tests/perf_buffer.c           | 2 +-
 tools/testing/selftests/bpf/prog_tests/perf_link.c             | 3 ++-
 tools/testing/selftests/bpf/prog_tests/probe_user.c            | 3 ++-
 .../selftests/bpf/prog_tests/raw_tp_writable_test_run.c        | 3 ++-
 tools/testing/selftests/bpf/prog_tests/select_reuseport.c      | 2 +-
 .../selftests/bpf/prog_tests/send_signal_sched_switch.c        | 3 ++-
 tools/testing/selftests/bpf/prog_tests/sk_storage_tracing.c    | 2 +-
 tools/testing/selftests/bpf/prog_tests/snprintf_btf.c          | 2 +-
 tools/testing/selftests/bpf/prog_tests/sock_fields.c           | 2 +-
 tools/testing/selftests/bpf/prog_tests/sockmap_listen.c        | 2 +-
 tools/testing/selftests/bpf/prog_tests/timer.c                 | 3 ++-
 tools/testing/selftests/bpf/prog_tests/timer_mim.c             | 2 +-
 tools/testing/selftests/bpf/prog_tests/tp_attach_query.c       | 2 +-
 tools/testing/selftests/bpf/prog_tests/trace_printk.c          | 2 +-
 tools/testing/selftests/bpf/prog_tests/trace_vprintk.c         | 2 +-
 tools/testing/selftests/bpf/prog_tests/trampoline_count.c      | 3 ++-
 tools/testing/selftests/bpf/prog_tests/xdp_attach.c            | 2 +-
 tools/testing/selftests/bpf/prog_tests/xdp_bonding.c           | 2 +-
 tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c     | 2 +-
 tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c     | 2 +-
 tools/testing/selftests/bpf/prog_tests/xdp_info.c              | 2 +-
 tools/testing/selftests/bpf/prog_tests/xdp_link.c              | 2 +-
 38 files changed, 48 insertions(+), 38 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c
index 85babb0487b3..b52ff8ce34db 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c
@@ -179,7 +179,7 @@ static void do_bpf_iter_setsockopt(struct bpf_iter_setsockopt *iter_skel,
 	free_fds(est_fds, nr_est);
 }
 
-void test_bpf_iter_setsockopt(void)
+void serial_test_bpf_iter_setsockopt(void)
 {
 	struct bpf_iter_setsockopt *iter_skel = NULL;
 	struct bpf_cubic *cubic_skel = NULL;
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c b/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
index 284d5921c345..eb8eeebe6935 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
@@ -3,7 +3,7 @@
 
 #define nr_iters 2
 
-void test_bpf_obj_id(void)
+void serial_test_bpf_obj_id(void)
 {
 	const __u64 array_magic_value = 0xfaceb00c;
 	const __u32 array_key = 0;
diff --git a/tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c b/tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c
index 876be0ecb654..621c57222191 100644
--- a/tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c
+++ b/tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c
@@ -363,7 +363,7 @@ static void test_shared(int parent_cgroup_fd, int child_cgroup_fd)
 	cg_storage_multi_shared__destroy(obj);
 }
 
-void test_cg_storage_multi(void)
+void serial_test_cg_storage_multi(void)
 {
 	int parent_cgroup_fd = -1, child_cgroup_fd = -1;
 
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
index 70e94e783070..5de485c7370f 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
@@ -21,7 +21,7 @@ static int prog_load(void)
 			       bpf_log_buf, BPF_LOG_BUF_SIZE);
 }
 
-void test_cgroup_attach_autodetach(void)
+void serial_test_cgroup_attach_autodetach(void)
 {
 	__u32 duration = 0, prog_cnt = 4, attach_flags;
 	int allow_prog[2] = {-1};
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
index 20bb8831dda6..731bea84d8ed 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
@@ -74,7 +74,7 @@ static int prog_load_cnt(int verdict, int val)
 	return ret;
 }
 
-void test_cgroup_attach_multi(void)
+void serial_test_cgroup_attach_multi(void)
 {
 	__u32 prog_ids[4], prog_cnt = 0, attach_flags, saved_prog_id;
 	int cg1 = 0, cg2 = 0, cg3 = 0, cg4 = 0, cg5 = 0, key = 0;
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
index 9e96f8d87fea..10d3c33821a7 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
@@ -23,7 +23,7 @@ static int prog_load(int verdict)
 			       bpf_log_buf, BPF_LOG_BUF_SIZE);
 }
 
-void test_cgroup_attach_override(void)
+void serial_test_cgroup_attach_override(void)
 {
 	int drop_prog = -1, allow_prog = -1, foo = -1, bar = -1;
 	__u32 duration = 0;
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
index 9091524131d6..9e6e6aad347c 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
@@ -24,7 +24,7 @@ int ping_and_check(int exp_calls, int exp_alt_calls)
 	return 0;
 }
 
-void test_cgroup_link(void)
+void serial_test_cgroup_link(void)
 {
 	struct {
 		const char *path;
diff --git a/tools/testing/selftests/bpf/prog_tests/check_mtu.c b/tools/testing/selftests/bpf/prog_tests/check_mtu.c
index 012068f33a0a..f73e6e36b74d 100644
--- a/tools/testing/selftests/bpf/prog_tests/check_mtu.c
+++ b/tools/testing/selftests/bpf/prog_tests/check_mtu.c
@@ -195,7 +195,7 @@ static void test_check_mtu_tc(__u32 mtu, __u32 ifindex)
 	test_check_mtu__destroy(skel);
 }
 
-void test_check_mtu(void)
+void serial_test_check_mtu(void)
 {
 	__u32 mtu_lo;
 
diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
index 2839f4270a26..9cff14a23bb7 100644
--- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
@@ -380,7 +380,8 @@ static void test_func_map_prog_compatibility(void)
 				     "./test_attach_probe.o");
 }
 
-void test_fexit_bpf2bpf(void)
+/* NOTE: affect other tests, must run in serial mode */
+void serial_test_fexit_bpf2bpf(void)
 {
 	if (test__start_subtest("target_no_callees"))
 		test_target_no_callees();
diff --git a/tools/testing/selftests/bpf/prog_tests/flow_dissector_load_bytes.c b/tools/testing/selftests/bpf/prog_tests/flow_dissector_load_bytes.c
index 0e8a4d2f023d..6093728497c7 100644
--- a/tools/testing/selftests/bpf/prog_tests/flow_dissector_load_bytes.c
+++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector_load_bytes.c
@@ -2,7 +2,7 @@
 #include <test_progs.h>
 #include <network_helpers.h>
 
-void test_flow_dissector_load_bytes(void)
+void serial_test_flow_dissector_load_bytes(void)
 {
 	struct bpf_flow_keys flow_keys;
 	__u32 duration = 0, retval, size;
diff --git a/tools/testing/selftests/bpf/prog_tests/flow_dissector_reattach.c b/tools/testing/selftests/bpf/prog_tests/flow_dissector_reattach.c
index 3931ede5c534..f0c6c226aba8 100644
--- a/tools/testing/selftests/bpf/prog_tests/flow_dissector_reattach.c
+++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector_reattach.c
@@ -628,7 +628,7 @@ static void run_tests(int netns)
 	}
 }
 
-void test_flow_dissector_reattach(void)
+void serial_test_flow_dissector_reattach(void)
 {
 	int err, new_net, saved_net;
 
diff --git a/tools/testing/selftests/bpf/prog_tests/get_branch_snapshot.c b/tools/testing/selftests/bpf/prog_tests/get_branch_snapshot.c
index 67e86f8d8677..4ef4f284b462 100644
--- a/tools/testing/selftests/bpf/prog_tests/get_branch_snapshot.c
+++ b/tools/testing/selftests/bpf/prog_tests/get_branch_snapshot.c
@@ -49,7 +49,7 @@ static void close_perf_events(void)
 	free(pfd_array);
 }
 
-void test_get_branch_snapshot(void)
+void serial_test_get_branch_snapshot(void)
 {
 	struct get_branch_snapshot *skel = NULL;
 	int err;
diff --git a/tools/testing/selftests/bpf/prog_tests/kfree_skb.c b/tools/testing/selftests/bpf/prog_tests/kfree_skb.c
index ddfb6bf97152..032a322d51f2 100644
--- a/tools/testing/selftests/bpf/prog_tests/kfree_skb.c
+++ b/tools/testing/selftests/bpf/prog_tests/kfree_skb.c
@@ -48,7 +48,8 @@ static void on_sample(void *ctx, int cpu, void *data, __u32 size)
 	*(bool *)ctx = true;
 }
 
-void test_kfree_skb(void)
+/* TODO: fix kernel panic caused by this test in parallel mode */
+void serial_test_kfree_skb(void)
 {
 	struct __sk_buff skb = {};
 	struct bpf_prog_test_run_attr tattr = {
diff --git a/tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c b/tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c
index 59adb4715394..7589c03fd26b 100644
--- a/tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c
+++ b/tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c
@@ -541,7 +541,7 @@ static void run_test(struct migrate_reuseport_test_case *test_case,
 	}
 }
 
-void test_migrate_reuseport(void)
+void serial_test_migrate_reuseport(void)
 {
 	struct test_migrate_reuseport *skel;
 	int i;
diff --git a/tools/testing/selftests/bpf/prog_tests/modify_return.c b/tools/testing/selftests/bpf/prog_tests/modify_return.c
index 97fec70c600b..b772fe30ce9b 100644
--- a/tools/testing/selftests/bpf/prog_tests/modify_return.c
+++ b/tools/testing/selftests/bpf/prog_tests/modify_return.c
@@ -53,7 +53,8 @@ static void run_test(__u32 input_retval, __u16 want_side_effect, __s16 want_ret)
 	modify_return__destroy(skel);
 }
 
-void test_modify_return(void)
+/* TODO: conflict with get_func_ip_test */
+void serial_test_modify_return(void)
 {
 	run_test(0 /* input_retval */,
 		 1 /* want_side_effect */,
diff --git a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
index 2535788e135f..24d493482ffc 100644
--- a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
+++ b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
@@ -78,7 +78,8 @@ static void test_ns_current_pid_tgid_new_ns(void)
 		return;
 }
 
-void test_ns_current_pid_tgid(void)
+/* TODO: use a different tracepoint */
+void serial_test_ns_current_pid_tgid(void)
 {
 	if (test__start_subtest("ns_current_pid_tgid_root_ns"))
 		test_current_pid_tgid(NULL);
diff --git a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
index 6490e9673002..6979aff4aab2 100644
--- a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
+++ b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
@@ -43,7 +43,7 @@ int trigger_on_cpu(int cpu)
 	return 0;
 }
 
-void test_perf_buffer(void)
+void serial_test_perf_buffer(void)
 {
 	int err, on_len, nr_on_cpus = 0, nr_cpus, i;
 	struct perf_buffer_opts pb_opts = {};
diff --git a/tools/testing/selftests/bpf/prog_tests/perf_link.c b/tools/testing/selftests/bpf/prog_tests/perf_link.c
index 74e5bd5f1c19..513b5349539c 100644
--- a/tools/testing/selftests/bpf/prog_tests/perf_link.c
+++ b/tools/testing/selftests/bpf/prog_tests/perf_link.c
@@ -23,7 +23,8 @@ static void burn_cpu(void)
 		++j;
 }
 
-void test_perf_link(void)
+/* TODO: often fails in concurrent mode */
+void serial_test_perf_link(void)
 {
 	struct test_perf_link *skel = NULL;
 	struct perf_event_attr attr;
diff --git a/tools/testing/selftests/bpf/prog_tests/probe_user.c b/tools/testing/selftests/bpf/prog_tests/probe_user.c
index 52fe157e2a90..abf890d066eb 100644
--- a/tools/testing/selftests/bpf/prog_tests/probe_user.c
+++ b/tools/testing/selftests/bpf/prog_tests/probe_user.c
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <test_progs.h>
 
-void test_probe_user(void)
+/* TODO: corrupts other tests uses connect() */
+void serial_test_probe_user(void)
 {
 	const char *prog_name = "handle_sys_connect";
 	const char *obj_file = "./test_probe_user.o";
diff --git a/tools/testing/selftests/bpf/prog_tests/raw_tp_writable_test_run.c b/tools/testing/selftests/bpf/prog_tests/raw_tp_writable_test_run.c
index 5c45424cac5f..ddefa1192e5d 100644
--- a/tools/testing/selftests/bpf/prog_tests/raw_tp_writable_test_run.c
+++ b/tools/testing/selftests/bpf/prog_tests/raw_tp_writable_test_run.c
@@ -3,7 +3,8 @@
 #include <test_progs.h>
 #include <linux/nbd.h>
 
-void test_raw_tp_writable_test_run(void)
+/* NOTE: conflict with other tests. */
+void serial_test_raw_tp_writable_test_run(void)
 {
 	__u32 duration = 0;
 	char error[4096];
diff --git a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c
index d40e9156c48d..3cfc910ab3c1 100644
--- a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c
+++ b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c
@@ -858,7 +858,7 @@ void test_map_type(enum bpf_map_type mt)
 	cleanup();
 }
 
-void test_select_reuseport(void)
+void serial_test_select_reuseport(void)
 {
 	saved_tcp_fo = read_int_sysctl(TCP_FO_SYSCTL);
 	if (saved_tcp_fo < 0)
diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal_sched_switch.c b/tools/testing/selftests/bpf/prog_tests/send_signal_sched_switch.c
index 189a34a7addb..15dacfcfaa6d 100644
--- a/tools/testing/selftests/bpf/prog_tests/send_signal_sched_switch.c
+++ b/tools/testing/selftests/bpf/prog_tests/send_signal_sched_switch.c
@@ -25,7 +25,8 @@ static void *worker(void *p)
 	return NULL;
 }
 
-void test_send_signal_sched_switch(void)
+/* NOTE: cause events loss */
+void serial_test_send_signal_sched_switch(void)
 {
 	struct test_send_signal_kern *skel;
 	pthread_t threads[THREAD_COUNT];
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_storage_tracing.c b/tools/testing/selftests/bpf/prog_tests/sk_storage_tracing.c
index 2b392590e8ca..547ae53cde74 100644
--- a/tools/testing/selftests/bpf/prog_tests/sk_storage_tracing.c
+++ b/tools/testing/selftests/bpf/prog_tests/sk_storage_tracing.c
@@ -105,7 +105,7 @@ static void do_test(void)
 		close(listen_fd);
 }
 
-void test_sk_storage_tracing(void)
+void serial_test_sk_storage_tracing(void)
 {
 	struct test_sk_storage_trace_itself *skel_itself;
 	int err;
diff --git a/tools/testing/selftests/bpf/prog_tests/snprintf_btf.c b/tools/testing/selftests/bpf/prog_tests/snprintf_btf.c
index 76e1f5fe18fa..dd41b826be30 100644
--- a/tools/testing/selftests/bpf/prog_tests/snprintf_btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/snprintf_btf.c
@@ -6,7 +6,7 @@
 /* Demonstrate that bpf_snprintf_btf succeeds and that various data types
  * are formatted correctly.
  */
-void test_snprintf_btf(void)
+void serial_test_snprintf_btf(void)
 {
 	struct netif_receive_skb *skel;
 	struct netif_receive_skb__bss *bss;
diff --git a/tools/testing/selftests/bpf/prog_tests/sock_fields.c b/tools/testing/selftests/bpf/prog_tests/sock_fields.c
index 577d619fb07e..fae40db4d81f 100644
--- a/tools/testing/selftests/bpf/prog_tests/sock_fields.c
+++ b/tools/testing/selftests/bpf/prog_tests/sock_fields.c
@@ -329,7 +329,7 @@ static void test(void)
 		close(listen_fd);
 }
 
-void test_sock_fields(void)
+void serial_test_sock_fields(void)
 {
 	struct bpf_link *egress_link = NULL, *ingress_link = NULL;
 	int parent_cg_fd = -1, child_cg_fd = -1;
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
index 5c5979046523..102c73a00402 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
@@ -2037,7 +2037,7 @@ static void run_tests(struct test_sockmap_listen *skel, struct bpf_map *map,
 	test_udp_unix_redir(skel, map, family);
 }
 
-void test_sockmap_listen(void)
+void serial_test_sockmap_listen(void)
 {
 	struct test_sockmap_listen *skel;
 
diff --git a/tools/testing/selftests/bpf/prog_tests/timer.c b/tools/testing/selftests/bpf/prog_tests/timer.c
index 25f40e1b9967..0f4e49e622cd 100644
--- a/tools/testing/selftests/bpf/prog_tests/timer.c
+++ b/tools/testing/selftests/bpf/prog_tests/timer.c
@@ -39,7 +39,8 @@ static int timer(struct timer *timer_skel)
 	return 0;
 }
 
-void test_timer(void)
+/* TODO: use pid filtering */
+void serial_test_timer(void)
 {
 	struct timer *timer_skel = NULL;
 	int err;
diff --git a/tools/testing/selftests/bpf/prog_tests/timer_mim.c b/tools/testing/selftests/bpf/prog_tests/timer_mim.c
index ced8f6cf347c..949a0617869d 100644
--- a/tools/testing/selftests/bpf/prog_tests/timer_mim.c
+++ b/tools/testing/selftests/bpf/prog_tests/timer_mim.c
@@ -52,7 +52,7 @@ static int timer_mim(struct timer_mim *timer_skel)
 	return 0;
 }
 
-void test_timer_mim(void)
+void serial_test_timer_mim(void)
 {
 	struct timer_mim_reject *timer_reject_skel = NULL;
 	libbpf_print_fn_t old_print_fn = NULL;
diff --git a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
index fb095e5cd9af..8652d0a46c87 100644
--- a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
+++ b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <test_progs.h>
 
-void test_tp_attach_query(void)
+void serial_test_tp_attach_query(void)
 {
 	const int num_progs = 3;
 	int i, j, bytes, efd, err, prog_fd[num_progs], pmu_fd[num_progs];
diff --git a/tools/testing/selftests/bpf/prog_tests/trace_printk.c b/tools/testing/selftests/bpf/prog_tests/trace_printk.c
index e47835f0a674..3f7a7141265e 100644
--- a/tools/testing/selftests/bpf/prog_tests/trace_printk.c
+++ b/tools/testing/selftests/bpf/prog_tests/trace_printk.c
@@ -8,7 +8,7 @@
 #define TRACEBUF	"/sys/kernel/debug/tracing/trace_pipe"
 #define SEARCHMSG	"testing,testing"
 
-void test_trace_printk(void)
+void serial_test_trace_printk(void)
 {
 	int err = 0, iter = 0, found = 0;
 	struct trace_printk__bss *bss;
diff --git a/tools/testing/selftests/bpf/prog_tests/trace_vprintk.c b/tools/testing/selftests/bpf/prog_tests/trace_vprintk.c
index 61a24e62e1a0..46101270cb1a 100644
--- a/tools/testing/selftests/bpf/prog_tests/trace_vprintk.c
+++ b/tools/testing/selftests/bpf/prog_tests/trace_vprintk.c
@@ -8,7 +8,7 @@
 #define TRACEBUF	"/sys/kernel/debug/tracing/trace_pipe"
 #define SEARCHMSG	"1,2,3,4,5,6,7,8,9,10"
 
-void test_trace_vprintk(void)
+void serial_test_trace_vprintk(void)
 {
 	int err = 0, iter = 0, found = 0;
 	struct trace_vprintk__bss *bss;
diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
index d7f5a931d7f3..fc146671b20a 100644
--- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
+++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
@@ -41,7 +41,8 @@ static struct bpf_link *load(struct bpf_object *obj, const char *name)
 	return bpf_program__attach_trace(prog);
 }
 
-void test_trampoline_count(void)
+/* TODO: use different target function to run in concurrent mode */
+void serial_test_trampoline_count(void)
 {
 	const char *fentry_name = "fentry/__set_task_comm";
 	const char *fexit_name = "fexit/__set_task_comm";
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_attach.c
index 15ef3531483e..4c4057262cd8 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_attach.c
@@ -4,7 +4,7 @@
 #define IFINDEX_LO 1
 #define XDP_FLAGS_REPLACE		(1U << 4)
 
-void test_xdp_attach(void)
+void serial_test_xdp_attach(void)
 {
 	__u32 duration = 0, id1, id2, id0 = 0, len;
 	struct bpf_object *obj1, *obj2, *obj3;
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c b/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
index ad3ba81b4048..faa22b84f2ee 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
@@ -519,7 +519,7 @@ static struct bond_test_case bond_test_cases[] = {
 	{ "xdp_bonding_xor_layer34", BOND_MODE_XOR, BOND_XMIT_POLICY_LAYER34, },
 };
 
-void test_xdp_bonding(void)
+void serial_test_xdp_bonding(void)
 {
 	libbpf_print_fn_t old_print_fn;
 	struct skeletons skeletons = {};
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
index 8755effd80b0..fd812bd43600 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
@@ -7,7 +7,7 @@
 
 #define IFINDEX_LO	1
 
-void test_xdp_cpumap_attach(void)
+void serial_test_xdp_cpumap_attach(void)
 {
 	struct test_xdp_with_cpumap_helpers *skel;
 	struct bpf_prog_info info = {};
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
index c72af030ff10..d4e9a9972a67 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
@@ -72,7 +72,7 @@ void test_neg_xdp_devmap_helpers(void)
 }
 
 
-void test_xdp_devmap_attach(void)
+void serial_test_xdp_devmap_attach(void)
 {
 	if (test__start_subtest("DEVMAP with programs in entries"))
 		test_xdp_with_devmap_helpers();
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_info.c b/tools/testing/selftests/bpf/prog_tests/xdp_info.c
index d2d7a283d72f..4e2a4fd56f67 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_info.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_info.c
@@ -4,7 +4,7 @@
 
 #define IFINDEX_LO 1
 
-void test_xdp_info(void)
+void serial_test_xdp_info(void)
 {
 	__u32 len = sizeof(struct bpf_prog_info), duration = 0, prog_id;
 	const char *file = "./xdp_dummy.o";
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_link.c b/tools/testing/selftests/bpf/prog_tests/xdp_link.c
index 46eed0a33c23..983ab0b47d30 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_link.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_link.c
@@ -6,7 +6,7 @@
 
 #define IFINDEX_LO 1
 
-void test_xdp_link(void)
+void serial_test_xdp_link(void)
 {
 	__u32 duration = 0, id1, id2, id0 = 0, prog_fd1, prog_fd2, err;
 	DECLARE_LIBBPF_OPTS(bpf_xdp_set_link_opts, opts, .old_fd = -1);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs
  2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
                   ` (13 preceding siblings ...)
  2021-10-06 18:56 ` [PATCH bpf-next v6 14/14] selfetest/bpf: make some tests serial Yucong Sun
@ 2021-10-08 22:26 ` Andrii Nakryiko
  2021-10-08 22:42   ` Andrii Nakryiko
  14 siblings, 1 reply; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 22:26 UTC (permalink / raw)
  To: Yucong Sun; +Cc: bpf, Andrii Nakryiko, Yucong Sun

On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
>
> This patch series adds "-j" parelell execution to test_progs, with "--debug" to
> display server/worker communications. Also, some Tests that often fails in
> parallel are marked as serial test, and it will run in sequence after parallel
> execution is done.
>
> This patch series also adds a error summary after all tests execution finished.
>

Huge milestone, good job! Applied most patches to bpf-next. See
comments below in respective patches.

We'll need to iterate on improving the stability of parallel mode, but
this is a great start. I've dropped a bunch of "fix up" patches where
I didn't feel confident yet about the approach. We should discuss it
independently from the parallelization changes in this patch set. See
some more thoughts below, but overall:

time sudo ./test_progs -j
...
Summary: 181/977 PASSED, 3 SKIPPED, 0 FAILED

real    0m36.949s
user    0m4.546s
sys     0m30.872s

VS

$ time sudo ./test_progs
...
Summary: 181/977 PASSED, 3 SKIPPED, 0 FAILED

real    1m3.031s
user    0m4.157s
sys     0m28.820s

2x speed up and the gap will just grow over time as we add more tests.
And that's also with bpf_verif_scale as is, which we should break up
into individual tests to parallelize them.

So few things worth mentioning:

1. To focus future efforts on parallelizing existing tests, we should
probably emit how long did the test take.

2. We are losing subtest progress when running in parallel mode. That
sucks. While it's not easy to parallelize subtests, it's easy to send
separate logs for each subtest and display them as they come. Let's do
that?

3. Parallel execution times are not consistent, once I got 30 seconds
(which is 8 seconds faster than sequential, I excluded
bpf_verif_scale), other times it was 45 seconds and more than 1
minute. Not sure what's going on there, but this doesn't look right.

4. A bunch of tests still fail from time to time (see examples below).
What's even scarier that once I got the "failed to determine
tracepoint perf event ID" message, subsequent sequential executions
kept failing. I don't see what selftest could have done to cause this,
so this is concerning and seems to point to the kernel.
/sys/kernel/debug and /sys/kernel/tracing directories were empty at
this point. cc Steven, is there any situation when tracefs can become
"defunct"?

#84 ns_current_pid_tgid:FAIL
test_current_pid_tgid:PASS:skel_open_load 0 nsec
test_current_pid_tgid:PASS:stat 0 nsec
libbpf: failed to determine tracepoint 'syscalls/sys_enter_nanosleep'
perf event ID: No such file or directory
libbpf: prog 'handler': failed to create tracepoint
'syscalls/sys_enter_nanosleep' perf event: No such file or directory
libbpf: failed to auto-attach program 'handler': -2
test_current_pid_tgid:FAIL:skel_attach skeleton attach failed: -2
#84/1 ns_current_pid_tgid/ns_current_pid_tgid_root_ns:FAIL
test_ns_current_pid_tgid_new_ns:PASS:clone 0 nsec
test_ns_current_pid_tgid_new_ns:PASS:waitpid 0 nsec
test_ns_current_pid_tgid_new_ns:FAIL:newns_pidtgid failed#84/2
ns_current_pid_tgid/ns_current_pid_tgid_new_ns:FAIL

#88 perf_buffer:FAIL
serial_test_perf_buffer:PASS:nr_cpus 0 nsec
serial_test_perf_buffer:PASS:nr_on_cpus 0 nsec
serial_test_perf_buffer:PASS:skel_load 0 nsec
libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf
event ID: No such file or directory
libbpf: prog 'handle_sys_enter': failed to create tracepoint
'raw_syscalls/sys_enter' perf event: No such file or directory
libbpf: failed to auto-attach program 'handle_sys_enter': -2
serial_test_perf_buffer:FAIL:attach_kprobe err -2

#110 send_signal_sched_switch:FAIL
serial_test_send_signal_sched_switch:PASS:skel_open_and_load 0 nsec
libbpf: failed to determine tracepoint 'syscalls/sys_enter_nanosleep'
perf event ID: No such file or directory
libbpf: prog 'send_signal_tp': failed to create tracepoint
'syscalls/sys_enter_nanosleep' perf event: No such file or directory
libbpf: failed to auto-attach program 'send_signal_tp': -2
serial_test_send_signal_sched_switch:FAIL:skel_attach skeleton attach failed

#161 tp_attach_query:FAIL
serial_test_tp_attach_query:FAIL:open err -1 errno 2

#163 trace_printk:FAIL
serial_test_trace_printk:PASS:trace_printk__open 0 nsec
serial_test_trace_printk:PASS:skel->rodata->fmt[0] 0 nsec
serial_test_trace_printk:PASS:trace_printk__load 0 nsec
serial_test_trace_printk:PASS:trace_printk__attach 0 nsec
serial_test_trace_printk:FAIL:fopen(TRACEBUF) unexpected error: -2

#164 trace_vprintk:FAIL
serial_test_trace_vprintk:PASS:trace_vprintk__open_and_load 0 nsec
serial_test_trace_vprintk:PASS:trace_vprintk__attach 0 nsec
serial_test_trace_vprintk:FAIL:fopen(TRACEBUF) unexpected error: -2

#46 fexit_stress:FAIL
test_fexit_stress:PASS:find_vmlinux_btf_id 0 nsec
test_fexit_stress:PASS:fexit loaded 0 nsec
test_fexit_stress:PASS:fexit attach failed 0 nsec
test_fexit_stress:PASS:fexit loaded 0 nsec

...

test_fexit_stress:PASS:fexit loaded 0 nsec
test_fexit_stress:PASS:fexit attach failed 0 nsec
test_fexit_stress:PASS:fexit loaded 0 nsec
test_fexit_stress:FAIL:fexit attach failed prog 37 failed: -7 err 7



> V6 -> V5:
>   * adding error summary logic for non parallel mode too.
>   * changed how serial tests are implemented, use main process instead of worker 0.
>   * fixed a dozen broken test when running in parallel.
>
> V5 -> V4:
>   * change to SOCK_SEQPACKET for close notification.
>   * move all debug output to "--debug" mode
>   * output log as test finish, and all error logs again after summary line.
>   * variable naming / style changes
>   * adds serial_test_name() to replace serial test lists.
>
>
> Yucong Sun (14):
>   selftests/bpf: Add parallelism to test_progs
>   selftests/bpf: Allow some tests to be executed in sequence
>   selftests/bpf: disable perf rate limiting when running tests.
>   selftests/bpf: add per worker cgroup suffix
>   selftests/bpf: adding read_perf_max_sample_freq() helper
>   selftests/bpf: fix race condition in enable_stats
>   selftests/bpf: make cgroup_v1v2 use its own port
>   selftests/bpf: adding a namespace reset for tc_redirect
>   selftests/bpf: Make uprobe tests use different attach functions.
>   selftests/bpf: adding pid filtering for atomics test
>   selftests/bpf: adding random delay for send_signal test
>   selftests/bpf: Fix pid check in fexit_sleep test
>   selftests/bpf: increase loop count for perf_branches
>   selfetest/bpf: make some tests serial
>
>  tools/testing/selftests/bpf/cgroup_helpers.c  |   6 +-
>  tools/testing/selftests/bpf/cgroup_helpers.h  |   2 +-
>  .../selftests/bpf/prog_tests/atomics.c        |   1 +
>  .../selftests/bpf/prog_tests/attach_probe.c   |   8 +-
>  .../selftests/bpf/prog_tests/bpf_cookie.c     |  10 +-
>  .../bpf/prog_tests/bpf_iter_setsockopt.c      |   2 +-
>  .../selftests/bpf/prog_tests/bpf_obj_id.c     |   2 +-
>  .../bpf/prog_tests/cg_storage_multi.c         |   2 +-
>  .../bpf/prog_tests/cgroup_attach_autodetach.c |   2 +-
>  .../bpf/prog_tests/cgroup_attach_multi.c      |   2 +-
>  .../bpf/prog_tests/cgroup_attach_override.c   |   2 +-
>  .../selftests/bpf/prog_tests/cgroup_link.c    |   2 +-
>  .../selftests/bpf/prog_tests/cgroup_v1v2.c    |   2 +-
>  .../selftests/bpf/prog_tests/check_mtu.c      |   2 +-
>  .../selftests/bpf/prog_tests/fexit_bpf2bpf.c  |   3 +-
>  .../prog_tests/flow_dissector_load_bytes.c    |   2 +-
>  .../bpf/prog_tests/flow_dissector_reattach.c  |   2 +-
>  .../bpf/prog_tests/get_branch_snapshot.c      |   2 +-
>  .../selftests/bpf/prog_tests/kfree_skb.c      |   3 +-
>  .../bpf/prog_tests/migrate_reuseport.c        |   2 +-
>  .../selftests/bpf/prog_tests/modify_return.c  |   3 +-
>  .../bpf/prog_tests/ns_current_pid_tgid.c      |   3 +-
>  .../selftests/bpf/prog_tests/perf_branches.c  |  10 +-
>  .../selftests/bpf/prog_tests/perf_buffer.c    |   2 +-
>  .../selftests/bpf/prog_tests/perf_link.c      |   5 +-
>  .../selftests/bpf/prog_tests/probe_user.c     |   3 +-
>  .../bpf/prog_tests/raw_tp_writable_test_run.c |   3 +-
>  .../bpf/prog_tests/select_reuseport.c         |   2 +-
>  .../selftests/bpf/prog_tests/send_signal.c    |   6 +-
>  .../bpf/prog_tests/send_signal_sched_switch.c |   3 +-
>  .../bpf/prog_tests/sk_storage_tracing.c       |   2 +-
>  .../selftests/bpf/prog_tests/snprintf_btf.c   |   2 +-
>  .../selftests/bpf/prog_tests/sock_fields.c    |   2 +-
>  .../selftests/bpf/prog_tests/sockmap_listen.c |   2 +-
>  .../bpf/prog_tests/stacktrace_build_id_nmi.c  |  19 +-
>  .../selftests/bpf/prog_tests/task_pt_regs.c   |   8 +-
>  .../selftests/bpf/prog_tests/tc_redirect.c    |  14 +
>  .../testing/selftests/bpf/prog_tests/timer.c  |   3 +-
>  .../selftests/bpf/prog_tests/timer_mim.c      |   2 +-
>  .../bpf/prog_tests/tp_attach_query.c          |   2 +-
>  .../selftests/bpf/prog_tests/trace_printk.c   |   2 +-
>  .../selftests/bpf/prog_tests/trace_vprintk.c  |   2 +-
>  .../bpf/prog_tests/trampoline_count.c         |   3 +-
>  .../selftests/bpf/prog_tests/xdp_attach.c     |   2 +-
>  .../selftests/bpf/prog_tests/xdp_bonding.c    |   2 +-
>  .../bpf/prog_tests/xdp_cpumap_attach.c        |   2 +-
>  .../bpf/prog_tests/xdp_devmap_attach.c        |   2 +-
>  .../selftests/bpf/prog_tests/xdp_info.c       |   2 +-
>  .../selftests/bpf/prog_tests/xdp_link.c       |   2 +-
>  tools/testing/selftests/bpf/progs/atomics.c   |  16 +
>  .../selftests/bpf/progs/connect4_dropper.c    |   2 +-
>  .../testing/selftests/bpf/progs/fexit_sleep.c |   4 +-
>  .../selftests/bpf/progs/test_enable_stats.c   |   2 +-
>  tools/testing/selftests/bpf/test_progs.c      | 671 +++++++++++++++++-
>  tools/testing/selftests/bpf/test_progs.h      |  37 +-
>  55 files changed, 790 insertions(+), 116 deletions(-)
>
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 01/14] selftests/bpf: Add parallelism to test_progs
  2021-10-06 18:56 ` [PATCH bpf-next v6 01/14] " Yucong Sun
@ 2021-10-08 22:26   ` Andrii Nakryiko
  0 siblings, 0 replies; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 22:26 UTC (permalink / raw)
  To: Yucong Sun; +Cc: bpf, Andrii Nakryiko, Yucong Sun

On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
>
> From: Yucong Sun <sunyucong@gmail.com>
>
> This patch adds "-j" mode to test_progs, executing tests in multiple
> process.  "-j" mode is optional, and works with all existing test
> selection mechanism, as well as "-v", "-l" etc.
>
> In "-j" mode, main process use UDS/DGRAM to communicate to each forked
> worker, commanding it to run tests and collect logs. After all tests are
> finished, a summary is printed. main process use multiple competing
> threads to dispatch work to worker, trying to keep them all busy.
>
> The test status will be printed as soon as it is finished, if there are
> error logs, it will be printed after the final summary line.
>
> By specifying "--debug", additional debug information on server/worker
> communication will be printed.
>
> Example output:
>   > ./test_progs -n 15-20 -j
>   [   12.801730] bpf_testmod: loading out-of-tree module taints kernel.
>   Launching 8 workers.
>   #20 btf_split:OK
>   #16 btf_endian:OK
>   #18 btf_module:OK
>   #17 btf_map_in_map:OK
>   #19 btf_skc_cls_ingress:OK
>   #15 btf_dump:OK
>   Summary: 6/20 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> ---
>  tools/testing/selftests/bpf/test_progs.c | 598 +++++++++++++++++++++--
>  tools/testing/selftests/bpf/test_progs.h |  36 +-
>  2 files changed, 600 insertions(+), 34 deletions(-)
>

Looks good overall, error summary is a great time saver, as well as
parallelisation itself, of course, once we hammer out all the initial
pains. But instead of trying to make it perfect, let's iterate and
improve as we give it more every day testing.

There were a few stylistic problems which I fixed up, but see below
also for some more error handling follow ups.

[...]

> @@ -794,11 +819,456 @@ void crash_handler(int signum)
>                 dump_test_log(env.test, true);
>         if (env.stdout)
>                 stdio_restore();
> -
> +       if (env.worker_id != -1)
> +               fprintf(stderr, "[%d]: ", env.worker_id);
>         fprintf(stderr, "Caught signal #%d!\nStack trace:\n", signum);
>         backtrace_symbols_fd(bt, sz, STDERR_FILENO);
>  }
>
> +void sigint_handler(int signum) {

static, { on another line

> +       for (int i = 0; i < env.workers; i++)

C89 doesn't allow declaring variables inside for(). Please don't
forget to run checkpatch.pl before submission. I've fixed in few more
places below.

> +               if (env.worker_socks[i] > 0)
> +                       close(env.worker_socks[i]);
> +}
> +
> +static int current_test_idx;
> +static pthread_mutex_t current_test_lock;
> +static pthread_mutex_t stdout_output_lock;
> +

[...]

> +       dump_test_log(test, test->error_cnt);
> +
> +       reset_affinity();
> +       restore_netns();
> +       if (test->need_cgroup_cleanup)
> +               cleanup_cgroup_environment();
> +}
> +
> +struct dispatch_data {
> +       int worker_id;
> +       int sock_fd;
> +};
> +
> +void *dispatch_thread(void *ctx)

static, function has to be global for a reason, fixed up

> +{
> +       struct dispatch_data *data = ctx;
> +       int sock_fd;
> +       FILE *log_fd = NULL;
> +
> +       sock_fd = data->sock_fd;
> +

[...]

> +       /* initializing tests */
>         for (i = 0; i < prog_test_cnt; i++) {
>                 struct prog_test_def *test = &prog_test_defs[i];
>
> -               env.test = test;
>                 test->test_num = i + 1;
> -
> -               if (!should_run(&env.test_selector,
> +               if (should_run(&env.test_selector,
>                                 test->test_num, test->test_name))
> +                       test->should_run = true;
> +               else
> +                       test->should_run = false;
> +       }
> +
> +       /* ignore workers if we are just listing */
> +       if (env.get_test_cnt || env.list_test_names)
> +               env.workers = 0;
> +
> +       /* launch workers if requested */
> +       env.worker_id = -1; /* main process */
> +       if (env.workers) {
> +               env.worker_pids = calloc(sizeof(__pid_t), env.workers);
> +               env.worker_socks = calloc(sizeof(int), env.workers);

check errors instead of SIGSEGV?

We should also free memory. I'd like test_progs to run under
LeakSanitizer in near future, if we don't free all the memory property
and clean up everything, we'll be getting lots of false positive from
LeakSanitizer at the process exit. So we need to be diligent about
stuff like this. Please follow up with fixes.

> +               if (env.debug)
> +                       fprintf(stdout, "Launching %d workers.\n", env.workers);
> +               for (int i = 0; i < env.workers; i++) {
> +                       int sv[2];
> +                       pid_t pid;
> +

[...]

>                 fprintf(env.stdout, "#%d %s:%s\n",
>                         test->test_num, test->test_name,
>                         test->error_cnt ? "FAIL" : (test->skip_cnt ? "SKIP" : "OK"));
>
> +               result = &test_results[i];
> +               result->error_cnt = test->error_cnt;
> +               if (env.log_buf) {
> +                       result->log_buf = strdup(env.log_buf);
> +                       result->log_cnt = env.log_cnt;
> +
> +                       free(env.log_buf);
> +                       env.log_buf = NULL;

unclear why this dance: strdup and free the original memory, would be
simpler to just use env.log_buf directly for result->log_buf and set
env.log_buf to NULL after?

> +                       env.log_cnt = 0;
> +               }
> +
>                 if (test->error_cnt)
>                         env.fail_cnt++;
>                 else
>                         env.succ_cnt++;
> -               skip_account();
>
> -               reset_affinity();
> -               restore_netns();
> -               if (test->need_cgroup_cleanup)
> -                       cleanup_cgroup_environment();
> +               skip_account();
> +               env.sub_succ_cnt += test->sub_succ_cnt;
>         }

[...]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 02/14] selftests/bpf: Allow some tests to be executed in sequence
  2021-10-06 18:56 ` [PATCH bpf-next v6 02/14] selftests/bpf: Allow some tests to be executed in sequence Yucong Sun
@ 2021-10-08 22:26   ` Andrii Nakryiko
  2021-10-08 23:06     ` sunyucong
  0 siblings, 1 reply; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 22:26 UTC (permalink / raw)
  To: Yucong Sun; +Cc: bpf, Andrii Nakryiko, Yucong Sun

On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
>
> From: Yucong Sun <sunyucong@gmail.com>
>
> This patch allows tests to define serial_test_name() instead of
> test_name(), and this will make test_progs execute those in sequence
> after all other tests finished executing concurrently.
>
> Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> ---
>  tools/testing/selftests/bpf/test_progs.c | 60 +++++++++++++++++++++---
>  1 file changed, 54 insertions(+), 6 deletions(-)
>

[...]

> @@ -1129,6 +1136,40 @@ static int server_main(void)
>         free(env.worker_current_test);
>         free(data);
>
> +       /* run serial tests */
> +       save_netns();
> +
> +       for (int i = 0; i < prog_test_cnt; i++) {
> +               struct prog_test_def *test = &prog_test_defs[i];
> +               struct test_result *result = &test_results[i];
> +
> +               if (!test->should_run || !test->run_serial_test)
> +                       continue;
> +
> +               stdio_hijack();
> +
> +               run_one_test(i);
> +
> +               stdio_restore();
> +               if (env.log_buf) {
> +                       result->log_cnt = env.log_cnt;
> +                       result->log_buf = strdup(env.log_buf);
> +
> +                       free(env.log_buf);
> +                       env.log_buf = NULL;
> +                       env.log_cnt = 0;
> +               }
> +               restore_netns();
> +
> +               fprintf(stdout, "#%d %s:%s\n",
> +                       test->test_num, test->test_name,
> +                       test->error_cnt ? "FAIL" : (test->skip_cnt ? "SKIP" : "OK"));
> +
> +               result->error_cnt = test->error_cnt;
> +               result->skip_cnt = test->skip_cnt;
> +               result->sub_succ_cnt = test->sub_succ_cnt;
> +       }
> +

Did you try to just reuse sequential running loop logic in main() for
this? I'd like to avoid the third test running loop copy, if possible.
What were the problems of reusing the sequential logic from main(),
they do the same work, no?


>         /* generate summary */
>         fflush(stderr);
>         fflush(stdout);
> @@ -1326,6 +1367,13 @@ int main(int argc, char **argv)
>                         test->should_run = true;
>                 else
>                         test->should_run = false;
> +
> +               if ((test->run_test == NULL && test->run_serial_test == NULL) ||
> +                   (test->run_test != NULL && test->run_serial_test != NULL)) {
> +                       fprintf(stderr, "Test %d:%s must have either test_%s() or serial_test_%sl() defined.\n",
> +                               test->test_num, test->test_name, test->test_name, test->test_name);
> +                       exit(EXIT_ERR_SETUP_INFRA);
> +               }
>         }
>
>         /* ignore workers if we are just listing */
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 03/14] selftests/bpf: disable perf rate limiting when running tests.
  2021-10-06 18:56 ` [PATCH bpf-next v6 03/14] selftests/bpf: disable perf rate limiting when running tests Yucong Sun
@ 2021-10-08 22:26   ` Andrii Nakryiko
  0 siblings, 0 replies; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 22:26 UTC (permalink / raw)
  To: Yucong Sun; +Cc: bpf, Andrii Nakryiko, Yucong Sun

On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
>
> When running tests concurrently, perf rate limiting will often cause
> some events to be skipped and make some tests flaky, disabling it making
> running tests more robust.
>
> Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> ---
>  tools/testing/selftests/bpf/test_progs.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 4e2028189471..2ac922f8aa2c 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -1376,6 +1376,8 @@ int main(int argc, char **argv)
>                 }
>         }
>
> +       system("echo 0 > /proc/sys/kernel/perf_cpu_time_max_percent");

I don't think we want to do this outside of QEMU, changing the global
state of the real system. I think this might have also been relevant
to me waiting for perf_branches test for about 20 seconds or so. I've
dropped this patch for now.


> +
>         /* ignore workers if we are just listing */
>         if (env.get_test_cnt || env.list_test_names)
>                 env.workers = 0;
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 08/14] selftests/bpf: adding a namespace reset for tc_redirect
  2021-10-06 18:56 ` [PATCH bpf-next v6 08/14] selftests/bpf: adding a namespace reset for tc_redirect Yucong Sun
@ 2021-10-08 22:27   ` Andrii Nakryiko
  2021-10-08 23:07     ` sunyucong
  0 siblings, 1 reply; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 22:27 UTC (permalink / raw)
  To: Yucong Sun; +Cc: bpf, Andrii Nakryiko, Yucong Sun

On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
>
> From: Yucong Sun <sunyucong@gmail.com>
>
> This patch delete ns_src/ns_dst/ns_redir namespaces before recreating
> them, making the test more robust.
>
> Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> ---
>  .../testing/selftests/bpf/prog_tests/tc_redirect.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> index e87bc4466d9a..25744136e131 100644
> --- a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> +++ b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> @@ -176,6 +176,18 @@ static int netns_setup_namespaces(const char *verb)
>         return 0;
>  }
>
> +static void netns_setup_namespaces_nofail(const char *verb)
> +{
> +       const char * const *ns = namespaces;
> +       char cmd[128];
> +
> +       while (*ns) {
> +               snprintf(cmd, sizeof(cmd), "ip netns %s %s", verb, *ns);
> +               system(cmd);

is this what's causing

Cannot remove namespace file "/var/run/netns/ns_src": No such file or directory
Cannot remove namespace file "/var/run/netns/ns_fwd": No such file or directory
Cannot remove namespace file "/var/run/netns/ns_dst": No such file or directory

?

I haven't applied it yet, let's see if there is a way to avoid
unnecessary "warnings".



> +               ns++;
> +       }
> +}
> +
>  struct netns_setup_result {
>         int ifindex_veth_src_fwd;
>         int ifindex_veth_dst_fwd;
> @@ -762,6 +774,8 @@ static void test_tc_redirect_peer_l3(struct netns_setup_result *setup_result)
>
>  static void *test_tc_redirect_run_tests(void *arg)
>  {
> +       netns_setup_namespaces_nofail("delete");
> +
>         RUN_TEST(tc_redirect_peer);
>         RUN_TEST(tc_redirect_peer_l3);
>         RUN_TEST(tc_redirect_neigh);
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 05/14] selftests/bpf: adding read_perf_max_sample_freq() helper
  2021-10-06 18:56 ` [PATCH bpf-next v6 05/14] selftests/bpf: adding read_perf_max_sample_freq() helper Yucong Sun
@ 2021-10-08 22:27   ` Andrii Nakryiko
  2021-10-08 22:49     ` sunyucong
  0 siblings, 1 reply; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 22:27 UTC (permalink / raw)
  To: Yucong Sun; +Cc: bpf, Andrii Nakryiko, Yucong Sun

On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
>
> From: Yucong Sun <sunyucong@gmail.com>
>
> This patch moved a helper function to test_progs and make all tests
> setting sampling frequency use it to read current perf_max_sample_freq,
> this will avoid triggering EINVAL error.
>
> Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> ---
>  .../selftests/bpf/prog_tests/bpf_cookie.c     |  2 +-
>  .../selftests/bpf/prog_tests/perf_branches.c  |  4 ++--
>  .../selftests/bpf/prog_tests/perf_link.c      |  2 +-
>  .../bpf/prog_tests/stacktrace_build_id_nmi.c  | 19 ++-----------------
>  tools/testing/selftests/bpf/test_progs.c      | 15 +++++++++++++++
>  tools/testing/selftests/bpf/test_progs.h      |  1 +
>  6 files changed, 22 insertions(+), 21 deletions(-)
>

We have trace_helper.c, seems like it would be better to have it
there? I haven't applied this patch yet.

[...]

> @@ -48,6 +31,8 @@ void test_stacktrace_build_id_nmi(void)
>         if (CHECK(err, "skel_load", "skeleton load failed: %d\n", err))
>                 goto cleanup;
>
> +       attr.sample_freq = read_perf_max_sample_freq();
> +
>         pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
>                          0 /* cpu 0 */, -1 /* group id */,
>                          0 /* flags */);
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 2ac922f8aa2c..66825313414b 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -1500,3 +1500,18 @@ int main(int argc, char **argv)
>
>         return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
>  }
> +
> +__u64 read_perf_max_sample_freq(void)
> +{
> +       __u64 sample_freq = 1000; /* fallback to 1000 on error */

previous default was 5000, message below still claims 5000, what's the
reason for changing it?



> +       FILE *f;
> +       __u32 duration = 0;
> +
> +       f = fopen("/proc/sys/kernel/perf_event_max_sample_rate", "r");
> +       if (f == NULL)
> +               return sample_freq;
> +       CHECK(fscanf(f, "%llu", &sample_freq) != 1, "Get max sample rate",
> +             "return default value: 5000,err %d\n", -errno);
> +       fclose(f);
> +       return sample_freq;
> +}
> diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
> index b239dc9fcef0..d5ca0d36cc96 100644
> --- a/tools/testing/selftests/bpf/test_progs.h
> +++ b/tools/testing/selftests/bpf/test_progs.h
> @@ -327,6 +327,7 @@ int extract_build_id(char *build_id, size_t size);
>  int kern_sync_rcu(void);
>  int trigger_module_test_read(int read_sz);
>  int trigger_module_test_write(int write_sz);
> +__u64 read_perf_max_sample_freq(void);
>
>  #ifdef __x86_64__
>  #define SYS_NANOSLEEP_KPROBE_NAME "__x64_sys_nanosleep"
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 09/14] selftests/bpf: Make uprobe tests use different attach functions.
  2021-10-06 18:56 ` [PATCH bpf-next v6 09/14] selftests/bpf: Make uprobe tests use different attach functions Yucong Sun
@ 2021-10-08 22:27   ` Andrii Nakryiko
  2021-10-08 22:46     ` sunyucong
  0 siblings, 1 reply; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 22:27 UTC (permalink / raw)
  To: Yucong Sun, Song Liu; +Cc: bpf, Andrii Nakryiko, Yucong Sun

On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
>
> From: Yucong Sun <sunyucong@gmail.com>
>
> Using same address on different processes of the same binary often fail
> with EINVAL, this patch make these tests use distinct methods, so they
> can run in parallel.
>
> Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/attach_probe.c | 8 ++++++--
>  tools/testing/selftests/bpf/prog_tests/bpf_cookie.c   | 8 ++++++--
>  tools/testing/selftests/bpf/prog_tests/task_pt_regs.c | 8 ++++++--
>  3 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> index 6c511dcd1465..eff36ba9c148 100644
> --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> @@ -5,6 +5,10 @@
>  /* this is how USDT semaphore is actually defined, except volatile modifier */
>  volatile unsigned short uprobe_ref_ctr __attribute__((unused)) __attribute((section(".probes")));
>
> +static int method() {

wrong style: { should be on separate line

> +       return get_base_addr();

there is nothing special about get_base_addr(), except that it's a
global function in a different file and won't be inlined, while this
method() approach has no such guarantee

I've dropped this patch for now.

But I'm surprised that attaching to the same uprobe few times doesn't
work. Song, is there anything in kernel that could cause this?


> +}
> +
>  void test_attach_probe(void)
>  {
>         DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, uprobe_opts);
> @@ -33,7 +37,7 @@ void test_attach_probe(void)
>         if (CHECK(base_addr < 0, "get_base_addr",
>                   "failed to find base addr: %zd", base_addr))
>                 return;
> -       uprobe_offset = get_uprobe_offset(&get_base_addr, base_addr);
> +       uprobe_offset = get_uprobe_offset(&method, base_addr);
>
>         ref_ctr_offset = get_rel_offset((uintptr_t)&uprobe_ref_ctr);
>         if (!ASSERT_GE(ref_ctr_offset, 0, "ref_ctr_offset"))
> @@ -98,7 +102,7 @@ void test_attach_probe(void)
>                 goto cleanup;
>
>         /* trigger & validate uprobe & uretprobe */
> -       get_base_addr();
> +       method();
>
>         if (CHECK(skel->bss->uprobe_res != 3, "check_uprobe_res",
>                   "wrong uprobe res: %d\n", skel->bss->uprobe_res))
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> index 19c9f7b53cfa..5ebd8ba988e2 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> @@ -8,6 +8,10 @@
>  #include <test_progs.h>
>  #include "test_bpf_cookie.skel.h"
>
> +static int method() {
> +       return get_base_addr();
> +}
> +
>  static void kprobe_subtest(struct test_bpf_cookie *skel)
>  {
>         DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts);
> @@ -66,7 +70,7 @@ static void uprobe_subtest(struct test_bpf_cookie *skel)
>         ssize_t base_addr;
>
>         base_addr = get_base_addr();
> -       uprobe_offset = get_uprobe_offset(&get_base_addr, base_addr);
> +       uprobe_offset = get_uprobe_offset(&method, base_addr);
>
>         /* attach two uprobes */
>         opts.bpf_cookie = 0x100;
> @@ -99,7 +103,7 @@ static void uprobe_subtest(struct test_bpf_cookie *skel)
>                 goto cleanup;
>
>         /* trigger uprobe && uretprobe */
> -       get_base_addr();
> +       method();
>
>         ASSERT_EQ(skel->bss->uprobe_res, 0x100 | 0x200, "uprobe_res");
>         ASSERT_EQ(skel->bss->uretprobe_res, 0x1000 | 0x2000, "uretprobe_res");
> diff --git a/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c b/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
> index 37c20b5ffa70..4d2f1435be90 100644
> --- a/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
> +++ b/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
> @@ -3,6 +3,10 @@
>  #include <test_progs.h>
>  #include "test_task_pt_regs.skel.h"
>
> +static int method() {
> +       return get_base_addr();
> +}
> +
>  void test_task_pt_regs(void)
>  {
>         struct test_task_pt_regs *skel;
> @@ -14,7 +18,7 @@ void test_task_pt_regs(void)
>         base_addr = get_base_addr();
>         if (!ASSERT_GT(base_addr, 0, "get_base_addr"))
>                 return;
> -       uprobe_offset = get_uprobe_offset(&get_base_addr, base_addr);
> +       uprobe_offset = get_uprobe_offset(&method, base_addr);
>
>         skel = test_task_pt_regs__open_and_load();
>         if (!ASSERT_OK_PTR(skel, "skel_open"))
> @@ -32,7 +36,7 @@ void test_task_pt_regs(void)
>         skel->links.handle_uprobe = uprobe_link;
>
>         /* trigger & validate uprobe */
> -       get_base_addr();
> +       method();
>
>         if (!ASSERT_EQ(skel->bss->uprobe_res, 1, "check_uprobe_res"))
>                 goto cleanup;
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 11/14] selftests/bpf: adding random delay for send_signal test
  2021-10-06 18:56 ` [PATCH bpf-next v6 11/14] selftests/bpf: adding random delay for send_signal test Yucong Sun
@ 2021-10-08 22:27   ` Andrii Nakryiko
  0 siblings, 0 replies; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 22:27 UTC (permalink / raw)
  To: Yucong Sun, Yonghong Song; +Cc: bpf, Andrii Nakryiko, Yucong Sun

On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
>
> From: Yucong Sun <sunyucong@gmail.com>
>
> This patch adds random delay on waiting for the signal to arrive,
> making the test more robust.
>
> Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/send_signal.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
> index 776916b61c40..6200256243f2 100644
> --- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
> +++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
> @@ -19,6 +19,7 @@ static void test_send_signal_common(struct perf_event_attr *attr,
>         int err = -1, pmu_fd = -1;
>         char buf[256];
>         pid_t pid;
> +       int attempts = 100;
>
>         if (!ASSERT_OK(pipe(pipe_c2p), "pipe_c2p"))
>                 return;
> @@ -63,7 +64,10 @@ static void test_send_signal_common(struct perf_event_attr *attr,
>                 ASSERT_EQ(read(pipe_p2c[0], buf, 1), 1, "pipe_read");
>
>                 /* wait a little for signal handler */
> -               sleep(1);
> +               while (attempts > 0 && !sigusr1_received) {

This is not reliable, sigusr1_received has to be volatile or
sig_atomic_t, please fix. I haven't applied this patch yet.

Also, previously we slept for a second, now we can, technically, sleep
only up to 100ms, would that cause any problems in practice? cc
Yonghong


> +                       attempts--;
> +                       usleep(500 + rand() % 500);
> +               }
>
>                 buf[0] = sigusr1_received ? '2' : '0';
>                 ASSERT_EQ(write(pipe_c2p[1], buf, 1), 1, "pipe_write");
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 13/14] selftests/bpf: increase loop count for perf_branches
  2021-10-06 18:56 ` [PATCH bpf-next v6 13/14] selftests/bpf: increase loop count for perf_branches Yucong Sun
@ 2021-10-08 22:27   ` Andrii Nakryiko
  2021-10-08 22:57     ` sunyucong
  0 siblings, 1 reply; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 22:27 UTC (permalink / raw)
  To: Yucong Sun; +Cc: bpf, Andrii Nakryiko, Yucong Sun

On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
>
> From: Yucong Sun <sunyucong@gmail.com>
>
> This make this test more likely to succeed.
>
> Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> ---

100 million iterations seems a bit excessive. Why one million loops
doesn't cause a single perf event? Can we make it more robust in some
other way that is not as slow? I've dropped it for now while we
discuss.


>  tools/testing/selftests/bpf/prog_tests/perf_branches.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/perf_branches.c b/tools/testing/selftests/bpf/prog_tests/perf_branches.c
> index 6b2e3dced619..d7e88b2c5f36 100644
> --- a/tools/testing/selftests/bpf/prog_tests/perf_branches.c
> +++ b/tools/testing/selftests/bpf/prog_tests/perf_branches.c
> @@ -16,7 +16,7 @@ static void check_good_sample(struct test_perf_branches *skel)
>         int duration = 0;
>
>         if (CHECK(!skel->bss->valid, "output not valid",
> -                "no valid sample from prog"))
> +                "no valid sample from prog\n"))
>                 return;
>
>         /*
> @@ -46,7 +46,7 @@ static void check_bad_sample(struct test_perf_branches *skel)
>         int duration = 0;
>
>         if (CHECK(!skel->bss->valid, "output not valid",
> -                "no valid sample from prog"))
> +                "no valid sample from prog\n"))
>                 return;
>
>         CHECK((required_size != -EINVAL && required_size != -ENOENT),
> @@ -84,7 +84,7 @@ static void test_perf_branches_common(int perf_fd,
>         if (CHECK(err, "set_affinity", "cpu #0, err %d\n", err))
>                 goto out_destroy;
>         /* spin the loop for a while (random high number) */
> -       for (i = 0; i < 1000000; ++i)
> +       for (i = 0; i < 100000000; ++i)
>                 ++j;
>
>         test_perf_branches__detach(skel);
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 14/14] selfetest/bpf: make some tests serial
  2021-10-06 18:56 ` [PATCH bpf-next v6 14/14] selfetest/bpf: make some tests serial Yucong Sun
@ 2021-10-08 22:27   ` Andrii Nakryiko
  2021-10-08 22:55     ` sunyucong
  0 siblings, 1 reply; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 22:27 UTC (permalink / raw)
  To: Yucong Sun; +Cc: bpf, Andrii Nakryiko, Yucong Sun

On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
>
> From: Yucong Sun <sunyucong@gmail.com>
>
> Change tests that often fails in parallel execution mode to serial.
>
> Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> ---

I hope we'll be able to parallelise these tests over time. See some
notes on cover letter for how to target this effort better. But here's
another thought I had while thinking about this.

Some tests are inherently testing unshareable resources, e.g., like
XDP tests. So we might never be able to completely parallelize all the
tests. But it probably will be too restrictive to just bunde all of
them into the one "serial" group of tests. It's a good starting point,
but I think we'll have to have something like "serialization" group,
where we'll be able to mark a bunch of tests like
"serial_xdp_<testname>" and each test with "serial_xdp_" prefix will
run sequentially relative to each other, but they might run completely
parallel to, say, "serial_perf_" tests. WDYT?


>  tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c   | 2 +-
>  tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c            | 2 +-
>  tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c      | 2 +-
>  .../selftests/bpf/prog_tests/cgroup_attach_autodetach.c        | 2 +-
>  tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c   | 2 +-
>  .../testing/selftests/bpf/prog_tests/cgroup_attach_override.c  | 2 +-
>  tools/testing/selftests/bpf/prog_tests/cgroup_link.c           | 2 +-
>  tools/testing/selftests/bpf/prog_tests/check_mtu.c             | 2 +-
>  tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c         | 3 ++-
>  .../selftests/bpf/prog_tests/flow_dissector_load_bytes.c       | 2 +-
>  .../testing/selftests/bpf/prog_tests/flow_dissector_reattach.c | 2 +-
>  tools/testing/selftests/bpf/prog_tests/get_branch_snapshot.c   | 2 +-
>  tools/testing/selftests/bpf/prog_tests/kfree_skb.c             | 3 ++-
>  tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c     | 2 +-
>  tools/testing/selftests/bpf/prog_tests/modify_return.c         | 3 ++-
>  tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c   | 3 ++-
>  tools/testing/selftests/bpf/prog_tests/perf_buffer.c           | 2 +-
>  tools/testing/selftests/bpf/prog_tests/perf_link.c             | 3 ++-
>  tools/testing/selftests/bpf/prog_tests/probe_user.c            | 3 ++-
>  .../selftests/bpf/prog_tests/raw_tp_writable_test_run.c        | 3 ++-
>  tools/testing/selftests/bpf/prog_tests/select_reuseport.c      | 2 +-
>  .../selftests/bpf/prog_tests/send_signal_sched_switch.c        | 3 ++-
>  tools/testing/selftests/bpf/prog_tests/sk_storage_tracing.c    | 2 +-
>  tools/testing/selftests/bpf/prog_tests/snprintf_btf.c          | 2 +-
>  tools/testing/selftests/bpf/prog_tests/sock_fields.c           | 2 +-
>  tools/testing/selftests/bpf/prog_tests/sockmap_listen.c        | 2 +-
>  tools/testing/selftests/bpf/prog_tests/timer.c                 | 3 ++-
>  tools/testing/selftests/bpf/prog_tests/timer_mim.c             | 2 +-
>  tools/testing/selftests/bpf/prog_tests/tp_attach_query.c       | 2 +-
>  tools/testing/selftests/bpf/prog_tests/trace_printk.c          | 2 +-
>  tools/testing/selftests/bpf/prog_tests/trace_vprintk.c         | 2 +-
>  tools/testing/selftests/bpf/prog_tests/trampoline_count.c      | 3 ++-
>  tools/testing/selftests/bpf/prog_tests/xdp_attach.c            | 2 +-
>  tools/testing/selftests/bpf/prog_tests/xdp_bonding.c           | 2 +-
>  tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c     | 2 +-
>  tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c     | 2 +-
>  tools/testing/selftests/bpf/prog_tests/xdp_info.c              | 2 +-
>  tools/testing/selftests/bpf/prog_tests/xdp_link.c              | 2 +-
>  38 files changed, 48 insertions(+), 38 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c
> index 85babb0487b3..b52ff8ce34db 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c
> @@ -179,7 +179,7 @@ static void do_bpf_iter_setsockopt(struct bpf_iter_setsockopt *iter_skel,
>         free_fds(est_fds, nr_est);
>  }
>

[...]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs
  2021-10-08 22:26 ` [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Andrii Nakryiko
@ 2021-10-08 22:42   ` Andrii Nakryiko
  2021-10-08 23:37     ` Steven Rostedt
  0 siblings, 1 reply; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 22:42 UTC (permalink / raw)
  To: Yucong Sun, Steven Rostedt; +Cc: bpf, Andrii Nakryiko, Yucong Sun

On Fri, Oct 8, 2021 at 3:26 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
> >
> > This patch series adds "-j" parelell execution to test_progs, with "--debug" to
> > display server/worker communications. Also, some Tests that often fails in
> > parallel are marked as serial test, and it will run in sequence after parallel
> > execution is done.
> >
> > This patch series also adds a error summary after all tests execution finished.
> >
>
> Huge milestone, good job! Applied most patches to bpf-next. See
> comments below in respective patches.
>
> We'll need to iterate on improving the stability of parallel mode, but
> this is a great start. I've dropped a bunch of "fix up" patches where
> I didn't feel confident yet about the approach. We should discuss it
> independently from the parallelization changes in this patch set. See
> some more thoughts below, but overall:
>
> time sudo ./test_progs -j
> ...
> Summary: 181/977 PASSED, 3 SKIPPED, 0 FAILED
>
> real    0m36.949s
> user    0m4.546s
> sys     0m30.872s
>
> VS
>
> $ time sudo ./test_progs
> ...
> Summary: 181/977 PASSED, 3 SKIPPED, 0 FAILED
>
> real    1m3.031s
> user    0m4.157s
> sys     0m28.820s
>
> 2x speed up and the gap will just grow over time as we add more tests.
> And that's also with bpf_verif_scale as is, which we should break up
> into individual tests to parallelize them.
>
> So few things worth mentioning:
>
> 1. To focus future efforts on parallelizing existing tests, we should
> probably emit how long did the test take.
>
> 2. We are losing subtest progress when running in parallel mode. That
> sucks. While it's not easy to parallelize subtests, it's easy to send
> separate logs for each subtest and display them as they come. Let's do
> that?
>
> 3. Parallel execution times are not consistent, once I got 30 seconds
> (which is 8 seconds faster than sequential, I excluded
> bpf_verif_scale), other times it was 45 seconds and more than 1
> minute. Not sure what's going on there, but this doesn't look right.
>
> 4. A bunch of tests still fail from time to time (see examples below).
> What's even scarier that once I got the "failed to determine
> tracepoint perf event ID" message, subsequent sequential executions
> kept failing. I don't see what selftest could have done to cause this,
> so this is concerning and seems to point to the kernel.
> /sys/kernel/debug and /sys/kernel/tracing directories were empty at
> this point. cc Steven, is there any situation when tracefs can become
> "defunct"?
>

Forgot to actually cc Steven, oops. Steven, I've run into the problem
when running a few selftests that do uprobe/kprobe attachment. At some
point, they started complaining that files like
/sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id don't
exist. And this condition persisted. When I checked
/sys/kernel/debug/tracing in QEMU, it was empty. Is this a known
problem?

> #84 ns_current_pid_tgid:FAIL
> test_current_pid_tgid:PASS:skel_open_load 0 nsec
> test_current_pid_tgid:PASS:stat 0 nsec
> libbpf: failed to determine tracepoint 'syscalls/sys_enter_nanosleep'
> perf event ID: No such file or directory
> libbpf: prog 'handler': failed to create tracepoint
> 'syscalls/sys_enter_nanosleep' perf event: No such file or directory
> libbpf: failed to auto-attach program 'handler': -2
> test_current_pid_tgid:FAIL:skel_attach skeleton attach failed: -2
> #84/1 ns_current_pid_tgid/ns_current_pid_tgid_root_ns:FAIL
> test_ns_current_pid_tgid_new_ns:PASS:clone 0 nsec
> test_ns_current_pid_tgid_new_ns:PASS:waitpid 0 nsec
> test_ns_current_pid_tgid_new_ns:FAIL:newns_pidtgid failed#84/2
> ns_current_pid_tgid/ns_current_pid_tgid_new_ns:FAIL
>
> #88 perf_buffer:FAIL
> serial_test_perf_buffer:PASS:nr_cpus 0 nsec
> serial_test_perf_buffer:PASS:nr_on_cpus 0 nsec
> serial_test_perf_buffer:PASS:skel_load 0 nsec
> libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf
> event ID: No such file or directory
> libbpf: prog 'handle_sys_enter': failed to create tracepoint
> 'raw_syscalls/sys_enter' perf event: No such file or directory
> libbpf: failed to auto-attach program 'handle_sys_enter': -2
> serial_test_perf_buffer:FAIL:attach_kprobe err -2
>
> #110 send_signal_sched_switch:FAIL
> serial_test_send_signal_sched_switch:PASS:skel_open_and_load 0 nsec
> libbpf: failed to determine tracepoint 'syscalls/sys_enter_nanosleep'
> perf event ID: No such file or directory
> libbpf: prog 'send_signal_tp': failed to create tracepoint
> 'syscalls/sys_enter_nanosleep' perf event: No such file or directory
> libbpf: failed to auto-attach program 'send_signal_tp': -2
> serial_test_send_signal_sched_switch:FAIL:skel_attach skeleton attach failed
>
> #161 tp_attach_query:FAIL
> serial_test_tp_attach_query:FAIL:open err -1 errno 2
>
> #163 trace_printk:FAIL
> serial_test_trace_printk:PASS:trace_printk__open 0 nsec
> serial_test_trace_printk:PASS:skel->rodata->fmt[0] 0 nsec
> serial_test_trace_printk:PASS:trace_printk__load 0 nsec
> serial_test_trace_printk:PASS:trace_printk__attach 0 nsec
> serial_test_trace_printk:FAIL:fopen(TRACEBUF) unexpected error: -2
>
> #164 trace_vprintk:FAIL
> serial_test_trace_vprintk:PASS:trace_vprintk__open_and_load 0 nsec
> serial_test_trace_vprintk:PASS:trace_vprintk__attach 0 nsec
> serial_test_trace_vprintk:FAIL:fopen(TRACEBUF) unexpected error: -2
>
> #46 fexit_stress:FAIL
> test_fexit_stress:PASS:find_vmlinux_btf_id 0 nsec
> test_fexit_stress:PASS:fexit loaded 0 nsec
> test_fexit_stress:PASS:fexit attach failed 0 nsec
> test_fexit_stress:PASS:fexit loaded 0 nsec
>
> ...
>
> test_fexit_stress:PASS:fexit loaded 0 nsec
> test_fexit_stress:PASS:fexit attach failed 0 nsec
> test_fexit_stress:PASS:fexit loaded 0 nsec
> test_fexit_stress:FAIL:fexit attach failed prog 37 failed: -7 err 7
>
>
>
> > V6 -> V5:
> >   * adding error summary logic for non parallel mode too.
> >   * changed how serial tests are implemented, use main process instead of worker 0.
> >   * fixed a dozen broken test when running in parallel.
> >
> > V5 -> V4:
> >   * change to SOCK_SEQPACKET for close notification.
> >   * move all debug output to "--debug" mode
> >   * output log as test finish, and all error logs again after summary line.
> >   * variable naming / style changes
> >   * adds serial_test_name() to replace serial test lists.
> >
> >
> > Yucong Sun (14):
> >   selftests/bpf: Add parallelism to test_progs
> >   selftests/bpf: Allow some tests to be executed in sequence
> >   selftests/bpf: disable perf rate limiting when running tests.
> >   selftests/bpf: add per worker cgroup suffix
> >   selftests/bpf: adding read_perf_max_sample_freq() helper
> >   selftests/bpf: fix race condition in enable_stats
> >   selftests/bpf: make cgroup_v1v2 use its own port
> >   selftests/bpf: adding a namespace reset for tc_redirect
> >   selftests/bpf: Make uprobe tests use different attach functions.
> >   selftests/bpf: adding pid filtering for atomics test
> >   selftests/bpf: adding random delay for send_signal test
> >   selftests/bpf: Fix pid check in fexit_sleep test
> >   selftests/bpf: increase loop count for perf_branches
> >   selfetest/bpf: make some tests serial
> >
> >  tools/testing/selftests/bpf/cgroup_helpers.c  |   6 +-
> >  tools/testing/selftests/bpf/cgroup_helpers.h  |   2 +-
> >  .../selftests/bpf/prog_tests/atomics.c        |   1 +
> >  .../selftests/bpf/prog_tests/attach_probe.c   |   8 +-
> >  .../selftests/bpf/prog_tests/bpf_cookie.c     |  10 +-
> >  .../bpf/prog_tests/bpf_iter_setsockopt.c      |   2 +-
> >  .../selftests/bpf/prog_tests/bpf_obj_id.c     |   2 +-
> >  .../bpf/prog_tests/cg_storage_multi.c         |   2 +-
> >  .../bpf/prog_tests/cgroup_attach_autodetach.c |   2 +-
> >  .../bpf/prog_tests/cgroup_attach_multi.c      |   2 +-
> >  .../bpf/prog_tests/cgroup_attach_override.c   |   2 +-
> >  .../selftests/bpf/prog_tests/cgroup_link.c    |   2 +-
> >  .../selftests/bpf/prog_tests/cgroup_v1v2.c    |   2 +-
> >  .../selftests/bpf/prog_tests/check_mtu.c      |   2 +-
> >  .../selftests/bpf/prog_tests/fexit_bpf2bpf.c  |   3 +-
> >  .../prog_tests/flow_dissector_load_bytes.c    |   2 +-
> >  .../bpf/prog_tests/flow_dissector_reattach.c  |   2 +-
> >  .../bpf/prog_tests/get_branch_snapshot.c      |   2 +-
> >  .../selftests/bpf/prog_tests/kfree_skb.c      |   3 +-
> >  .../bpf/prog_tests/migrate_reuseport.c        |   2 +-
> >  .../selftests/bpf/prog_tests/modify_return.c  |   3 +-
> >  .../bpf/prog_tests/ns_current_pid_tgid.c      |   3 +-
> >  .../selftests/bpf/prog_tests/perf_branches.c  |  10 +-
> >  .../selftests/bpf/prog_tests/perf_buffer.c    |   2 +-
> >  .../selftests/bpf/prog_tests/perf_link.c      |   5 +-
> >  .../selftests/bpf/prog_tests/probe_user.c     |   3 +-
> >  .../bpf/prog_tests/raw_tp_writable_test_run.c |   3 +-
> >  .../bpf/prog_tests/select_reuseport.c         |   2 +-
> >  .../selftests/bpf/prog_tests/send_signal.c    |   6 +-
> >  .../bpf/prog_tests/send_signal_sched_switch.c |   3 +-
> >  .../bpf/prog_tests/sk_storage_tracing.c       |   2 +-
> >  .../selftests/bpf/prog_tests/snprintf_btf.c   |   2 +-
> >  .../selftests/bpf/prog_tests/sock_fields.c    |   2 +-
> >  .../selftests/bpf/prog_tests/sockmap_listen.c |   2 +-
> >  .../bpf/prog_tests/stacktrace_build_id_nmi.c  |  19 +-
> >  .../selftests/bpf/prog_tests/task_pt_regs.c   |   8 +-
> >  .../selftests/bpf/prog_tests/tc_redirect.c    |  14 +
> >  .../testing/selftests/bpf/prog_tests/timer.c  |   3 +-
> >  .../selftests/bpf/prog_tests/timer_mim.c      |   2 +-
> >  .../bpf/prog_tests/tp_attach_query.c          |   2 +-
> >  .../selftests/bpf/prog_tests/trace_printk.c   |   2 +-
> >  .../selftests/bpf/prog_tests/trace_vprintk.c  |   2 +-
> >  .../bpf/prog_tests/trampoline_count.c         |   3 +-
> >  .../selftests/bpf/prog_tests/xdp_attach.c     |   2 +-
> >  .../selftests/bpf/prog_tests/xdp_bonding.c    |   2 +-
> >  .../bpf/prog_tests/xdp_cpumap_attach.c        |   2 +-
> >  .../bpf/prog_tests/xdp_devmap_attach.c        |   2 +-
> >  .../selftests/bpf/prog_tests/xdp_info.c       |   2 +-
> >  .../selftests/bpf/prog_tests/xdp_link.c       |   2 +-
> >  tools/testing/selftests/bpf/progs/atomics.c   |  16 +
> >  .../selftests/bpf/progs/connect4_dropper.c    |   2 +-
> >  .../testing/selftests/bpf/progs/fexit_sleep.c |   4 +-
> >  .../selftests/bpf/progs/test_enable_stats.c   |   2 +-
> >  tools/testing/selftests/bpf/test_progs.c      | 671 +++++++++++++++++-
> >  tools/testing/selftests/bpf/test_progs.h      |  37 +-
> >  55 files changed, 790 insertions(+), 116 deletions(-)
> >
> > --
> > 2.30.2
> >

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 09/14] selftests/bpf: Make uprobe tests use different attach functions.
  2021-10-08 22:27   ` Andrii Nakryiko
@ 2021-10-08 22:46     ` sunyucong
  2021-10-09  3:13       ` Andrii Nakryiko
  0 siblings, 1 reply; 39+ messages in thread
From: sunyucong @ 2021-10-08 22:46 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Yucong Sun, Song Liu, bpf, Andrii Nakryiko

On Fri, Oct 8, 2021 at 3:27 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
> >
> > From: Yucong Sun <sunyucong@gmail.com>
> >
> > Using same address on different processes of the same binary often fail
> > with EINVAL, this patch make these tests use distinct methods, so they
> > can run in parallel.
> >
> > Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> > ---
> >  tools/testing/selftests/bpf/prog_tests/attach_probe.c | 8 ++++++--
> >  tools/testing/selftests/bpf/prog_tests/bpf_cookie.c   | 8 ++++++--
> >  tools/testing/selftests/bpf/prog_tests/task_pt_regs.c | 8 ++++++--
> >  3 files changed, 18 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > index 6c511dcd1465..eff36ba9c148 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > @@ -5,6 +5,10 @@
> >  /* this is how USDT semaphore is actually defined, except volatile modifier */
> >  volatile unsigned short uprobe_ref_ctr __attribute__((unused)) __attribute((section(".probes")));
> >
> > +static int method() {
>
> wrong style: { should be on separate line
>
> > +       return get_base_addr();
>
> there is nothing special about get_base_addr(), except that it's a
> global function in a different file and won't be inlined, while this
> method() approach has no such guarantee
>
> I've dropped this patch for now.
>
> But I'm surprised that attaching to the same uprobe few times doesn't
> work. Song, is there anything in kernel that could cause this?


libbpf: uprobe perf_event_open() failed: Invalid argument
libbpf: prog 'handle_uprobe': failed to create uprobe
'/proc/self/exe:0x144d59' perf event: Invalid argument
uprobe_subtest:FAIL:link1 unexpected error: -22

The problem only happens when several different processes of the same
binary are trying to attach uprobe on the same function. I am guessing
it is due to address space randomization ?

I traced through the code and the EINVAL is returned right after this warning

[    1.375901] ref_ctr_offset mismatch. inode: 0x55a0 offset: 0x144d59
ref_ctr_offset(old): 0x554a00 ref_ctr_offset(new): 0x0


This could be easily be reproduced by    ./test_progs -t
attach_probe,bpf_cookie,test_pt_regs -j

>
>
> > +}
> > +
> >  void test_attach_probe(void)
> >  {
> >         DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, uprobe_opts);
> > @@ -33,7 +37,7 @@ void test_attach_probe(void)
> >         if (CHECK(base_addr < 0, "get_base_addr",
> >                   "failed to find base addr: %zd", base_addr))
> >                 return;
> > -       uprobe_offset = get_uprobe_offset(&get_base_addr, base_addr);
> > +       uprobe_offset = get_uprobe_offset(&method, base_addr);
> >
> >         ref_ctr_offset = get_rel_offset((uintptr_t)&uprobe_ref_ctr);
> >         if (!ASSERT_GE(ref_ctr_offset, 0, "ref_ctr_offset"))
> > @@ -98,7 +102,7 @@ void test_attach_probe(void)
> >                 goto cleanup;
> >
> >         /* trigger & validate uprobe & uretprobe */
> > -       get_base_addr();
> > +       method();
> >
> >         if (CHECK(skel->bss->uprobe_res != 3, "check_uprobe_res",
> >                   "wrong uprobe res: %d\n", skel->bss->uprobe_res))
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > index 19c9f7b53cfa..5ebd8ba988e2 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > @@ -8,6 +8,10 @@
> >  #include <test_progs.h>
> >  #include "test_bpf_cookie.skel.h"
> >
> > +static int method() {
> > +       return get_base_addr();
> > +}
> > +
> >  static void kprobe_subtest(struct test_bpf_cookie *skel)
> >  {
> >         DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts);
> > @@ -66,7 +70,7 @@ static void uprobe_subtest(struct test_bpf_cookie *skel)
> >         ssize_t base_addr;
> >
> >         base_addr = get_base_addr();
> > -       uprobe_offset = get_uprobe_offset(&get_base_addr, base_addr);
> > +       uprobe_offset = get_uprobe_offset(&method, base_addr);
> >
> >         /* attach two uprobes */
> >         opts.bpf_cookie = 0x100;
> > @@ -99,7 +103,7 @@ static void uprobe_subtest(struct test_bpf_cookie *skel)
> >                 goto cleanup;
> >
> >         /* trigger uprobe && uretprobe */
> > -       get_base_addr();
> > +       method();
> >
> >         ASSERT_EQ(skel->bss->uprobe_res, 0x100 | 0x200, "uprobe_res");
> >         ASSERT_EQ(skel->bss->uretprobe_res, 0x1000 | 0x2000, "uretprobe_res");
> > diff --git a/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c b/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
> > index 37c20b5ffa70..4d2f1435be90 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
> > @@ -3,6 +3,10 @@
> >  #include <test_progs.h>
> >  #include "test_task_pt_regs.skel.h"
> >
> > +static int method() {
> > +       return get_base_addr();
> > +}
> > +
> >  void test_task_pt_regs(void)
> >  {
> >         struct test_task_pt_regs *skel;
> > @@ -14,7 +18,7 @@ void test_task_pt_regs(void)
> >         base_addr = get_base_addr();
> >         if (!ASSERT_GT(base_addr, 0, "get_base_addr"))
> >                 return;
> > -       uprobe_offset = get_uprobe_offset(&get_base_addr, base_addr);
> > +       uprobe_offset = get_uprobe_offset(&method, base_addr);
> >
> >         skel = test_task_pt_regs__open_and_load();
> >         if (!ASSERT_OK_PTR(skel, "skel_open"))
> > @@ -32,7 +36,7 @@ void test_task_pt_regs(void)
> >         skel->links.handle_uprobe = uprobe_link;
> >
> >         /* trigger & validate uprobe */
> > -       get_base_addr();
> > +       method();
> >
> >         if (!ASSERT_EQ(skel->bss->uprobe_res, 1, "check_uprobe_res"))
> >                 goto cleanup;
> > --
> > 2.30.2
> >

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 05/14] selftests/bpf: adding read_perf_max_sample_freq() helper
  2021-10-08 22:27   ` Andrii Nakryiko
@ 2021-10-08 22:49     ` sunyucong
  2022-03-08 20:22       ` sunyucong
  0 siblings, 1 reply; 39+ messages in thread
From: sunyucong @ 2021-10-08 22:49 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Yucong Sun, bpf, Andrii Nakryiko

On Fri, Oct 8, 2021 at 3:27 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
> >
> > From: Yucong Sun <sunyucong@gmail.com>
> >
> > This patch moved a helper function to test_progs and make all tests
> > setting sampling frequency use it to read current perf_max_sample_freq,
> > this will avoid triggering EINVAL error.
> >
> > Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> > ---
> >  .../selftests/bpf/prog_tests/bpf_cookie.c     |  2 +-
> >  .../selftests/bpf/prog_tests/perf_branches.c  |  4 ++--
> >  .../selftests/bpf/prog_tests/perf_link.c      |  2 +-
> >  .../bpf/prog_tests/stacktrace_build_id_nmi.c  | 19 ++-----------------
> >  tools/testing/selftests/bpf/test_progs.c      | 15 +++++++++++++++
> >  tools/testing/selftests/bpf/test_progs.h      |  1 +
> >  6 files changed, 22 insertions(+), 21 deletions(-)
> >
>
> We have trace_helper.c, seems like it would be better to have it
> there? I haven't applied this patch yet.

I did look at that file, but the content was not really related, so
didn't go with it, of course we can :-D

>
> [...]
>
> > @@ -48,6 +31,8 @@ void test_stacktrace_build_id_nmi(void)
> >         if (CHECK(err, "skel_load", "skeleton load failed: %d\n", err))
> >                 goto cleanup;
> >
> > +       attr.sample_freq = read_perf_max_sample_freq();
> > +
> >         pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
> >                          0 /* cpu 0 */, -1 /* group id */,
> >                          0 /* flags */);
> > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> > index 2ac922f8aa2c..66825313414b 100644
> > --- a/tools/testing/selftests/bpf/test_progs.c
> > +++ b/tools/testing/selftests/bpf/test_progs.c
> > @@ -1500,3 +1500,18 @@ int main(int argc, char **argv)
> >
> >         return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
> >  }
> > +
> > +__u64 read_perf_max_sample_freq(void)
> > +{
> > +       __u64 sample_freq = 1000; /* fallback to 1000 on error */
>
> previous default was 5000, message below still claims 5000, what's the
> The reason for changing it?

This is from my observation that on my machine it frequently dip down
to 3000,  the test doesn't really rely on 5000 either, they were fine
with 1000 even.

>
>
>
> > +       FILE *f;
> > +       __u32 duration = 0;
> > +
> > +       f = fopen("/proc/sys/kernel/perf_event_max_sample_rate", "r");
> > +       if (f == NULL)
> > +               return sample_freq;
> > +       CHECK(fscanf(f, "%llu", &sample_freq) != 1, "Get max sample rate",
> > +             "return default value: 5000,err %d\n", -errno);
> > +       fclose(f);
> > +       return sample_freq;
> > +}
> > diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
> > index b239dc9fcef0..d5ca0d36cc96 100644
> > --- a/tools/testing/selftests/bpf/test_progs.h
> > +++ b/tools/testing/selftests/bpf/test_progs.h
> > @@ -327,6 +327,7 @@ int extract_build_id(char *build_id, size_t size);
> >  int kern_sync_rcu(void);
> >  int trigger_module_test_read(int read_sz);
> >  int trigger_module_test_write(int write_sz);
> > +__u64 read_perf_max_sample_freq(void);
> >
> >  #ifdef __x86_64__
> >  #define SYS_NANOSLEEP_KPROBE_NAME "__x64_sys_nanosleep"
> > --
> > 2.30.2
> >

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 14/14] selfetest/bpf: make some tests serial
  2021-10-08 22:27   ` Andrii Nakryiko
@ 2021-10-08 22:55     ` sunyucong
  2021-10-09  3:14       ` Andrii Nakryiko
  0 siblings, 1 reply; 39+ messages in thread
From: sunyucong @ 2021-10-08 22:55 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Yucong Sun, bpf, Andrii Nakryiko

On Fri, Oct 8, 2021 at 3:27 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
> >
> > From: Yucong Sun <sunyucong@gmail.com>
> >
> > Change tests that often fails in parallel execution mode to serial.
> >
> > Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> > ---
>
> I hope we'll be able to parallelise these tests over time. See some
> notes on cover letter for how to target this effort better. But here's
> another thought I had while thinking about this.
>
> Some tests are inherently testing unshareable resources, e.g., like
> XDP tests. So we might never be able to completely parallelize all the
> tests. But it probably will be too restrictive to just bunde all of
> them into the one "serial" group of tests. It's a good starting point,
> but I think we'll have to have something like "serialization" group,
> where we'll be able to mark a bunch of tests like
> "serial_xdp_<testname>" and each test with "serial_xdp_" prefix will
> run sequentially relative to each other, but they might run completely
> parallel to, say, "serial_perf_" tests. WDYT?

I think someone was talking about being able to attach multiple XDP
programs in the future?

another approach I want to suggest is for each test to test for
EBUSY/EEXIST error code on attach and simply sleep/retry,  Then in the
future when it doesn't return this error there is nothing to change on
test side.
>
>
> >  tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c   | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c            | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c      | 2 +-
> >  .../selftests/bpf/prog_tests/cgroup_attach_autodetach.c        | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c   | 2 +-
> >  .../testing/selftests/bpf/prog_tests/cgroup_attach_override.c  | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/cgroup_link.c           | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/check_mtu.c             | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c         | 3 ++-
> >  .../selftests/bpf/prog_tests/flow_dissector_load_bytes.c       | 2 +-
> >  .../testing/selftests/bpf/prog_tests/flow_dissector_reattach.c | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/get_branch_snapshot.c   | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/kfree_skb.c             | 3 ++-
> >  tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c     | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/modify_return.c         | 3 ++-
> >  tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c   | 3 ++-
> >  tools/testing/selftests/bpf/prog_tests/perf_buffer.c           | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/perf_link.c             | 3 ++-
> >  tools/testing/selftests/bpf/prog_tests/probe_user.c            | 3 ++-
> >  .../selftests/bpf/prog_tests/raw_tp_writable_test_run.c        | 3 ++-
> >  tools/testing/selftests/bpf/prog_tests/select_reuseport.c      | 2 +-
> >  .../selftests/bpf/prog_tests/send_signal_sched_switch.c        | 3 ++-
> >  tools/testing/selftests/bpf/prog_tests/sk_storage_tracing.c    | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/snprintf_btf.c          | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/sock_fields.c           | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/sockmap_listen.c        | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/timer.c                 | 3 ++-
> >  tools/testing/selftests/bpf/prog_tests/timer_mim.c             | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/tp_attach_query.c       | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/trace_printk.c          | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/trace_vprintk.c         | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/trampoline_count.c      | 3 ++-
> >  tools/testing/selftests/bpf/prog_tests/xdp_attach.c            | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/xdp_bonding.c           | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c     | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c     | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/xdp_info.c              | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/xdp_link.c              | 2 +-
> >  38 files changed, 48 insertions(+), 38 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c
> > index 85babb0487b3..b52ff8ce34db 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c
> > @@ -179,7 +179,7 @@ static void do_bpf_iter_setsockopt(struct bpf_iter_setsockopt *iter_skel,
> >         free_fds(est_fds, nr_est);
> >  }
> >
>
> [...]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 13/14] selftests/bpf: increase loop count for perf_branches
  2021-10-08 22:27   ` Andrii Nakryiko
@ 2021-10-08 22:57     ` sunyucong
  0 siblings, 0 replies; 39+ messages in thread
From: sunyucong @ 2021-10-08 22:57 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Yucong Sun, bpf, Andrii Nakryiko

On Fri, Oct 8, 2021 at 3:27 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
> >
> > From: Yucong Sun <sunyucong@gmail.com>
> >
> > This make this test more likely to succeed.
> >
> > Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> > ---
>
> 100 million iterations seems a bit excessive. Why one million loops
> doesn't cause a single perf event? Can we make it more robust in some
> other way that is not as slow? I've dropped it for now while we
> discuss.

I don't know, without this patch the test constantly fails for me
regardless of serial or parallel mode.
I think it could be something related to compiler optimizations or hardware?

>
>
> >  tools/testing/selftests/bpf/prog_tests/perf_branches.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/perf_branches.c b/tools/testing/selftests/bpf/prog_tests/perf_branches.c
> > index 6b2e3dced619..d7e88b2c5f36 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/perf_branches.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/perf_branches.c
> > @@ -16,7 +16,7 @@ static void check_good_sample(struct test_perf_branches *skel)
> >         int duration = 0;
> >
> >         if (CHECK(!skel->bss->valid, "output not valid",
> > -                "no valid sample from prog"))
> > +                "no valid sample from prog\n"))
> >                 return;
> >
> >         /*
> > @@ -46,7 +46,7 @@ static void check_bad_sample(struct test_perf_branches *skel)
> >         int duration = 0;
> >
> >         if (CHECK(!skel->bss->valid, "output not valid",
> > -                "no valid sample from prog"))
> > +                "no valid sample from prog\n"))
> >                 return;
> >
> >         CHECK((required_size != -EINVAL && required_size != -ENOENT),
> > @@ -84,7 +84,7 @@ static void test_perf_branches_common(int perf_fd,
> >         if (CHECK(err, "set_affinity", "cpu #0, err %d\n", err))
> >                 goto out_destroy;
> >         /* spin the loop for a while (random high number) */
> > -       for (i = 0; i < 1000000; ++i)
> > +       for (i = 0; i < 100000000; ++i)
> >                 ++j;
> >
> >         test_perf_branches__detach(skel);
> > --
> > 2.30.2
> >

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 02/14] selftests/bpf: Allow some tests to be executed in sequence
  2021-10-08 22:26   ` Andrii Nakryiko
@ 2021-10-08 23:06     ` sunyucong
  2021-10-09  3:17       ` Andrii Nakryiko
  0 siblings, 1 reply; 39+ messages in thread
From: sunyucong @ 2021-10-08 23:06 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Yucong Sun, bpf, Andrii Nakryiko

On Fri, Oct 8, 2021 at 3:26 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
> >
> > From: Yucong Sun <sunyucong@gmail.com>
> >
> > This patch allows tests to define serial_test_name() instead of
> > test_name(), and this will make test_progs execute those in sequence
> > after all other tests finished executing concurrently.
> >
> > Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> > ---
> >  tools/testing/selftests/bpf/test_progs.c | 60 +++++++++++++++++++++---
> >  1 file changed, 54 insertions(+), 6 deletions(-)
> >
>
> [...]
>
> > @@ -1129,6 +1136,40 @@ static int server_main(void)
> >         free(env.worker_current_test);
> >         free(data);
> >
> > +       /* run serial tests */
> > +       save_netns();
> > +
> > +       for (int i = 0; i < prog_test_cnt; i++) {
> > +               struct prog_test_def *test = &prog_test_defs[i];
> > +               struct test_result *result = &test_results[i];
> > +
> > +               if (!test->should_run || !test->run_serial_test)
> > +                       continue;
> > +
> > +               stdio_hijack();
> > +
> > +               run_one_test(i);
> > +
> > +               stdio_restore();
> > +               if (env.log_buf) {
> > +                       result->log_cnt = env.log_cnt;
> > +                       result->log_buf = strdup(env.log_buf);
> > +
> > +                       free(env.log_buf);
> > +                       env.log_buf = NULL;
> > +                       env.log_cnt = 0;
> > +               }
> > +               restore_netns();
> > +
> > +               fprintf(stdout, "#%d %s:%s\n",
> > +                       test->test_num, test->test_name,
> > +                       test->error_cnt ? "FAIL" : (test->skip_cnt ? "SKIP" : "OK"));
> > +
> > +               result->error_cnt = test->error_cnt;
> > +               result->skip_cnt = test->skip_cnt;
> > +               result->sub_succ_cnt = test->sub_succ_cnt;
> > +       }
> > +
>
> Did you try to just reuse sequential running loop logic in main() for
> this? I'd like to avoid the third test running loop copy, if possible.
> What were the problems of reusing the sequential logic from main(),
> they do the same work, no?

Well, yes and no

The loop itself is small/simple enough, I'm not sure there is a value
to extract them to a common function with multiple arguments.
I think the main issue that needs to be refactored is that log
printing still works differently in serial mode or parallel mode,  it
works now, but I would like to get rid of the old dump_test_log()
function.

>
>
> >         /* generate summary */
> >         fflush(stderr);
> >         fflush(stdout);
> > @@ -1326,6 +1367,13 @@ int main(int argc, char **argv)
> >                         test->should_run = true;
> >                 else
> >                         test->should_run = false;
> > +
> > +               if ((test->run_test == NULL && test->run_serial_test == NULL) ||
> > +                   (test->run_test != NULL && test->run_serial_test != NULL)) {
> > +                       fprintf(stderr, "Test %d:%s must have either test_%s() or serial_test_%sl() defined.\n",
> > +                               test->test_num, test->test_name, test->test_name, test->test_name);
> > +                       exit(EXIT_ERR_SETUP_INFRA);
> > +               }
> >         }
> >
> >         /* ignore workers if we are just listing */
> > --
> > 2.30.2
> >

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 08/14] selftests/bpf: adding a namespace reset for tc_redirect
  2021-10-08 22:27   ` Andrii Nakryiko
@ 2021-10-08 23:07     ` sunyucong
  0 siblings, 0 replies; 39+ messages in thread
From: sunyucong @ 2021-10-08 23:07 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Yucong Sun, bpf, Andrii Nakryiko

On Fri, Oct 8, 2021 at 3:27 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
> >
> > From: Yucong Sun <sunyucong@gmail.com>
> >
> > This patch delete ns_src/ns_dst/ns_redir namespaces before recreating
> > them, making the test more robust.
> >
> > Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> > ---
> >  .../testing/selftests/bpf/prog_tests/tc_redirect.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> > index e87bc4466d9a..25744136e131 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> > @@ -176,6 +176,18 @@ static int netns_setup_namespaces(const char *verb)
> >         return 0;
> >  }
> >
> > +static void netns_setup_namespaces_nofail(const char *verb)
> > +{
> > +       const char * const *ns = namespaces;
> > +       char cmd[128];
> > +
> > +       while (*ns) {
> > +               snprintf(cmd, sizeof(cmd), "ip netns %s %s", verb, *ns);
> > +               system(cmd);
>
> is this what's causing
>
> Cannot remove namespace file "/var/run/netns/ns_src": No such file or directory
> Cannot remove namespace file "/var/run/netns/ns_fwd": No such file or directory
> Cannot remove namespace file "/var/run/netns/ns_dst": No such file or directory
>
> ?
>
> I haven't applied it yet, let's see if there is a way to avoid
> unnecessary "warnings".

we could just change this line

+               snprintf(cmd, sizeof(cmd), "ip netns %s %s > /dev/null
2>&1", verb, *ns);

to get rid of the warning

>
>
>
> > +               ns++;
> > +       }
> > +}
> > +
> >  struct netns_setup_result {
> >         int ifindex_veth_src_fwd;
> >         int ifindex_veth_dst_fwd;
> > @@ -762,6 +774,8 @@ static void test_tc_redirect_peer_l3(struct netns_setup_result *setup_result)
> >
> >  static void *test_tc_redirect_run_tests(void *arg)
> >  {
> > +       netns_setup_namespaces_nofail("delete");
> > +
> >         RUN_TEST(tc_redirect_peer);
> >         RUN_TEST(tc_redirect_peer_l3);
> >         RUN_TEST(tc_redirect_neigh);
> > --
> > 2.30.2
> >

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs
  2021-10-08 22:42   ` Andrii Nakryiko
@ 2021-10-08 23:37     ` Steven Rostedt
  2021-10-09  3:20       ` Andrii Nakryiko
  0 siblings, 1 reply; 39+ messages in thread
From: Steven Rostedt @ 2021-10-08 23:37 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Yucong Sun, bpf, Andrii Nakryiko, Yucong Sun

On Fri, 8 Oct 2021 15:42:31 -0700
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> orgot to actually cc Steven, oops. Steven, I've run into the problem
> when running a few selftests that do uprobe/kprobe attachment. At some
> point, they started complaining that files like
> /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id don't
> exist. And this condition persisted. When I checked
> /sys/kernel/debug/tracing in QEMU, it was empty. Is this a known
> problem?

The tracefs directory should automatically be mounted in the debugfs
"tracing" directory when debugfs is mounted.

Does /sys/kernel/tracing exist?

-- Steve

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 09/14] selftests/bpf: Make uprobe tests use different attach functions.
  2021-10-08 22:46     ` sunyucong
@ 2021-10-09  3:13       ` Andrii Nakryiko
  0 siblings, 0 replies; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-09  3:13 UTC (permalink / raw)
  To: sunyucong; +Cc: Yucong Sun, Song Liu, bpf, Andrii Nakryiko

On Fri, Oct 8, 2021 at 3:47 PM sunyucong@gmail.com <sunyucong@gmail.com> wrote:
>
> On Fri, Oct 8, 2021 at 3:27 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
> > >
> > > From: Yucong Sun <sunyucong@gmail.com>
> > >
> > > Using same address on different processes of the same binary often fail
> > > with EINVAL, this patch make these tests use distinct methods, so they
> > > can run in parallel.
> > >
> > > Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> > > ---
> > >  tools/testing/selftests/bpf/prog_tests/attach_probe.c | 8 ++++++--
> > >  tools/testing/selftests/bpf/prog_tests/bpf_cookie.c   | 8 ++++++--
> > >  tools/testing/selftests/bpf/prog_tests/task_pt_regs.c | 8 ++++++--
> > >  3 files changed, 18 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > > index 6c511dcd1465..eff36ba9c148 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > > @@ -5,6 +5,10 @@
> > >  /* this is how USDT semaphore is actually defined, except volatile modifier */
> > >  volatile unsigned short uprobe_ref_ctr __attribute__((unused)) __attribute((section(".probes")));
> > >
> > > +static int method() {
> >
> > wrong style: { should be on separate line
> >
> > > +       return get_base_addr();
> >
> > there is nothing special about get_base_addr(), except that it's a
> > global function in a different file and won't be inlined, while this
> > method() approach has no such guarantee
> >
> > I've dropped this patch for now.
> >
> > But I'm surprised that attaching to the same uprobe few times doesn't
> > work. Song, is there anything in kernel that could cause this?
>
>
> libbpf: uprobe perf_event_open() failed: Invalid argument
> libbpf: prog 'handle_uprobe': failed to create uprobe
> '/proc/self/exe:0x144d59' perf event: Invalid argument
> uprobe_subtest:FAIL:link1 unexpected error: -22
>
> The problem only happens when several different processes of the same
> binary are trying to attach uprobe on the same function. I am guessing
> it is due to address space randomization ?

nope, we don't use address space randomization, it's the
ref_ctr_offset (normally used for USDT semaphore)

>
> I traced through the code and the EINVAL is returned right after this warning
>
> [    1.375901] ref_ctr_offset mismatch. inode: 0x55a0 offset: 0x144d59
> ref_ctr_offset(old): 0x554a00 ref_ctr_offset(new): 0x0

ah, ref_ctr_offset is probably enforced by the kernel to be the same
across all uprobes. attach_probe is the only one that's testing
ref_ctr_offset, so it should be enough to modify only that one, just
make sure you are using a non-inlined function for this.

>
>
> This could be easily be reproduced by    ./test_progs -t
> attach_probe,bpf_cookie,test_pt_regs -j
>
> >
> >
> > > +}
> > > +

[...]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 14/14] selfetest/bpf: make some tests serial
  2021-10-08 22:55     ` sunyucong
@ 2021-10-09  3:14       ` Andrii Nakryiko
  0 siblings, 0 replies; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-09  3:14 UTC (permalink / raw)
  To: sunyucong; +Cc: Yucong Sun, bpf, Andrii Nakryiko

On Fri, Oct 8, 2021 at 3:55 PM sunyucong@gmail.com <sunyucong@gmail.com> wrote:
>
> On Fri, Oct 8, 2021 at 3:27 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
> > >
> > > From: Yucong Sun <sunyucong@gmail.com>
> > >
> > > Change tests that often fails in parallel execution mode to serial.
> > >
> > > Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> > > ---
> >
> > I hope we'll be able to parallelise these tests over time. See some
> > notes on cover letter for how to target this effort better. But here's
> > another thought I had while thinking about this.
> >
> > Some tests are inherently testing unshareable resources, e.g., like
> > XDP tests. So we might never be able to completely parallelize all the
> > tests. But it probably will be too restrictive to just bunde all of
> > them into the one "serial" group of tests. It's a good starting point,
> > but I think we'll have to have something like "serialization" group,
> > where we'll be able to mark a bunch of tests like
> > "serial_xdp_<testname>" and each test with "serial_xdp_" prefix will
> > run sequentially relative to each other, but they might run completely
> > parallel to, say, "serial_perf_" tests. WDYT?
>
> I think someone was talking about being able to attach multiple XDP
> programs in the future?
>

that's all built on top of single XDP attach point using a bunch of
conventions, so that's not a solution here

> another approach I want to suggest is for each test to test for
> EBUSY/EEXIST error code on attach and simply sleep/retry,  Then in the
> future when it doesn't return this error there is nothing to change on
> test side.

This won't work reliably, sometimes test just makes some system-wide
assumptions or modifications, it's not always resulting in EBUSY

> >
> >
> > >  tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c   | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c            | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c      | 2 +-
> > >  .../selftests/bpf/prog_tests/cgroup_attach_autodetach.c        | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c   | 2 +-
> > >  .../testing/selftests/bpf/prog_tests/cgroup_attach_override.c  | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/cgroup_link.c           | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/check_mtu.c             | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c         | 3 ++-
> > >  .../selftests/bpf/prog_tests/flow_dissector_load_bytes.c       | 2 +-
> > >  .../testing/selftests/bpf/prog_tests/flow_dissector_reattach.c | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/get_branch_snapshot.c   | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/kfree_skb.c             | 3 ++-
> > >  tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c     | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/modify_return.c         | 3 ++-
> > >  tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c   | 3 ++-
> > >  tools/testing/selftests/bpf/prog_tests/perf_buffer.c           | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/perf_link.c             | 3 ++-
> > >  tools/testing/selftests/bpf/prog_tests/probe_user.c            | 3 ++-
> > >  .../selftests/bpf/prog_tests/raw_tp_writable_test_run.c        | 3 ++-
> > >  tools/testing/selftests/bpf/prog_tests/select_reuseport.c      | 2 +-
> > >  .../selftests/bpf/prog_tests/send_signal_sched_switch.c        | 3 ++-
> > >  tools/testing/selftests/bpf/prog_tests/sk_storage_tracing.c    | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/snprintf_btf.c          | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/sock_fields.c           | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/sockmap_listen.c        | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/timer.c                 | 3 ++-
> > >  tools/testing/selftests/bpf/prog_tests/timer_mim.c             | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/tp_attach_query.c       | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/trace_printk.c          | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/trace_vprintk.c         | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/trampoline_count.c      | 3 ++-
> > >  tools/testing/selftests/bpf/prog_tests/xdp_attach.c            | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/xdp_bonding.c           | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c     | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c     | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/xdp_info.c              | 2 +-
> > >  tools/testing/selftests/bpf/prog_tests/xdp_link.c              | 2 +-
> > >  38 files changed, 48 insertions(+), 38 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c
> > > index 85babb0487b3..b52ff8ce34db 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c
> > > @@ -179,7 +179,7 @@ static void do_bpf_iter_setsockopt(struct bpf_iter_setsockopt *iter_skel,
> > >         free_fds(est_fds, nr_est);
> > >  }
> > >
> >
> > [...]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 02/14] selftests/bpf: Allow some tests to be executed in sequence
  2021-10-08 23:06     ` sunyucong
@ 2021-10-09  3:17       ` Andrii Nakryiko
  0 siblings, 0 replies; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-09  3:17 UTC (permalink / raw)
  To: sunyucong; +Cc: Yucong Sun, bpf, Andrii Nakryiko

On Fri, Oct 8, 2021 at 4:06 PM sunyucong@gmail.com <sunyucong@gmail.com> wrote:
>
> On Fri, Oct 8, 2021 at 3:26 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote:
> > >
> > > From: Yucong Sun <sunyucong@gmail.com>
> > >
> > > This patch allows tests to define serial_test_name() instead of
> > > test_name(), and this will make test_progs execute those in sequence
> > > after all other tests finished executing concurrently.
> > >
> > > Signed-off-by: Yucong Sun <sunyucong@gmail.com>
> > > ---
> > >  tools/testing/selftests/bpf/test_progs.c | 60 +++++++++++++++++++++---
> > >  1 file changed, 54 insertions(+), 6 deletions(-)
> > >
> >
> > [...]
> >
> > > @@ -1129,6 +1136,40 @@ static int server_main(void)
> > >         free(env.worker_current_test);
> > >         free(data);
> > >
> > > +       /* run serial tests */
> > > +       save_netns();
> > > +
> > > +       for (int i = 0; i < prog_test_cnt; i++) {
> > > +               struct prog_test_def *test = &prog_test_defs[i];
> > > +               struct test_result *result = &test_results[i];
> > > +
> > > +               if (!test->should_run || !test->run_serial_test)
> > > +                       continue;
> > > +
> > > +               stdio_hijack();
> > > +
> > > +               run_one_test(i);
> > > +
> > > +               stdio_restore();
> > > +               if (env.log_buf) {
> > > +                       result->log_cnt = env.log_cnt;
> > > +                       result->log_buf = strdup(env.log_buf);
> > > +
> > > +                       free(env.log_buf);
> > > +                       env.log_buf = NULL;
> > > +                       env.log_cnt = 0;
> > > +               }
> > > +               restore_netns();
> > > +
> > > +               fprintf(stdout, "#%d %s:%s\n",
> > > +                       test->test_num, test->test_name,
> > > +                       test->error_cnt ? "FAIL" : (test->skip_cnt ? "SKIP" : "OK"));
> > > +
> > > +               result->error_cnt = test->error_cnt;
> > > +               result->skip_cnt = test->skip_cnt;
> > > +               result->sub_succ_cnt = test->sub_succ_cnt;
> > > +       }
> > > +
> >
> > Did you try to just reuse sequential running loop logic in main() for
> > this? I'd like to avoid the third test running loop copy, if possible.
> > What were the problems of reusing the sequential logic from main(),
> > they do the same work, no?
>
> Well, yes and no
>
> The loop itself is small/simple enough, I'm not sure there is a value
> to extract them to a common function with multiple arguments.

The loop has netns save/restore, stdio hijacking, output formatting,
we might add some more logic later. I'm mainly asking because there is
already a sequential loop in the main, and I was wondering if we can
reuse that (as in, let it run regardless of -j option, just run only
serial tests if -j is specified).


> I think the main issue that needs to be refactored is that log
> printing still works differently in serial mode or parallel mode,  it
> works now, but I would like to get rid of the old dump_test_log()
> function.
>
> >
> >
> > >         /* generate summary */
> > >         fflush(stderr);
> > >         fflush(stdout);
> > > @@ -1326,6 +1367,13 @@ int main(int argc, char **argv)
> > >                         test->should_run = true;
> > >                 else
> > >                         test->should_run = false;
> > > +
> > > +               if ((test->run_test == NULL && test->run_serial_test == NULL) ||
> > > +                   (test->run_test != NULL && test->run_serial_test != NULL)) {
> > > +                       fprintf(stderr, "Test %d:%s must have either test_%s() or serial_test_%sl() defined.\n",
> > > +                               test->test_num, test->test_name, test->test_name, test->test_name);
> > > +                       exit(EXIT_ERR_SETUP_INFRA);
> > > +               }
> > >         }
> > >
> > >         /* ignore workers if we are just listing */
> > > --
> > > 2.30.2
> > >

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs
  2021-10-08 23:37     ` Steven Rostedt
@ 2021-10-09  3:20       ` Andrii Nakryiko
  2021-10-09  3:28         ` Steven Rostedt
  0 siblings, 1 reply; 39+ messages in thread
From: Andrii Nakryiko @ 2021-10-09  3:20 UTC (permalink / raw)
  To: Steven Rostedt, Jussi Maki; +Cc: Yucong Sun, bpf, Andrii Nakryiko, Yucong Sun

On Fri, Oct 8, 2021 at 4:37 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri, 8 Oct 2021 15:42:31 -0700
> Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> > orgot to actually cc Steven, oops. Steven, I've run into the problem
> > when running a few selftests that do uprobe/kprobe attachment. At some
> > point, they started complaining that files like
> > /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id don't
> > exist. And this condition persisted. When I checked
> > /sys/kernel/debug/tracing in QEMU, it was empty. Is this a known
> > problem?
>
> The tracefs directory should automatically be mounted in the debugfs
> "tracing" directory when debugfs is mounted.

Yeah, we do that for our testing environment and it works reliably.
This time we started running a bunch of tests completely in parallel,
and in one case I ran into this situation that caused "permanent
damage". Unfortunately I haven't checked if debugfs is still mounted
or not.

Yucong mentioned that one of the selftests (tc_redirect.c) is
manipulating /sys mount, so I wonder if this could have caused some
consequences like this... Jussi, any idea?

>
> Does /sys/kernel/tracing exist?

Yeah, it was there, but was empty.

>
> -- Steve

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs
  2021-10-09  3:20       ` Andrii Nakryiko
@ 2021-10-09  3:28         ` Steven Rostedt
  0 siblings, 0 replies; 39+ messages in thread
From: Steven Rostedt @ 2021-10-09  3:28 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Jussi Maki, Yucong Sun, bpf, Andrii Nakryiko, Yucong Sun

On Fri, 8 Oct 2021 20:20:38 -0700
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> > Does /sys/kernel/tracing exist?  
> 
> Yeah, it was there, but was empty.

And would be filled with: mount -t tracefs nodev /sys/kernel/tracing

-- Steve

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH bpf-next v6 05/14] selftests/bpf: adding read_perf_max_sample_freq() helper
  2021-10-08 22:49     ` sunyucong
@ 2022-03-08 20:22       ` sunyucong
  0 siblings, 0 replies; 39+ messages in thread
From: sunyucong @ 2022-03-08 20:22 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Yucong Sun, bpf, Andrii Nakryiko

the patch can still be found here  :
https://www.spinics.net/lists/bpf/msg47375.html

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2022-03-08 20:23 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-06 18:56 [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Yucong Sun
2021-10-06 18:56 ` [PATCH bpf-next v6 01/14] " Yucong Sun
2021-10-08 22:26   ` Andrii Nakryiko
2021-10-06 18:56 ` [PATCH bpf-next v6 02/14] selftests/bpf: Allow some tests to be executed in sequence Yucong Sun
2021-10-08 22:26   ` Andrii Nakryiko
2021-10-08 23:06     ` sunyucong
2021-10-09  3:17       ` Andrii Nakryiko
2021-10-06 18:56 ` [PATCH bpf-next v6 03/14] selftests/bpf: disable perf rate limiting when running tests Yucong Sun
2021-10-08 22:26   ` Andrii Nakryiko
2021-10-06 18:56 ` [PATCH bpf-next v6 04/14] selftests/bpf: add per worker cgroup suffix Yucong Sun
2021-10-06 18:56 ` [PATCH bpf-next v6 05/14] selftests/bpf: adding read_perf_max_sample_freq() helper Yucong Sun
2021-10-08 22:27   ` Andrii Nakryiko
2021-10-08 22:49     ` sunyucong
2022-03-08 20:22       ` sunyucong
2021-10-06 18:56 ` [PATCH bpf-next v6 06/14] selftests/bpf: fix race condition in enable_stats Yucong Sun
2021-10-06 18:56 ` [PATCH bpf-next v6 07/14] selftests/bpf: make cgroup_v1v2 use its own port Yucong Sun
2021-10-06 18:56 ` [PATCH bpf-next v6 08/14] selftests/bpf: adding a namespace reset for tc_redirect Yucong Sun
2021-10-08 22:27   ` Andrii Nakryiko
2021-10-08 23:07     ` sunyucong
2021-10-06 18:56 ` [PATCH bpf-next v6 09/14] selftests/bpf: Make uprobe tests use different attach functions Yucong Sun
2021-10-08 22:27   ` Andrii Nakryiko
2021-10-08 22:46     ` sunyucong
2021-10-09  3:13       ` Andrii Nakryiko
2021-10-06 18:56 ` [PATCH bpf-next v6 10/14] selftests/bpf: adding pid filtering for atomics test Yucong Sun
2021-10-06 18:56 ` [PATCH bpf-next v6 11/14] selftests/bpf: adding random delay for send_signal test Yucong Sun
2021-10-08 22:27   ` Andrii Nakryiko
2021-10-06 18:56 ` [PATCH bpf-next v6 12/14] selftests/bpf: Fix pid check in fexit_sleep test Yucong Sun
2021-10-06 18:56 ` [PATCH bpf-next v6 13/14] selftests/bpf: increase loop count for perf_branches Yucong Sun
2021-10-08 22:27   ` Andrii Nakryiko
2021-10-08 22:57     ` sunyucong
2021-10-06 18:56 ` [PATCH bpf-next v6 14/14] selfetest/bpf: make some tests serial Yucong Sun
2021-10-08 22:27   ` Andrii Nakryiko
2021-10-08 22:55     ` sunyucong
2021-10-09  3:14       ` Andrii Nakryiko
2021-10-08 22:26 ` [PATCH bpf-next v6 00/14] selftests/bpf: Add parallelism to test_progs Andrii Nakryiko
2021-10-08 22:42   ` Andrii Nakryiko
2021-10-08 23:37     ` Steven Rostedt
2021-10-09  3:20       ` Andrii Nakryiko
2021-10-09  3:28         ` Steven Rostedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.