Re: [dpdk-stable] [PATCH] net/mlx5: fix deadlock due to buffered slots in Rx SW ring

[Yongseok Koh <yskoh@mellanox.com>]

My apologies. I put a wrong list of recipients.
Please disregard this email, will send out again.

Thanks
Yongseok

> On Oct 6, 2017, at 3:13 PM, Yongseok Koh <yskoh@mellanox.com> wrote:
> 
> 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
> Cc: martin.weiser@allegro-packets.com
> 
> Reported-by: Martin Weiser <martin.weiser@allegro-packets.com>
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
> 
> 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 | 20 ++++++++------------
> 1 file changed, 8 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..3f92ce559 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,11 @@ 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 += 2)
> +		_mm_storeu_si128((__m128i *)&(*rxq->elts)[elts_idx + i],
> +				 _mm_set1_epi64x((uintptr_t)&rxq->fake_mbuf));
> 	rte_wmb();
> 	*rxq->rq_db = rte_cpu_to_be_32(rxq->rq_ci);
> }
> @@ -639,13 +644,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 +1033,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
>