| Message ID | 20210215154638.4627-4-maciej.fijalkowski@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Introduce bpf_link in libbpf's xsk | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 10 maintainers not CCed: jonathan.lemon@gmail.com kuba@kernel.org hawk@kernel.org yhs@fb.com bjorn@kernel.org davem@davemloft.net john.fastabend@gmail.com kpsingh@kernel.org songliubraving@fb.com kafai@fb.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 85 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
Maciej Fijalkowski wrote: > With the introduction of bpf_link in xsk's libbpf part, there's no > further need for explicit unload of prog on xdpsock's termination. When > process dies, the bpf_link's refcount will be decremented and resources > will be unloaded/freed under the hood in case when there are no more > active users. > > While at it, don't dump stats on error path. > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > --- Can we also use it from selftests prog so we have a test that is run there using this? Looks like xdpxceiver.c could do the link step as well?
On 2021-02-15 21:24, John Fastabend wrote: > Maciej Fijalkowski wrote: >> With the introduction of bpf_link in xsk's libbpf part, there's no >> further need for explicit unload of prog on xdpsock's termination. When >> process dies, the bpf_link's refcount will be decremented and resources >> will be unloaded/freed under the hood in case when there are no more >> active users. >> >> While at it, don't dump stats on error path. >> >> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> >> --- > > Can we also use it from selftests prog so we have a test that is run there > using this? Looks like xdpxceiver.c could do the link step as well? > Yes! Good point! Björn
On Tue, Feb 16, 2021 at 10:22:15AM +0100, Björn Töpel wrote: > On 2021-02-15 21:24, John Fastabend wrote: > > Maciej Fijalkowski wrote: > > > With the introduction of bpf_link in xsk's libbpf part, there's no > > > further need for explicit unload of prog on xdpsock's termination. When > > > process dies, the bpf_link's refcount will be decremented and resources > > > will be unloaded/freed under the hood in case when there are no more > > > active users. > > > > > > While at it, don't dump stats on error path. > > > > > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > > --- > > > > Can we also use it from selftests prog so we have a test that is run there > > using this? Looks like xdpxceiver.c could do the link step as well? > > > > Yes! Good point! Somehow John's mail didn't end up in my inbox. xdpxceiver is using libbpf API for socket handling (create/delete), so with that set included it is working on bpf_link. > > > Björn
diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c index db0cb73513a5..96246313e342 100644 --- a/samples/bpf/xdpsock_user.c +++ b/samples/bpf/xdpsock_user.c @@ -96,7 +96,6 @@ static int opt_xsk_frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE; static int opt_timeout = 1000; static bool opt_need_wakeup = true; static u32 opt_num_xsks = 1; -static u32 prog_id; static bool opt_busy_poll; static bool opt_reduced_cap; @@ -462,59 +461,37 @@ static void *poller(void *arg) return NULL; } -static void remove_xdp_program(void) +static void int_exit(int sig) { - u32 curr_prog_id = 0; - int cmd = CLOSE_CONN; - - if (bpf_get_link_xdp_id(opt_ifindex, &curr_prog_id, opt_xdp_flags)) { - printf("bpf_get_link_xdp_id failed\n"); - exit(EXIT_FAILURE); - } - if (prog_id == curr_prog_id) - bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags); - else if (!curr_prog_id) - printf("couldn't find a prog id on a given interface\n"); - else - printf("program on interface changed, not removing\n"); - - if (opt_reduced_cap) { - if (write(sock, &cmd, sizeof(int)) < 0) { - fprintf(stderr, "Error writing into stream socket: %s", strerror(errno)); - exit(EXIT_FAILURE); - } - } + benchmark_done = true; } -static void int_exit(int sig) +static void __exit_with_error(int error, const char *file, const char *func, + int line) { - benchmark_done = true; + fprintf(stderr, "%s:%s:%i: errno: %d/\"%s\"\n", file, func, + line, error, strerror(error)); + exit(EXIT_FAILURE); } +#define exit_with_error(error) __exit_with_error(error, __FILE__, __func__, __LINE__) + static void xdpsock_cleanup(void) { struct xsk_umem *umem = xsks[0]->umem->umem; - int i; + int i, cmd = CLOSE_CONN; dump_stats(); for (i = 0; i < num_socks; i++) xsk_socket__delete(xsks[i]->xsk); (void)xsk_umem__delete(umem); - remove_xdp_program(); -} -static void __exit_with_error(int error, const char *file, const char *func, - int line) -{ - fprintf(stderr, "%s:%s:%i: errno: %d/\"%s\"\n", file, func, - line, error, strerror(error)); - dump_stats(); - remove_xdp_program(); - exit(EXIT_FAILURE); + if (opt_reduced_cap) { + if (write(sock, &cmd, sizeof(int)) < 0) + exit_with_error(errno); + } } -#define exit_with_error(error) __exit_with_error(error, __FILE__, __func__, \ - __LINE__) static void swap_mac_addresses(void *data) { struct ether_header *eth = (struct ether_header *)data; @@ -880,10 +857,6 @@ static struct xsk_socket_info *xsk_configure_socket(struct xsk_umem_info *umem, if (ret) exit_with_error(-ret); - ret = bpf_get_link_xdp_id(opt_ifindex, &prog_id, opt_xdp_flags); - if (ret) - exit_with_error(-ret); - xsk->app_stats.rx_empty_polls = 0; xsk->app_stats.fill_fail_polls = 0; xsk->app_stats.copy_tx_sendtos = 0;
With the introduction of bpf_link in xsk's libbpf part, there's no further need for explicit unload of prog on xdpsock's termination. When process dies, the bpf_link's refcount will be decremented and resources will be unloaded/freed under the hood in case when there are no more active users. While at it, don't dump stats on error path. Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> --- samples/bpf/xdpsock_user.c | 55 ++++++++++---------------------------- 1 file changed, 14 insertions(+), 41 deletions(-)