DPDK patches and discussions
 help / color / Atom feed
* [dpdk-dev] [PATCH] af_xdp: avoid deadlock due to empty fill queue
@ 2020-09-17  8:06 Li RongQing
  2020-09-18  9:27 ` Loftus, Ciara
  0 siblings, 1 reply; 3+ messages in thread
From: Li RongQing @ 2020-09-17  8:06 UTC (permalink / raw)
  To: dev

when receive packets, it is possible to fail to reserve
fill queue, since buffer ring is shared between tx and rx,
and maybe not available temporary. at last, both fill
queue and rx queue are empty.

then kernel side will be unable to receive packets due to
empty fill queue, and dpdk will be unable to reserve fill
queue because dpdk has not pakcets to receive, at last
deadlock will happen

so move reserve fill queue before xsk_ring_cons__peek
to fix it

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 drivers/net/af_xdp/rte_eth_af_xdp.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 7ce4ad04a..2dc9cab27 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -304,6 +304,10 @@ af_xdp_rx_cp(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	uint32_t free_thresh = fq->size >> 1;
 	struct rte_mbuf *mbufs[ETH_AF_XDP_RX_BATCH_SIZE];
 
+	if (xsk_prod_nb_free(fq, free_thresh) >= free_thresh)
+		(void)reserve_fill_queue(umem, ETH_AF_XDP_RX_BATCH_SIZE, NULL);
+
+
 	if (unlikely(rte_pktmbuf_alloc_bulk(rxq->mb_pool, mbufs, nb_pkts) != 0))
 		return 0;
 
@@ -317,9 +321,6 @@ af_xdp_rx_cp(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		goto out;
 	}
 
-	if (xsk_prod_nb_free(fq, free_thresh) >= free_thresh)
-		(void)reserve_fill_queue(umem, ETH_AF_XDP_RX_BATCH_SIZE, NULL);
-
 	for (i = 0; i < rcvd; i++) {
 		const struct xdp_desc *desc;
 		uint64_t addr;
-- 
2.16.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] af_xdp: avoid deadlock due to empty fill queue
  2020-09-17  8:06 [dpdk-dev] [PATCH] af_xdp: avoid deadlock due to empty fill queue Li RongQing
@ 2020-09-18  9:27 ` Loftus, Ciara
  2020-09-18 11:24   ` Li,Rongqing
  0 siblings, 1 reply; 3+ messages in thread
From: Loftus, Ciara @ 2020-09-18  9:27 UTC (permalink / raw)
  To: Li RongQing, dev; +Cc: stable

> when receive packets, it is possible to fail to reserve
> fill queue, since buffer ring is shared between tx and rx,
> and maybe not available temporary. at last, both fill
> queue and rx queue are empty.
> 
> then kernel side will be unable to receive packets due to
> empty fill queue, and dpdk will be unable to reserve fill
> queue because dpdk has not pakcets to receive, at last
> deadlock will happen
> 
> so move reserve fill queue before xsk_ring_cons__peek
> to fix it
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>

Thanks for the fix. I tested and saw no significant performance drop.

Minor: the first line of the commit should read "net/af_xdp: ...."

Acked-by: Ciara Loftus <ciara.loftus@intel.com>

CC-ing stable as I think this fix should be considered for inclusion.

Thanks,
Ciara

> ---
>  drivers/net/af_xdp/rte_eth_af_xdp.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
> b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index 7ce4ad04a..2dc9cab27 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -304,6 +304,10 @@ af_xdp_rx_cp(void *queue, struct rte_mbuf **bufs,
> uint16_t nb_pkts)
>  	uint32_t free_thresh = fq->size >> 1;
>  	struct rte_mbuf *mbufs[ETH_AF_XDP_RX_BATCH_SIZE];
> 
> +	if (xsk_prod_nb_free(fq, free_thresh) >= free_thresh)
> +		(void)reserve_fill_queue(umem,
> ETH_AF_XDP_RX_BATCH_SIZE, NULL);
> +
> +
>  	if (unlikely(rte_pktmbuf_alloc_bulk(rxq->mb_pool, mbufs, nb_pkts)
> != 0))
>  		return 0;
> 
> @@ -317,9 +321,6 @@ af_xdp_rx_cp(void *queue, struct rte_mbuf **bufs,
> uint16_t nb_pkts)
>  		goto out;
>  	}
> 
> -	if (xsk_prod_nb_free(fq, free_thresh) >= free_thresh)
> -		(void)reserve_fill_queue(umem,
> ETH_AF_XDP_RX_BATCH_SIZE, NULL);
> -
>  	for (i = 0; i < rcvd; i++) {
>  		const struct xdp_desc *desc;
>  		uint64_t addr;
> --
> 2.16.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] af_xdp: avoid deadlock due to empty fill queue
  2020-09-18  9:27 ` Loftus, Ciara
@ 2020-09-18 11:24   ` Li,Rongqing
  0 siblings, 0 replies; 3+ messages in thread
From: Li,Rongqing @ 2020-09-18 11:24 UTC (permalink / raw)
  To: Loftus, Ciara, dev; +Cc: stable



+AD4- -----Original Message-----
+AD4- From: Loftus, Ciara +AFs-mailto:ciara.loftus+AEA-intel.com+AF0-
+AD4- Sent: Friday, September 18, 2020 5:27 PM
+AD4- To: Li,Rongqing +ADw-lirongqing+AEA-baidu.com+AD4AOw- dev+AEA-dpdk.org
+AD4- Cc: stable+AEA-dpdk.org
+AD4- Subject: RE: +AFs-dpdk-dev+AF0- +AFs-PATCH+AF0- af+AF8-xdp: avoid deadlock due to empty fill queue
+AD4- 
+AD4- +AD4- when receive packets, it is possible to fail to reserve fill queue,
+AD4- +AD4- since buffer ring is shared between tx and rx, and maybe not available
+AD4- +AD4- temporary. at last, both fill queue and rx queue are empty.
+AD4- +AD4-
+AD4- +AD4- then kernel side will be unable to receive packets due to empty fill
+AD4- +AD4- queue, and dpdk will be unable to reserve fill queue because dpdk has
+AD4- +AD4- not pakcets to receive, at last deadlock will happen
+AD4- +AD4-
+AD4- +AD4- so move reserve fill queue before xsk+AF8-ring+AF8-cons+AF8AXw-peek to fix it
+AD4- +AD4-
+AD4- +AD4- Signed-off-by: Li RongQing +ADw-lirongqing+AEA-baidu.com+AD4-
+AD4- 
+AD4- Thanks for the fix. I tested and saw no significant performance drop.
+AD4- 
+AD4- Minor: the first line of the commit should read +ACI-net/af+AF8-xdp: ....+ACI-
+AD4- 
+AD4- Acked-by: Ciara Loftus +ADw-ciara.loftus+AEA-intel.com+AD4-
+AD4- 
+AD4- CC-ing stable as I think this fix should be considered for inclusion.
+AD4- 
+AD4- Thanks,
+AD4- Ciara
+AD4- 

Thanks, I will send v2

-Li


+AD4- +AD4- ---
+AD4- +AD4-  drivers/net/af+AF8-xdp/rte+AF8-eth+AF8-af+AF8-xdp.c +AHw- 7 +-+-+-+----
+AD4- +AD4-  1 file changed, 4 insertions(+-), 3 deletions(-)
+AD4- +AD4-
+AD4- +AD4- diff --git a/drivers/net/af+AF8-xdp/rte+AF8-eth+AF8-af+AF8-xdp.c
+AD4- +AD4- b/drivers/net/af+AF8-xdp/rte+AF8-eth+AF8-af+AF8-xdp.c
+AD4- +AD4- index 7ce4ad04a..2dc9cab27 100644
+AD4- +AD4- --- a/drivers/net/af+AF8-xdp/rte+AF8-eth+AF8-af+AF8-xdp.c
+AD4- +AD4- +-+-+- b/drivers/net/af+AF8-xdp/rte+AF8-eth+AF8-af+AF8-xdp.c
+AD4- +AD4- +AEAAQA- -304,6 +-304,10 +AEAAQA- af+AF8-xdp+AF8-rx+AF8-cp(void +ACo-queue, struct rte+AF8-mbuf +ACoAKg-bufs,
+AD4- +AD4- uint16+AF8-t nb+AF8-pkts)
+AD4- +AD4-  	uint32+AF8-t free+AF8-thresh +AD0- fq-+AD4-size +AD4APg- 1+ADs-
+AD4- +AD4-  	struct rte+AF8-mbuf +ACo-mbufs+AFs-ETH+AF8-AF+AF8-XDP+AF8-RX+AF8-BATCH+AF8-SIZE+AF0AOw-
+AD4- +AD4-
+AD4- +AD4- +-	if (xsk+AF8-prod+AF8-nb+AF8-free(fq, free+AF8-thresh) +AD4APQ- free+AF8-thresh)
+AD4- +AD4- +-		(void)reserve+AF8-fill+AF8-queue(umem,
+AD4- +AD4- ETH+AF8-AF+AF8-XDP+AF8-RX+AF8-BATCH+AF8-SIZE, NULL)+ADs-
+AD4- +AD4- +-
+AD4- +AD4- +-
+AD4- +AD4-  	if (unlikely(rte+AF8-pktmbuf+AF8-alloc+AF8-bulk(rxq-+AD4-mb+AF8-pool, mbufs, nb+AF8-pkts) +ACEAPQ-
+AD4- +AD4- 0))
+AD4- +AD4-  		return 0+ADs-
+AD4- +AD4-
+AD4- +AD4- +AEAAQA- -317,9 +-321,6 +AEAAQA- af+AF8-xdp+AF8-rx+AF8-cp(void +ACo-queue, struct rte+AF8-mbuf +ACoAKg-bufs,
+AD4- +AD4- uint16+AF8-t nb+AF8-pkts)
+AD4- +AD4-  		goto out+ADs-
+AD4- +AD4-  	+AH0-
+AD4- +AD4-
+AD4- +AD4- -	if (xsk+AF8-prod+AF8-nb+AF8-free(fq, free+AF8-thresh) +AD4APQ- free+AF8-thresh)
+AD4- +AD4- -		(void)reserve+AF8-fill+AF8-queue(umem,
+AD4- +AD4- ETH+AF8-AF+AF8-XDP+AF8-RX+AF8-BATCH+AF8-SIZE, NULL)+ADs-
+AD4- +AD4- -
+AD4- +AD4-  	for (i +AD0- 0+ADs- i +ADw- rcvd+ADs- i+-+-) +AHs-
+AD4- +AD4-  		const struct xdp+AF8-desc +ACo-desc+ADs-
+AD4- +AD4-  		uint64+AF8-t addr+ADs-
+AD4- +AD4- --
+AD4- +AD4- 2.16.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17  8:06 [dpdk-dev] [PATCH] af_xdp: avoid deadlock due to empty fill queue Li RongQing
2020-09-18  9:27 ` Loftus, Ciara
2020-09-18 11:24   ` Li,Rongqing

DPDK patches and discussions

Archives are clonable:
	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox