From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx02.arubanetworks.com (sjcbarracuda02.arubanetworks.com [104.36.248.60]) by dpdk.org (Postfix) with ESMTP id B55FD5A85 for ; Thu, 23 Jul 2015 09:31:03 +0200 (CEST) X-ASG-Debug-ID: 1437636662-03d1342ac1a2000001-TfluYd Received: from sjc-exch10hc-01.arubanetworks.com ([10.1.8.45]) by mx02.arubanetworks.com with ESMTP id 1YYv65fzjVZW1Ud2 (version=TLSv1 cipher=AES128-SHA bits=128 verify=NO); Thu, 23 Jul 2015 00:31:02 -0700 (PDT) X-Barracuda-Envelope-From: vmohare@arubanetworks.com Received: from BOREAL.arubanetworks.com ([::1]) by sjc-exch10hc-01.arubanetworks.com ([fe80::58e7:e72c:3b0d:5534%11]) with mapi id 14.03.0158.001; Thu, 23 Jul 2015 00:31:01 -0700 From: Vithal S Mohare To: Stephen Hemminger , "mac_leehk@yahoo.com.hk" Thread-Topic: [dpdk-dev] [PATCH] The VMXNET3 PMD can't receive packet suddenly after a lot of traffic coming in. The root cause is due to mbuf allocation fail in vmxnet3_post_rx_bufs() and there is no error handling when it is called from vmxnet3_recv_pkts(). Th... X-ASG-Orig-Subj: RE: [dpdk-dev] [PATCH] The VMXNET3 PMD can't receive packet suddenly after a lot of traffic coming in. The root cause is due to mbuf allocation fail in vmxnet3_post_rx_bufs() and there is no error handling when it is called from vmxnet3_recv_pkts(). Th... Thread-Index: AQHQxQhQtwsaIYm4DUOGwuDulNen9J3opsvA Date: Thu, 23 Jul 2015 07:31:01 +0000 Message-ID: <98DB008FA2AC6644B40AD8C766FAB271020CB2EC2B@BOREAL.arubanetworks.com> References: <1437616135-5364-1-git-send-email-mac_leehk@yahoo.com.hk> <20150722222731.497b2abc@uryu.home.lan> In-Reply-To: <20150722222731.497b2abc@uryu.home.lan> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [59.93.73.191] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Barracuda-Connect: UNKNOWN[10.1.8.45] X-Barracuda-Start-Time: 1437636662 X-Barracuda-Encrypted: AES128-SHA X-Barracuda-URL: https://mx01.arubanetworks.com:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at arubanetworks.com X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=7.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.21001 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] The VMXNET3 PMD can't receive packet suddenly after a lot of traffic coming in. The root cause is due to mbuf allocation fail in vmxnet3_post_rx_bufs() and there is no error handling when it is called from vmxnet3_recv_pkts(). Th... 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: Thu, 23 Jul 2015 07:31:04 -0000 How about the below changes? I have been using below changes and helping to= resolve the issue. =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D =3D=3D=3D=3D=3D dpdk/lib/librte_pmd_vmxnet3/vmxnet3_ring.h#3 edit (text) = =3D=3D=3D=3D=3D =20 @@ -155,10 +155,11 @@ typedef struct vmxnet3_tx_queue { struct vmxnet3_rxq= _stats { uint64_t drop_total; uint64_t drop_err; uint64_t drop_fcs; uint64_t rx_buf_alloc_failure; + uint64_t rx_buf_replenish; }; =20 typedef struct vmxnet3_rx_queue { struct rte_mempool *mp; struct vmxnet3_hw *hw; =3D=3D=3D=3D=3D dpdk/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c#5 edit (text) = =3D=3D=3D=3D=3D =20 @@ -645,10 +645,32 @@ rcd_done: break; } } } =20 + /* VMXNET3 + * In the above loop, vmxnet3_post_rx_bufs would fai if all the mbufs = currently allocated. + * In such scenarios where hw device hasn't left with any of 'rx' desc= riptors, packets from + * network will not be 'DMA'd to driver. While the only way to refres= h 'rxd' back to hw is + * though above i.e. when packet is received from hw. So, there is po= tential dead-lock. + * + * Now, to break the deadlock, vmxnet3_post_rx_bufs() is triggered bel= ow when the poll=20 + * goes empty 'rcd'. vmxnet3_post_rx_bufs() is no-op if all the descr= iptors are allocated + * in hw + */ + + if (rcd->gen !=3D rxq->comp_ring.gen) { + ring_idx =3D (uint8_t)((rcd->rqID =3D=3D rxq->qid1) ? 0 : 1); + if (vmxnet3_post_rx_bufs(rxq, ring_idx) > 0 ) { + if (unlikely(rxq->shared->ctrl.updateRxProd)) { + VMXNET3_WRITE_BAR0_REG(hw, rxprod_reg[ring_idx] + (rxq->queue_id= * VMXNET3_REG_ALIGN), + rxq->cmd_ring[ring_idx].next2fill); + } + rxq->stats.rx_buf_replenish++; =20 + } + } + return (nb_rx); } =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D -----Original Message----- From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger Sent: 23 July 2015 AM 10:58 To: mac_leehk@yahoo.com.hk Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] The VMXNET3 PMD can't receive packet sudden= ly after a lot of traffic coming in. The root cause is due to mbuf allocati= on fail in vmxnet3_post_rx_bufs() and there is no error handling when it is= called from vmxnet3_recv_pkts(). Th... On Thu, 23 Jul 2015 09:48:55 +0800 mac_leehk@yahoo.com.hk wrote: > From: marco Thank you for addressing a real bug.=20 But there are several issues with the patch as submitted: * the standard way to handle allocation failure in network drivers is to d= rop the received packet and reuse the available data buffer (mbuf) for the next = packet. It looks like your code would just stop receiving which could cause dead= lock. * the mail is formatted in a manner than is incompatible with merging into= git. All submissions should have a short < 60 character Subject with a summar= y followed by a description. I don't know what mail client you used but e= verything is smashed into the Subject. * all patches require a Signed-off-by with a real name for Developer's Cer= tificate Of Origin * the style is wrong, indentation is a mess please indent with tabs not sp= aces. * avoid extra comments, often in code too many comments are worse than too= few Please rework your patch and resubmit it. > drivers/net/vmxnet3/vmxnet3_rxtx.c | 54 ++++++++++++++++++++++++++++++= +++++- > 1 file changed, 53 insertions(+), 1 deletion(-) mode change 100644=20 > =3D> 100755 drivers/net/vmxnet3/vmxnet3_rxtx.c >=20 > diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c=20 > b/drivers/net/vmxnet3/vmxnet3_rxtx.c > old mode 100644 > new mode 100755 > index 39ad6ef..d560bbb > --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c > +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c > @@ -421,6 +421,51 @@ vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **= tx_pkts, > return nb_tx; > } > =20 > +static inline void > +vmxnet3_renew_desc(vmxnet3_rx_queue_t *rxq, uint8_t ring_id,struct=20 > +rte_mbuf *mbuf) { > + uint32_t val =3D 0; > + struct vmxnet3_cmd_ring *ring =3D &rxq->cmd_ring[ring_id]; > + > + struct Vmxnet3_RxDesc *rxd; > + vmxnet3_buf_info_t *buf_info =3D &ring->buf_info[ring->next2fill]; > + > + rxd =3D (struct Vmxnet3_RxDesc *)(ring->base + ring->next2fill); > + > + if (ring->rid =3D=3D 0) { > + /* Usually: One HEAD type buf per packet > + * val =3D (ring->next2fill % rxq->hw->bufs_per_pkt) ? > + * VMXNET3_RXD_BTYPE_BODY : VMXNET3_RXD_BTYPE_HEAD; > + */ > + > + /* We use single packet buffer so all heads here */ > + val =3D VMXNET3_RXD_BTYPE_HEAD; > + } else { > + /* All BODY type buffers for 2nd ring; which won't be used at all by ES= Xi */ > + val =3D VMXNET3_RXD_BTYPE_BODY; > + } > + > + /* > + * Load mbuf pointer into buf_info[ring_size] > + * buf_info structure is equivalent to cookie for virtio-virtqueue > + */ > + buf_info->m =3D mbuf; > + buf_info->len =3D (uint16_t)(mbuf->buf_len - > + RTE_PKTMBUF_HEADROOM); > + buf_info->bufPA =3D RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mbuf); > + > + /* Load Rx Descriptor with the buffer's GPA */ > + rxd->addr =3D buf_info->bufPA; > + > + /* After this point rxd->addr MUST not be NULL */ > + rxd->btype =3D val; > + rxd->len =3D buf_info->len; > + /* Flip gen bit at the end to change ownership */ > + rxd->gen =3D ring->gen; > + > + vmxnet3_cmd_ring_adv_next2fill(ring); > + > +} > /* > * Allocates mbufs and clusters. Post rx descriptors with buffer detail= s > * so that device can receive packets in those buffers. > @@ -575,8 +620,15 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **= rx_pkts, uint16_t nb_pkts) > } > =20 > while (rcd->gen =3D=3D rxq->comp_ring.gen) { > + struct rte_mbuf *rep; > if (nb_rx >=3D nb_pkts) > break; > + =09 > + rep =3D rte_rxmbuf_alloc(rxq->mp); > + if (rep =3D=3D NULL) { > + rxq->stats.rx_buf_alloc_failure++; > + break; > + } > =20 > idx =3D rcd->rxdIdx; > ring_idx =3D (uint8_t)((rcd->rqID =3D=3D rxq->qid1) ? 0 : 1); @@ -657,= 7=20 > +709,7 @@ rcd_done: > VMXNET3_INC_RING_IDX_ONLY(rxq->cmd_ring[ring_idx].next2comp,=20 > rxq->cmd_ring[ring_idx].size); > =20 > /* It's time to allocate some new buf and renew descriptors */ > - vmxnet3_post_rx_bufs(rxq, ring_idx); > + vmxnet3_renew_desc(rxq, ring_idx,rep); > if (unlikely(rxq->shared->ctrl.updateRxProd)) { > VMXNET3_WRITE_BAR0_REG(hw, rxprod_reg[ring_idx] + (rxq->queue_id * VM= XNET3_REG_ALIGN), > rxq->cmd_ring[ring_idx].next2fill);