* [PATCH bpf] libbpf: unmap rings when umem deleted
@ 2022-03-01 13:26 lic121
2022-03-02 7:29 ` Magnus Karlsson
2022-03-08 6:03 ` Andrii Nakryiko
0 siblings, 2 replies; 6+ messages in thread
From: lic121 @ 2022-03-01 13:26 UTC (permalink / raw)
To: bpf
Cc: Björn Töpel, Magnus Karlsson, Jonathan Lemon,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, David S. Miller, Jakub Kicinski,
Jesper Dangaard Brouer, netdev, linux-kernel, lic121
xsk_umem__create() does mmap for fill/comp rings, but xsk_umem__delete()
doesn't do the unmap. This works fine for regular cases, because
xsk_socket__delete() does unmap for the rings. But for the case that
xsk_socket__create_shared() fails, umem rings are not unmapped.
fill_save/comp_save are checked to determine if rings have already be
unmapped by xsk. If fill_save and comp_save are NULL, it means that the
rings have already been used by xsk. Then they are supposed to be
unmapped by xsk_socket__delete(). Otherwise, xsk_umem__delete() does the
unmap.
Fixes: 2f6324a3937f ("libbpf: Support shared umems between queues and devices")
Signed-off-by: lic121 <lic121@chinatelecom.cn>
---
tools/lib/bpf/xsk.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index edafe56..32a2f57 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -1193,12 +1193,23 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
int xsk_umem__delete(struct xsk_umem *umem)
{
+ struct xdp_mmap_offsets off;
+ int err;
+
if (!umem)
return 0;
if (umem->refcount)
return -EBUSY;
+ err = xsk_get_mmap_offsets(umem->fd, &off);
+ if (!err && umem->fill_save && umem->comp_save) {
+ munmap(umem->fill_save->ring - off.fr.desc,
+ off.fr.desc + umem->config.fill_size * sizeof(__u64));
+ munmap(umem->comp_save->ring - off.cr.desc,
+ off.cr.desc + umem->config.comp_size * sizeof(__u64));
+ }
+
close(umem->fd);
free(umem);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] libbpf: unmap rings when umem deleted
2022-03-01 13:26 [PATCH bpf] libbpf: unmap rings when umem deleted lic121
@ 2022-03-02 7:29 ` Magnus Karlsson
2022-03-02 8:48 ` Björn Töpel
2022-03-08 6:03 ` Andrii Nakryiko
1 sibling, 1 reply; 6+ messages in thread
From: Magnus Karlsson @ 2022-03-02 7:29 UTC (permalink / raw)
To: lic121
Cc: bpf, Björn Töpel, Magnus Karlsson, Jonathan Lemon,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, David S. Miller, Jakub Kicinski,
Jesper Dangaard Brouer, Network Development, open list
On Tue, Mar 1, 2022 at 6:57 PM lic121 <lic121@chinatelecom.cn> wrote:
>
> xsk_umem__create() does mmap for fill/comp rings, but xsk_umem__delete()
> doesn't do the unmap. This works fine for regular cases, because
> xsk_socket__delete() does unmap for the rings. But for the case that
> xsk_socket__create_shared() fails, umem rings are not unmapped.
>
> fill_save/comp_save are checked to determine if rings have already be
> unmapped by xsk. If fill_save and comp_save are NULL, it means that the
> rings have already been used by xsk. Then they are supposed to be
> unmapped by xsk_socket__delete(). Otherwise, xsk_umem__delete() does the
> unmap.
Thanks for the fix. Please note that the AF_XDP support in libbpf has
been deprecated and moved to libxdp
(https://github.com/xdp-project/xdp-tools). The code will be
completely removed in the libbpf 1.0 release. Could I take your patch
and apply it to libxdp instead and fix the bug there? I have not
checked, but it is likely present there as well. And that is the code
base we will be using going forward.
> Fixes: 2f6324a3937f ("libbpf: Support shared umems between queues and devices")
> Signed-off-by: lic121 <lic121@chinatelecom.cn>
> ---
> tools/lib/bpf/xsk.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index edafe56..32a2f57 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -1193,12 +1193,23 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
>
> int xsk_umem__delete(struct xsk_umem *umem)
> {
> + struct xdp_mmap_offsets off;
> + int err;
> +
> if (!umem)
> return 0;
>
> if (umem->refcount)
> return -EBUSY;
>
> + err = xsk_get_mmap_offsets(umem->fd, &off);
> + if (!err && umem->fill_save && umem->comp_save) {
> + munmap(umem->fill_save->ring - off.fr.desc,
> + off.fr.desc + umem->config.fill_size * sizeof(__u64));
> + munmap(umem->comp_save->ring - off.cr.desc,
> + off.cr.desc + umem->config.comp_size * sizeof(__u64));
> + }
> +
> close(umem->fd);
> free(umem);
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] libbpf: unmap rings when umem deleted
2022-03-02 7:29 ` Magnus Karlsson
@ 2022-03-02 8:48 ` Björn Töpel
2022-03-02 9:06 ` lic121
0 siblings, 1 reply; 6+ messages in thread
From: Björn Töpel @ 2022-03-02 8:48 UTC (permalink / raw)
To: Magnus Karlsson
Cc: lic121, bpf, Magnus Karlsson, Jonathan Lemon, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, Network Development,
open list
On Wed, 2 Mar 2022 at 08:29, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> On Tue, Mar 1, 2022 at 6:57 PM lic121 <lic121@chinatelecom.cn> wrote:
[...]
> > Signed-off-by: lic121 <lic121@chinatelecom.cn>
In addition to Magnus' comments; Please use your full name, as
outlined in Documentation/process/5.Posting.rst.
Cheers!
Björn
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] libbpf: unmap rings when umem deleted
2022-03-02 8:48 ` Björn Töpel
@ 2022-03-02 9:06 ` lic121
2022-03-02 9:08 ` Magnus Karlsson
0 siblings, 1 reply; 6+ messages in thread
From: lic121 @ 2022-03-02 9:06 UTC (permalink / raw)
To: Björn Töpel
Cc: Magnus Karlsson, bpf, Magnus Karlsson, Jonathan Lemon,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, David S. Miller, Jakub Kicinski,
Jesper Dangaard Brouer, Network Development, open list
On Wed, Mar 02, 2022 at 09:48:33AM +0100, Björn Töpel wrote:
> On Wed, 2 Mar 2022 at 08:29, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> > On Tue, Mar 1, 2022 at 6:57 PM lic121 <lic121@chinatelecom.cn> wrote:
> [...]
> > > Signed-off-by: lic121 <lic121@chinatelecom.cn>
>
> In addition to Magnus' comments; Please use your full name, as
> outlined in Documentation/process/5.Posting.rst.
Thanks for the review Björn.
Magnus, please let me know if you can correct the full name when you
apply the patch? Otherwise I can create a MR in libxdp repo. Thanks
full name: Cheng Li
>
> Cheers!
> Björn
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] libbpf: unmap rings when umem deleted
2022-03-02 9:06 ` lic121
@ 2022-03-02 9:08 ` Magnus Karlsson
0 siblings, 0 replies; 6+ messages in thread
From: Magnus Karlsson @ 2022-03-02 9:08 UTC (permalink / raw)
To: lic121
Cc: Björn Töpel, bpf, Magnus Karlsson, Jonathan Lemon,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, David S. Miller, Jakub Kicinski,
Jesper Dangaard Brouer, Network Development, open list
On Wed, Mar 2, 2022 at 10:06 AM lic121 <lic121@chinatelecom.cn> wrote:
>
> On Wed, Mar 02, 2022 at 09:48:33AM +0100, Björn Töpel wrote:
> > On Wed, 2 Mar 2022 at 08:29, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> > > On Tue, Mar 1, 2022 at 6:57 PM lic121 <lic121@chinatelecom.cn> wrote:
> > [...]
> > > > Signed-off-by: lic121 <lic121@chinatelecom.cn>
> >
> > In addition to Magnus' comments; Please use your full name, as
> > outlined in Documentation/process/5.Posting.rst.
>
> Thanks for the review Björn.
> Magnus, please let me know if you can correct the full name when you
> apply the patch? Otherwise I can create a MR in libxdp repo. Thanks
I will add it.
Thanks: Magnus
> full name: Cheng Li
>
> >
> > Cheers!
> > Björn
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] libbpf: unmap rings when umem deleted
2022-03-01 13:26 [PATCH bpf] libbpf: unmap rings when umem deleted lic121
2022-03-02 7:29 ` Magnus Karlsson
@ 2022-03-08 6:03 ` Andrii Nakryiko
1 sibling, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2022-03-08 6:03 UTC (permalink / raw)
To: lic121
Cc: bpf, Björn Töpel, Magnus Karlsson, Jonathan Lemon,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, David S. Miller, Jakub Kicinski,
Jesper Dangaard Brouer, Networking, open list
On Tue, Mar 1, 2022 at 5:26 AM lic121 <lic121@chinatelecom.cn> wrote:
>
> xsk_umem__create() does mmap for fill/comp rings, but xsk_umem__delete()
> doesn't do the unmap. This works fine for regular cases, because
> xsk_socket__delete() does unmap for the rings. But for the case that
> xsk_socket__create_shared() fails, umem rings are not unmapped.
>
> fill_save/comp_save are checked to determine if rings have already be
> unmapped by xsk. If fill_save and comp_save are NULL, it means that the
> rings have already been used by xsk. Then they are supposed to be
> unmapped by xsk_socket__delete(). Otherwise, xsk_umem__delete() does the
> unmap.
>
> Fixes: 2f6324a3937f ("libbpf: Support shared umems between queues and devices")
> Signed-off-by: lic121 <lic121@chinatelecom.cn>
> ---
Applied to bpf-next as well. Changed the name to Cheng Li while
applying. Thanks.
> tools/lib/bpf/xsk.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index edafe56..32a2f57 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -1193,12 +1193,23 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
>
> int xsk_umem__delete(struct xsk_umem *umem)
> {
> + struct xdp_mmap_offsets off;
> + int err;
> +
> if (!umem)
> return 0;
>
> if (umem->refcount)
> return -EBUSY;
>
> + err = xsk_get_mmap_offsets(umem->fd, &off);
> + if (!err && umem->fill_save && umem->comp_save) {
> + munmap(umem->fill_save->ring - off.fr.desc,
> + off.fr.desc + umem->config.fill_size * sizeof(__u64));
> + munmap(umem->comp_save->ring - off.cr.desc,
> + off.cr.desc + umem->config.comp_size * sizeof(__u64));
> + }
> +
> close(umem->fd);
> free(umem);
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-08 6:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-01 13:26 [PATCH bpf] libbpf: unmap rings when umem deleted lic121
2022-03-02 7:29 ` Magnus Karlsson
2022-03-02 8:48 ` Björn Töpel
2022-03-02 9:06 ` lic121
2022-03-02 9:08 ` Magnus Karlsson
2022-03-08 6:03 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).