RDMA: Change capability fields in ib_device_attr from int to u32
Subsystems (19)
PATCH v8 1/1 RDMA: Change capability fields in ib_device_attr from int to u32
The capability counter fields in struct ib_device_attr are declared as signed int, but these values are inherently non-negative. Drivers maintain their cached caps as u32 and assign them directly into these int fields; if a cap exceeds INT_MAX the implicit narrowing yields a negative value visible to the IB core.
Change the signed int capability fields to u32 to match the underlying nature of the data. Also update consumers across the IB core, ULPs, NVMe-oF target, RDS, and NFS/RDMA so the new u32 values are not forced back through signed int or u8 via min()/min_t() or narrowing local variables.
Suggested-by Jason Gunthorpe <[email protected]>
Signed-off-by Erni Sri Satya Vennela <[email protected]>
---
Changes in v8:
* Convert the remaining non-negative counter fields max_ee_rd_atom, max_ee_init_rd_atom, max_ee, max_rdd, max_raw_ipv6_qp and max_srq_wr to u32; keep max_srq as int (its consumer compares it against ib_device.num_comp_vectors, still int).
* Drop all remaining min_t() where plain min() now works.
* Make the srq_size module parameters unsigned int so the srq_size min() stays a plain min().
* Replace the ternary-inside-min() with the simpler "if (x) x--;".
* Reorder the send_queue_depth min() to min(value, CONST) to match the sibling site.
* Restore reverse xmas-tree declaration order.
* Collapse the min()/min3() assignments that now fit onto a single line within 100 columns.
* Print the now-u32 fields with %u instead of %d.
Changes in v7:
* Drop min_t() in all sites where a plain min() (or min3()) works cleanly
* Guard nvme/host/rdma.c num_inline_segments computation against a device reporting max_send_sge == 0, so the u32 subtract cannot wrap to UINT_MAX.
* Use %u when printing the newly-u32 capability fields
in diagnostic messages.
Changes in v6:
* Fix subject prefix: net-next -> rdma-next.
Changes in v5:
* Add U8_MAX clamps in iser_verbs, nvme/host, nvme/target, isert,
* rds/ib_cm, smbdirect/connect and smbdirect/accept where u32 capability fields were directly narrowed into u8 rdma_conn_param fields without clamping.
* Guard the inline_sge_count calculation in nvmet_rdma_find_get_device() to prevent u32 underflow when both max_sge_rd and max_recv_sge are zero.
* Expand type migration to 9 additional fields (max_mw, max_raw_ethy_qp, max_mcast_grp, max_mcast_qp_attach, max_total_mcast_qp_attach, max_ah, max_srq, max_srq_wr, max_srq_sge)
* Fix min_t(int,...) in svc_rdma_transport; min_t(u32,...) in ipoib, srpt, nvme/target, rds/ib, rtrs-clt, rtrs-srv, xprtrdma/verbsdd.
* Fix frwr_ops.c u32 underflow guard (reorder check before subtraction)
* Change sc_max_send_sges to unsigned int, inline_sge_count to u32
* Fix %d -> %u in rxe_qp, rxe_srq, ipoib_cm, ib_isert, svc_rdma_transport
* Update commit message.
Changes in v4:
* Drop clamping the values in mana_ib_query_device, instead update the props values from int to u32.
Changes in v3:
* Drop clamping from mana_ib_gd_query_adapter_caps(). The internal u32 caps cache does not need to be clamped.
* Move all clamping exclusively to mana_ib_query_device(), which is the only place the cached u32 values are narrowed into the signed int fields of struct ib_device_attr.
* Reframe commit message: this is a u32-to-int type boundary fix, not a CVM/untrusted-hardware hardening patch.
Changes in v2:
* Update patch title.
--- drivers/infiniband/core/cq.c | 3 +- drivers/infiniband/hw/qedr/verbs.c | 2 +- drivers/infiniband/sw/rxe/rxe_qp.c | 22 +++++----- drivers/infiniband/sw/rxe/rxe_srq.c | 16 +++---- drivers/infiniband/ulp/ipoib/ipoib_cm.c | 10 ++--- drivers/infiniband/ulp/ipoib/ipoib_verbs.c | 3 +- drivers/infiniband/ulp/iser/iser_verbs.c | 5 +-- drivers/infiniband/ulp/isert/ib_isert.c | 7 ++- drivers/infiniband/ulp/rtrs/rtrs-clt.c | 11 ++--- drivers/infiniband/ulp/rtrs/rtrs-srv.c | 11 ++--- drivers/infiniband/ulp/srp/ib_srp.c | 2 +- drivers/infiniband/ulp/srpt/ib_srpt.c | 21 +++++---- drivers/nvme/host/rdma.c | 8 ++-- drivers/nvme/target/rdma.c | 13 +++--- fs/smb/smbdirect/accept.c | 5 ++- fs/smb/smbdirect/connect.c | 5 ++- fs/smb/smbdirect/connection.c | 8 ++-- include/linux/sunrpc/svc_rdma.h | 4 +- include/rdma/ib_verbs.h | 50 +++++++++++----------- net/rds/ib.c | 10 ++--- net/rds/ib_cm.c | 10 ++--- net/sunrpc/xprtrdma/frwr_ops.c | 7 +-- net/sunrpc/xprtrdma/svc_rdma_transport.c | 5 +-- net/sunrpc/xprtrdma/verbs.c | 2 +- 24 files changed, 117 insertions(+), 123 deletions(-)
drivers/infiniband/core/cq.c +1 -2
@@ -393,8 +393,7 @@ static int ib_alloc_cqs(struct ib_device *dev, unsigned int nr_cqes,* a reasonable batch size so that we can share CQs between * multiple users instead of allocating a larger number of CQs. */ - nr_cqes = min_t(unsigned int, dev->attrs.max_cqe, -max (nr_cqes,IB_MAX_SHARED_CQ_SZ )); + nr_cqes =min (dev->attrs.max_cqe,max (nr_cqes,IB_MAX_SHARED_CQ_SZ )); nr_cqs = min_t(unsigned int, dev->num_comp_vectors, num_online_cpus());for (i =0 ; i < nr_cqs; i++) { cq =ib_alloc_cq (dev,NULL , nr_cqes, i, poll_ctx);
drivers/infiniband/hw/qedr/verbs.c +1 -1
@@ -151,7 +151,7 @@ int qedr_query_device(struct ib_device *ibdev, attr->max_qp_init_rd_atom =1 << (fls (qattr->max_qp_req_rd_atomic_resc) -1 ); attr->max_qp_rd_atom = -min (1 << (fls (qattr->max_qp_resp_rd_atomic_resc) -1 ), +min (1U << (fls (qattr->max_qp_resp_rd_atomic_resc) -1 ), attr->max_qp_init_rd_atom); attr->max_srq = qattr->max_srq;
drivers/infiniband/sw/rxe/rxe_qp.c +11 -11
@@ -67,27 +67,27 @@ static int rxe_qp_chk_cap(struct rxe_dev *rxe, struct ib_qp_cap *cap, int has_srq) {if (cap->max_send_wr > rxe->attr.max_qp_wr) { -rxe_dbg_dev (rxe,"invalid send wr = %u > %d\n" , - cap->max_send_wr, rxe->attr.max_qp_wr); +rxe_dbg_dev (rxe,"invalid send wr = %u > %u\n" , + cap->max_send_wr, rxe->attr.max_qp_wr); goto err1; }if (cap->max_send_sge > rxe->attr.max_send_sge) { -rxe_dbg_dev (rxe,"invalid send sge = %u > %d\n" , - cap->max_send_sge, rxe->attr.max_send_sge); +rxe_dbg_dev (rxe,"invalid send sge = %u > %u\n" , + cap->max_send_sge, rxe->attr.max_send_sge); goto err1; }if (!has_srq) {if (cap->max_recv_wr > rxe->attr.max_qp_wr) { -rxe_dbg_dev (rxe,"invalid recv wr = %u > %d\n" , - cap->max_recv_wr, rxe->attr.max_qp_wr); +rxe_dbg_dev (rxe,"invalid recv wr = %u > %u\n" , + cap->max_recv_wr, rxe->attr.max_qp_wr); goto err1; }if (cap->max_recv_sge > rxe->attr.max_recv_sge) { -rxe_dbg_dev (rxe,"invalid recv sge = %u > %d\n" , - cap->max_recv_sge, rxe->attr.max_recv_sge); +rxe_dbg_dev (rxe,"invalid recv sge = %u > %u\n" , + cap->max_recv_sge, rxe->attr.max_recv_sge); goto err1; } }
@@ -537,9 +537,9 @@ int rxe_qp_chk_attr(struct rxe_dev *rxe, struct rxe_qp *qp,if (mask &IB_QP_MAX_QP_RD_ATOMIC ) {if (attr->max_rd_atomic > rxe->attr.max_qp_rd_atom) { -rxe_dbg_qp (qp,"invalid max_rd_atomic %d > %d\n" , - attr->max_rd_atomic, - rxe->attr.max_qp_rd_atom); +rxe_dbg_qp (qp,"invalid max_rd_atomic %u > %u\n" , + attr->max_rd_atomic, + rxe->attr.max_qp_rd_atom); goto err1; } }
drivers/infiniband/sw/rxe/rxe_srq.c +8 -8
@@ -13,8 +13,8 @@ int rxe_srq_chk_init(struct rxe_dev *rxe, struct ib_srq_init_attr *init)struct ib_srq_attr *attr = &init->attr;if (attr->max_wr > rxe->attr.max_srq_wr) { -rxe_dbg_dev (rxe,"max_wr(%d) > max_srq_wr(%d)\n" , - attr->max_wr, rxe->attr.max_srq_wr); +rxe_dbg_dev (rxe,"max_wr(%u) > max_srq_wr(%u)\n" , + attr->max_wr, rxe->attr.max_srq_wr); goto err1; }
@@ -27,8 +27,8 @@ int rxe_srq_chk_init(struct rxe_dev *rxe, struct ib_srq_init_attr *init) attr->max_wr =RXE_MIN_SRQ_WR ;if (attr->max_sge > rxe->attr.max_srq_sge) { -rxe_dbg_dev (rxe,"max_sge(%d) > max_srq_sge(%d)\n" , - attr->max_sge, rxe->attr.max_srq_sge); +rxe_dbg_dev (rxe,"max_sge(%u) > max_srq_sge(%u)\n" , + attr->max_sge, rxe->attr.max_srq_sge); goto err1; }
@@ -107,8 +107,8 @@ int rxe_srq_chk_attr(struct rxe_dev *rxe, struct rxe_srq *srq,if (mask &IB_SRQ_MAX_WR ) {if (attr->max_wr > rxe->attr.max_srq_wr) { -rxe_dbg_srq (srq,"max_wr(%d) > max_srq_wr(%d)\n" , - attr->max_wr, rxe->attr.max_srq_wr); +rxe_dbg_srq (srq,"max_wr(%u) > max_srq_wr(%u)\n" , + attr->max_wr, rxe->attr.max_srq_wr); goto err1; }
@@ -129,8 +129,8 @@ int rxe_srq_chk_attr(struct rxe_dev *rxe, struct rxe_srq *srq,if (mask &IB_SRQ_LIMIT ) {if (attr->srq_limit > rxe->attr.max_srq_wr) { -rxe_dbg_srq (srq,"srq_limit(%d) > max_srq_wr(%d)\n" , - attr->srq_limit, rxe->attr.max_srq_wr); +rxe_dbg_srq (srq,"srq_limit(%u) > max_srq_wr(%u)\n" , + attr->srq_limit, rxe->attr.max_srq_wr); goto err1; }
drivers/infiniband/ulp/ipoib/ipoib_cm.c +5 -5
@@ -1071,8 +1071,7 @@ static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_struct ib_qp *tx_qp;if (dev->features &NETIF_F_SG ) - attr.cap.max_send_sge =min_t (u32, priv->ca->attrs.max_send_sge, -MAX_SKB_FRAGS +1 ); + attr.cap.max_send_sge =min (priv->ca->attrs.max_send_sge,MAX_SKB_FRAGS +1 ); tx_qp =ib_create_qp (priv->pd, &attr); tx->max_send_sge = attr.cap.max_send_sge;
@@ -1582,7 +1581,8 @@ static void ipoib_cm_create_srq(struct net_device *dev, int max_sge)int ipoib_cm_dev_init (struct net_device *dev) {struct ipoib_dev_priv *priv =ipoib_priv (dev); -int max_srq_sge, i; +u32 max_srq_sge; +int i;u8 addr;INIT_LIST_HEAD (&priv->cm.passive_ids);
@@ -1600,9 +1600,9 @@ int ipoib_cm_dev_init(struct net_device *dev)skb_queue_head_init (&priv->cm.skb_queue); -ipoib_dbg (priv,"max_srq_sge=%d\n" , priv->ca->attrs.max_srq_sge); +ipoib_dbg (priv,"max_srq_sge=%u\n" , priv->ca->attrs.max_srq_sge); - max_srq_sge =min_t (int,IPOIB_CM_RX_SG , priv->ca->attrs.max_srq_sge); + max_srq_sge =min (priv->ca->attrs.max_srq_sge,IPOIB_CM_RX_SG );ipoib_cm_create_srq (dev, max_srq_sge);if (ipoib_cm_has_srq (dev)) { priv->cm.max_cm_mtu = max_srq_sge *PAGE_SIZE -0x10 ;
drivers/infiniband/ulp/ipoib/ipoib_verbs.c +1 -2
@@ -147,8 +147,7 @@ int ipoib_transport_dev_init(struct net_device *dev, struct ib_device *ca) .cap = { .max_send_wr = ipoib_sendq_size, .max_recv_wr = ipoib_recvq_size, - .max_send_sge =min_t (u32, priv->ca->attrs.max_send_sge, - MAX_SKB_FRAGS + 1), + .max_send_sge = min(priv->ca->attrs.max_send_sge, MAX_SKB_FRAGS + 1), .max_recv_sge =IPOIB_UD_RX_SG }, .sq_sig_type = IB_SIGNAL_ALL_WR,
drivers/infiniband/ulp/iser/iser_verbs.c +2 -3
@@ -244,8 +244,7 @@ static int iser_create_ib_conn_res(struct ib_conn *ib_conn) max_send_wr =ISER_QP_SIG_MAX_REQ_DTOS +1 ; else max_send_wr =ISER_QP_MAX_REQ_DTOS +1 ; - max_send_wr = min_t(unsigned int, max_send_wr, - (unsigned int )ib_dev->attrs.max_qp_wr); + max_send_wr =min (max_send_wr, ib_dev->attrs.max_qp_wr); cq_size = max_send_wr +ISER_QP_MAX_RECV_DTOS ; ib_conn->cq =ib_cq_pool_get (ib_dev, cq_size,-1 ,IB_POLL_SOFTIRQ );
@@ -589,7 +588,7 @@ static void iser_route_handler(struct rdma_cm_id *cma_id) goto failure;memset (&conn_param,0 ,sizeof conn_param); - conn_param.responder_resources = ib_dev->attrs.max_qp_rd_atom; + conn_param.responder_resources =min (ib_dev->attrs.max_qp_rd_atom,U8_MAX ); conn_param.initiator_depth =1 ; conn_param.retry_count =7 ; conn_param.rnr_retry_count =6 ;
drivers/infiniband/ulp/isert/ib_isert.c +3 -4
@@ -214,9 +214,9 @@ isert_create_device_ib_res(struct isert_device *device)struct ib_device *ib_dev = device->ib_device;int ret; -isert_dbg ("devattr->max_send_sge: %d devattr->max_recv_sge %d\n" , +isert_dbg ("devattr->max_send_sge: %u devattr->max_recv_sge %u\n" , ib_dev->attrs.max_send_sge, ib_dev->attrs.max_recv_sge); -isert_dbg ("devattr->max_sge_rd: %d\n" , ib_dev->attrs.max_sge_rd); +isert_dbg ("devattr->max_sge_rd: %u\n" , ib_dev->attrs.max_sge_rd); device->pd =ib_alloc_pd (ib_dev,0 );if (IS_ERR (device->pd)) {
@@ -381,8 +381,7 @@ isert_set_nego_params(struct isert_conn *isert_conn,struct ib_device_attr *attr = &isert_conn->device->ib_device->attrs;/* Set max inflight RDMA READ requests */ - isert_conn->initiator_depth =min_t (u8, param->initiator_depth, - attr->max_qp_init_rd_atom); + isert_conn->initiator_depth =min (param->initiator_depth, attr->max_qp_init_rd_atom);isert_dbg ("Using initiator_depth: %u\n" , isert_conn->initiator_depth);if (param->private_data) {
drivers/infiniband/ulp/rtrs/rtrs-clt.c +4 -7
@@ -1681,8 +1681,7 @@ static int create_con_cq_qp(struct rtrs_clt_con *con)* + 2 for drain and heartbeat * in case qp gets into error state. */ - max_send_wr = -min_t (int, wr_limit,SERVICE_CON_QUEUE_DEPTH *2 +2 ); + max_send_wr =min (wr_limit,SERVICE_CON_QUEUE_DEPTH *2 +2 ); max_recv_wr = max_send_wr; }else {/*
@@ -1698,11 +1697,9 @@ static int create_con_cq_qp(struct rtrs_clt_con *con) wr_limit = clt_path->s.dev->ib_dev->attrs.max_qp_wr;/* Shared between connections */ clt_path->s.dev_ref++; - max_send_wr =min_t (int, wr_limit, -/* QD * (REQ + RSP + FR REGS or INVS) + drain */ - clt_path->queue_depth *4 +1 ); - max_recv_wr =min_t (int, wr_limit, - clt_path->queue_depth *3 +1 ); +/* QD * (REQ + RSP + FR REGS or INVS) + drain */ + max_send_wr =min (wr_limit, clt_path->queue_depth *4 +1 ); + max_recv_wr =min (wr_limit, clt_path->queue_depth *3 +1 ); max_send_sge =2 ; }atomic_set (&con->c.sq_wr_avail, max_send_wr);
drivers/infiniband/ulp/rtrs/rtrs-srv.c +3 -8
@@ -1731,21 +1731,16 @@ static int create_con(struct rtrs_srv_path *srv_path,* All receive and all send (each requiring invalidate) * + 2 for drain and heartbeat */ - max_send_wr =min_t (int, wr_limit, -SERVICE_CON_QUEUE_DEPTH *2 +2 ); + max_send_wr =min (wr_limit,SERVICE_CON_QUEUE_DEPTH *2 +2 ); max_recv_wr = max_send_wr; s->signal_interval =min_not_zero (srv->queue_depth, (size_t )SERVICE_CON_QUEUE_DEPTH ); }else {/* when always_invlaidate enalbed, we need linv+rinv+mr+imm */ if (always_invalidate) - max_send_wr = -min_t (int, wr_limit, - srv->queue_depth * (1 +4 ) +1 ); + max_send_wr =min (wr_limit, srv->queue_depth * (1 +4 ) +1 );else - max_send_wr = -min_t (int, wr_limit, - srv->queue_depth * (1 +2 ) +1 ); + max_send_wr =min (wr_limit, srv->queue_depth * (1 +2 ) +1 ); max_recv_wr = srv->queue_depth +1 ; }
drivers/infiniband/ulp/srp/ib_srp.c +1 -1
@@ -557,7 +557,7 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch) init_attr->cap.max_send_wr = m * target->queue_size; init_attr->cap.max_recv_wr = target->queue_size +1 ; init_attr->cap.max_recv_sge =1 ; - init_attr->cap.max_send_sge =min (SRP_MAX_SGE , attr->max_send_sge); + init_attr->cap.max_send_sge =min (attr->max_send_sge,SRP_MAX_SGE ); init_attr->sq_sig_type =IB_SIGNAL_REQ_WR ; init_attr->qp_type =IB_QPT_RC ; init_attr->send_cq = send_cq;
drivers/infiniband/ulp/srpt/ib_srpt.c +10 -11
@@ -77,8 +77,8 @@ module_param(srp_max_req_size, int, 0444);MODULE_PARM_DESC (srp_max_req_size,"Maximum size of SRP request messages in bytes." ); -static int srpt_srq_size =DEFAULT_SRPT_SRQ_SIZE ; -module_param (srpt_srq_size, int,0444 ); +static unsigned int srpt_srq_size =DEFAULT_SRPT_SRQ_SIZE ; +module_param (srpt_srq_size, uint,0444 );MODULE_PARM_DESC (srpt_srq_size,"Shared receive queue (SRQ) size." );
@@ -405,8 +405,7 @@ static void srpt_get_ioc(struct srpt_port *sport, u32 slot,if (sdev->use_srq) send_queue_depth = sdev->srq_size;else - send_queue_depth =min (MAX_SRPT_RQ_SIZE , - sdev->device->attrs.max_qp_wr); + send_queue_depth =min (sdev->device->attrs.max_qp_wr,MAX_SRPT_RQ_SIZE );memset (iocp,0 ,sizeof (*iocp));strcpy (iocp->id_string,SRPT_ID_STRING );
@@ -1850,7 +1849,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)struct srpt_port *sport = ch->sport;struct srpt_device *sdev = sport->sdev;const struct ib_device_attr *attrs = &sdev->device->attrs; -int sq_size = sport->port_attrib.srp_sq_size; +u32 sq_size = sport->port_attrib.srp_sq_size;int i, ret;WARN_ON (ch->rq_size <1 );
@@ -1911,13 +1910,13 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)bool retry = sq_size >MIN_SRPT_SQ_SIZE ;if (retry) { -pr_debug ("failed to create queue pair with sq_size = %d (%d) - retrying\n" , +pr_debug ("failed to create queue pair with sq_size = %u (%d) - retrying\n" , sq_size, ret);ib_cq_pool_put (ch->cq, ch->cq_size); sq_size =max (sq_size /2 ,MIN_SRPT_SQ_SIZE ); goto retry; }else { -pr_err ("failed to create queue pair with sq_size = %d (%d)\n" , +pr_err ("failed to create queue pair with sq_size = %u (%d)\n" , sq_size, ret); goto err_destroy_cq; }
@@ -1925,7 +1924,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)atomic_set (&ch->sq_wr_avail, qp_init->cap.max_send_wr); -pr_debug ("%s: max_cqe= %d max_sge= %d sq_size = %d ch= %p\n" , +pr_debug ("%s: max_cqe= %d max_sge= %d sq_size = %u ch= %p\n" , __func__, ch->cq->cqe, qp_init->cap.max_send_sge, qp_init->cap.max_send_wr, ch);
@@ -2298,7 +2297,7 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev,* depth to avoid that the initiator driver has to report QUEUE_FULL * to the SCSI mid-layer. */ - ch->rq_size =min (MAX_SRPT_RQ_SIZE , sdev->device->attrs.max_qp_wr); + ch->rq_size =min (sdev->device->attrs.max_qp_wr,MAX_SRPT_RQ_SIZE );spin_lock_init (&ch->spinlock); ch->state =CH_CONNECTING ;INIT_LIST_HEAD (&ch->cmd_wait_list);
@@ -3136,7 +3135,7 @@ static int srpt_alloc_srq(struct srpt_device *sdev)return PTR_ERR (srq); } -pr_debug ("create SRQ #wr= %d max_allow=%d dev= %s\n" , sdev->srq_size, +pr_debug ("create SRQ #wr= %d max_allow=%u dev= %s\n" , sdev->srq_size, sdev->device->attrs.max_srq_wr,dev_name (&device->dev)); sdev->req_buf_cache =srpt_cache_get (srp_max_req_size);
@@ -3951,7 +3950,7 @@ static int __init srpt_init_module(void)if (srpt_srq_size <MIN_SRPT_SRQ_SIZE || srpt_srq_size >MAX_SRPT_SRQ_SIZE ) { -pr_err ("invalid value %d for kernel module parameter srpt_srq_size -- must be in the range [%d..%d].\n" , +pr_err ("invalid value %u for kernel module parameter srpt_srq_size -- must be in the range [%d..%d].\n" , srpt_srq_size,MIN_SRPT_SRQ_SIZE ,MAX_SRPT_SRQ_SIZE ); goto out; }
drivers/nvme/host/rdma.c +5 -3
@@ -394,8 +394,10 @@ nvme_rdma_find_get_device(struct rdma_cm_id *cm_id) goto out_free_pd; } - ndev->num_inline_segments =min (NVME_RDMA_MAX_INLINE_SEGMENTS , - ndev->dev->attrs.max_send_sge -1 ); + ndev->num_inline_segments = ndev->dev->attrs.max_send_sge; +if (ndev->num_inline_segments) + ndev->num_inline_segments--; + ndev->num_inline_segments =min (ndev->num_inline_segments,NVME_RDMA_MAX_INLINE_SEGMENTS );list_add (&ndev->entry, &device_list); out_unlock:mutex_unlock (&device_list_mutex);
@@ -1847,7 +1849,7 @@ static int nvme_rdma_route_resolved(struct nvme_rdma_queue *queue) param.qp_num = queue->qp->qp_num; param.flow_control =1 ; - param.responder_resources = queue->device->dev->attrs.max_qp_rd_atom; + param.responder_resources =min (queue->device->dev->attrs.max_qp_rd_atom,U8_MAX );/* maximum retry count */ param.retry_count =7 ; param.rnr_retry_count =7 ;
drivers/nvme/target/rdma.c +8 -5
@@ -152,7 +152,7 @@ static const struct kernel_param_ops srq_size_ops = { .get = param_get_int, }; -static int nvmet_rdma_srq_size =1024 ; +static unsigned int nvmet_rdma_srq_size =1024 ;module_param_cb (srq_size, &srq_size_ops, &nvmet_rdma_srq_size,0644 );MODULE_PARM_DESC (srq_size,"set Shared Receive Queue (SRQ) size, should >= 256 (default: 1024)" );
@@ -1197,7 +1197,7 @@ nvmet_rdma_find_get_device(struct rdma_cm_id *cm_id)struct nvmet_port *nport = port->nport;struct nvmet_rdma_device *ndev;int inline_page_count; -int inline_sge_count; +u32 inline_sge_count;int ret;mutex_lock (&device_list_mutex);
@@ -1213,7 +1213,9 @@ nvmet_rdma_find_get_device(struct rdma_cm_id *cm_id) inline_page_count =num_pages (nport->inline_data_size); inline_sge_count =max (cm_id->device->attrs.max_sge_rd, - cm_id->device->attrs.max_recv_sge) -1 ; + cm_id->device->attrs.max_recv_sge); +if (inline_sge_count) + inline_sge_count--;if (inline_page_count > inline_sge_count) { pr_warn("inline_data_size %d cannot be supported by device %s. Reducing to %lu.\n", nport->inline_data_size, cm_id->device->name,
@@ -1553,8 +1555,9 @@ static int nvmet_rdma_cm_accept(struct rdma_cm_id *cm_id, param.rnr_retry_count =7 ; param.flow_control =1 ; - param.initiator_depth =min_t (u8, p->initiator_depth, - queue->dev->device->attrs.max_qp_init_rd_atom); + param.initiator_depth =min3 (p->initiator_depth, + queue->dev->device->attrs.max_qp_init_rd_atom, +U8_MAX ); param.private_data = &priv; param.private_data_len =sizeof (priv); priv.recfmt =cpu_to_le16 (NVME_RDMA_CM_FMT_1_0 );
fs/smb/smbdirect/accept.c +3 -2
@@ -32,8 +32,9 @@ int smbdirect_accept_connect_request(struct smbdirect_socket *sc,/* * First set what the we as server are able to support */ - sp->initiator_depth =min_t (u8, sp->initiator_depth, - sc->ib.dev->attrs.max_qp_rd_atom); + sp->initiator_depth =min3 (sp->initiator_depth, + sc->ib.dev->attrs.max_qp_rd_atom, +U8_MAX ); peer_initiator_depth = param->initiator_depth; peer_responder_resources = param->responder_resources;
fs/smb/smbdirect/connect.c +3 -2
@@ -182,8 +182,9 @@ static int smbdirect_connect_rdma_connect(struct smbdirect_socket *sc)if (sc->ib.dev->attrs.kernel_cap_flags &IBK_SG_GAPS_REG ) sc->mr_io.type =IB_MR_TYPE_SG_GAPS ; - sp->responder_resources =min_t (u8, sp->responder_resources, - sc->ib.dev->attrs.max_qp_rd_atom); + sp->responder_resources =min3 (sp->responder_resources, + sc->ib.dev->attrs.max_qp_rd_atom, +U8_MAX );smbdirect_log_rdma_mr (sc,SMBDIRECT_LOG_INFO ,"responder_resources=%d\n" , sp->responder_resources);
fs/smb/smbdirect/connection.c +4 -4
@@ -287,7 +287,7 @@ int smbdirect_connection_create_qp(struct smbdirect_socket *sc) qp_cap.max_send_wr > sc->ib.dev->attrs.max_qp_wr) {pr_err ("Possible CQE overrun: max_send_wr %d\n" , qp_cap.max_send_wr); - pr_err("device %.*s reporting max_cqe %d max_qp_wr %d\n", + pr_err("device %.*s reporting max_cqe %u max_qp_wr %u\n", IB_DEVICE_NAME_MAX, sc->ib.dev->name, sc->ib.dev->attrs.max_cqe,
@@ -302,7 +302,7 @@ int smbdirect_connection_create_qp(struct smbdirect_socket *sc) max_send_wr >= sc->ib.dev->attrs.max_qp_wr)) {pr_err ("Possible CQE overrun: rdma_send_wr %d + max_send_wr %d = %d\n" , rdma_send_wr, qp_cap.max_send_wr, max_send_wr); - pr_err("device %.*s reporting max_cqe %d max_qp_wr %d\n", + pr_err("device %.*s reporting max_cqe %u max_qp_wr %u\n", IB_DEVICE_NAME_MAX, sc->ib.dev->name, sc->ib.dev->attrs.max_cqe,
@@ -316,7 +316,7 @@ int smbdirect_connection_create_qp(struct smbdirect_socket *sc) qp_cap.max_recv_wr > sc->ib.dev->attrs.max_qp_wr) {pr_err ("Possible CQE overrun: max_recv_wr %d\n" , qp_cap.max_recv_wr); - pr_err("device %.*s reporting max_cqe %d max_qp_wr %d\n", + pr_err("device %.*s reporting max_cqe %u max_qp_wr %u\n", IB_DEVICE_NAME_MAX, sc->ib.dev->name, sc->ib.dev->attrs.max_cqe,
@@ -328,7 +328,7 @@ int smbdirect_connection_create_qp(struct smbdirect_socket *sc)if (qp_cap.max_send_sge > sc->ib.dev->attrs.max_send_sge || qp_cap.max_recv_sge > sc->ib.dev->attrs.max_recv_sge) { - pr_err("device %.*s max_send_sge/max_recv_sge = %d/%d too small\n", + pr_err("device %.*s max_send_sge/max_recv_sge = %u/%u too small\n", IB_DEVICE_NAME_MAX, sc->ib.dev->name, sc->ib.dev->attrs.max_send_sge,
include/linux/sunrpc/svc_rdma.h +2 -2
@@ -78,8 +78,8 @@ struct svcxprt_rdma {struct rdma_cm_id *sc_cm_id;/* RDMA connection id */ struct list_head sc_accept_q;/* Conn. waiting accept */ struct rpcrdma_notification sc_rn;/* removal notification */ -int sc_ord;/* RDMA read limit */ -int sc_max_send_sges; +u32 sc_ord;/* RDMA read limit */ +unsigned int sc_max_send_sges;bool sc_snd_w_inv;/* OK to use Send With Invalidate */ atomic_t sc_sq_avail;/* SQEs ready to be consumed */
include/rdma/ib_verbs.h +25 -25
61 lines not on this page · raw message
net/rds/ib.c +5 -5
@@ -162,12 +162,12 @@ static int rds_ib_add_one(struct ib_device *device) IB_ODP_SUPPORT_READ); rds_ibdev->max_1m_mrs = device->attrs.max_mr ? - min_t(unsigned int, (device->attrs.max_mr / 2), - rds_ib_mr_1m_pool_size) : rds_ib_mr_1m_pool_size; +min (device->attrs.max_mr /2 , + rds_ib_mr_1m_pool_size) : rds_ib_mr_1m_pool_size; rds_ibdev->max_8k_mrs = device->attrs.max_mr ? - min_t(unsigned int, ((device->attrs.max_mr / 2) * RDS_MR_8K_SCALE), - rds_ib_mr_8k_pool_size) : rds_ib_mr_8k_pool_size; +min ((device->attrs.max_mr /2 ) *RDS_MR_8K_SCALE , + rds_ib_mr_8k_pool_size) : rds_ib_mr_8k_pool_size; rds_ibdev->max_initiator_depth = device->attrs.max_qp_init_rd_atom; rds_ibdev->max_responder_resources = device->attrs.max_qp_rd_atom;
@@ -204,7 +204,7 @@ static int rds_ib_add_one(struct ib_device *device) goto put_dev; } -rdsdebug ("RDS/IB: max_mr = %d, max_wrs = %d, max_sge = %d, max_1m_mrs = %d, max_8k_mrs = %d\n" , +rdsdebug ("RDS/IB: max_mr = %u, max_wrs = %d, max_sge = %d, max_1m_mrs = %d, max_8k_mrs = %d\n" , device->attrs.max_mr, rds_ibdev->max_wrs, rds_ibdev->max_sge, rds_ibdev->max_1m_mrs, rds_ibdev->max_8k_mrs);
net/rds/ib_cm.c +5 -5
@@ -173,11 +173,11 @@ static void rds_ib_cm_fill_conn_param(struct rds_connection *conn,memset (conn_param,0 ,sizeof (struct rdma_conn_param )); - conn_param->responder_resources = -min_t (u32, rds_ibdev->max_responder_resources, max_responder_resources); - conn_param->initiator_depth = -min_t (u32, rds_ibdev->max_initiator_depth, max_initiator_depth); - conn_param->retry_count = min_t(unsigned int, rds_ib_retry_count, 7); + conn_param->responder_resources =min3 (rds_ibdev->max_responder_resources, + max_responder_resources,U8_MAX ); + conn_param->initiator_depth =min3 (rds_ibdev->max_initiator_depth, + max_initiator_depth,U8_MAX ); + conn_param->retry_count =min (rds_ib_retry_count,7U ); conn_param->rnr_retry_count =7 ;if (dp) {
net/sunrpc/xprtrdma/frwr_ops.c +4 -3
@@ -172,8 +172,9 @@ int frwr_mr_init(struct rpcrdma_xprt *r_xprt, struct rpcrdma_mr *mr)int frwr_query_device (struct rpcrdma_ep *ep,const struct ib_device *device) {const struct ib_device_attr *attrs = &device->attrs; -int max_qp_wr, depth, delta;unsigned int max_sge; +u32 max_qp_wr; +int depth, delta;if (!(attrs->device_cap_flags &IB_DEVICE_MEM_MGT_EXTENSIONS ) || attrs->max_fast_reg_page_list_len ==0 ) {
@@ -229,10 +230,10 @@ int frwr_query_device(struct rpcrdma_ep *ep, const struct ib_device *device) } max_qp_wr = attrs->max_qp_wr; +if (max_qp_wr <RPCRDMA_BACKWARD_WRS +1 +RPCRDMA_MIN_SLOT_TABLE ) +return -ENOMEM ; max_qp_wr -=RPCRDMA_BACKWARD_WRS ; max_qp_wr -=1 ; -if (max_qp_wr <RPCRDMA_MIN_SLOT_TABLE ) -return -ENOMEM ;if (ep->re_max_requests > max_qp_wr) ep->re_max_requests = max_qp_wr; ep->re_attr.cap.max_send_wr = ep->re_max_requests * depth;
net/sunrpc/xprtrdma/svc_rdma_transport.c +2 -3
@@ -544,8 +544,7 @@ static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt)set_bit (RDMAXPRT_CONN_PENDING , &newxprt->sc_flags);memset (&conn_param,0 ,sizeof conn_param); conn_param.responder_resources =0 ; - conn_param.initiator_depth =min_t (int, newxprt->sc_ord, - dev->attrs.max_qp_init_rd_atom); + conn_param.initiator_depth =min (newxprt->sc_ord, dev->attrs.max_qp_init_rd_atom);if (!conn_param.initiator_depth) { ret = -EINVAL ;trace_svcrdma_initdepth_err (newxprt, ret);
@@ -570,7 +569,7 @@ static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt)dprintk (" local address : %pIS:%u\n" , sap,rpc_get_port (sap)); sap = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;dprintk (" remote address : %pIS:%u\n" , sap,rpc_get_port (sap)); -dprintk (" max_sge : %d\n" , newxprt->sc_max_send_sges); +dprintk (" max_sge : %u\n" , newxprt->sc_max_send_sges);dprintk (" sq_depth : %d\n" , newxprt->sc_sq_depth);dprintk (" rdma_rw_ctxs : %d\n" , ctxts);dprintk (" max_requests : %d\n" , newxprt->sc_max_requests);
net/sunrpc/xprtrdma/verbs.c +1 -1
@@ -453,7 +453,7 @@ static int rpcrdma_ep_create(struct rpcrdma_xprt *r_xprt)/* Client offers RDMA Read but does not initiate */ ep->re_remote_cma.initiator_depth =0 ; ep->re_remote_cma.responder_resources = -min_t (int,U8_MAX , device->attrs.max_qp_rd_atom); +min (device->attrs.max_qp_rd_atom,U8_MAX );/* Limit transport retries so client can detect server * GID changes quickly. RPC layer handles re-establishing
Signature · 1 line
2.34.1
Quoted 12 lines · Erni Sri Satya Vennela
The capability counter fields in struct
ib_device_attrare declared as signed int, but these values are inherently non-negative. Drivers maintain their cached caps as u32 and assign them directly into these int fields; if a cap exceedsINT_MAXthe implicit narrowing yields a negative value visible to the IB core.Change the signed int capability fields to u32 to match the underlying nature of the data. Also update consumers across the IB core, ULPs, NVMe-oF target, RDS, and NFS/RDMA so the new u32 values are not forced back through signed int or u8 via min()/min_t() or narrowing local variables.
Quoted 3 lines
Suggested-by Jason Gunthorpe <[email protected]>Signed-off-by Erni Sri Satya Vennela <[email protected]>
Hi,
Just a friendly follow-up on this patch. The Sashiko review mentioned a low-priority item, and I'd appreciate any guidance on whether the change is needed.
https://sashiko.dev/#/patchset/20260619203107.606359-1-ernis%40linux.microsoft.com
Thanks,
Vennela
Quoted 13 lines · Erni Sri Satya Vennela
Quoted 11 lines · Erni Sri Satya Vennela
The capability counter fields in struct
ib_device_attrare declared as signed int, but these values are inherently non-negative. Drivers maintain their cached caps as u32 and assign them directly into these int fields; if a cap exceedsINT_MAXthe implicit narrowing yields a negative value visible to the IB core.Change the signed int capability fields to u32 to match the underlying nature of the data. Also update consumers across the IB core, ULPs, NVMe-oF target, RDS, and NFS/RDMA so the new u32 values are not forced back through signed int or u8 via min()/min_t() or narrowing local variables.
Quoted 3 lines
Just a friendly follow-up on this patch. The Sashiko review mentioned a low-priority item, and I'd appreciate any guidance on whether the change is needed.
As I read this it needs to be fixed, the problem is that there is a potential of a weird case (not sure they are IRL) when somebody can use INT_MIN (in representation of signed number) to actually mean 0x80000000 size.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 18 lines · Andy Shevchenko
Quoted 13 lines · Erni Sri Satya Vennela
Quoted 11 lines · Erni Sri Satya Vennela
The capability counter fields in struct
ib_device_attrare declared as signed int, but these values are inherently non-negative. Drivers maintain their cached caps as u32 and assign them directly into these int fields; if a cap exceedsINT_MAXthe implicit narrowing yields a negative value visible to the IB core.Change the signed int capability fields to u32 to match the underlying nature of the data. Also update consumers across the IB core, ULPs, NVMe-oF target, RDS, and NFS/RDMA so the new u32 values are not forced back through signed int or u8 via min()/min_t() or narrowing local variables.
Quoted 3 lines
Just a friendly follow-up on this patch. The Sashiko review mentioned a low-priority item, and I'd appreciate any guidance on whether the change is needed.
Quoted 11 lines
As I read this it needs to be fixed, the problem is that there is a potential of a weird case (not sure they are IRL) when somebody can use
INT_MIN(in representation of signed number) to actually mean0x80000000size.Signature · 2 lines
With Best Regards, Andy Shevchenko
Thankyou for the confirmation, Andy.
I'll add this change in the next version.
- Vennela