fix issue iovisor#16: don't enable all tracepoints

these changes use a more targeted approach in enabling/disabling probes.
When enabling k[ret]probes we use the current pid to make the probe unique
system-wide; this allows multiple instances of ply to run.  Also remove
global enable/disable of kprobes/tracepoints as it is not needed.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>

src/include/ply/ply.h

@@ -53,6 +53,7 @@ struct globals {
 	int debug:1;
 	int dump:1;
 	int timeout;
+	pid_t self;
 
 	size_t map_nelem;
 

src/ply.c

@@ -22,6 +22,7 @@
 #include <signal.h>
 #include <stdio.h>
 #include <sys/resource.h>
+#include <sys/types.h>
 #include <unistd.h>
 
 #include <ply/ast.h>
@@ -194,9 +195,10 @@ int main(int argc, char **argv)
 	node_t *probe, *script;
 	prog_t *prog = NULL;
 	pvdr_t *pvdr;
-	FILE *sfp, *enable;
+	FILE *sfp;
 	int err = 0, num, total;
 
+	G.self = getpid();
 	scriptfp = stdin;
 	err = parse_opts(argc, argv, &sfp);
 	if (err)
@@ -269,24 +271,10 @@ int main(int argc, char **argv)
 	siginterrupt(SIGINT, 1);
 	signal(SIGINT, term);
 
-	enable = fopen("/sys/kernel/debug/tracing/events/enable", "w");
-	if (!enable) {
-		perror("unable to enable probes");
-		err = -errno;
-		goto err;
-	}
-
-	fputs("1\n", enable);
-	fflush(enable);
-	rewind(enable);
-
 	fprintf(stderr, "%d probe%s active\n", total, (total == 1) ? "" : "s");
 	err = evpipe_loop(evp, &term_sig, 0);
 
 	fprintf(stderr, "de-activating probes\n");
-	fputs("0\n", enable);
-	fflush(enable);
-	fclose(enable);
 
 	map_teardown(script);
 
@@ -297,8 +285,6 @@ int main(int argc, char **argv)
 			break;
 	}
 
-	/* disable all kprobe events */
-	fclose(fopen("/sys/kernel/debug/tracing/kprobe_events", "w"));
 done:
 err:
 	if (prog)

src/pvdr/kprobe.c

@@ -39,6 +39,12 @@
 #include <ply/ply.h>
 #include <ply/pvdr.h>
 
+#define KPROBE_MAXLEN   0x100
+
+/*
+ * Structure used for internal representation of kprobes, kretprobes and
+ * tracepoints.
+ */
 typedef struct kprobe {
 	const char *type;
 	FILE *ctrl;
@@ -56,6 +62,8 @@ typedef struct profile {
 	kprobe_t *kp;
 } profile_t;
 
+#define	KPROBE_MAXLEN	0x100
+
 static int probe_event_id(kprobe_t *kp, const char *path)
 {
 	FILE *fp;
@@ -86,16 +94,19 @@ static int probe_attach(kprobe_t *kp, int id)
 	gfd = kp->efds.len ? kp->efds.fds[0] : -1;
 	efd = perf_event_open(&attr, -1, 0, gfd, 0);
 	if (efd < 0) {
+		_d("could not open perf_event: %s", strerror(errno));
 		return -errno;
 	}
 
 	if (ioctl(efd, PERF_EVENT_IOC_ENABLE, 0)) {
 		close(efd);
+		_d("could not enable probe: %s", strerror(errno));
 		return -errno;
 	}
 
 	if (ioctl(efd, PERF_EVENT_IOC_SET_BPF, kp->bfd)) {
 		close(efd);
+		_d("could not set BPF program: %s", strerror(errno));
 		return -errno;
 	}
 
@@ -117,7 +128,7 @@ static kprobe_t *probe_load(enum bpf_prog_type type,
 {
 	kprobe_t *kp;
 
-	kp = malloc(sizeof(*kp));
+	kp = calloc(1, sizeof(*kp));
 	assert(kp);
 
 	kp->efds.fds = calloc(1, sizeof(*kp->efds.fds));
@@ -142,23 +153,31 @@ static kprobe_t *probe_load(enum bpf_prog_type type,
 	return kp;
 }
 
-static int kprobe_teardown(kprobe_t *kp)
+static int probe_teardown_events(kprobe_t *kp)
 {
 	int i;
 
 	for (i = 0; i < kp->efds.len; i++)
 		close(kp->efds.fds[i]);
-
 	free(kp->efds.fds);
-	free(kp);
+
 	return 0;
 }
 
 static int probe_teardown(node_t *probe)
 {
-	return kprobe_teardown(probe->dyn->probe.pvdr_priv);
-}
+	int err;
 
+	kprobe_t *kp = probe->dyn->probe.pvdr_priv;
+
+	err = probe_teardown_events(kp);
+
+	if (kp->ctrl)
+		fclose(kp->ctrl);
+	free(kp);
+
+	return err;
+}
 
 /* TRACEPOINT provider */
 #ifdef LINUX_HAS_TRACEPOINT
@@ -185,10 +204,8 @@ static int trace_load(node_t *probe, prog_t *prog)
 	probe->dyn->probe.pvdr_priv = kp;
 
 	func = strchr(probe->string, ':') + 1;
-	/* if (strchr(func, '?') || strchr(func, '*')) */
-	/* 	return kprobe_attach_pattern(kp, func); */
-	/* else */
-		return trace_attach(kp, func);
+
+	return trace_attach(kp, func);
 }
 
 const module_t *trace_modules[] = {
@@ -219,48 +236,88 @@ pvdr_t trace_pvdr = {
 
 /* KPROBE provider */
 
-static int kprobe_event_id(kprobe_t *kp, const char *func)
+static int kprobe_event_id(kprobe_t *kp, const char *func, long offs)
 {
-	char ev_name[0x100];
+	char ev_name[KPROBE_MAXLEN];
+
+	snprintf(ev_name, sizeof(ev_name), "kprobes/%s_%s_%ld_%d", kp->type,
+		 func, offs, G.self);
+
+	return probe_event_id(kp, ev_name);
+}
+
+/*
+ * Set attach state for function/offset to attached if attach is 1, otherwise
+ * detach.
+ *
+ * In order to make k[ret]probes unique to this instance of ply, we name
+ * the probes [type]_[func]_[offset]_[pid], where
+ *	- type is the type of probe (p for kprobes, r for kretprobes);
+ *	- func and offset are function name and offset
+ *	- pid is process id of this process.
+ *
+ * For example, p_kfree_skb_0_1234 is the kprobe for kfree_skb at offset 0
+ * created by process 1234.
+ *
+ * Making probes unique like this allows us to have multiple k[ret]probes
+ * active for different instances of ply running simultaneously, and it
+ * gives us a way to clean up after ourselves only when done.
+ */
+static int kprobe_setattach(kprobe_t *kp, const char *func_and_offset,
+			    int attach)
+{
+	char func[KPROBE_MAXLEN];
 	const char *offstr;
 	long offs = 0;
 	int funclen;
+	int id, err;
 
-	offstr = strchrnul(func, '+');
+	offstr = strchrnul(func_and_offset, '+');
 	if (*offstr) {
 		offs = strtol(offstr, NULL, 0);
 		if (offs < 0) {
 			_e("unknown offset in probe '%s'", func);
 			return -EINVAL;
 		}
 	}
+	funclen = (int)(offstr - func_and_offset);
+	snprintf(func, funclen+1, "%*.*s", funclen, funclen, func_and_offset);
+
+	assert(kp->ctrl);
+	_d("%s %s+%x", attach ? "attaching to" : "detaching from", func, offs);
+	fseek(kp->ctrl, 0, SEEK_END);
+	if (attach)
+		err = fprintf(kp->ctrl, "%s:%s_%s_%ld_%d %s+%ld\n", kp->type,
+			      kp->type, func, offs, G.self, func, offs);
+	else
+		err = fprintf(kp->ctrl, "-:%s_%s_%ld_%d\n", kp->type,
+			      func, offs, G.self);
+	if (err < 0)
+		err = -errno;
+	else
+		err = 0;
 
-	funclen = (int)(offstr - func);
-
-	fprintf(kp->ctrl, "%s %*.*s+%ld\n", kp->type, funclen, funclen, func, offs);
 	fflush(kp->ctrl);
 
-	snprintf(ev_name, sizeof(ev_name), "kprobes/%s_%*.*s_%ld", kp->type,
-		 funclen, funclen, func, offs);
+	/* If detaching or something went wrong, we're done... */
+	if (!attach || err)
+		return err;
 
-	return probe_event_id(kp, ev_name);
-}
-
-static int kprobe_attach(kprobe_t *kp, const char *func)
-{
-	int id;
-
-	id = kprobe_event_id(kp, func);
+	id = kprobe_event_id(kp, func, offs);
 	if (id < 0)
 		return id;
 
 	return probe_attach(kp, id);
 }
 
-static int kprobe_attach_pattern(kprobe_t *kp, const char *pattern)
+static int kprobe_setattach_pattern(kprobe_t *kp, const char *pattern,
+				    int attach)
 {
 	int i, err;
 
+	if (!strchr(pattern, '?') && !strchr(pattern, '*'))
+		return kprobe_setattach(kp, pattern, attach);
+
 	if (!G.ksyms) {
 		_e("probe wildcards not supported without KALLSYMS support");
 		return -ENOSYS;
@@ -272,9 +329,11 @@ static int kprobe_attach_pattern(kprobe_t *kp, const char *pattern)
 		if (fnmatch(pattern, k->sym, 0))
 			continue;
 
-		err = kprobe_attach(kp, k->sym);
+		err = kprobe_setattach(kp, k->sym, attach);
 		if (err == -EEXIST || err == -ENOENT) {
-			_w("'%s' will not be probed", k->sym);
+			_w("'%s' will not be probed: %s", k->sym,
+			   err == -EEXIST ? "probe already exists" :
+			   "probe not found");
 			err = 0;
 		}
 	}
@@ -299,14 +358,20 @@ static int kprobe_load(node_t *probe, prog_t *prog, const char *probestring,
 		_eno("unable to open kprobe_events");
 		return -errno;
 	}
+
 	*kpp = kp;
 	func = strchr(probestring, ':') + 1;
-	if (strchr(func, '?') || strchr(func, '*'))
-		return kprobe_attach_pattern(kp, func);
-	else
-		return kprobe_attach(kp, func);
+	return kprobe_setattach_pattern(kp, func, 1);
 }
 
+static int kprobe_detach(kprobe_t *kp, const char *probestring)
+{
+	char *func;
+
+	func = strchr(probestring, ':') + 1;
+
+	return kprobe_setattach_pattern(kp, func, 0);
+}
 
 const module_t *kprobe_modules[] = {
 	&kprobe_module,
@@ -355,14 +420,42 @@ static int kprobe_setup(node_t *probe, prog_t *prog)
 			   (kprobe_t **)&probe->dyn->probe.pvdr_priv);
 }
 
+static int kprobe_destroy(kprobe_t *kp, const char *pattern)
+{
+	int err1, err2;
+
+	/* preserve first error we hit but drive on... */
+	err1 = probe_teardown_events(kp);
+
+	/*
+	 * we need to detach after closing all event fds relating to probe,
+	 * otherwise we cannot remove the associated event from
+	 * /sys/kernel/debug/tracing/kprobe_events.
+	 */
+	err2 = kprobe_detach(kp, pattern);
+
+	fclose(kp->ctrl);
+
+	free(kp);
+
+	return err1 ? err1 : err2;
+}
+
+static int kprobe_teardown(node_t *probe)
+{
+	kprobe_t *kp = probe->dyn->probe.pvdr_priv;
+
+	return kprobe_destroy(kp, probe->string);
+}
+
 pvdr_t kprobe_pvdr = {
 	.name = "kprobe",
 
 	.dflt    = kprobe_default,
 	.resolve = kprobe_resolve,
 
 	.setup      = kprobe_setup,
-	.teardown   = probe_teardown,
+	.teardown   = kprobe_teardown,
 };
 
 
@@ -420,7 +513,7 @@ pvdr_t kretprobe_pvdr = {
 	.resolve = kretprobe_resolve,
 
 	.setup      = kretprobe_setup,
-	.teardown   = probe_teardown,
+	.teardown   = kprobe_teardown,
 };
 
 /* PROFILE provider */
@@ -438,7 +531,7 @@ static void profile_destroy(profile_t *profile)
 		return;
 
 	if (profile->kp)
-		(void) kprobe_teardown(profile->kp);
+		kprobe_destroy(profile->kp, "kprobe:perf_swevent_hrtimer");
 	for (i = 0; i < profile->num; i++) {
 		if (profile->efds[i] > 0)
 			close(profile->efds[i]);