[PATCH rdma-next 1/1] RDMA/mana_ib: Adopt robust udata

[PATCH rdma-next 1/1] RDMA/mana_ib: Adopt robust udata

[Konstantin Taranov]

From: Konstantin Taranov <kotaranov@microsoft.com>

Enable the uverbs robust udata interface in mana_ib by setting
uverbs_robust_udata and converting the driver to the new udata
handling model.

The alloc_ucontext() request is extended to receive userspace
capability flags, providing a foundation for future feature
negotiation between rdma-core and the kernel driver.

Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
---
 drivers/infiniband/hw/mana/cq.c      | 10 ++++--
 drivers/infiniband/hw/mana/device.c  |  1 +
 drivers/infiniband/hw/mana/main.c    | 36 +++++++++++++++++-----
 drivers/infiniband/hw/mana/mana_ib.h |  4 +++
 drivers/infiniband/hw/mana/mr.c      | 15 ++++++++-
 drivers/infiniband/hw/mana/qp.c      | 46 +++++++++++++++++++++-------
 drivers/infiniband/hw/mana/wq.c      | 16 ++++++++--
 include/uapi/rdma/mana-abi.h         | 14 +++++++--
 8 files changed, 115 insertions(+), 27 deletions(-)

diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
index f2547989f..a5757847b 100644
--- a/drivers/infiniband/hw/mana/cq.c
+++ b/drivers/infiniband/hw/mana/cq.c
@@ -27,7 +27,8 @@ int mana_ib_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
 	is_rnic_cq = mana_ib_is_rnic(mdev);
 
 	if (udata) {
-		err = ib_copy_validate_udata_in(udata, ucmd, buf_addr);
+		err = ib_copy_validate_udata_in_cm(udata, ucmd, buf_addr,
+						   MANA_IB_CREATE_RNIC_CQ);
 		if (err)
 			return err;
 
@@ -105,6 +106,11 @@ int mana_ib_destroy_cq(struct ib_cq *ibcq, struct ib_udata *udata)
 	struct mana_ib_cq *cq = container_of(ibcq, struct mana_ib_cq, ibcq);
 	struct ib_device *ibdev = ibcq->device;
 	struct mana_ib_dev *mdev;
+	int err;
+
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return err;
 
 	mdev = container_of(ibdev, struct mana_ib_dev, ib_dev);
 
@@ -117,7 +123,7 @@ int mana_ib_destroy_cq(struct ib_cq *ibcq, struct ib_udata *udata)
 
 	mana_ib_destroy_queue(mdev, &cq->queue);
 
-	return 0;
+	return ib_respond_empty_udata(udata);
 }
 
 static void mana_ib_cq_handler(void *ctx, struct gdma_queue *gdma_cq)
diff --git a/drivers/infiniband/hw/mana/device.c b/drivers/infiniband/hw/mana/device.c
index 9811570ab..a8d19586a 100644
--- a/drivers/infiniband/hw/mana/device.c
+++ b/drivers/infiniband/hw/mana/device.c
@@ -15,6 +15,7 @@ static const struct ib_device_ops mana_ib_dev_ops = {
 	.owner = THIS_MODULE,
 	.driver_id = RDMA_DRIVER_MANA,
 	.uverbs_abi_ver = MANA_IB_UVERBS_ABI_VERSION,
+	.uverbs_robust_udata = true,
 
 	.add_gid = mana_ib_gd_add_gid,
 	.alloc_mw = mana_ib_alloc_mw,
diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
index a86f80b91..d8f93502b 100644
--- a/drivers/infiniband/hw/mana/main.c
+++ b/drivers/infiniband/hw/mana/main.c
@@ -77,6 +77,10 @@ int mana_ib_alloc_pd(struct ib_pd *ibpd, struct ib_udata *udata)
 	struct gdma_context *gc;
 	int err;
 
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return err;
+
 	dev = container_of(ibdev, struct mana_ib_dev, ib_dev);
 	gc = mdev_to_gc(dev);
 
@@ -98,7 +102,7 @@ int mana_ib_alloc_pd(struct ib_pd *ibpd, struct ib_udata *udata)
 
 	mutex_init(&pd->vport_mutex);
 	pd->vport_use_count = 0;
-	return 0;
+	return ib_respond_empty_udata(udata);
 }
 
 int mana_ib_dealloc_pd(struct ib_pd *ibpd, struct ib_udata *udata)
@@ -109,6 +113,11 @@ int mana_ib_dealloc_pd(struct ib_pd *ibpd, struct ib_udata *udata)
 	struct gdma_destroy_pd_req req = {};
 	struct mana_ib_dev *dev;
 	struct gdma_context *gc;
+	int err;
+
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return err;
 
 	dev = container_of(ibdev, struct mana_ib_dev, ib_dev);
 	gc = mdev_to_gc(dev);
@@ -118,7 +127,11 @@ int mana_ib_dealloc_pd(struct ib_pd *ibpd, struct ib_udata *udata)
 
 	req.pd_handle = pd->pd_handle;
 
-	return mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
+	err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
+	if (err)
+		return err;
+
+	return ib_respond_empty_udata(udata);
 }
 
 static int mana_gd_destroy_doorbell_page(struct gdma_context *gc,
@@ -169,26 +182,33 @@ int mana_ib_alloc_ucontext(struct ib_ucontext *ibcontext,
 	struct mana_ib_ucontext *ucontext =
 		container_of(ibcontext, struct mana_ib_ucontext, ibucontext);
 	struct ib_device *ibdev = ibcontext->device;
+	struct mana_ib_uctx_req ucmd;
 	struct mana_ib_dev *mdev;
 	struct gdma_context *gc;
 	int doorbell_page;
 	int ret;
 
+	if (udata->inlen) {
+		ret = ib_copy_validate_udata_in_cm(udata, ucmd, comp_mask, 0);
+		if (ret)
+			return ret;
+	}
+
 	mdev = container_of(ibdev, struct mana_ib_dev, ib_dev);
 	gc = mdev_to_gc(mdev);
 
 	/* Allocate a doorbell page index */
 	ret = mana_gd_allocate_doorbell_page(gc, &doorbell_page);
-	if (ret) {
-		ibdev_dbg(ibdev, "Failed to allocate doorbell page %d\n", ret);
+	if (ret)
 		return ret;
-	}
-
-	ibdev_dbg(ibdev, "Doorbell page allocated %d\n", doorbell_page);
 
 	ucontext->doorbell = doorbell_page;
+	ucontext->client_caps1 = ucmd.client_caps1;
+	ucontext->client_caps2 = ucmd.client_caps2;
+	ucontext->client_caps3 = ucmd.client_caps3;
+	ucontext->client_caps4 = ucmd.client_caps4;
 
-	return 0;
+	return ib_respond_empty_udata(udata);
 }
 
 void mana_ib_dealloc_ucontext(struct ib_ucontext *ibcontext)
diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
index c9c94e86a..f0c605914 100644
--- a/drivers/infiniband/hw/mana/mana_ib.h
+++ b/drivers/infiniband/hw/mana/mana_ib.h
@@ -202,6 +202,10 @@ struct mana_ib_qp {
 struct mana_ib_ucontext {
 	struct ib_ucontext ibucontext;
 	u32 doorbell;
+	u64 client_caps1;
+	u64 client_caps2;
+	u64 client_caps3;
+	u64 client_caps4;
 };
 
 struct mana_ib_rwq_ind_table {
diff --git a/drivers/infiniband/hw/mana/mr.c b/drivers/infiniband/hw/mana/mr.c
index 030bfdcff..2a85e6a04 100644
--- a/drivers/infiniband/hw/mana/mr.c
+++ b/drivers/infiniband/hw/mana/mr.c
@@ -113,6 +113,10 @@ struct ib_mr *mana_ib_reg_user_mr(struct ib_pd *ibpd, u64 start, u64 length,
 	if (dmah)
 		return ERR_PTR(-EOPNOTSUPP);
 
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return ERR_PTR(err);
+
 	dev = container_of(ibdev, struct mana_ib_dev, ib_dev);
 
 	ibdev_dbg(ibdev,
@@ -327,6 +331,11 @@ int mana_ib_alloc_mw(struct ib_mw *ibmw, struct ib_udata *udata)
 {
 	struct mana_ib_dev *mdev = container_of(ibmw->device, struct mana_ib_dev, ib_dev);
 	struct mana_ib_pd *pd = container_of(ibmw->pd, struct mana_ib_pd, ibpd);
+	int err;
+
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return err;
 
 	return mana_ib_gd_create_mw(mdev, pd, ibmw);
 }
@@ -346,6 +355,10 @@ int mana_ib_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
 	struct mana_ib_dev *dev;
 	int err;
 
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return err;
+
 	dev = container_of(ibdev, struct mana_ib_dev, ib_dev);
 
 	err = mana_ib_gd_destroy_mr(dev, mr->mr_handle);
@@ -357,7 +370,7 @@ int mana_ib_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
 
 	kfree(mr);
 
-	return 0;
+	return ib_respond_empty_udata(udata);
 }
 
 static int mana_ib_gd_alloc_dm(struct mana_ib_dev *mdev, struct mana_ib_dm *dm,
diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
index 39d9cdcc5..727cd4c94 100644
--- a/drivers/infiniband/hw/mana/qp.c
+++ b/drivers/infiniband/hw/mana/qp.c
@@ -606,10 +606,8 @@ static int mana_ib_create_ud_qp(struct ib_qp *ibqp, struct ib_pd *ibpd,
 	u32 doorbell, queue_size;
 	int i, err;
 
-	if (udata) {
-		ibdev_dbg(&mdev->ib_dev, "User-level UD QPs are not supported\n");
+	if (udata)
 		return -EOPNOTSUPP;
-	}
 
 	for (i = 0; i < MANA_UD_QUEUE_TYPE_MAX; ++i) {
 		queue_size = mana_ib_queue_size(attr, i);
@@ -700,6 +698,11 @@ static int mana_ib_gd_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 	struct gdma_context *gc = mdev_to_gc(mdev);
 	struct mana_port_context *mpc;
 	struct net_device *ndev;
+	int err;
+
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return err;
 
 	mana_gd_init_req_hdr(&req.hdr, MANA_IB_SET_QP_STATE, sizeof(req), sizeof(resp));
 
@@ -752,7 +755,11 @@ static int mana_ib_gd_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 		req.ah_attr.flow_label = attr->ah_attr.grh.flow_label;
 	}
 
-	return mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
+	err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
+	if (err)
+		return err;
+
+	return ib_respond_empty_udata(udata);
 }
 
 int mana_ib_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
@@ -779,7 +786,11 @@ static int mana_ib_destroy_qp_rss(struct mana_ib_qp *qp,
 	struct net_device *ndev;
 	struct mana_ib_wq *wq;
 	struct ib_wq *ibwq;
-	int i;
+	int i, err;
+
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return err;
 
 	ndev = mana_ib_get_netdev(qp->ibqp.device, qp->port);
 	mpc = netdev_priv(ndev);
@@ -807,7 +818,7 @@ static int mana_ib_destroy_qp_rss(struct mana_ib_qp *qp,
 		mana_destroy_wq_obj(mpc, GDMA_RQ, wq->rx_object);
 	}
 
-	return 0;
+	return ib_respond_empty_udata(udata);
 }
 
 static int mana_ib_destroy_qp_raw(struct mana_ib_qp *qp, struct ib_udata *udata)
@@ -818,6 +829,11 @@ static int mana_ib_destroy_qp_raw(struct mana_ib_qp *qp, struct ib_udata *udata)
 	struct mana_port_context *mpc;
 	struct net_device *ndev;
 	struct mana_ib_pd *pd;
+	int err;
+
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return err;
 
 	ndev = mana_ib_get_netdev(qp->ibqp.device, qp->port);
 	mpc = netdev_priv(ndev);
@@ -829,14 +845,18 @@ static int mana_ib_destroy_qp_raw(struct mana_ib_qp *qp, struct ib_udata *udata)
 
 	mana_ib_uncfg_vport(mdev, pd, qp->port);
 
-	return 0;
+	return ib_respond_empty_udata(udata);
 }
 
 static int mana_ib_destroy_rc_qp(struct mana_ib_qp *qp, struct ib_udata *udata)
 {
 	struct mana_ib_dev *mdev =
 		container_of(qp->ibqp.device, struct mana_ib_dev, ib_dev);
-	int i;
+	int i, err;
+
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return err;
 
 	mana_table_remove_qp(mdev, qp);
 
@@ -847,14 +867,18 @@ static int mana_ib_destroy_rc_qp(struct mana_ib_qp *qp, struct ib_udata *udata)
 	for (i = 0; i < MANA_RC_QUEUE_TYPE_MAX; ++i)
 		mana_ib_destroy_queue(mdev, &qp->rc_qp.queues[i]);
 
-	return 0;
+	return ib_respond_empty_udata(udata);
 }
 
 static int mana_ib_destroy_ud_qp(struct mana_ib_qp *qp, struct ib_udata *udata)
 {
 	struct mana_ib_dev *mdev =
 		container_of(qp->ibqp.device, struct mana_ib_dev, ib_dev);
-	int i;
+	int i, err;
+
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return err;
 
 	mana_remove_qp_from_cqs(qp);
 	mana_table_remove_qp(mdev, qp);
@@ -869,7 +893,7 @@ static int mana_ib_destroy_ud_qp(struct mana_ib_qp *qp, struct ib_udata *udata)
 	for (i = 0; i < MANA_UD_QUEUE_TYPE_MAX; ++i)
 		mana_ib_destroy_queue(mdev, &qp->ud_qp.queues[i]);
 
-	return 0;
+	return ib_respond_empty_udata(udata);
 }
 
 int mana_ib_destroy_qp(struct ib_qp *ibqp, struct ib_udata *udata)
diff --git a/drivers/infiniband/hw/mana/wq.c b/drivers/infiniband/hw/mana/wq.c
index 5c2134a0b..619c91e67 100644
--- a/drivers/infiniband/hw/mana/wq.c
+++ b/drivers/infiniband/hw/mana/wq.c
@@ -55,6 +55,11 @@ int mana_ib_destroy_wq(struct ib_wq *ibwq, struct ib_udata *udata)
 	struct mana_ib_wq *wq = container_of(ibwq, struct mana_ib_wq, ibwq);
 	struct ib_device *ib_dev = ibwq->device;
 	struct mana_ib_dev *mdev;
+	int err;
+
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return err;
 
 	mdev = container_of(ib_dev, struct mana_ib_dev, ib_dev);
 
@@ -62,18 +67,25 @@ int mana_ib_destroy_wq(struct ib_wq *ibwq, struct ib_udata *udata)
 
 	kfree(wq);
 
-	return 0;
+	return ib_respond_empty_udata(udata);
 }
 
 int mana_ib_create_rwq_ind_table(struct ib_rwq_ind_table *ib_rwq_ind_table,
 				 struct ib_rwq_ind_table_init_attr *init_attr,
 				 struct ib_udata *udata)
 {
+	int err;
+
+	err = ib_is_udata_in_empty(udata);
+	if (err)
+		return err;
+
 	/*
 	 * There is no additional data in ind_table to be maintained by this
 	 * driver, do nothing
 	 */
-	return 0;
+
+	return ib_respond_empty_udata(udata);
 }
 
 int mana_ib_destroy_rwq_ind_table(struct ib_rwq_ind_table *ib_rwq_ind_tbl)
diff --git a/include/uapi/rdma/mana-abi.h b/include/uapi/rdma/mana-abi.h
index a75bf32b8..e6fdcefed 100644
--- a/include/uapi/rdma/mana-abi.h
+++ b/include/uapi/rdma/mana-abi.h
@@ -25,9 +25,9 @@ enum mana_ib_create_cq_flags {
 
 struct mana_ib_create_cq {
 	__aligned_u64 buf_addr;
-	__u16	flags;
-	__u16	reserved0;
-	__u32	reserved1;
+	__u16   comp_mask;
+	__u16   reserved0;
+	__u32   reserved1;
 };
 
 struct mana_ib_create_cq_resp {
@@ -68,6 +68,14 @@ enum mana_ib_rx_hash_function_flags {
 	MANA_IB_RX_HASH_FUNC_TOEPLITZ = 1 << 0,
 };
 
+struct mana_ib_uctx_req {
+	__aligned_u64 client_caps1;
+	__aligned_u64 client_caps2;
+	__aligned_u64 client_caps3;
+	__aligned_u64 client_caps4;
+	__aligned_u64 comp_mask;
+};
+
 struct mana_ib_create_qp_rss {
 	__aligned_u64 rx_hash_fields_mask;
 	__u8 rx_hash_function;
-- 
2.43.0

Re: [PATCH rdma-next 1/1] RDMA/mana_ib: Adopt robust udata

[Jacob Moroni]

> +struct mana_ib_uctx_req {
> +       __aligned_u64 client_caps1;
> +       __aligned_u64 client_caps2;
> +       __aligned_u64 client_caps3;
> +       __aligned_u64 client_caps4;
> +       __aligned_u64 comp_mask;
> +};
> +

I am curious about the addition of these unused "client_caps" fields.

I guess the idea is to be able to reject older providers that lack support for
some mandatory feature in the future - like if a new HW variant breaks the
descriptor ABI or something and therefore requires a provider update?

My main question is: how come they need to be added now as opposed
to extending the structure later?

I'm not proposing any changes, just trying to understand the intent.

Thanks,
Jake

Re: [PATCH rdma-next 1/1] RDMA/mana_ib: Adopt robust udata

[Konstantin Taranov]

> > +struct mana_ib_uctx_req {
> > +       __aligned_u64 client_caps1;
> > +       __aligned_u64 client_caps2;
> > +       __aligned_u64 client_caps3;
> > +       __aligned_u64 client_caps4;
> > +       __aligned_u64 comp_mask;
> > +};
> > +
> 
> I am curious about the addition of these unused "client_caps" fields.
> 
> I guess the idea is to be able to reject older providers that lack support for
> some mandatory feature in the future - like if a new HW variant breaks the
> descriptor ABI or something and therefore requires a provider update?

Not really. The capability bits will be used for simpler integration of new features,
and not to reject older providers. To reject an older provider, I could use the abi version.

> 
> My main question is: how come they need to be added now as opposed to
> extending the structure later?
> 

these client caps allow mana_ib to integrate easier client wide optimizations or feature enablement.
Once we have some important code in the rdma-core that the kernel should know about, we can declare it
as a capability of a client (meaning a client can do additionally X). Important aspect that the capability is
an addition, and it is not a new behavior (meaning a client is backwards compatible). So it is handy for bug
fixes and new features, rather than overwhelming other udata requests via chained changes.

For example, the upcoming patch after this one is that WQEs in the rdma-core will be of fixed size.
The HW supports all sizes, but the knowledge that all WQEs are the same will allow the HW to apply optimizations.
So, this fixed size can be defined as a capability. Then in the kernel code for QP creation, we can add HW flag from the client
capability (so a simple line in the kernel. 1) if the cap present then add a certain flag to HWC).
Otherwise, the change would be more complex: new response for alloc_ucontext, and then new request for various qp create.

A similar idea was in bnxt_re (see BNXT_RE_COMP_MASK_REQ_UCNTX_POW2_SUPPORT and uctx->cmask),
but I think there was misunderstanding as the cap field was named as comp_mask and now bnxt_re is locked to 2 capability bits.
As it is the first rdma-core ioctl, there is no way to know which comp_mask is allowed. With the wave of robust udata,
providers will be locked to one udata request format for alloc_ucontext() without a chance of extending.
That is why, I try to introduce the idea now.

All in all, I believe it would be beneficial for kernel to get some initial feedback from rdma-core,
and I think it was an initial goal of having udata request in alloc_ucontext().

- Konstantin

> I'm not proposing any changes, just trying to understand the intent.
> 
> Thanks,
> Jake

Re: [PATCH rdma-next 1/1] RDMA/mana_ib: Adopt robust udata

[Jacob Moroni]

Thanks for explaining. I think it makes sense.

> Otherwise, the change would be more complex: new response for alloc_ucontex

Do you still need new response fields/bits for alloc_ucontext so that
the client knows whether it can actually use the new capability - like
a driver "ack"?

The fixed WQE capability may be special in that it sounds more like
a one-way hint, but I am wondering about features that require explicit
kernel support (just hypothetical) before the client can actually use it,
like maybe features that require activation in HW, etc.

> With the wave of robust udata,
> providers will be locked to one udata request format for alloc_ucontext() without a chance of extending.
> That is why, I try to introduce the idea now.

Good point - I was wondering if alloc_ucontext would be an exception to
the robust udata policy since it's the first part of the handshake. As in,
clients would be allowed to provide non-zero udata input that the driver
may not understand and it would be up to the driver to acknowledge
features or not by sending a response. Seems kind of similar to how I
think your solution would work.

Otherwise, it's kind of locked in as you said. Enforcing a real comp_mask
in alloc_ucontext would mean that clients wouldn't be compatible with older
kernels since context allocation is too early for them to know which bits
are allowed or not. I could be misunderstanding though.

Thanks,
Jake

Re: [PATCH rdma-next 1/1] RDMA/mana_ib: Adopt robust udata

[Konstantin Taranov]

> -----Original Message-----
> From: Jacob Moroni <jmoroni@google.com>
> Sent: Thursday, 25 June 2026 15:29
> To: Konstantin Taranov <kotaranov@microsoft.com>
> Cc: Konstantin Taranov <kotaranov@linux.microsoft.com>;
> shirazsaleem@microsoft.com; Long Li <longli@microsoft.com>;
> jgg@ziepe.ca; leon@kernel.org; linux-rdma@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH rdma-next 1/1] RDMA/mana_ib: Adopt
> robust udata
> 
> Thanks for explaining. I think it makes sense.
> 
> > Otherwise, the change would be more complex: new response for
> > alloc_ucontex
> 
> Do you still need new response fields/bits for alloc_ucontext so that the
> client knows whether it can actually use the new capability - like a driver
> "ack"?

I do think that an ack per capability bit offers anything useful. Imagine if I did not have a
client cap bit, I could still grow responses as I want and I could send back to the client
that certain kernel feature is available. So, the new clients could use it without telling the kernel
its caps in the first place. So, the main use of the client caps is to add client-wide backward compatible features,
which allow to reduce boiler plate code later in the kernel. E.g., I do not plan to use that caps for features
that require kernel support. Soon I will resend the patch with short PDIDs. There is no benefit in telling the
kernel that rdma-core has a cap to request short PDIDs, as kernel must support it.

I know that drivers often ack the bits with the same flags, but I do not see what it enables.
Having separately defined client and kernel capabilities seems more universal to me.
So in upcoming patches I will do add kernel capabilities in the response (e.g., for short PDs).

-Konstantin

> 
> The fixed WQE capability may be special in that it sounds more like a one-
> way hint, but I am wondering about features that require explicit kernel
> support (just hypothetical) before the client can actually use it, like maybe
> features that require activation in HW, etc.
> 
> > With the wave of robust udata,
> > providers will be locked to one udata request format for alloc_ucontext()
> without a chance of extending.
> > That is why, I try to introduce the idea now.
> 
> Good point - I was wondering if alloc_ucontext would be an exception to the
> robust udata policy since it's the first part of the handshake. As in, clients
> would be allowed to provide non-zero udata input that the driver may not
> understand and it would be up to the driver to acknowledge features or not
> by sending a response. Seems kind of similar to how I think your solution
> would work.
> 
> Otherwise, it's kind of locked in as you said. Enforcing a real comp_mask in
> alloc_ucontext would mean that clients wouldn't be compatible with older
> kernels since context allocation is too early for them to know which bits are
> allowed or not. I could be misunderstanding though.
> 
> Thanks,
> Jake

Re: [PATCH rdma-next 1/1] RDMA/mana_ib: Adopt robust udata

[Jason Gunthorpe]

On Tue, Jun 23, 2026 at 04:44:43AM -0700, Konstantin Taranov wrote:

> +struct mana_ib_uctx_req {
> +	__aligned_u64 client_caps1;
> +	__aligned_u64 client_caps2;
> +	__aligned_u64 client_caps3;
> +	__aligned_u64 client_caps4;
> +	__aligned_u64 comp_mask;
> +};

I think Jacob has it right, This is looking pretty gross.

If you want such a complex protocol then use the new ioctl format and
declare sensible driver specific attributes. Nobody has converted
alloc_ucontext yet, but that is usually not hard to do.

Then you can use the existing mandatory/optional schemes to pass
whatever is appropriate with full easy compat.

So maybe split this part out, the rst of the udata fiddling looks OK

Jason

Re: [PATCH rdma-next 1/1] RDMA/mana_ib: Adopt robust udata

[Konstantin Taranov]

> On Tue, Jun 23, 2026 at 04:44:43AM -0700, Konstantin Taranov wrote:
> 
> > +struct mana_ib_uctx_req {
> > +	__aligned_u64 client_caps1;
> > +	__aligned_u64 client_caps2;
> > +	__aligned_u64 client_caps3;
> > +	__aligned_u64 client_caps4;
> > +	__aligned_u64 comp_mask;
> > +};
> 
> I think Jacob has it right, This is looking pretty gross.
> 

Jason, I sent a response to Jacob earlier. I am not sure you saw it.
The problem of enforcing robust udata on alloc user context is that it blocks
any future additions to the request. Since it is the first IOCTL there is no way
to negotiate the request format. And that is why I wanted to reserve some fields now.

> If you want such a complex protocol then use the new ioctl format and
> declare sensible driver specific attributes. Nobody has converted
> alloc_ucontext yet, but that is usually not hard to do.

I have a question. If I add now ib_is_udata_in_empty() to alloc user context, would I be able to
adopt new ioctl format and extend the udata for alloc user context?

Konstantin

> 
> Then you can use the existing mandatory/optional schemes to pass whatever
> is appropriate with full easy compat.
> 
> So maybe split this part out, the rst of the udata fiddling looks OK
> 
> Jason

Re: [PATCH rdma-next 1/1] RDMA/mana_ib: Adopt robust udata

[Jason Gunthorpe]

On Fri, Jul 10, 2026 at 08:54:03AM +0000, Konstantin Taranov wrote:
> 
> > On Tue, Jun 23, 2026 at 04:44:43AM -0700, Konstantin Taranov wrote:
> > 
> > > +struct mana_ib_uctx_req {
> > > +	__aligned_u64 client_caps1;
> > > +	__aligned_u64 client_caps2;
> > > +	__aligned_u64 client_caps3;
> > > +	__aligned_u64 client_caps4;
> > > +	__aligned_u64 comp_mask;
> > > +};
> > 
> > I think Jacob has it right, This is looking pretty gross.
> > 
> 
> Jason, I sent a response to Jacob earlier. I am not sure you saw it.
> The problem of enforcing robust udata on alloc user context is that it blocks
> any future additions to the request. Since it is the first IOCTL there is no way
> to negotiate the request format. And that is why I wanted to reserve some fields now.

With the udata system you can't implement a scheme where you send
unknown bits into the kernel and it returns back the ones it
understands. That protocol just isn't supported in the 'zero means
backwards compat' semantic we are using for udata.

So I'd rather not see deviation from that normal protocol (I know a
few other places have but lets not grow)

> > If you want such a complex protocol then use the new ioctl format and
> > declare sensible driver specific attributes. Nobody has converted
> > alloc_ucontext yet, but that is usually not hard to do.
> 
> I have a question. If I add now ib_is_udata_in_empty() to alloc user
> context, would I be able to adopt new ioctl format and extend the
> udata for alloc user context?

Yep, they are orthogonal things. the "udata" is only the legacly uhw
in/out structs. The ioctl mechanism is uattrs based and completely
different.

So you can set an optional client_caps uattr and implement this
protocol there.

Jason