From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5C558A2EEB for ; Tue, 10 Sep 2019 17:11:34 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 334DD1C21A; Tue, 10 Sep 2019 17:11:34 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 6056D1BEEB; Tue, 10 Sep 2019 17:11:29 +0200 (CEST) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Sep 2019 08:11:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,489,1559545200"; d="scan'208";a="184195826" Received: from yexl-server.sh.intel.com (HELO localhost) ([10.67.117.5]) by fmsmga008.fm.intel.com with ESMTP; 10 Sep 2019 08:11:26 -0700 Date: Tue, 10 Sep 2019 23:09:13 +0800 From: Ye Xiaolong To: "Zhang, Qi Z" Cc: "Yigit, Ferruh" , "Loftus, Ciara" , "dev@dpdk.org" , "stable@dpdk.org" , "Karlsson, Magnus" Message-ID: <20190910150913.GE6732@intel.com> References: <20190909161247.61801-1-xiaolong.ye@intel.com> <039ED4275CED7440929022BC67E7061153D93589@SHSMSX105.ccr.corp.intel.com> <20190910135347.GD6732@intel.com> <039ED4275CED7440929022BC67E7061153D938D9@SHSMSX105.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <039ED4275CED7440929022BC67E7061153D938D9@SHSMSX105.ccr.corp.intel.com> User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-stable] [PATCH] net/af_xdp: fix Tx halt when no recv packets X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" On 09/10, Zhang, Qi Z wrote: > > >> -----Original Message----- >> From: Ye, Xiaolong >> Sent: Tuesday, September 10, 2019 9:54 PM >> To: Zhang, Qi Z >> Cc: Yigit, Ferruh ; Loftus, Ciara >> ; dev@dpdk.org; stable@dpdk.org; Karlsson, >> Magnus >> Subject: Re: [PATCH] net/af_xdp: fix Tx halt when no recv packets >> >> On 09/10, Zhang, Qi Z wrote: >> > >> > >> >> -----Original Message----- >> >> From: Ye, Xiaolong >> >> Sent: Tuesday, September 10, 2019 12:13 AM >> >> To: Yigit, Ferruh ; Loftus, Ciara >> >> ; Ye, Xiaolong ; >> >> Zhang, Qi Z >> >> Cc: dev@dpdk.org; stable@dpdk.org >> >> Subject: [PATCH] net/af_xdp: fix Tx halt when no recv packets >> >> >> >> The kernel only consumes Tx packets if we have some Rx traffic on >> >> specified queue or we have called send(). So we need to issue a >> >> send() even when the allocation fails so that kernel will start to consume >> packets again. >> > >> >So "allocation fails" means " xsk_ring_prod__reserve" fail right? >> >> Yes. >> >> >I don't understand when xsk_ring_prod__needs_wakeup is true why kernel >> >will stop Tx packet at this situation would you share more insight? >> >> Actually, the fail case is xsk_ring_prod__needs_wakeup is false, then we >> can't issue a send() when xsk_ring_prod__reserve fails. > >Sorry, I think my question should be for the case when xsk_ring_prod__needs_wakeup is false, >I don't understand why we need to handle different at below two situations >1. when xsk_ring_prod__reserve fails >2. normal tx scenario. >My understanding is when xsk_ring_prod__needs_wakeup(tx) is false, which means Tx is ongoing, we don't need to wake up kernel to continue. > The problem is that kernel does not guarantee that all entries are sent for Tx. There are a number of reasons that this might not happen, but usually some Rx packet will at some point in time in the very short future trigger further Tx processing and the packets will be sent. But if you only have Tx processing and no Rx at all, you have to trigger a sento() again. Thanks, Xiaolong >> >> Thanks, >> Xiaolong >> >> > >> >Thanks >> >Qi >> > >> >> >> >> Commit 45bba02c95b0 ("net/af_xdp: support need wakeup feature") >> >> breaks above rule by adding some condition to send, this patch fixes >> >> it while still keeps the need_wakeup feature for Tx. >> >> >> >> Fixes: 45bba02c95b0 ("net/af_xdp: support need wakeup feature") >> >> Cc: stable@dpdk.org >> >> >> >> Signed-off-by: Xiaolong Ye >> >> --- >> >> drivers/net/af_xdp/rte_eth_af_xdp.c | 28 >> >> ++++++++++++++-------------- >> >> 1 file changed, 14 insertions(+), 14 deletions(-) >> >> >> >> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c >> >> b/drivers/net/af_xdp/rte_eth_af_xdp.c >> >> index 41ed5b2af..e496e9aaa 100644 >> >> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c >> >> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c >> >> @@ -286,19 +286,16 @@ kick_tx(struct pkt_tx_queue *txq) { >> >> struct xsk_umem_info *umem = txq->pair->umem; >> >> >> >> -#if defined(XDP_USE_NEED_WAKEUP) >> >> - if (xsk_ring_prod__needs_wakeup(&txq->tx)) >> >> -#endif >> >> - while (send(xsk_socket__fd(txq->pair->xsk), NULL, >> >> - 0, MSG_DONTWAIT) < 0) { >> >> - /* some thing unexpected */ >> >> - if (errno != EBUSY && errno != EAGAIN && errno != EINTR) >> >> - break; >> >> - >> >> - /* pull from completion queue to leave more space */ >> >> - if (errno == EAGAIN) >> >> - pull_umem_cq(umem, ETH_AF_XDP_TX_BATCH_SIZE); >> >> - } >> >> + while (send(xsk_socket__fd(txq->pair->xsk), NULL, >> >> + 0, MSG_DONTWAIT) < 0) { >> >> + /* some thing unexpected */ >> >> + if (errno != EBUSY && errno != EAGAIN && errno != EINTR) >> >> + break; >> >> + >> >> + /* pull from completion queue to leave more space */ >> >> + if (errno == EAGAIN) >> >> + pull_umem_cq(umem, ETH_AF_XDP_TX_BATCH_SIZE); >> >> + } >> >> pull_umem_cq(umem, ETH_AF_XDP_TX_BATCH_SIZE); } >> >> >> >> @@ -367,7 +364,10 @@ eth_af_xdp_tx(void *queue, struct rte_mbuf >> >> **bufs, uint16_t nb_pkts) >> >> >> >> xsk_ring_prod__submit(&txq->tx, nb_pkts); >> >> >> >> - kick_tx(txq); >> >> +#if defined(XDP_USE_NEED_WAKEUP) >> >> + if (xsk_ring_prod__needs_wakeup(&txq->tx)) >> >> +#endif >> >> + kick_tx(txq); >> >> >> >> txq->stats.tx_pkts += nb_pkts; >> >> txq->stats.tx_bytes += tx_bytes; >> >> -- >> >> 2.17.1 >> >