On 10/2/20 3:36 PM, Magnus Karlsson wrote:
From: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
Fix a compatibility problem when the old XDP_SHARED_UMEM mode is used
together with the xsk_socket__create() call. In the old XDP_SHARED_UMEM
mode, only sharing of the same device and queue id was allowed, and in
this mode, the fill ring and completion ring were shared between the
AF_XDP sockets. Therfore, it was perfectly fine to call the
xsk_socket__create() API for each socket and not use the new
xsk_socket__create_shared() API. This behaviour was ruined by the
commit introducing XDP_SHARED_UMEM support between different devices
and/or queue ids. This patch restores the ability to use
xsk_socket__create in these circumstances so that backward
compatibility is not broken.
We also make sure that a user that uses the
xsk_socket__create_shared() api for the first socket in the old
XDP_SHARED_UMEM mode above, gets and error message if the user tries
to feed a fill ring or a completion ring that is not the same as the
ones used for the umem registration. Previously, libbpf would just
have silently ignored the supplied fill and completion rings and just
taken them from the umem. Better to provide an error to the user.
Signed-off-by: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
Fixes: 2f6324a3937f ("libbpf: Support shared umems between queues and devices")
---
tools/lib/bpf/xsk.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index 30b4ca5..5b61932 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -705,7 +705,7 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
struct xsk_ctx *ctx;
int err, ifindex;
- if (!umem || !xsk_ptr || !(rx || tx) || !fill || !comp)
+ if (!umem || !xsk_ptr || !(rx || tx))
return -EFAULT;
xsk = calloc(1, sizeof(*xsk));
@@ -735,12 +735,24 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
ctx = xsk_get_ctx(umem, ifindex, queue_id);
if (!ctx) {
+ if (!fill || !comp) {
+ err = -EFAULT;
+ goto out_socket;
+ }
+
ctx = xsk_create_ctx(xsk, umem, ifindex, ifname, queue_id,
fill, comp);
if (!ctx) {
err = -ENOMEM;
goto out_socket;
}
+ } else if ((fill && ctx->fill != fill) || (comp && ctx->comp != comp)) {
+ /* If the xsk_socket__create_shared() api is used for the first socket
+ * registration, then make sure the fill and completion rings supplied
+ * are the same as the ones used to register the umem. If not, bail out.
+ */
+ err = -EINVAL;
+ goto out_socket;
This looks buggy. You got a valid ctx in this path which was ctx->refcount++'ed. By just
going to out_socket you'll leak this libbpf internal refcount.
}
xsk->ctx = ctx;
