bnxt_re/lib: Support direct verbs#1654
Conversation
8042660 to
2617428
Compare
2617428 to
7c19ba0
Compare
7c19ba0 to
130c041
Compare
|
Please take the debug_file stuff out of here, I'm not keen on every driver implementing their own stuff. We already have tracing frameworks supported, why not use one? |
| struct bnxt_re_dv_db_region_attr *attr); | ||
| int bnxt_re_dv_get_default_db_region(struct ibv_context *ibvctx, | ||
| struct bnxt_re_dv_db_region_attr *out); | ||
| void *bnxt_re_dv_umem_reg(struct ibv_context *ibvctx, |
There was a problem hiding this comment.
void *? That shouldn't be. Why do you have these functions when there is no kernel side?
130c041 to
c841ce1
Compare
|
On Thu, Dec 18, 2025 at 1:31 AM Jason Gunthorpe ***@***.***> wrote:
*jgunthorpe* left a comment (linux-rdma/rdma-core#1654)
<#1654 (comment)>
Please take the debug_file stuff out of here, I'm not keen on every driver
implementing their own stuff. We already have tracing frameworks supported,
why not use one?
Deleted the debug log patch from the series and pushed the next version.
Thanks,
-Harsha
… —
Reply to this email directly, view it on GitHub
<#1654 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGTY2BJYE3NGLRRWOYN2OD34CGZCXAVCNFSM6AAAAACK7AEXQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTMNRWHE2DSNBQHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
On Thu, Dec 18, 2025 at 1:32 AM Jason Gunthorpe ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In providers/bnxt_re/bnxt_re_dv.h
<#1654 (comment)>
:
> +struct bnxt_re_dv_umem_reg_attr {
+ void *addr;
+ size_t size;
+ uint32_t access_flags;
+ uint64_t pgsz_bitmap;
+ uint64_t comp_mask;
+ int dmabuf_fd;
+};
+
+struct bnxt_re_dv_db_region_attr *
+bnxt_re_dv_alloc_db_region(struct ibv_context *ctx);
+int bnxt_re_dv_free_db_region(struct ibv_context *ctx,
+ struct bnxt_re_dv_db_region_attr *attr);
+int bnxt_re_dv_get_default_db_region(struct ibv_context *ibvctx,
+ struct bnxt_re_dv_db_region_attr *out);
+void *bnxt_re_dv_umem_reg(struct ibv_context *ibvctx,
void *? That shouldn't be. Why do you have these functions when there is
no kernel side?
It is a void *, since we want to pass an opaque handle to the application.
This handle is passed back into the library during object creation. The
application provides this handle along with an offset into this memory area
to create the object (see CQ/QP creation DV APIs and commit messages).
Thanks,
-Harsha
… —
Reply to this email directly, view it on GitHub
<#1654 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGTY2BPWWJYFCN6B4PI3OWT4CGZGZAVCNFSM6AAAAACK7AEXQ2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTKOBZGQZTKMBUGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
So give it an opaque type, don't just randomly use void *s. Again,why is this stuff here when there is no kernel side for umems? |
|
On Thu, Jan 8, 2026 at 1:28 AM Jason Gunthorpe ***@***.***> wrote:
*jgunthorpe* left a comment (linux-rdma/rdma-core#1654)
<#1654 (comment)>
So give it an opaque type, don't just randomly use void *s.
Are you suggesting to use a pointer to the forward declaration of that
structure? Is that what you meant by "give it an opaque type"? Please
clarify.
Again,why is this stuff here when there is no kernel side for umems?
The application registers the addr and the total size of the memory area
with the library, initially using this API. Later during each object
creation, the library uses this information to provide the actual va
(base-addr + offset) for mapping/pinning, to the kernel driver. This has
been captured in the kernel driver review thread:
***@***.***/
… —
Reply to this email directly, view it on GitHub
<#1654 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGTY2BIR6EFHFZXC2CENTI34FVQP5AVCNFSM6AAAAACK7AEXQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMRQGUZTOMBUGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Here's the pointer to the kernel review thread for reference: |
|
Just have some 'struct bnxt_umem' as a forward declaration that never sees an actual definition. It is now an opaque type you can use instead of void. You also need to write man pages for these functions, see providers/mlx5/man/mlx5dv_devx_umem_reg.3.md for instance. |
c841ce1 to
37a3df4
Compare
|
On Fri, Jan 9, 2026 at 11:44 PM Jason Gunthorpe ***@***.***> wrote:
*jgunthorpe* left a comment (linux-rdma/rdma-core#1654)
<#1654 (comment)>
Just have some 'struct bnxt_umem' as a forward declaration that never sees
an actual definition. It is now an opaque type you can use instead of void.
Done, pushed the updated patch-set.
You also need to write man pages for these functions, see
providers/mlx5/man/mlx5dv_devx_umem_reg.3.md for instance.
Sure, I will push this in the next revision if there are no other code
related comments.
… —
Reply to this email directly, view it on GitHub
<#1654 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGTY2BIJHOVXBUWPLFMOGR34F7VWVAVCNFSM6AAAAACK7AEXQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMZQGA2DCMRWGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
While looking into changes needed in the bnxt_re kernel driver to process DBR passed through driver specific attributes (during QP creation), I noticed that the corresponding libibverbs support seems to be missing in cmd_qp.c. |
|
As the first user you'd need to do a similar thing as CQ. |
37a3df4 to
d6d9097
Compare
|
On Thu, Jan 15, 2026 at 10:19 PM Jason Gunthorpe ***@***.***> wrote:
*jgunthorpe* left a comment (linux-rdma/rdma-core#1654)
<#1654 (comment)>
As the first user you'd need to do a similar thing as CQ.
Pushed the next version with a new create_qp function in libibverbs.
Thanks,
-Harsha
… —
Reply to this email directly, view it on GitHub
<#1654 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGTY2BPOYJBYLUJ4JB5E2LL4G7AIXAVCNFSM6AAAAACK7AEXQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTONJVHAZTGNJUGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
294a076 to
3d8c351
Compare
d43117e to
362804c
Compare
362804c to
cc0e95c
Compare
7a10dd5 to
24a1e0f
Compare
24a1e0f to
e56695b
Compare
530f887 to
28d4255
Compare
| fill_attr_in_ptr(cmdb, attr_id, storage); | ||
| cmdb->fallback_ioctl_only = 1; | ||
| } | ||
|
|
There was a problem hiding this comment.
Does not make sense to have this as separate helper. Your caller checks "buf->fd ==-1". You can do the check inside fill_attr_in_buf_umem and fill up things (like type) appropriately.
There was a problem hiding this comment.
Done, will update it in the next version.
| }; | ||
|
|
||
| struct bnxt_re_dv_umem_reg_attr { | ||
| void *addr; |
There was a problem hiding this comment.
This duplicates the layout of struct ibv_buf. Why you can't use it directly?
There was a problem hiding this comment.
This is a user facing structure and we don't want to change it as it would impact applications that are already using it.
There was a problem hiding this comment.
This puzzles me. How an application can use this struct if you are only introducing it now?
There was a problem hiding this comment.
OOB solution which already exists is being upstreamed.
There was a problem hiding this comment.
Wait, you say that you form the API of a feature currently being pushed upon some out of tree code and use it as argument the API can't be changed? I have troubles to understand that reasoning, sorry.
There was a problem hiding this comment.
The structure layouts are slightly different. We have added a comp_mask for vendor specific extensions.
struct bnxt_re_dv_umem_reg_attr {
void *addr;
size_t size;
uint64_t comp_mask;
int dmabuf_fd;
};
struct ibv_buf {
void *addr;
size_t size;
struct ibv_pd *pd;
int dmabuf_fd;
};
Also, since this is a Direct Verb to the user application, we want to maintain the same ABI for both our inbox and out-of-tree libraries, so that the applications can work seamlessly on both installations. So unless you see a design level issue here, we would prefer to keep this as-is and avoid any impact to existing customers that are using out-of-tree packages.
There was a problem hiding this comment.
Why you have comp_mask here is a mystery. Not needed (and if really, really need it, you can embed ibv_buf - so no argument). Basically they are same structures. Don't screw-up the API in a name of out-of-tree code. That is fundamentally wrong. Please fix.
fill_attr_in_buf_umem() only handles dmabuf-backed buffers. Extend it for user virtual address buffers. Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Extract HSI definitions (WQE/CQE structures and enums) from bnxt_re-abi.h into a standalone bnxt_re-hsi.h and add it to publish_headers() so it is installed under infiniband/. This will make HSI definitions available to DV applications, that implement their own datapath. Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> Reviewed-by: Selvin Xavier <selvin.xavier@broadcom.com>
The following Direct Verb (DV) APIs have been implemented in this patch. Doorbell Region Direct Verbs: - bnxt_re_dv_alloc_db_region(): This will allow the appliation to create extra doorbell regions and use the associated doorbell page index in dv_create_qp() and use the associated DB address while ringing the doorbell. - bnxt_re_dv_free_db_region(): Free the allocated doorbell region. - bnxt_re_dv_get_default_db_region(): Return the default doorbell page index and doorbell page address associated with the ucontext. Umem Registration Direct Verbs: - bnxt_re_dv_umem_reg(): Register the user memory to be used by the application with the library. Application can register a large chunk of memory and use it during subsequent resource creation DV APIs. Note that the API terminates in the library and the app specified memory params (addr, len) are saved and a umem-handle is returned. That is, there is no ioctl to the driver at this point to map/pin the user memory. This memory is mapped/pinned later when the application creates the required resources (CQ/QP) using respective direct verbs. This is implemented in the next patch in this series. - bnxt_re_dv_umem_dereg(): Deregister the user memory specified by the umem-handle. Co-developed-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Reviewed-by: Selvin Xavier <selvin.xavier@broadcom.com>
The following Direct Verb (DV) APIs have been implemented in this patch. These are implemented by enhancing the driver specific udata in existing verbs. CQ Direct Verbs: - bnxt_re_dv_create_cq(): Create a CQ of requested size (cqe). The application must have already registered this memory using bnxt_re_dv_umem_reg(). The CQ umem-handle and umem-offset provided by the application are translated into an address for mapping and passed to the driver. - bnxt_re_dv_destroy_cq(): Destroy the DV_CQ created earlier. Note: Some applications might want to allocate memory for all CQs in one big chunk and then register that entire memory once using dv_umem_reg(). At the time of creating each individual resource, the application should pass a specific offset/length in the umem registered memory. Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> Co-developed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Reviewed-by: Selvin Xavier <selvin.xavier@broadcom.com> fix
The following Direct Verb (DV) APIs have been implemented in this patch. These are implemented by enhancing the driver specific udata in existing verbs. QP Direct Verbs: - bnxt_re_dv_create_qp(): Create a QP using specified params (struct bnxt_re_dv_qp_init_attr). The application must have already registered SQ/RQ memory using bnxt_re_dv_umem_reg(). The SQ/RQ umem-handle and umem-offset provided by the application are translated into an address for mapping and passed to the driver. - bnxt_re_dv_destroy_qp(): Destroy the DV_QP created earlier. - bnxt_re_dv_modify_qp(): Modify QP attributes of the DV_QP. - bnxt_re_dv_query_qp(): Return QP attributes of the DV_QP. Note: Some applications might want to allocate memory for all QPs in one big chunk and then register that entire memory once using dv_umem_reg(). At the time of creating each individual resource, the application should pass a specific offset/length in the umem registered memory. Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> Reviewed-by: Selvin Xavier <selvin.xavier@broadcom.com>
Document direct verbs (DV) feature and related DV APIs. The man page describes these DV APIs: - Doorbell regions (db) - User memory (umem) - Completion Queues (cq) Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> Reviewed-by: Selvin Xavier <selvin.xavier@broadcom.com>
Document QP related DV APIs:
bnxt_re_dv_create_qp
bnxt_re_dv_destroy_qp
bnxt_re_dv_modify_qp
bnxt_re_dv_query_qp
Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Reviewed-by: Selvin Xavier <selvin.xavier@broadcom.com>
694f4b4 to
5fddf2f
Compare
4 participants
Hi,
This patchset supports Direct Verbs in the bnxt_re library.
This is required by vendor specific applications that need to manage
the HW resources directly and to implement the datapath in the
application.
To support this, the library and the driver are being enhanced to
provide Direct Verbs using which the application can allocate and
manage the HW resources (Queues, Doorbell etc) . The Direct Verbs
enable the application to implement the control path.
Patch#1 Add a helper for VA-backed buffer descriptors
Patch#2 Export HSI file
Patch#3 Support DBR and UMEM verbs
Patch#4 Support CQ verbs
Patch#5 Support QP verbs
Patch#6 Add man pages
Patch#7 Add QP man pages
Thanks,
-Harsha