From 5a301bb9f1b33764384f25b749e30091a5abefce Mon Sep 17 00:00:00 2001 From: Kamal Heib Date: Wed, 9 Jan 2019 15:28:29 +0200 Subject: [PATCH 01/10] docs/pvrdma: Update rdmacm-mux documentation Before running the rdmacm-mux need to make sure that both the ib_cm and rdma_cm kernel modules are unloaded. Reviewed-by: Yuval Shaia Signed-off-by: Kamal Heib Message-Id: <20190109132829.19164-1-kamalheib1@gmail.com> Signed-off-by: Marcel Apfelbaum --- docs/pvrdma.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/pvrdma.txt b/docs/pvrdma.txt index 5175251b47..81959378b1 100644 --- a/docs/pvrdma.txt +++ b/docs/pvrdma.txt @@ -99,6 +99,9 @@ MAD layer to send and receive RDMA-CM MAD packets. To build rdmacm-mux run # make rdmacm-mux +Before running the rdmacm-mux make sure that both ib_cm and rdma_cm kernel +modules aren't loaded, otherwise the rdmacm-mux service will fail to start. + The application accepts 3 command line arguments and exposes a UNIX socket to pass control and data to it. -d rdma-device-name Name of RDMA device to register with From ffef47754a1fdae2fdcea267bd98172dc5a877c5 Mon Sep 17 00:00:00 2001 From: Yuval Shaia Date: Wed, 9 Jan 2019 21:41:23 +0200 Subject: [PATCH 02/10] hw/pvrdma: Remove max-sge command-line param This parameter has no effect, fix it. The function init_dev_caps sets the front-end's max-sge to MAX_SGE. Then it checks backend's max-sge and adjust it accordingly (we can't send more than what the device supports). On send and recv we need to make sure the num_sge in the WQE does not exceeds the backend device capability. This check is done in pvrdma level so check on rdma level is deleted. Signed-off-by: Yuval Shaia Message-Id: <20190109194123.3468-1-yuval.shaia@oracle.com> Reviewed-by: Marcel Apfelbaum Signed-off-by: Marcel Apfelbaum --- docs/pvrdma.txt | 1 - hw/rdma/rdma_backend.c | 23 ++--------------------- hw/rdma/rdma_backend.h | 11 +++++++++++ hw/rdma/vmw/pvrdma_main.c | 10 +++++----- hw/rdma/vmw/pvrdma_qp_ops.c | 24 ++++++++++++++++++++++++ 5 files changed, 42 insertions(+), 27 deletions(-) diff --git a/docs/pvrdma.txt b/docs/pvrdma.txt index 81959378b1..0f0dd8a7e5 100644 --- a/docs/pvrdma.txt +++ b/docs/pvrdma.txt @@ -156,7 +156,6 @@ Ethernet function can be used for other Ethernet purposes such as IP. specify the port to use. If not set 1 will be used. - dev-caps-max-mr-size: The maximum size of MR. - dev-caps-max-qp: Maximum number of QPs. -- dev-caps-max-sge: Maximum number of SGE elements in WR. - dev-caps-max-cq: Maximum number of CQs. - dev-caps-max-mr: Maximum number of MRs. - dev-caps-max-pd: Maximum number of PDs. diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c index c28bfbd44d..16dca69ee9 100644 --- a/hw/rdma/rdma_backend.c +++ b/hw/rdma/rdma_backend.c @@ -32,17 +32,6 @@ #include "rdma_rm.h" #include "rdma_backend.h" -/* Vendor Errors */ -#define VENDOR_ERR_FAIL_BACKEND 0x201 -#define VENDOR_ERR_TOO_MANY_SGES 0x202 -#define VENDOR_ERR_NOMEM 0x203 -#define VENDOR_ERR_QP0 0x204 -#define VENDOR_ERR_INV_NUM_SGE 0x205 -#define VENDOR_ERR_MAD_SEND 0x206 -#define VENDOR_ERR_INVLKEY 0x207 -#define VENDOR_ERR_MR_SMALL 0x208 -#define VENDOR_ERR_INV_MAD_BUFF 0x209 - #define THR_NAME_LEN 16 #define THR_POLL_TO 5000 @@ -475,11 +464,6 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev, } pr_dbg("num_sge=%d\n", num_sge); - if (!num_sge || num_sge > MAX_SGE) { - pr_dbg("invalid num_sge=%d\n", num_sge); - complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_INV_NUM_SGE, ctx); - return; - } bctx = g_malloc0(sizeof(*bctx)); bctx->up_ctx = ctx; @@ -602,11 +586,6 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev, } pr_dbg("num_sge=%d\n", num_sge); - if (!num_sge || num_sge > MAX_SGE) { - pr_dbg("invalid num_sge=%d\n", num_sge); - complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_INV_NUM_SGE, ctx); - return; - } bctx = g_malloc0(sizeof(*bctx)); bctx->up_ctx = ctx; @@ -942,6 +921,8 @@ static int init_device_caps(RdmaBackendDev *backend_dev, return -EIO; } + dev_attr->max_sge = MAX_SGE; + CHK_ATTR(dev_attr, backend_dev->dev_attr, max_mr_size, "%" PRId64); CHK_ATTR(dev_attr, backend_dev->dev_attr, max_qp, "%d"); CHK_ATTR(dev_attr, backend_dev->dev_attr, max_sge, "%d"); diff --git a/hw/rdma/rdma_backend.h b/hw/rdma/rdma_backend.h index 8cae40f827..a9ba40ae48 100644 --- a/hw/rdma/rdma_backend.h +++ b/hw/rdma/rdma_backend.h @@ -22,6 +22,17 @@ #include "rdma_rm_defs.h" #include "rdma_backend_defs.h" +/* Vendor Errors */ +#define VENDOR_ERR_FAIL_BACKEND 0x201 +#define VENDOR_ERR_TOO_MANY_SGES 0x202 +#define VENDOR_ERR_NOMEM 0x203 +#define VENDOR_ERR_QP0 0x204 +#define VENDOR_ERR_INV_NUM_SGE 0x205 +#define VENDOR_ERR_MAD_SEND 0x206 +#define VENDOR_ERR_INVLKEY 0x207 +#define VENDOR_ERR_MR_SMALL 0x208 +#define VENDOR_ERR_INV_MAD_BUFF 0x209 + /* Add definition for QP0 and QP1 as there is no userspace enums for them */ enum ibv_special_qp_type { IBV_QPT_SMI = 0, diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c index 838ad8a949..d2bdb5ba8c 100644 --- a/hw/rdma/vmw/pvrdma_main.c +++ b/hw/rdma/vmw/pvrdma_main.c @@ -43,7 +43,6 @@ static Property pvrdma_dev_properties[] = { DEFINE_PROP_UINT64("dev-caps-max-mr-size", PVRDMADev, dev_attr.max_mr_size, MAX_MR_SIZE), DEFINE_PROP_INT32("dev-caps-max-qp", PVRDMADev, dev_attr.max_qp, MAX_QP), - DEFINE_PROP_INT32("dev-caps-max-sge", PVRDMADev, dev_attr.max_sge, MAX_SGE), DEFINE_PROP_INT32("dev-caps-max-cq", PVRDMADev, dev_attr.max_cq, MAX_CQ), DEFINE_PROP_INT32("dev-caps-max-mr", PVRDMADev, dev_attr.max_mr, MAX_MR), DEFINE_PROP_INT32("dev-caps-max-pd", PVRDMADev, dev_attr.max_pd, MAX_PD), @@ -549,8 +548,9 @@ static void init_dev_caps(PVRDMADev *dev) sizeof(struct pvrdma_rq_wqe_hdr)); dev->dev_attr.max_qp_wr = pg_tbl_bytes / - (wr_sz + sizeof(struct pvrdma_sge) * MAX_SGE) - - TARGET_PAGE_SIZE; /* First page is ring state */ + (wr_sz + sizeof(struct pvrdma_sge) * + dev->dev_attr.max_sge) - TARGET_PAGE_SIZE; + /* First page is ring state ^^^^ */ pr_dbg("max_qp_wr=%d\n", dev->dev_attr.max_qp_wr); dev->dev_attr.max_cqe = pg_tbl_bytes / sizeof(struct pvrdma_cqe) - @@ -626,8 +626,6 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp) init_regs(pdev); - init_dev_caps(dev); - rc = init_msix(pdev, errp); if (rc) { goto out; @@ -640,6 +638,8 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp) goto out; } + init_dev_caps(dev); + rc = rdma_rm_init(&dev->rdma_dev_res, &dev->dev_attr, errp); if (rc) { goto out; diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c index 300471a4c9..465bee8641 100644 --- a/hw/rdma/vmw/pvrdma_qp_ops.c +++ b/hw/rdma/vmw/pvrdma_qp_ops.c @@ -121,6 +121,16 @@ static void pvrdma_qp_ops_comp_handler(void *ctx, struct ibv_wc *wc) g_free(ctx); } +static void complete_with_error(uint32_t vendor_err, void *ctx) +{ + struct ibv_wc wc = {0}; + + wc.status = IBV_WC_GENERAL_ERR; + wc.vendor_err = vendor_err; + + pvrdma_qp_ops_comp_handler(ctx, &wc); +} + void pvrdma_qp_ops_fini(void) { rdma_backend_unregister_comp_handler(); @@ -182,6 +192,13 @@ int pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle) return -EIO; } + if (wqe->hdr.num_sge > dev->dev_attr.max_sge) { + pr_dbg("Invalid num_sge=%d (max %d)\n", wqe->hdr.num_sge, + dev->dev_attr.max_sge); + complete_with_error(VENDOR_ERR_INV_NUM_SGE, comp_ctx); + continue; + } + rdma_backend_post_send(&dev->backend_dev, &qp->backend_qp, qp->qp_type, (struct ibv_sge *)&wqe->sge[0], wqe->hdr.num_sge, sgid_idx, sgid, @@ -227,6 +244,13 @@ int pvrdma_qp_recv(PVRDMADev *dev, uint32_t qp_handle) comp_ctx->cqe.qp = qp_handle; comp_ctx->cqe.opcode = IBV_WC_RECV; + if (wqe->hdr.num_sge > dev->dev_attr.max_sge) { + pr_dbg("Invalid num_sge=%d (max %d)\n", wqe->hdr.num_sge, + dev->dev_attr.max_sge); + complete_with_error(VENDOR_ERR_INV_NUM_SGE, comp_ctx); + continue; + } + rdma_backend_post_recv(&dev->backend_dev, &dev->rdma_dev_res, &qp->backend_qp, qp->qp_type, (struct ibv_sge *)&wqe->sge[0], wqe->hdr.num_sge, From 732d948ca52d1a91a0930acfd7610598d4e78903 Mon Sep 17 00:00:00 2001 From: Yuval Shaia Date: Wed, 9 Jan 2019 22:19:56 +0200 Subject: [PATCH 03/10] hw/rdma: Delete unused struct member MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This member is used only in init_device_caps function, make it local. Signed-off-by: Yuval Shaia Reviewed-by: Marcel Apfelbaum Reviewed-by: Philippe Mathieu-Daudé --- hw/rdma/rdma_backend.c | 26 ++++++++++++++------------ hw/rdma/rdma_backend_defs.h | 1 - 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c index 16dca69ee9..b49edaacaf 100644 --- a/hw/rdma/rdma_backend.c +++ b/hw/rdma/rdma_backend.c @@ -917,23 +917,25 @@ void rdma_backend_destroy_qp(RdmaBackendQP *qp) static int init_device_caps(RdmaBackendDev *backend_dev, struct ibv_device_attr *dev_attr) { - if (ibv_query_device(backend_dev->context, &backend_dev->dev_attr)) { + struct ibv_device_attr bk_dev_attr; + + if (ibv_query_device(backend_dev->context, &bk_dev_attr)) { return -EIO; } dev_attr->max_sge = MAX_SGE; - CHK_ATTR(dev_attr, backend_dev->dev_attr, max_mr_size, "%" PRId64); - CHK_ATTR(dev_attr, backend_dev->dev_attr, max_qp, "%d"); - CHK_ATTR(dev_attr, backend_dev->dev_attr, max_sge, "%d"); - CHK_ATTR(dev_attr, backend_dev->dev_attr, max_qp_wr, "%d"); - CHK_ATTR(dev_attr, backend_dev->dev_attr, max_cq, "%d"); - CHK_ATTR(dev_attr, backend_dev->dev_attr, max_cqe, "%d"); - CHK_ATTR(dev_attr, backend_dev->dev_attr, max_mr, "%d"); - CHK_ATTR(dev_attr, backend_dev->dev_attr, max_pd, "%d"); - CHK_ATTR(dev_attr, backend_dev->dev_attr, max_qp_rd_atom, "%d"); - CHK_ATTR(dev_attr, backend_dev->dev_attr, max_qp_init_rd_atom, "%d"); - CHK_ATTR(dev_attr, backend_dev->dev_attr, max_ah, "%d"); + CHK_ATTR(dev_attr, bk_dev_attr, max_mr_size, "%" PRId64); + CHK_ATTR(dev_attr, bk_dev_attr, max_qp, "%d"); + CHK_ATTR(dev_attr, bk_dev_attr, max_sge, "%d"); + CHK_ATTR(dev_attr, bk_dev_attr, max_qp_wr, "%d"); + CHK_ATTR(dev_attr, bk_dev_attr, max_cq, "%d"); + CHK_ATTR(dev_attr, bk_dev_attr, max_cqe, "%d"); + CHK_ATTR(dev_attr, bk_dev_attr, max_mr, "%d"); + CHK_ATTR(dev_attr, bk_dev_attr, max_pd, "%d"); + CHK_ATTR(dev_attr, bk_dev_attr, max_qp_rd_atom, "%d"); + CHK_ATTR(dev_attr, bk_dev_attr, max_qp_init_rd_atom, "%d"); + CHK_ATTR(dev_attr, bk_dev_attr, max_ah, "%d"); return 0; } diff --git a/hw/rdma/rdma_backend_defs.h b/hw/rdma/rdma_backend_defs.h index 1e5c3dd3bf..15ae8b970e 100644 --- a/hw/rdma/rdma_backend_defs.h +++ b/hw/rdma/rdma_backend_defs.h @@ -41,7 +41,6 @@ typedef struct RdmaCmMux { } RdmaCmMux; typedef struct RdmaBackendDev { - struct ibv_device_attr dev_attr; RdmaBackendThread comp_thread; PCIDevice *dev; RdmaDeviceResources *rdma_dev_res; From 26fd869556bb9513bf82ad9573c906b2a631576e Mon Sep 17 00:00:00 2001 From: Yuval Shaia Date: Wed, 9 Jan 2019 22:15:59 +0200 Subject: [PATCH 04/10] hw/pvrdma: Post CQE when receive invalid gid index This error should propagate back to guest. Spotted by Coverity: CID 1398595 Fixes: 2b05705dc8 Reported-by: Li Qiang Signed-off-by: Yuval Shaia Reviewed-by: Marcel Apfelbaum --- hw/rdma/rdma_backend.h | 1 + hw/rdma/vmw/pvrdma_qp_ops.c | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/rdma/rdma_backend.h b/hw/rdma/rdma_backend.h index a9ba40ae48..5114c90e67 100644 --- a/hw/rdma/rdma_backend.h +++ b/hw/rdma/rdma_backend.h @@ -32,6 +32,7 @@ #define VENDOR_ERR_INVLKEY 0x207 #define VENDOR_ERR_MR_SMALL 0x208 #define VENDOR_ERR_INV_MAD_BUFF 0x209 +#define VENDOR_ERR_INV_GID_IDX 0x210 /* Add definition for QP0 and QP1 as there is no userspace enums for them */ enum ibv_special_qp_type { diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c index 465bee8641..0565eba981 100644 --- a/hw/rdma/vmw/pvrdma_qp_ops.c +++ b/hw/rdma/vmw/pvrdma_qp_ops.c @@ -178,7 +178,8 @@ int pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle) sgid = rdma_rm_get_gid(&dev->rdma_dev_res, wqe->hdr.wr.ud.av.gid_index); if (!sgid) { pr_dbg("Fail to get gid for idx %d\n", wqe->hdr.wr.ud.av.gid_index); - return -EIO; + complete_with_error(VENDOR_ERR_INV_GID_IDX, comp_ctx); + continue; } pr_dbg("sgid_id=%d, sgid=0x%llx\n", wqe->hdr.wr.ud.av.gid_index, sgid->global.interface_id); @@ -189,7 +190,8 @@ int pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle) if (sgid_idx <= 0) { pr_dbg("Fail to get bk sgid_idx for sgid_idx %d\n", wqe->hdr.wr.ud.av.gid_index); - return -EIO; + complete_with_error(VENDOR_ERR_INV_GID_IDX, comp_ctx); + continue; } if (wqe->hdr.num_sge > dev->dev_attr.max_sge) { From 5bb8b739879f255fb3587238611334584ddcd99b Mon Sep 17 00:00:00 2001 From: Yuval Shaia Date: Wed, 9 Jan 2019 22:21:40 +0200 Subject: [PATCH 05/10] hw/pvrdma: Make function pvrdma_qp_send/recv return void. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The functions handles errors internaly, callers have nothing to do with the return value. Signed-off-by: Yuval Shaia Message-Id: <20190109202140.4051-1-yuval.shaia@oracle.com> Reviewed-by: Marcel Apfelbaum Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Marcel Apfelbaum --- hw/rdma/vmw/pvrdma_qp_ops.c | 14 ++++++-------- hw/rdma/vmw/pvrdma_qp_ops.h | 4 ++-- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c index 0565eba981..ce5a60e184 100644 --- a/hw/rdma/vmw/pvrdma_qp_ops.c +++ b/hw/rdma/vmw/pvrdma_qp_ops.c @@ -143,7 +143,7 @@ int pvrdma_qp_ops_init(void) return 0; } -int pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle) +void pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle) { RdmaRmQP *qp; PvrdmaSqWqe *wqe; @@ -155,7 +155,8 @@ int pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle) qp = rdma_rm_get_qp(&dev->rdma_dev_res, qp_handle); if (unlikely(!qp)) { - return -EINVAL; + pr_dbg("Invalid qpn\n"); + return; } ring = (PvrdmaRing *)qp->opaque; @@ -212,11 +213,9 @@ int pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle) wqe = pvrdma_ring_next_elem_read(ring); } - - return 0; } -int pvrdma_qp_recv(PVRDMADev *dev, uint32_t qp_handle) +void pvrdma_qp_recv(PVRDMADev *dev, uint32_t qp_handle) { RdmaRmQP *qp; PvrdmaRqWqe *wqe; @@ -226,7 +225,8 @@ int pvrdma_qp_recv(PVRDMADev *dev, uint32_t qp_handle) qp = rdma_rm_get_qp(&dev->rdma_dev_res, qp_handle); if (unlikely(!qp)) { - return -EINVAL; + pr_dbg("Invalid qpn\n"); + return; } ring = &((PvrdmaRing *)qp->opaque)[1]; @@ -262,8 +262,6 @@ int pvrdma_qp_recv(PVRDMADev *dev, uint32_t qp_handle) wqe = pvrdma_ring_next_elem_read(ring); } - - return 0; } void pvrdma_cq_poll(RdmaDeviceResources *dev_res, uint32_t cq_handle) diff --git a/hw/rdma/vmw/pvrdma_qp_ops.h b/hw/rdma/vmw/pvrdma_qp_ops.h index ac46bf7fdf..31cb48ba29 100644 --- a/hw/rdma/vmw/pvrdma_qp_ops.h +++ b/hw/rdma/vmw/pvrdma_qp_ops.h @@ -20,8 +20,8 @@ int pvrdma_qp_ops_init(void); void pvrdma_qp_ops_fini(void); -int pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle); -int pvrdma_qp_recv(PVRDMADev *dev, uint32_t qp_handle); +void pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle); +void pvrdma_qp_recv(PVRDMADev *dev, uint32_t qp_handle); void pvrdma_cq_poll(RdmaDeviceResources *dev_res, uint32_t cq_handle); #endif From a1aa88b7dcac2b22d74908f9276d5d145d8e65a0 Mon Sep 17 00:00:00 2001 From: Yuval Shaia Date: Wed, 16 Jan 2019 17:15:38 +0200 Subject: [PATCH 06/10] hw/rdma: Verify that ptr is not NULL before freeing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To cover the case where fini() was called even when init() fails make sure objects are not NULL before calling to non-null-safe destructors. Signed-off-by: Yuval Shaia Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20190116151538.14088-1-yuval.shaia@oracle.com> Signed-off-by: Marcel Apfelbaum --- hw/rdma/rdma_backend.c | 6 ++++-- hw/rdma/rdma_rm.c | 7 ++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c index b49edaacaf..3ee5172c96 100644 --- a/hw/rdma/rdma_backend.c +++ b/hw/rdma/rdma_backend.c @@ -1066,8 +1066,10 @@ static void mad_fini(RdmaBackendDev *backend_dev) pr_dbg("Stopping MAD\n"); disable_rdmacm_mux_async(backend_dev); qemu_chr_fe_disconnect(backend_dev->rdmacm_mux.chr_be); - qlist_destroy_obj(QOBJECT(backend_dev->recv_mads_list.list)); - qemu_mutex_destroy(&backend_dev->recv_mads_list.lock); + if (backend_dev->recv_mads_list.list) { + qlist_destroy_obj(QOBJECT(backend_dev->recv_mads_list.list)); + qemu_mutex_destroy(&backend_dev->recv_mads_list.lock); + } } int rdma_backend_get_gid_index(RdmaBackendDev *backend_dev, diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c index f5b1295890..8bf241e91f 100644 --- a/hw/rdma/rdma_rm.c +++ b/hw/rdma/rdma_rm.c @@ -41,6 +41,9 @@ static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl, static inline void res_tbl_free(RdmaRmResTbl *tbl) { + if (!tbl->bitmap) { + return; + } qemu_mutex_destroy(&tbl->lock); g_free(tbl->tbl); g_free(tbl->bitmap); @@ -655,5 +658,7 @@ void rdma_rm_fini(RdmaDeviceResources *dev_res, RdmaBackendDev *backend_dev, res_tbl_free(&dev_res->cq_tbl); res_tbl_free(&dev_res->pd_tbl); - g_hash_table_destroy(dev_res->qp_hash); + if (dev_res->qp_hash) { + g_hash_table_destroy(dev_res->qp_hash); + } } From a5fe209d71f311afb0035ce39e9472f9b31445e9 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Thu, 3 Jan 2019 05:12:51 -0800 Subject: [PATCH 07/10] hw: rdma: fix an off-by-one issue In rdma_rm_get_backend_gid_index(), the 'sgid_idx' is used to index the array 'dev_res->port.gid_tbl' which size is MAX_PORT_GIDS. Current the 'sgid_idx' may be MAX_PORT_GIDS thus cause an off-by-one issue. Spotted by Coverity: CID 1398594 Signed-off-by: Li Qiang Message-Id: <20190103131251.49271-1-liq3ea@163.com> Signed-off-by: Marcel Apfelbaum --- hw/rdma/rdma_rm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c index 8bf241e91f..268ff633a4 100644 --- a/hw/rdma/rdma_rm.c +++ b/hw/rdma/rdma_rm.c @@ -579,7 +579,7 @@ int rdma_rm_del_gid(RdmaDeviceResources *dev_res, RdmaBackendDev *backend_dev, int rdma_rm_get_backend_gid_index(RdmaDeviceResources *dev_res, RdmaBackendDev *backend_dev, int sgid_idx) { - if (unlikely(sgid_idx < 0 || sgid_idx > MAX_PORT_GIDS)) { + if (unlikely(sgid_idx < 0 || sgid_idx >= MAX_PORT_GIDS)) { pr_dbg("Got invalid sgid_idx %d\n", sgid_idx); return -EINVAL; } From 4a3d55a097695e17a3a39efc0434db95501241b2 Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Fri, 18 Jan 2019 14:46:12 +0200 Subject: [PATCH 08/10] contrib/rdmacm-mux: remove Wno-format-truncation flag The flag is not recognized by some CLANG versions. Add proper constraints in code instead. Reviewed-by: Yuval Shaia Tested-by: Cornelia Huck Message-Id: <20190118124614.24548-2-marcel.apfelbaum@gmail.com> Signed-off-by: Marcel Apfelbaum --- contrib/rdmacm-mux/Makefile.objs | 2 +- contrib/rdmacm-mux/main.c | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/contrib/rdmacm-mux/Makefile.objs b/contrib/rdmacm-mux/Makefile.objs index be3eacb6f7..e1ff4fe569 100644 --- a/contrib/rdmacm-mux/Makefile.objs +++ b/contrib/rdmacm-mux/Makefile.objs @@ -1,4 +1,4 @@ ifdef CONFIG_PVRDMA -CFLAGS += -libumad -Wno-format-truncation +CFLAGS += -libumad rdmacm-mux-obj-y = main.o endif diff --git a/contrib/rdmacm-mux/main.c b/contrib/rdmacm-mux/main.c index 835a7f9214..64676030c5 100644 --- a/contrib/rdmacm-mux/main.c +++ b/contrib/rdmacm-mux/main.c @@ -42,6 +42,8 @@ /* The below can be override by command line parameter */ #define UNIX_SOCKET_PATH "/var/run/rdmacm-mux" +/* Has format %s-%s-%d" -- */ +#define SOCKET_PATH_MAX (PATH_MAX - NAME_MAX - sizeof(int) - 2) #define RDMA_PORT_NUM 1 typedef struct RdmaCmServerArgs { @@ -95,7 +97,7 @@ static void help(const char *progname) static void parse_args(int argc, char *argv[]) { int c; - char unix_socket_path[PATH_MAX]; + char unix_socket_path[SOCKET_PATH_MAX]; strcpy(server.args.rdma_dev_name, ""); strcpy(unix_socket_path, UNIX_SOCKET_PATH); @@ -113,7 +115,7 @@ static void parse_args(int argc, char *argv[]) case 's': /* This is temporary, final name will build below */ - strncpy(unix_socket_path, optarg, PATH_MAX); + strncpy(unix_socket_path, optarg, SOCKET_PATH_MAX); break; case 'p': From 555b3d67bc64bbcc3005f6619ab42552616bd383 Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Fri, 18 Jan 2019 14:46:13 +0200 Subject: [PATCH 09/10] hw/rdma: modify struct initialization Do not initialize structs with {0} since some CLANG versions do not support it. Use {} construct instead. Reviewed-by: Yuval Shaia Tested-by: Cornelia Huck Message-Id: <20190118124614.24548-3-marcel.apfelbaum@gmail.com> Reviewed-by: Cornelia Huck Signed-off-by: Marcel Apfelbaum --- contrib/rdmacm-mux/main.c | 6 +++--- hw/rdma/rdma_backend.c | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/contrib/rdmacm-mux/main.c b/contrib/rdmacm-mux/main.c index 64676030c5..ae88c77a1e 100644 --- a/contrib/rdmacm-mux/main.c +++ b/contrib/rdmacm-mux/main.c @@ -350,7 +350,7 @@ static int get_fd(const char *mad, int *fd, __be64 *gid_ifid) static void *umad_recv_thread_func(void *args) { int rc; - RdmaCmMuxMsg msg = {0}; + RdmaCmMuxMsg msg = {}; int fd = -2; msg.hdr.msg_type = RDMACM_MUX_MSG_TYPE_REQ; @@ -387,7 +387,7 @@ static void *umad_recv_thread_func(void *args) static int read_and_process(int fd) { int rc; - RdmaCmMuxMsg msg = {0}; + RdmaCmMuxMsg msg = {}; struct umad_hdr *hdr; uint32_t *comm_id = 0; uint16_t attr_id; @@ -744,7 +744,7 @@ static void signal_handler(int sig, siginfo_t *siginfo, void *context) static int init(void) { int rc; - struct sigaction sig = {0}; + struct sigaction sig = {}; rc = init_listener(); if (rc) { diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c index 3ee5172c96..fd571f21e5 100644 --- a/hw/rdma/rdma_backend.c +++ b/hw/rdma/rdma_backend.c @@ -179,7 +179,7 @@ static inline int rdmacm_mux_can_process_async(RdmaBackendDev *backend_dev) static int check_mux_op_status(CharBackend *mad_chr_be) { - RdmaCmMuxMsg msg = {0}; + RdmaCmMuxMsg msg = {}; int ret; pr_dbg("Reading response\n"); @@ -376,7 +376,7 @@ static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res, static int mad_send(RdmaBackendDev *backend_dev, uint8_t sgid_idx, union ibv_gid *sgid, struct ibv_sge *sge, uint32_t num_sge) { - RdmaCmMuxMsg msg = {0}; + RdmaCmMuxMsg msg = {}; char *hdr, *data; int ret; @@ -1097,7 +1097,7 @@ int rdma_backend_get_gid_index(RdmaBackendDev *backend_dev, int rdma_backend_add_gid(RdmaBackendDev *backend_dev, const char *ifname, union ibv_gid *gid) { - RdmaCmMuxMsg msg = {0}; + RdmaCmMuxMsg msg = {}; int ret; pr_dbg("0x%llx, 0x%llx\n", @@ -1123,7 +1123,7 @@ int rdma_backend_add_gid(RdmaBackendDev *backend_dev, const char *ifname, int rdma_backend_del_gid(RdmaBackendDev *backend_dev, const char *ifname, union ibv_gid *gid) { - RdmaCmMuxMsg msg = {0}; + RdmaCmMuxMsg msg = {}; int ret; pr_dbg("0x%llx, 0x%llx\n", From 0f645ba16c6b76ccf2076d38460aa998198893bc Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Fri, 18 Jan 2019 14:46:14 +0200 Subject: [PATCH 10/10] contrib/rdmacm-mux: fix clang compilation Fix Commit a5d2f6f877 (contrib/rdmacm-mux: Add implementation of RDMA User MAD multiplexer). The above commit introduces a new contrib target, adding a global dependency to libumad library in case pvrdma configuration option is enabled. Clang forbids it: clang-6.0: error: -libumad: 'linker' input unused [-Werror,-Wunused-command-line-argument] Fix by limiting the scope to the rdmacm-mux target itself. Reported-by: Cornelia Huck Reviewed-by: Yuval Shaia Tested-by: Cornelia Huck Message-Id: <20190118124614.24548-4-marcel.apfelbaum@gmail.com> Signed-off-by: Marcel Apfelbaum --- Makefile | 2 ++ contrib/rdmacm-mux/Makefile.objs | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index dccba1dca2..c9bdb67274 100644 --- a/Makefile +++ b/Makefile @@ -581,6 +581,8 @@ vhost-user-scsi$(EXESUF): $(vhost-user-scsi-obj-y) libvhost-user.a $(call LINK, $^) vhost-user-blk$(EXESUF): $(vhost-user-blk-obj-y) libvhost-user.a $(call LINK, $^) + +rdmacm-mux$(EXESUF): LIBS += "-libumad" rdmacm-mux$(EXESUF): $(rdmacm-mux-obj-y) $(COMMON_LDADDS) $(call LINK, $^) diff --git a/contrib/rdmacm-mux/Makefile.objs b/contrib/rdmacm-mux/Makefile.objs index e1ff4fe569..3df744af89 100644 --- a/contrib/rdmacm-mux/Makefile.objs +++ b/contrib/rdmacm-mux/Makefile.objs @@ -1,4 +1,3 @@ ifdef CONFIG_PVRDMA -CFLAGS += -libumad rdmacm-mux-obj-y = main.o endif