Re: [PATCH rdma-next v2 01/15] RDMA/core: Introduce generic buffer descriptor infrastructure for umem

Michael Margolin
2026-04-16T12:10:16Z

On Mon, Apr 13, 2026 at 08:22:05PM +0200, Jiri Pirko wrote:
> >> >While I like the idea of standardizing the way we pass buffer
> >> >information to the kernel, the list thing looks like over generalization
> >> >to me, especially after Leon's refactoring of CQ creation. Maybe we can
> >> >add buffer as a new attribute type that can be used for multiple
> >> >parameters in a command, and have a helper with the code below that
> >> >takes an attribute id and returns a umem object, letting each handler
> >> >store it. This would also make it easier for drivers to pass their
> >> >private buffers using this infrastructure.
> >>
> >> Currently we have set of attrs (4) to pass CQ umem. I tried to make this
> >> very smooth for all possible uverbs, passing single attr of array of
> >> structs describing a buffer. Uverb attr api knows how to work with
> >> arrays, all clicks.
> >>
> >> Drivers can easily pass their specific buffers over this list too. I
> >> didn't implement it as there was no need, but the idea is to have index>X
> >> for driver specific indexes.
> >
> >Why do we need to invent a new way instead of just adding another
> >argument in a command, that consists of all the info needed to pass a
> >buffer? Also how can this work for objects that have only private umem?
>
> You can put the buf array attr to any uverb, some may not have
> "standard" indexes.
>
Not sure I fully follow your idea here, can you elaborate on how you
plan to reserve index>X range in an enum used as index into dynamic
array?

> >
> >> What's the benefit of passing per-uverb attrs with a struct? Perhaps I'm
> >> missing something.
> >
> >Mostly simplification by untying two unrelated things:
> >1) way of passing args to kernel
> >2) object lifetime management
>
> Could you be more specific please?
>
>
> >
> >And also significantly reducing the amount of code changes required to
> >achieve this.
>
> Significantly? I'm not sure I follow, but I guess that is related to my
> previous question. I'm not sure I understand what you have exacly in
> mind. Regarding UAPI, I think I understand, but regarding kernel
> internals, I don't :(
>

I imagine the changes for getting umem using the new mechanism in
create CQ command are about:

Define a new optional buffer attribute:

- UVERBS_ATTR_UHW());
+ UVERBS_ATTR_UHW(),
+ UVERBS_ATTR_BUFFER(UVERBS_ATTR_CREATE_CQ_BUFFER,
+ UA_OPTIONAL));

Get umem from the new attribute if available or fallback to existing
attributes:

- umem = uverbs_create_cq_get_umem(ib_dev, attrs);
- if (IS_ERR(umem)) {
- ret = PTR_ERR(umem);
- goto err_event_file;
- }
+ if (uverbs_attr_is_valid(attrs, UVERBS_ATTR_CREATE_CQ_BUFFER)) {
+ ret = uverbs_get_umem(&umem, attrs, UVERBS_ATTR_CREATE_CQ_BUFFER);
+ if (ret)
+ goto err_event_file;
+ } else {
+ umem = uverbs_create_cq_get_umem(ib_dev, attrs);
+ if (IS_ERR(umem)) {
+ ret = PTR_ERR(umem);
+ goto err_event_file;
+ }
+ }

Drivers don't need to change.

> >> >> + * Optional array of struct ib_uverbs_buffer_desc describing memory regions
> >> >> + * backed by dma-buf or user virtual address. Can be added to any method
> >> >> + * that needs external buffer support.
> >> >> + * Each entry carries an index field selecting the per-command buffer slot.
> >> >> + * Use ib_umem_list_create() to map them and ib_umem_list_load() to access.
> >> >> + */
> >> >> +#define UVERBS_ATTR_BUFFERS() \
> >> >> + UVERBS_ATTR_PTR_IN(UVERBS_ATTR_BUFFERS, \
> >> >> + UVERBS_ATTR_MIN_SIZE( \
> >> >> + sizeof(struct ib_uverbs_buffer_desc)), \
> >> >> + UA_OPTIONAL, \
> >> >> + UA_ALLOC_AND_COPY)
> >> >> +
> >> >> /* =================================================
> >> >> * Parsing infrastructure
> >> >> * =================================================
> >> >> diff --git a/include/uapi/rdma/ib_user_ioctl_cmds.h b/include/uapi/rdma/ib_user_ioctl_cmds.h
> >> >> index 72041c1b0ea5..10aa6568abf1 100644
> >> >> --- a/include/uapi/rdma/ib_user_ioctl_cmds.h
> >> >> +++ b/include/uapi/rdma/ib_user_ioctl_cmds.h
> >> >> @@ -64,6 +64,7 @@ enum {
> >> >> UVERBS_ATTR_UHW_IN = UVERBS_ID_DRIVER_NS,
> >> >> UVERBS_ATTR_UHW_OUT,
> >> >> UVERBS_ID_DRIVER_NS_WITH_UHW,
> >> >> + UVERBS_ATTR_BUFFERS,

I don't think you can add anything here as it overlaps with driver
specific attributes. I suggest defining per command attr id and passing
it by caller into ib_umem_list_create.

> >> >> };
> >> >>
> >> >> enum uverbs_methods_device {
> >> >> diff --git a/include/uapi/rdma/ib_user_ioctl_verbs.h b/include/uapi/rdma/ib_user_ioctl_verbs.h
> >> >> index 90c5cd8e7753..41ed9f75b4de 100644
> >> >> --- a/include/uapi/rdma/ib_user_ioctl_verbs.h
> >> >> +++ b/include/uapi/rdma/ib_user_ioctl_verbs.h
> >> >> @@ -273,4 +273,31 @@ struct ib_uverbs_gid_entry {
> >> >> __u32 netdev_ifindex; /* It is 0 if there is no netdev associated with it */
> >> >> };
> >> >>
> >> >> +enum ib_uverbs_buffer_type {
> >> >> + IB_UVERBS_BUFFER_TYPE_DMABUF,
> >> >> + IB_UVERBS_BUFFER_TYPE_VA,
> >> >> +};
> >> >> +
> >> >> +/*
> >> >> + * Describes a single buffer backed by dma-buf or user virtual address.
> >> >> + * Passed as an array via UVERBS_ATTR_BUFFERS. Each uverb command that
> >> >> + * accepts this attribute defines its own per-command buffer slot enum.
> >> >> + * The index field selects the buffer slot this descriptor maps to.
> >> >> + *
> >> >> + * @fd: dma-buf file descriptor (valid for IB_UVERBS_BUFFER_TYPE_DMABUF)
> >> >> + * @type: buffer type from enum ib_uverbs_buffer_type
> >> >> + * @index: per-command buffer slot index
> >> >> + * @reserved: must be zero
> >> >> + * @addr: offset within dma-buf, or user virtual address for VA
> >> >> + * @length: buffer length in bytes
> >> >> + */
> >> >> +struct ib_uverbs_buffer_desc {
> >> >> + __s32 fd;
> >> >> + __u32 type;
> >> >> + __u32 index;
> >> >> + __u32 reserved;
> >> >> + __aligned_u64 addr;
> >> >> + __aligned_u64 length;
> >> >> +};
> >> >> +
> >> >> #endif
> >> >> --
> >> >> 2.53.0
> >> >>