Daniel T. Lee wrote: > The bpf_program__attach of libbpf(using bpf_link) is much more intuitive > than the previous method using ioctl. > > bpf_program__attach_perf_event manages the enable of perf_event and > attach of BPF programs to it, so there's no neeed to do this > directly with ioctl. > > In addition, bpf_link provides consistency in the use of API because it > allows disable (detach, destroy) for multiple events to be treated as > one bpf_link__destroy. > > This commit refactors samples that attach the bpf program to perf_event > by using libbbpf instead of ioctl. Also the bpf_load in the samples were > removed and migrated to use libbbpf API. > > Signed-off-by: Daniel T. Lee <danieltimlee@xxxxxxxxx> > --- [...] > > int main(int argc, char **argv) > { > + int prog_fd, *pmu_fd, opt, freq = DEFAULT_FREQ, secs = DEFAULT_SECS; > + struct bpf_program *prog; > + struct bpf_object *obj; > + struct bpf_link **link; > char filename[256]; > - int *pmu_fd, opt, freq = DEFAULT_FREQ, secs = DEFAULT_SECS; > > /* process arguments */ > while ((opt = getopt(argc, argv, "F:h")) != -1) { > @@ -165,36 +170,47 @@ int main(int argc, char **argv) > /* create perf FDs for each CPU */ > nr_cpus = sysconf(_SC_NPROCESSORS_CONF); > pmu_fd = malloc(nr_cpus * sizeof(int)); > - if (pmu_fd == NULL) { > - fprintf(stderr, "ERROR: malloc of pmu_fd\n"); > + link = malloc(nr_cpus * sizeof(struct bpf_link *)); > + if (pmu_fd == NULL || link == NULL) { > + fprintf(stderr, "ERROR: malloc of pmu_fd/link\n"); > return 1; > } > > /* load BPF program */ > snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]); > - if (load_bpf_file(filename)) { > + if (bpf_prog_load(filename, BPF_PROG_TYPE_PERF_EVENT, &obj, &prog_fd)) { > fprintf(stderr, "ERROR: loading BPF program (errno %d):\n", > errno); > - if (strcmp(bpf_log_buf, "") == 0) > - fprintf(stderr, "Try: ulimit -l unlimited\n"); > - else > - fprintf(stderr, "%s", bpf_log_buf); > return 1; > } > + > + prog = bpf_program__next(NULL, obj); > + if (!prog) { > + printf("finding a prog in obj file failed\n"); > + return 1; > + } > + > + map_fd = bpf_object__find_map_fd_by_name(obj, "ip_map"); > + if (map_fd < 0) { > + printf("finding a ip_map map in obj file failed\n"); > + return 1; > + } > + > signal(SIGINT, int_exit); > signal(SIGTERM, int_exit); > > /* do sampling */ > printf("Sampling at %d Hertz for %d seconds. Ctrl-C also ends.\n", > freq, secs); > - if (sampling_start(pmu_fd, freq) != 0) > + if (sampling_start(pmu_fd, freq, prog, link) != 0) > return 1; > sleep(secs); > - sampling_end(pmu_fd); > + sampling_end(link); > free(pmu_fd); > + free(link); Not really a problem with this patch but on error we don't free() memory but then on normal exit there is a free() its a bit inconsistent. How about adding free on errors as well? > > /* output sample counts */ > - print_ip_map(map_fd[0]); > + print_ip_map(map_fd); > > return 0; > } [...] > static void print_ksym(__u64 addr) > @@ -137,6 +136,7 @@ static inline int generate_load(void) > static void test_perf_event_all_cpu(struct perf_event_attr *attr) > { > int nr_cpus = sysconf(_SC_NPROCESSORS_CONF); > + struct bpf_link **link = malloc(nr_cpus * sizeof(struct bpf_link *)); need to check if its null? Its not going to be very friendly to segfault later. Or maybe I'm missing the check. > int *pmu_fd = malloc(nr_cpus * sizeof(int)); > int i, error = 0; > > @@ -151,8 +151,12 @@ static void test_perf_event_all_cpu(struct perf_event_attr *attr) > error = 1; > goto all_cpu_err; > } > - assert(ioctl(pmu_fd[i], PERF_EVENT_IOC_SET_BPF, prog_fd[0]) == 0); > - assert(ioctl(pmu_fd[i], PERF_EVENT_IOC_ENABLE) == 0); > + link[i] = bpf_program__attach_perf_event(prog, pmu_fd[i]); > + if (link[i] < 0) { > + printf("bpf_program__attach_perf_event failed\n"); > + error = 1; > + goto all_cpu_err; > + } > } > > if (generate_load() < 0) { > @@ -161,11 +165,11 @@ static void test_perf_event_all_cpu(struct perf_event_attr *attr) > } > print_stacks(); > all_cpu_err: > - for (i--; i >= 0; i--) { > - ioctl(pmu_fd[i], PERF_EVENT_IOC_DISABLE); > - close(pmu_fd[i]); > - } > + for (i--; i >= 0; i--) > + bpf_link__destroy(link[i]); > + > free(pmu_fd); > + free(link); > if (error) > int_exit(0); > } Thanks, John