| Message ID | 20210330113419.4616-4-ciara.loftus@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | AF_XDP Socket Creation Fixes | 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 |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | fail | 1 blamed authors not CCed: daniel@iogearbox.net; 9 maintainers not CCed: jonathan.lemon@gmail.com yhs@fb.com kpsingh@kernel.org daniel@iogearbox.net andrii@kernel.org kafai@fb.com ast@kernel.org john.fastabend@gmail.com songliubraving@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 | fail | Link |
| netdev/checkpatch | warning | WARNING: line length of 81 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Tue, 30 Mar 2021 at 14:05, Ciara Loftus <ciara.loftus@intel.com> wrote: > > Prior to this commit xsk_socket__create(_shared) always attempted to create > the rx and tx rings for the socket. However this causes an issue when the > socket being setup is that which shares the fd with the UMEM. If a > previous call to this function failed with this socket after the rings were > set up, a subsequent call would always fail because the rings are not torn > down after the first call and when we try to set them up again we encounter > an error because they already exist. Solve this by remembering whether the > rings were set up by introducing a new flag to struct xsk_umem, and > checking it before setting up the rings for sockets which share the fd > with the UMEM. > > Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets") > > Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> > --- > tools/lib/bpf/xsk.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c > index d4991ddff05a..12110bba4cc0 100644 > --- a/tools/lib/bpf/xsk.c > +++ b/tools/lib/bpf/xsk.c > @@ -14,6 +14,7 @@ > #include <unistd.h> > #include <arpa/inet.h> > #include <asm/barrier.h> > +#include <linux/bitops.h> > #include <linux/compiler.h> > #include <linux/ethtool.h> > #include <linux/filter.h> > @@ -46,6 +47,9 @@ > #define PF_XDP AF_XDP > #endif > > +#define XDP_RX_RING_SETUP_DONE BIT(0) > +#define XDP_TX_RING_SETUP_DONE BIT(1) > + > enum xsk_prog { > XSK_PROG_FALLBACK, > XSK_PROG_REDIRECT_FLAGS, > @@ -59,6 +63,7 @@ struct xsk_umem { > int fd; > int refcount; > struct list_head ctx_list; > + __u8 ring_setup_status; Are we envisioning any more flags here? Otherwise, just a simple bool/__u8 stating ring_setup_complete or ring_is_setup and just use true/false and a simple check w/o bitwise and. Björn
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c index d4991ddff05a..12110bba4cc0 100644 --- a/tools/lib/bpf/xsk.c +++ b/tools/lib/bpf/xsk.c @@ -14,6 +14,7 @@ #include <unistd.h> #include <arpa/inet.h> #include <asm/barrier.h> +#include <linux/bitops.h> #include <linux/compiler.h> #include <linux/ethtool.h> #include <linux/filter.h> @@ -46,6 +47,9 @@ #define PF_XDP AF_XDP #endif +#define XDP_RX_RING_SETUP_DONE BIT(0) +#define XDP_TX_RING_SETUP_DONE BIT(1) + enum xsk_prog { XSK_PROG_FALLBACK, XSK_PROG_REDIRECT_FLAGS, @@ -59,6 +63,7 @@ struct xsk_umem { int fd; int refcount; struct list_head ctx_list; + __u8 ring_setup_status; }; struct xsk_ctx { @@ -855,6 +860,7 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr, struct xsk_ctx *ctx; int err, ifindex; bool unmap = umem->fill_save != fill; + bool rx_setup_done = false, tx_setup_done = false; if (!umem || !xsk_ptr || !(rx || tx)) return -EFAULT; @@ -882,6 +888,8 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr, } } else { xsk->fd = umem->fd; + rx_setup_done = umem->ring_setup_status & XDP_RX_RING_SETUP_DONE; + tx_setup_done = umem->ring_setup_status & XDP_TX_RING_SETUP_DONE; } ctx = xsk_get_ctx(umem, ifindex, queue_id); @@ -900,7 +908,7 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr, } xsk->ctx = ctx; - if (rx) { + if (rx && !rx_setup_done) { err = setsockopt(xsk->fd, SOL_XDP, XDP_RX_RING, &xsk->config.rx_size, sizeof(xsk->config.rx_size)); @@ -908,8 +916,10 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr, err = -errno; goto out_put_ctx; } + if (xsk->fd == umem->fd) + umem->ring_setup_status |= XDP_RX_RING_SETUP_DONE; } - if (tx) { + if (tx && !tx_setup_done) { err = setsockopt(xsk->fd, SOL_XDP, XDP_TX_RING, &xsk->config.tx_size, sizeof(xsk->config.tx_size)); @@ -917,6 +927,8 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr, err = -errno; goto out_put_ctx; } + if (xsk->fd == umem->fd) + umem->ring_setup_status |= XDP_TX_RING_SETUP_DONE; } err = xsk_get_mmap_offsets(xsk->fd, &off);
Prior to this commit xsk_socket__create(_shared) always attempted to create the rx and tx rings for the socket. However this causes an issue when the socket being setup is that which shares the fd with the UMEM. If a previous call to this function failed with this socket after the rings were set up, a subsequent call would always fail because the rings are not torn down after the first call and when we try to set them up again we encounter an error because they already exist. Solve this by remembering whether the rings were set up by introducing a new flag to struct xsk_umem, and checking it before setting up the rings for sockets which share the fd with the UMEM. Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets") Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> --- tools/lib/bpf/xsk.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)