DPDK patches and discussions
 help / color / mirror / Atom feed
From: Yongseok Koh <yskoh@mellanox.com>
To: adrien.mazarguil@6wind.com, nelio.laranjeiro@6wind.com
Cc: martin.weiser@allegro-packets.com, dev@dpdk.org,
	Yongseok Koh <yskoh@mellanox.com>,
	stable@dpdk.org
Subject: [dpdk-dev] [PATCH v2] net/mlx5: fix deadlock due to buffered slots in Rx SW ring
Date: Sat,  7 Oct 2017 15:10:35 -0700	[thread overview]
Message-ID: <09b07aed3dd9a25ae6ee575a725924dd66f04e79.1507413575.git.yskoh@mellanox.com> (raw)
In-Reply-To: <20171006221948.24238-1-yskoh@mellanox.com>

When replenishing Rx ring, there're always buffered slots reserved between
consumed entries and HW owned entries. These have to be filled with fake
mbufs to protect from possible overflow rather than optimistically
expecting successful replenishment which can cause deadlock with
small-sized queue.

Fixes: 951649d21edf ("net/mlx5: fix overflow of Rx SW ring")
Cc: stable@dpdk.org

Reported-by: Martin Weiser <martin.weiser@allegro-packets.com>
Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
v2:
* Replace vector st/ld with regular assignment as performance gain is negligible
for the short loop. This is to make the function shareable with ARM NEON.

This patch actually reverts "net/mlx5: fix overflow of Rx SW ring". If possible,
these two can be squashed by disregrading the old one.

 drivers/net/mlx5/mlx5_rxtx_vec_sse.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_sse.c b/drivers/net/mlx5/mlx5_rxtx_vec_sse.c
index 075dce908..4de8b7333 100644
--- a/drivers/net/mlx5/mlx5_rxtx_vec_sse.c
+++ b/drivers/net/mlx5/mlx5_rxtx_vec_sse.c
@@ -548,7 +548,7 @@ rxq_replenish_bulk_mbuf(struct rxq *rxq, uint16_t n)
 {
 	const uint16_t q_n = 1 << rxq->elts_n;
 	const uint16_t q_mask = q_n - 1;
-	const uint16_t elts_idx = rxq->rq_ci & q_mask;
+	uint16_t elts_idx = rxq->rq_ci & q_mask;
 	struct rte_mbuf **elts = &(*rxq->elts)[elts_idx];
 	volatile struct mlx5_wqe_data_seg *wq = &(*rxq->wqes)[elts_idx];
 	unsigned int i;
@@ -566,6 +566,10 @@ rxq_replenish_bulk_mbuf(struct rxq *rxq, uint16_t n)
 		wq[i].addr = rte_cpu_to_be_64((uintptr_t)elts[i]->buf_addr +
 					      RTE_PKTMBUF_HEADROOM);
 	rxq->rq_ci += n;
+	/* Prevent overflowing into consumed mbufs. */
+	elts_idx = rxq->rq_ci & q_mask;
+	for (i = 0; i < MLX5_VPMD_DESCS_PER_LOOP; ++i)
+		(*rxq->elts)[elts_idx + i] = &rxq->fake_mbuf;
 	rte_wmb();
 	*rxq->rq_db = rte_cpu_to_be_32(rxq->rq_ci);
 }
@@ -639,13 +643,6 @@ rxq_cq_decompress_v(struct rxq *rxq,
 	RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, hash) !=
 			 offsetof(struct rte_mbuf, rx_descriptor_fields1) + 12);
 	/*
-	 * Not to overflow elts array. Decompress next time after mbuf
-	 * replenishment.
-	 */
-	if (unlikely(mcqe_n + MLX5_VPMD_DESCS_PER_LOOP >
-		     (uint16_t)(rxq->rq_ci - rxq->cq_ci)))
-		return;
-	/*
 	 * A. load mCQEs into a 128bit register.
 	 * B. store rearm data to mbuf.
 	 * C. combine data from mCQEs with rx_descriptor_fields1.
@@ -1035,10 +1032,8 @@ rxq_burst_v(struct rxq *rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 	}
 	elts_idx = rxq->rq_pi & q_mask;
 	elts = &(*rxq->elts)[elts_idx];
-	pkts_n = RTE_MIN(pkts_n - rcvd_pkt,
-			 (uint16_t)(rxq->rq_ci - rxq->cq_ci));
-	/* Not to overflow pkts/elts array. */
-	pkts_n = RTE_ALIGN_FLOOR(pkts_n, MLX5_VPMD_DESCS_PER_LOOP);
+	/* Not to overflow pkts array. */
+	pkts_n = RTE_ALIGN_FLOOR(pkts_n - rcvd_pkt, MLX5_VPMD_DESCS_PER_LOOP);
 	/* Not to cross queue end. */
 	pkts_n = RTE_MIN(pkts_n, q_n - elts_idx);
 	if (!pkts_n)
-- 
2.11.0

  reply	other threads:[~2017-10-07 22:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-06 22:19 [dpdk-dev] [PATCH] " Yongseok Koh
2017-10-07 22:10 ` Yongseok Koh [this message]
2017-10-10 14:04 ` [dpdk-dev] [PATCH v3] " Yongseok Koh
2017-10-11  0:43   ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=09b07aed3dd9a25ae6ee575a725924dd66f04e79.1507413575.git.yskoh@mellanox.com \
    --to=yskoh@mellanox.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=martin.weiser@allegro-packets.com \
    --cc=nelio.laranjeiro@6wind.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).