AF_XDP Socket Creation Fixes

If the call to xsk_socket__create fails, the user may want to retry the socket creation using the same umem. Ensure that the umem is in the same state on exit if the call fails by:
1. ensuring the umem _save pointers are unmodified.
2. not unmapping the set of umem rings that were set up with the umem during xsk_umem__create, since those maps existed before the call to xsk_socket__create and should remain in tact even in the event of failure.

Fixes 2f6324a3937f ("libbpf: Support shared umems between queues and devices")

Signed-off-by Ciara Loftus <[email protected]>

---
 tools/lib/bpf/xsk.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

tools/lib/bpf/xsk.c +1 -9

index 443b0cfb45e8..d4991ddff05a 100644
@@ -743,21 +743,23 @@ static struct xsk_ctx *xsk_get_ctx(struct xsk_umem *umem, int ifindex,
        return NULL;
 }
 
-static void xsk_put_ctx(struct xsk_ctx *ctx)
+static void xsk_put_ctx(struct xsk_ctx *ctx, bool unmap)
 {
        struct xsk_umem *umem = ctx->umem;
        struct xdp_mmap_offsets off;
        int err;
 
        if (--ctx->refcount == 0) {
-               err = xsk_get_mmap_offsets(umem->fd, &off);
-               if (!err) {
-                       munmap(ctx->fill->ring - off.fr.desc,
-                              off.fr.desc + umem->config.fill_size *
-                              sizeof(__u64));
-                       munmap(ctx->comp->ring - off.cr.desc,
-                              off.cr.desc + umem->config.comp_size *
-                              sizeof(__u64));

tools/lib/bpf/xsk.c +5 -5

                }
 
                list_del(&ctx->list);

@@ -797,8 +799,6 @@ static struct xsk_ctx *xsk_create_ctx(struct xsk_socket *xsk,
        memcpy(ctx->ifname, ifname, IFNAMSIZ - 1);
        ctx->ifname[IFNAMSIZ - 1] = '\0';
 
-       umem->fill_save = NULL;
-       umem->comp_save = NULL;
        ctx->fill = fill;
        ctx->comp = comp;
        list_add(&ctx->list, &umem->ctx_list);

@@ -854,6 +854,7 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
        struct xsk_socket *xsk;
        struct xsk_ctx *ctx;
        int err, ifindex;
+       bool unmap = umem->fill_save != fill;
 
        if (!umem || !xsk_ptr || !(rx || tx))
                return -EFAULT;

@@ -994,6 +995,8 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
        }
 
        *xsk_ptr = xsk;
+       umem->fill_save = NULL;
+       umem->comp_save = NULL;
        return 0;
 
 out_mmap_tx:

@@ -1005,7 +1008,7 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
                munmap(rx_map, off.rx.desc +
                       xsk->config.rx_size * sizeof(struct xdp_desc));
 out_put_ctx:
-       xsk_put_ctx(ctx);
+       xsk_put_ctx(ctx, unmap);
 out_socket:
        if (--umem->refcount)
                close(xsk->fd);

@@ -1071,7 +1074,7 @@ void xsk_socket__delete(struct xsk_socket *xsk)
                }
        }
 
-       xsk_put_ctx(ctx);
+       xsk_put_ctx(ctx, true);
 
        umem->refcount--;
        /* Do not close an fd that also has an associated umem connected
--

2.17.1

Quoted 1 line · Ciara Loftus

Quoted 9 lines

        if (--ctx->refcount == 0) {

tools/lib/bpf/xsk.c +0 -8

-               err = xsk_get_mmap_offsets(umem->fd, &off);
-               if (!err) {
-                       munmap(ctx->fill->ring - off.fr.desc,
-                              off.fr.desc + umem->config.fill_size *
-                              sizeof(__u64));
-                       munmap(ctx->comp->ring - off.cr.desc,
-                              off.cr.desc + umem->config.comp_size *
-                              sizeof(__u64));

tools/lib/bpf/xsk.c static struct xsk_ctx *xsk_get_ctx(struct xsk_umem *umem, int ifindex, · line 762

Quoted 10 lines

tools/lib/bpf/xsk.c +10 -0

+               if (unmap) {
+                       err = xsk_get_mmap_offsets(umem->fd, &off);
+                       if (!err) {
+                               munmap(ctx->fill->ring - off.fr.desc,
+                                      off.fr.desc + umem->config.fill_size *
+                               sizeof(__u64));
+                               munmap(ctx->comp->ring - off.cr.desc,
+                                      off.cr.desc + umem->config.comp_size *
+                               sizeof(__u64));
+                       }

The whole function increases indent, since it changes anyway could you write it as:
{
if (--ctx->refcount)
return;
if (!unmap)
goto out_free;
err = xsk_get
if (err)
goto out_free;
munmap();
out_free:
list_del
free
}

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 <[email protected]>

---
 tools/lib/bpf/xsk.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

tools/lib/bpf/xsk.c +4 -0

index d4991ddff05a..12110bba4cc0 100644
@@ -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,