| Message ID | 20201009160353.1529-1-danieltimlee@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | samples: bpf: Refactor XDP programs with libbpf | expand |
On Fri, Oct 9, 2020 at 9:04 AM Daniel T. Lee <danieltimlee@gmail.com> wrote: > > To avoid confusion caused by the increasing fragmentation of the BPF > Loader program, this commit would like to convert the previous bpf_load > loader with the libbpf loader. > > Thanks to libbpf's bpf_link interface, managing the tracepoint BPF > program is much easier. bpf_program__attach_tracepoint manages the > enable of tracepoint event and attach of BPF programs to it with a > single interface bpf_link, so there is no need to manage event_fd and > prog_fd separately. > > And due to addition of generic bpf_program__attach() to libbpf, it is > now possible to attach BPF programs with __attach() instead of > explicitly calling __attach_<type>(). > > This patchset refactors xdp_monitor with using this libbpf API, and the > bpf_load is removed and migrated to libbpf. Also, attach_tracepoint() > is replaced with the generic __attach() method in xdp_redirect_cpu. > Moreover, maps in kern program have been converted to BTF-defined map. > > Daniel T. Lee (3): > samples: bpf: Refactor xdp_monitor with libbpf > samples: bpf: Replace attach_tracepoint() to attach() in > xdp_redirect_cpu > samples: bpf: refactor XDP kern program maps with BTF-defined map > > samples/bpf/Makefile | 4 +- > samples/bpf/xdp_monitor_kern.c | 60 ++++++------ > samples/bpf/xdp_monitor_user.c | 144 +++++++++++++++++++++------- > samples/bpf/xdp_redirect_cpu_user.c | 138 +++++++++++++------------- > samples/bpf/xdp_sample_pkts_kern.c | 14 ++- > samples/bpf/xdp_sample_pkts_user.c | 1 - > 6 files changed, 211 insertions(+), 150 deletions(-) > > -- > 2.25.1 > Thanks for this clean up, Daniel! It's great! I left a few nits here and there in the appropriate patches. There still seem to be a bunch of users of bpf_load.c, which would be nice to get rid of completely. But before you go do that, consider integrating BPF skeleton into samples/bpf Makefile. That way instead of all those look ups of maps/programs by name, you'd be writing a straightforward skel->maps.my_map and similar short and non-failing code. This should make the overall time spent on conversion much smaller (and more pleasant, IMO). You've dealt with a lot of samples/bpf reworking, so it should be too hard for you to figure out the best way to do this, but check selftests/bpf's Makefile, if you need some ideas. Or just ask for help. Thanks!
On Sat, Oct 10, 2020 at 3:30 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Fri, Oct 9, 2020 at 9:04 AM Daniel T. Lee <danieltimlee@gmail.com> wrote: > > > > To avoid confusion caused by the increasing fragmentation of the BPF > > Loader program, this commit would like to convert the previous bpf_load > > loader with the libbpf loader. > > > > Thanks to libbpf's bpf_link interface, managing the tracepoint BPF > > program is much easier. bpf_program__attach_tracepoint manages the > > enable of tracepoint event and attach of BPF programs to it with a > > single interface bpf_link, so there is no need to manage event_fd and > > prog_fd separately. > > > > And due to addition of generic bpf_program__attach() to libbpf, it is > > now possible to attach BPF programs with __attach() instead of > > explicitly calling __attach_<type>(). > > > > This patchset refactors xdp_monitor with using this libbpf API, and the > > bpf_load is removed and migrated to libbpf. Also, attach_tracepoint() > > is replaced with the generic __attach() method in xdp_redirect_cpu. > > Moreover, maps in kern program have been converted to BTF-defined map. > > > > Daniel T. Lee (3): > > samples: bpf: Refactor xdp_monitor with libbpf > > samples: bpf: Replace attach_tracepoint() to attach() in > > xdp_redirect_cpu > > samples: bpf: refactor XDP kern program maps with BTF-defined map > > > > samples/bpf/Makefile | 4 +- > > samples/bpf/xdp_monitor_kern.c | 60 ++++++------ > > samples/bpf/xdp_monitor_user.c | 144 +++++++++++++++++++++------- > > samples/bpf/xdp_redirect_cpu_user.c | 138 +++++++++++++------------- > > samples/bpf/xdp_sample_pkts_kern.c | 14 ++- > > samples/bpf/xdp_sample_pkts_user.c | 1 - > > 6 files changed, 211 insertions(+), 150 deletions(-) > > > > -- > > 2.25.1 > > > > Thanks for this clean up, Daniel! It's great! I left a few nits here > and there in the appropriate patches. > > There still seem to be a bunch of users of bpf_load.c, which would be > nice to get rid of completely. But before you go do that, consider > integrating BPF skeleton into samples/bpf Makefile. That way instead > of all those look ups of maps/programs by name, you'd be writing a > straightforward skel->maps.my_map and similar short and non-failing > code. This should make the overall time spent on conversion much > smaller (and more pleasant, IMO). > > You've dealt with a lot of samples/bpf reworking, so it should be too > hard for you to figure out the best way to do this, but check > selftests/bpf's Makefile, if you need some ideas. Or just ask for > help. Thanks! Thanks for the great feedback! Thank you for letting me know about the BPF features that I can apply. Currently, I'm not familiar with the BPF skeleton yet, but I'll take a good look at the BPF skeleton to apply it in a more advanced form. Thank you for your time and effort for the review.
To avoid confusion caused by the increasing fragmentation of the BPF Loader program, this commit would like to convert the previous bpf_load loader with the libbpf loader. Thanks to libbpf's bpf_link interface, managing the tracepoint BPF program is much easier. bpf_program__attach_tracepoint manages the enable of tracepoint event and attach of BPF programs to it with a single interface bpf_link, so there is no need to manage event_fd and prog_fd separately. And due to addition of generic bpf_program__attach() to libbpf, it is now possible to attach BPF programs with __attach() instead of explicitly calling __attach_<type>(). This patchset refactors xdp_monitor with using this libbpf API, and the bpf_load is removed and migrated to libbpf. Also, attach_tracepoint() is replaced with the generic __attach() method in xdp_redirect_cpu. Moreover, maps in kern program have been converted to BTF-defined map. Daniel T. Lee (3): samples: bpf: Refactor xdp_monitor with libbpf samples: bpf: Replace attach_tracepoint() to attach() in xdp_redirect_cpu samples: bpf: refactor XDP kern program maps with BTF-defined map samples/bpf/Makefile | 4 +- samples/bpf/xdp_monitor_kern.c | 60 ++++++------ samples/bpf/xdp_monitor_user.c | 144 +++++++++++++++++++++------- samples/bpf/xdp_redirect_cpu_user.c | 138 +++++++++++++------------- samples/bpf/xdp_sample_pkts_kern.c | 14 ++- samples/bpf/xdp_sample_pkts_user.c | 1 - 6 files changed, 211 insertions(+), 150 deletions(-)