From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f169.google.com (mail-wi0-f169.google.com [209.85.212.169]) by dpdk.org (Postfix) with ESMTP id 1B389924A for ; Mon, 5 Oct 2015 19:55:16 +0200 (CEST) Received: by wicge5 with SMTP id ge5so132048129wic.0 for ; Mon, 05 Oct 2015 10:55:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=RmPCnPnRIrv61lFLNHEZYN2HvuPC+G8dTVoLDPIxf1Y=; b=BqykdOo/58GlysHI0zpnV1mRrG/nNLs715uFMqGwFUH9bG6tn6Xi7dq6i1a0pTdhcw JOQ7oyLr4fTlgOQuJoEdtxiy/h71C1Q+DOMyNIi9dKauESpIOtIfCLOIFp/bQUnIschy MwBK3rmGHrCx65BqbFPQ2XYJG7SOafkeasMY2Cpp8Zx5aSI+se4+aYUncfcJ7xpkDMqB lU0vorLPlStZPzlpr4rgfMNSVM9fBGOkbayPOEMlcus11f+sCP1PYX8yO+9/OABKaSqG ZpCTxmgbwEIvluiHRpncwHRndazRAxtfwSLl7y5TuW9pGgVWmcdRwbVV34CZeZGCkAMJ zQ0w== X-Gm-Message-State: ALoCoQl2Ud2DhcMXLjR55Gs7nVxlg/I6eEUBSAcSF4iv3IMuNtCMkqzskUwYXknYJGj6lRVqEvRX X-Received: by 10.194.184.73 with SMTP id es9mr37864694wjc.122.1444067715979; Mon, 05 Oct 2015 10:55:15 -0700 (PDT) Received: from 6wind.com (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by smtp.gmail.com with ESMTPSA id 12sm28075675wjw.15.2015.10.05.10.55.14 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Mon, 05 Oct 2015 10:55:15 -0700 (PDT) From: Adrien Mazarguil To: dev@dpdk.org Date: Mon, 5 Oct 2015 19:54:37 +0200 Message-Id: <1444067692-29645-3-git-send-email-adrien.mazarguil@6wind.com> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1444067692-29645-1-git-send-email-adrien.mazarguil@6wind.com> References: <1444067692-29645-1-git-send-email-adrien.mazarguil@6wind.com> Subject: [dpdk-dev] [PATCH 02/17] mlx5: get rid of the WR structure in RX queue elements X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Oct 2015 17:55:16 -0000 Removing this structure reduces the size of SG and non-SG RX queue elements significantly to improve performance. An nice side effect is that the mbuf pointer is now fully stored in struct rxq_elt instead of relying on the WR ID data offset hack. Signed-off-by: Adrien Mazarguil Signed-off-by: Olga Shern Signed-off-by: Or Ami Signed-off-by: Nelio Laranjeiro --- drivers/net/mlx5/mlx5.h | 18 ----- drivers/net/mlx5/mlx5_rxq.c | 162 ++++++++++++++++++++---------------------- drivers/net/mlx5/mlx5_rxtx.c | 33 +++------ drivers/net/mlx5/mlx5_rxtx.h | 4 +- drivers/net/mlx5/mlx5_utils.h | 2 - 5 files changed, 87 insertions(+), 132 deletions(-) diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 459dc3d..a818703 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -118,24 +118,6 @@ struct priv { rte_spinlock_t lock; /* Lock for control functions. */ }; -/* Work Request ID data type (64 bit). */ -typedef union { - struct { - uint32_t id; - uint16_t offset; - } data; - uint64_t raw; -} wr_id_t; - -/* Compile-time check. */ -static inline void wr_id_t_check(void) -{ - wr_id_t check[1 + (2 * -!(sizeof(wr_id_t) == sizeof(uint64_t)))]; - - (void)check; - (void)wr_id_t_check; -} - /** * Lock private structure to protect it from concurrent access in the * control path. diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c index 8cfad17..c938d2d 100644 --- a/drivers/net/mlx5/mlx5_rxq.c +++ b/drivers/net/mlx5/mlx5_rxq.c @@ -97,16 +97,10 @@ rxq_alloc_elts_sp(struct rxq *rxq, unsigned int elts_n, for (i = 0; (i != elts_n); ++i) { unsigned int j; struct rxq_elt_sp *elt = &(*elts)[i]; - struct ibv_recv_wr *wr = &elt->wr; struct ibv_sge (*sges)[RTE_DIM(elt->sges)] = &elt->sges; /* These two arrays must have the same size. */ assert(RTE_DIM(elt->sges) == RTE_DIM(elt->bufs)); - /* Configure WR. */ - wr->wr_id = i; - wr->next = &(*elts)[(i + 1)].wr; - wr->sg_list = &(*sges)[0]; - wr->num_sge = RTE_DIM(*sges); /* For each SGE (segment). */ for (j = 0; (j != RTE_DIM(elt->bufs)); ++j) { struct ibv_sge *sge = &(*sges)[j]; @@ -149,8 +143,6 @@ rxq_alloc_elts_sp(struct rxq *rxq, unsigned int elts_n, assert(sge->length == rte_pktmbuf_tailroom(buf)); } } - /* The last WR pointer must be NULL. */ - (*elts)[(i - 1)].wr.next = NULL; DEBUG("%p: allocated and configured %u WRs (%zu segments)", (void *)rxq, elts_n, (elts_n * RTE_DIM((*elts)[0].sges))); rxq->elts_n = elts_n; @@ -242,7 +234,6 @@ rxq_alloc_elts(struct rxq *rxq, unsigned int elts_n, struct rte_mbuf **pool) /* For each WR (packet). */ for (i = 0; (i != elts_n); ++i) { struct rxq_elt *elt = &(*elts)[i]; - struct ibv_recv_wr *wr = &elt->wr; struct ibv_sge *sge = &(*elts)[i].sge; struct rte_mbuf *buf; @@ -258,16 +249,7 @@ rxq_alloc_elts(struct rxq *rxq, unsigned int elts_n, struct rte_mbuf **pool) ret = ENOMEM; goto error; } - /* Configure WR. Work request ID contains its own index in - * the elts array and the offset between SGE buffer header and - * its data. */ - WR_ID(wr->wr_id).id = i; - WR_ID(wr->wr_id).offset = - (((uintptr_t)buf->buf_addr + RTE_PKTMBUF_HEADROOM) - - (uintptr_t)buf); - wr->next = &(*elts)[(i + 1)].wr; - wr->sg_list = sge; - wr->num_sge = 1; + elt->buf = buf; /* Headroom is reserved by rte_pktmbuf_alloc(). */ assert(DATA_OFF(buf) == RTE_PKTMBUF_HEADROOM); /* Buffer is supposed to be empty. */ @@ -282,21 +264,7 @@ rxq_alloc_elts(struct rxq *rxq, unsigned int elts_n, struct rte_mbuf **pool) sge->lkey = rxq->mr->lkey; /* Redundant check for tailroom. */ assert(sge->length == rte_pktmbuf_tailroom(buf)); - /* Make sure elts index and SGE mbuf pointer can be deduced - * from WR ID. */ - if ((WR_ID(wr->wr_id).id != i) || - ((void *)((uintptr_t)sge->addr - - WR_ID(wr->wr_id).offset) != buf)) { - ERROR("%p: cannot store index and offset in WR ID", - (void *)rxq); - sge->addr = 0; - rte_pktmbuf_free(buf); - ret = EOVERFLOW; - goto error; - } } - /* The last WR pointer must be NULL. */ - (*elts)[(i - 1)].wr.next = NULL; DEBUG("%p: allocated and configured %u single-segment WRs", (void *)rxq, elts_n); rxq->elts_n = elts_n; @@ -309,14 +277,10 @@ error: assert(pool == NULL); for (i = 0; (i != RTE_DIM(*elts)); ++i) { struct rxq_elt *elt = &(*elts)[i]; - struct rte_mbuf *buf; + struct rte_mbuf *buf = elt->buf; - if (elt->sge.addr == 0) - continue; - assert(WR_ID(elt->wr.wr_id).id == i); - buf = (void *)((uintptr_t)elt->sge.addr - - WR_ID(elt->wr.wr_id).offset); - rte_pktmbuf_free_seg(buf); + if (buf != NULL) + rte_pktmbuf_free_seg(buf); } rte_free(elts); } @@ -345,14 +309,10 @@ rxq_free_elts(struct rxq *rxq) return; for (i = 0; (i != RTE_DIM(*elts)); ++i) { struct rxq_elt *elt = &(*elts)[i]; - struct rte_mbuf *buf; + struct rte_mbuf *buf = elt->buf; - if (elt->sge.addr == 0) - continue; - assert(WR_ID(elt->wr.wr_id).id == i); - buf = (void *)((uintptr_t)elt->sge.addr - - WR_ID(elt->wr.wr_id).offset); - rte_pktmbuf_free_seg(buf); + if (buf != NULL) + rte_pktmbuf_free_seg(buf); } rte_free(elts); } @@ -552,7 +512,6 @@ rxq_rehash(struct rte_eth_dev *dev, struct rxq *rxq) struct rte_mbuf **pool; unsigned int i, k; struct ibv_exp_qp_attr mod; - struct ibv_recv_wr *bad_wr; int err; int parent = (rxq == &priv->rxq_parent); @@ -673,11 +632,8 @@ rxq_rehash(struct rte_eth_dev *dev, struct rxq *rxq) for (i = 0; (i != RTE_DIM(*elts)); ++i) { struct rxq_elt *elt = &(*elts)[i]; - struct rte_mbuf *buf = (void *) - ((uintptr_t)elt->sge.addr - - WR_ID(elt->wr.wr_id).offset); + struct rte_mbuf *buf = elt->buf; - assert(WR_ID(elt->wr.wr_id).id == i); pool[k++] = buf; } } @@ -701,17 +657,36 @@ rxq_rehash(struct rte_eth_dev *dev, struct rxq *rxq) rxq->elts_n = 0; rte_free(rxq->elts.sp); rxq->elts.sp = NULL; - /* Post WRs. */ - err = ibv_post_recv(tmpl.qp, - (tmpl.sp ? - &(*tmpl.elts.sp)[0].wr : - &(*tmpl.elts.no_sp)[0].wr), - &bad_wr); + /* Post SGEs. */ + assert(tmpl.if_qp != NULL); + if (tmpl.sp) { + struct rxq_elt_sp (*elts)[tmpl.elts_n] = tmpl.elts.sp; + + for (i = 0; (i != RTE_DIM(*elts)); ++i) { + err = tmpl.if_qp->recv_sg_list + (tmpl.qp, + (*elts)[i].sges, + RTE_DIM((*elts)[i].sges)); + if (err) + break; + } + } else { + struct rxq_elt (*elts)[tmpl.elts_n] = tmpl.elts.no_sp; + + for (i = 0; (i != RTE_DIM(*elts)); ++i) { + err = tmpl.if_qp->recv_burst( + tmpl.qp, + &(*elts)[i].sge, + 1); + if (err) + break; + } + } if (err) { - ERROR("%p: ibv_post_recv() failed for WR %p: %s", - (void *)dev, - (void *)bad_wr, - strerror(err)); + ERROR("%p: failed to post SGEs with error %d", + (void *)dev, err); + /* Set err because it does not contain a valid errno value. */ + err = EIO; goto skip_rtr; } mod = (struct ibv_exp_qp_attr){ @@ -764,10 +739,10 @@ rxq_setup(struct rte_eth_dev *dev, struct rxq *rxq, uint16_t desc, struct ibv_exp_res_domain_init_attr rd; } attr; enum ibv_exp_query_intf_status status; - struct ibv_recv_wr *bad_wr; struct rte_mbuf *buf; int ret = 0; int parent = (rxq == &priv->rxq_parent); + unsigned int i; (void)conf; /* Thresholds configuration (ignored). */ /* @@ -903,28 +878,7 @@ skip_mr: (void *)dev, strerror(ret)); goto error; } - ret = ibv_post_recv(tmpl.qp, - (tmpl.sp ? - &(*tmpl.elts.sp)[0].wr : - &(*tmpl.elts.no_sp)[0].wr), - &bad_wr); - if (ret) { - ERROR("%p: ibv_post_recv() failed for WR %p: %s", - (void *)dev, - (void *)bad_wr, - strerror(ret)); - goto error; - } skip_alloc: - mod = (struct ibv_exp_qp_attr){ - .qp_state = IBV_QPS_RTR - }; - ret = ibv_exp_modify_qp(tmpl.qp, &mod, IBV_EXP_QP_STATE); - if (ret) { - ERROR("%p: QP state to IBV_QPS_RTR failed: %s", - (void *)dev, strerror(ret)); - goto error; - } /* Save port ID. */ tmpl.port_id = dev->data->port_id; DEBUG("%p: RTE port ID: %u", (void *)rxq, tmpl.port_id); @@ -950,6 +904,46 @@ skip_alloc: (void *)dev, status); goto error; } + /* Post SGEs. */ + if (!parent && tmpl.sp) { + struct rxq_elt_sp (*elts)[tmpl.elts_n] = tmpl.elts.sp; + + for (i = 0; (i != RTE_DIM(*elts)); ++i) { + ret = tmpl.if_qp->recv_sg_list + (tmpl.qp, + (*elts)[i].sges, + RTE_DIM((*elts)[i].sges)); + if (ret) + break; + } + } else if (!parent) { + struct rxq_elt (*elts)[tmpl.elts_n] = tmpl.elts.no_sp; + + for (i = 0; (i != RTE_DIM(*elts)); ++i) { + ret = tmpl.if_qp->recv_burst( + tmpl.qp, + &(*elts)[i].sge, + 1); + if (ret) + break; + } + } + if (ret) { + ERROR("%p: failed to post SGEs with error %d", + (void *)dev, ret); + /* Set ret because it does not contain a valid errno value. */ + ret = EIO; + goto error; + } + mod = (struct ibv_exp_qp_attr){ + .qp_state = IBV_QPS_RTR + }; + ret = ibv_exp_modify_qp(tmpl.qp, &mod, IBV_EXP_QP_STATE); + if (ret) { + ERROR("%p: QP state to IBV_QPS_RTR failed: %s", + (void *)dev, strerror(ret)); + goto error; + } /* Clean up rxq in case we're reinitializing it. */ DEBUG("%p: cleaning-up old rxq just in case", (void *)rxq); rxq_cleanup(rxq); diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c index 8db4f3f..06712cb 100644 --- a/drivers/net/mlx5/mlx5_rxtx.c +++ b/drivers/net/mlx5/mlx5_rxtx.c @@ -610,8 +610,6 @@ mlx5_rx_burst_sp(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) return 0; for (i = 0; (i != pkts_n); ++i) { struct rxq_elt_sp *elt = &(*elts)[elts_head]; - struct ibv_recv_wr *wr = &elt->wr; - uint64_t wr_id = wr->wr_id; unsigned int len; unsigned int pkt_buf_len; struct rte_mbuf *pkt_buf = NULL; /* Buffer returned in pkts. */ @@ -621,12 +619,6 @@ mlx5_rx_burst_sp(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) uint32_t flags; /* Sanity checks. */ -#ifdef NDEBUG - (void)wr_id; -#endif - assert(wr_id < rxq->elts_n); - assert(wr->sg_list == elt->sges); - assert(wr->num_sge == RTE_DIM(elt->sges)); assert(elts_head < rxq->elts_n); assert(rxq->elts_head < rxq->elts_n); ret = rxq->if_cq->poll_length_flags(rxq->cq, NULL, NULL, @@ -675,6 +667,7 @@ mlx5_rx_burst_sp(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) struct rte_mbuf *rep; unsigned int seg_tailroom; + assert(seg != NULL); /* * Fetch initial bytes of packet descriptor into a * cacheline while allocating rep. @@ -686,9 +679,8 @@ mlx5_rx_burst_sp(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) * Unable to allocate a replacement mbuf, * repost WR. */ - DEBUG("rxq=%p, wr_id=%" PRIu64 ":" - " can't allocate a new mbuf", - (void *)rxq, wr_id); + DEBUG("rxq=%p: can't allocate a new mbuf", + (void *)rxq); if (pkt_buf != NULL) { *pkt_buf_next = NULL; rte_pktmbuf_free(pkt_buf); @@ -818,18 +810,13 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) return mlx5_rx_burst_sp(dpdk_rxq, pkts, pkts_n); for (i = 0; (i != pkts_n); ++i) { struct rxq_elt *elt = &(*elts)[elts_head]; - struct ibv_recv_wr *wr = &elt->wr; - uint64_t wr_id = wr->wr_id; unsigned int len; - struct rte_mbuf *seg = (void *)((uintptr_t)elt->sge.addr - - WR_ID(wr_id).offset); + struct rte_mbuf *seg = elt->buf; struct rte_mbuf *rep; uint32_t flags; /* Sanity checks. */ - assert(WR_ID(wr_id).id < rxq->elts_n); - assert(wr->sg_list == &elt->sge); - assert(wr->num_sge == 1); + assert(seg != NULL); assert(elts_head < rxq->elts_n); assert(rxq->elts_head < rxq->elts_n); ret = rxq->if_cq->poll_length_flags(rxq->cq, NULL, NULL, @@ -880,9 +867,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) * Unable to allocate a replacement mbuf, * repost WR. */ - DEBUG("rxq=%p, wr_id=%" PRIu32 ":" - " can't allocate a new mbuf", - (void *)rxq, WR_ID(wr_id).id); + DEBUG("rxq=%p: can't allocate a new mbuf", + (void *)rxq); /* Increment out of memory counters. */ ++rxq->stats.rx_nombuf; ++rxq->priv->dev->data->rx_mbuf_alloc_failed; @@ -892,10 +878,7 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) /* Reconfigure sge to use rep instead of seg. */ elt->sge.addr = (uintptr_t)rep->buf_addr + RTE_PKTMBUF_HEADROOM; assert(elt->sge.lkey == rxq->mr->lkey); - WR_ID(wr->wr_id).offset = - (((uintptr_t)rep->buf_addr + RTE_PKTMBUF_HEADROOM) - - (uintptr_t)rep); - assert(WR_ID(wr->wr_id).id == WR_ID(wr_id).id); + elt->buf = rep; /* Add SGE to array for repost. */ sges[i] = elt->sge; diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h index 0eb1e98..aec67f6 100644 --- a/drivers/net/mlx5/mlx5_rxtx.h +++ b/drivers/net/mlx5/mlx5_rxtx.h @@ -81,16 +81,14 @@ struct mlx5_txq_stats { /* RX element (scattered packets). */ struct rxq_elt_sp { - struct ibv_recv_wr wr; /* Work Request. */ struct ibv_sge sges[MLX5_PMD_SGE_WR_N]; /* Scatter/Gather Elements. */ struct rte_mbuf *bufs[MLX5_PMD_SGE_WR_N]; /* SGEs buffers. */ }; /* RX element. */ struct rxq_elt { - struct ibv_recv_wr wr; /* Work Request. */ struct ibv_sge sge; /* Scatter/Gather Element. */ - /* mbuf pointer is derived from WR_ID(wr.wr_id).offset. */ + struct rte_mbuf *buf; /* SGE buffer. */ }; struct priv; diff --git a/drivers/net/mlx5/mlx5_utils.h b/drivers/net/mlx5/mlx5_utils.h index 8ff075b..f1fad18 100644 --- a/drivers/net/mlx5/mlx5_utils.h +++ b/drivers/net/mlx5/mlx5_utils.h @@ -161,6 +161,4 @@ pmd_drv_log_basename(const char *s) \ snprintf(name, sizeof(name), __VA_ARGS__) -#define WR_ID(o) (((wr_id_t *)&(o))->data) - #endif /* RTE_PMD_MLX5_UTILS_H_ */ -- 2.1.0