* [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(). The RXD will not have "free" mbuf for it but the counter still increment. Finally, no packet can be received. This fix is allocate the mbuf first, if the allocation is failed, then don't receive any packet and the packet will remain in RXD to prevent any packet drop.If the allocation is sucess, the vmxnet3_post_rx_bufs() will call vmxnet3_renew_desc() and RXD will be renew inside.
@ 2015-07-23 1:48 mac_leehk
2015-07-23 5:27 ` Stephen Hemminger
0 siblings, 1 reply; 5+ messages in thread
From: mac_leehk @ 2015-07-23 1:48 UTC (permalink / raw)
To: dev
From: marco <marco@ubuntu.(none)>
---
drivers/net/vmxnet3/vmxnet3_rxtx.c | 54 +++++++++++++++++++++++++++++++++++-
1 file changed, 53 insertions(+), 1 deletion(-)
mode change 100644 => 100755 drivers/net/vmxnet3/vmxnet3_rxtx.c
diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c 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;
}
+static inline void
+vmxnet3_renew_desc(vmxnet3_rx_queue_t *rxq, uint8_t ring_id,struct rte_mbuf *mbuf)
+{
+ uint32_t val = 0;
+ struct vmxnet3_cmd_ring *ring = &rxq->cmd_ring[ring_id];
+
+ struct Vmxnet3_RxDesc *rxd;
+ vmxnet3_buf_info_t *buf_info = &ring->buf_info[ring->next2fill];
+
+ rxd = (struct Vmxnet3_RxDesc *)(ring->base + ring->next2fill);
+
+ if (ring->rid == 0) {
+ /* Usually: One HEAD type buf per packet
+ * val = (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 = VMXNET3_RXD_BTYPE_HEAD;
+ } else {
+ /* All BODY type buffers for 2nd ring; which won't be used at all by ESXi */
+ val = 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 = mbuf;
+ buf_info->len = (uint16_t)(mbuf->buf_len -
+ RTE_PKTMBUF_HEADROOM);
+ buf_info->bufPA = RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mbuf);
+
+ /* Load Rx Descriptor with the buffer's GPA */
+ rxd->addr = buf_info->bufPA;
+
+ /* After this point rxd->addr MUST not be NULL */
+ rxd->btype = val;
+ rxd->len = buf_info->len;
+ /* Flip gen bit at the end to change ownership */
+ rxd->gen = ring->gen;
+
+ vmxnet3_cmd_ring_adv_next2fill(ring);
+
+}
/*
* Allocates mbufs and clusters. Post rx descriptors with buffer details
* 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)
}
while (rcd->gen == rxq->comp_ring.gen) {
+ struct rte_mbuf *rep;
if (nb_rx >= nb_pkts)
break;
+
+ rep = rte_rxmbuf_alloc(rxq->mp);
+ if (rep == NULL) {
+ rxq->stats.rx_buf_alloc_failure++;
+ break;
+ }
idx = rcd->rxdIdx;
ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1);
@@ -657,7 +709,7 @@ rcd_done:
VMXNET3_INC_RING_IDX_ONLY(rxq->cmd_ring[ring_idx].next2comp, rxq->cmd_ring[ring_idx].size);
/* 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 * VMXNET3_REG_ALIGN),
rxq->cmd_ring[ring_idx].next2fill);
--
1.7.9.5
^ permalink raw reply [flat|nested] 5+ messages in thread
* 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(). The RXD will not have "free" mbuf for it but the counter still increment. Finally, no packet can be received. This fix is allocate the mbuf first, if the allocation is failed, then don't receive any packet and the packet will remain in RXD to prevent any packet drop.If the allocation is sucess, the vmxnet3_post_rx_bufs() will call vmxnet3_renew_desc() and RXD will be renew inside.
2015-07-23 1:48 [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(). The RXD will not have "free" mbuf for it but the counter still increment. Finally, no packet can be received. This fix is allocate the mbuf first, if the allocation is failed, then don't receive any packet and the packet will remain in RXD to prevent any packet drop.If the allocation is sucess, the vmxnet3_post_rx_bufs() will call vmxnet3_renew_desc() and RXD will be renew inside mac_leehk
@ 2015-07-23 5:27 ` Stephen Hemminger
2015-07-23 5:44 ` [dpdk-dev] 回覆︰ " MAC Lee
2015-07-23 7:31 ` [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 Vithal S Mohare
0 siblings, 2 replies; 5+ messages in thread
From: Stephen Hemminger @ 2015-07-23 5:27 UTC (permalink / raw)
To: mac_leehk; +Cc: dev
On Thu, 23 Jul 2015 09:48:55 +0800
mac_leehk@yahoo.com.hk wrote:
> From: marco <marco@ubuntu.(none)>
Thank you for addressing a real bug.
But there are several issues with the patch as submitted:
* the standard way to handle allocation failure in network drivers is to drop 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 deadlock.
* 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 summary
followed by a description. I don't know what mail client you used but everything
is smashed into the Subject.
* all patches require a Signed-off-by with a real name for Developer's Certificate Of Origin
* the style is wrong, indentation is a mess please indent with tabs not spaces.
* 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 => 100755 drivers/net/vmxnet3/vmxnet3_rxtx.c
>
> diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c 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;
> }
>
> +static inline void
> +vmxnet3_renew_desc(vmxnet3_rx_queue_t *rxq, uint8_t ring_id,struct rte_mbuf *mbuf)
> +{
> + uint32_t val = 0;
> + struct vmxnet3_cmd_ring *ring = &rxq->cmd_ring[ring_id];
> +
> + struct Vmxnet3_RxDesc *rxd;
> + vmxnet3_buf_info_t *buf_info = &ring->buf_info[ring->next2fill];
> +
> + rxd = (struct Vmxnet3_RxDesc *)(ring->base + ring->next2fill);
> +
> + if (ring->rid == 0) {
> + /* Usually: One HEAD type buf per packet
> + * val = (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 = VMXNET3_RXD_BTYPE_HEAD;
> + } else {
> + /* All BODY type buffers for 2nd ring; which won't be used at all by ESXi */
> + val = 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 = mbuf;
> + buf_info->len = (uint16_t)(mbuf->buf_len -
> + RTE_PKTMBUF_HEADROOM);
> + buf_info->bufPA = RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mbuf);
> +
> + /* Load Rx Descriptor with the buffer's GPA */
> + rxd->addr = buf_info->bufPA;
> +
> + /* After this point rxd->addr MUST not be NULL */
> + rxd->btype = val;
> + rxd->len = buf_info->len;
> + /* Flip gen bit at the end to change ownership */
> + rxd->gen = ring->gen;
> +
> + vmxnet3_cmd_ring_adv_next2fill(ring);
> +
> +}
> /*
> * Allocates mbufs and clusters. Post rx descriptors with buffer details
> * 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)
> }
>
> while (rcd->gen == rxq->comp_ring.gen) {
> + struct rte_mbuf *rep;
> if (nb_rx >= nb_pkts)
> break;
> +
> + rep = rte_rxmbuf_alloc(rxq->mp);
> + if (rep == NULL) {
> + rxq->stats.rx_buf_alloc_failure++;
> + break;
> + }
>
> idx = rcd->rxdIdx;
> ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1);
> @@ -657,7 +709,7 @@ rcd_done:
> VMXNET3_INC_RING_IDX_ONLY(rxq->cmd_ring[ring_idx].next2comp, rxq->cmd_ring[ring_idx].size);
>
> /* 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 * VMXNET3_REG_ALIGN),
> rxq->cmd_ring[ring_idx].next2fill);
^ permalink raw reply [flat|nested] 5+ messages in thread
* [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(). The RXD will not have "free" mbuf for it but the counter still increment. Finally, no packet can be received. This fix is allocate the mbuf first, if the allocation is failed, then don't receive any packet and the packet will remain in RXD to prevent any packet drop.If the allocation is sucess, the vmxnet3_post_rx_bufs() will call vmxnet3_renew_desc() and RXD will be renew inside.
2015-07-23 5:27 ` Stephen Hemminger
@ 2015-07-23 5:44 ` MAC Lee
2015-07-23 7:31 ` [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 Vithal S Mohare
1 sibling, 0 replies; 5+ messages in thread
From: MAC Lee @ 2015-07-23 5:44 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 13880 bytes --]
Hi Stephen,  I have question about point 1 and want to discuss with you. Why it will cause deadlock if just stop receiving it in this cases?  And I also found VMXNET3 PMD have the same bug in RX. I will rework the patch and submit later. Thanks!
Best Regards,Marco
Stephen Hemminger <stephen@networkplumber.org> æ¼ 2015å¹´07æ23æ¥ (é±å) 1:27 PM 寫éï¹
On Thu, 23 Jul 2015 09:48:55 +0800
mac_leehk@yahoo.com.hk wrote:
> From: marco <marco@ubuntu.(none)>
Thank you for addressing a real bug.
But there are several issues with the patch as submitted:
* the standard way to handle allocation failure in network drivers is to drop 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 deadlock.
* 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 summary
 followed by a description. I don't know what mail client you used but everything
 is smashed into the Subject.
* all patches require a Signed-off-by with a real name for Developer's Certificate Of Origin
* the style is wrong, indentation is a mess please indent with tabs not spaces.
* 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 => 100755 drivers/net/vmxnet3/vmxnet3_rxtx.c
>
> diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c 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;
>Â }
>Â
> +static inline void
> +vmxnet3_renew_desc(vmxnet3_rx_queue_t *rxq, uint8_t ring_id,struct rte_mbuf *mbuf)
> +{
> +   uint32_t val = 0;
> +Â Â Â struct vmxnet3_cmd_ring *ring = &rxq->cmd_ring[ring_id];
> +
> +Â Â Â struct Vmxnet3_RxDesc *rxd;
> +Â Â Â vmxnet3_buf_info_t *buf_info = &ring->buf_info[ring->next2fill];
> +
> +Â Â Â rxd = (struct Vmxnet3_RxDesc *)(ring->base + ring->next2fill);
> +
> +Â Â Â if (ring->rid == 0) {
> +Â Â Â /* Usually: One HEAD type buf per packet
> +Â Â Â * val = (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 = VMXNET3_RXD_BTYPE_HEAD;
> +Â Â Â } else {
> +Â Â Â /* All BODY type buffers for 2nd ring; which won't be used at all by ESXi */
> +Â Â Â Â Â Â val = 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 = mbuf;
> +Â Â Â buf_info->len = (uint16_t)(mbuf->buf_len -
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â RTE_PKTMBUF_HEADROOM);
> +Â Â Â buf_info->bufPA = RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mbuf);
> +
> +Â Â Â /* Load Rx Descriptor with the buffer's GPA */
> +Â Â Â rxd->addr = buf_info->bufPA;
> +
> +Â Â Â /* After this point rxd->addr MUST not be NULL */
> +Â Â Â rxd->btype = val;
> +Â Â Â rxd->len = buf_info->len;
> +Â Â Â /* Flip gen bit at the end to change ownership */
> +Â Â Â rxd->gen = ring->gen;
> +
> +Â Â Â vmxnet3_cmd_ring_adv_next2fill(ring);
> +
> +}
>Â /*
>Â *Â Allocates mbufs and clusters. Post rx descriptors with buffer details
>Â *Â 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)
>Â Â Â Â }
>Â
>Â Â Â Â while (rcd->gen == rxq->comp_ring.gen) {
> +Â Â Â Â Â Â Â struct rte_mbuf *rep;
>Â Â Â Â Â Â Â if (nb_rx >= nb_pkts)
>Â Â Â Â Â Â Â Â Â Â break;
> +Â Â Â Â Â Â
> +Â Â Â Â Â Â rep = rte_rxmbuf_alloc(rxq->mp);
> +Â Â Â Â Â Â Â if (rep == NULL) {
> +Â Â Â Â Â Â Â Â Â Â Â Â rxq->stats.rx_buf_alloc_failure++;
> +Â Â Â Â Â Â Â Â Â Â Â Â break;
> +Â Â Â Â Â Â Â }
>Â
>Â Â Â Â Â Â Â idx = rcd->rxdIdx;
>Â Â Â Â Â Â Â ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1);
> @@ -657,7 +709,7 @@ rcd_done:
>Â Â Â Â Â Â Â VMXNET3_INC_RING_IDX_ONLY(rxq->cmd_ring[ring_idx].next2comp, rxq->cmd_ring[ring_idx].size);
>Â
>Â Â Â Â Â Â Â /* 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 * VMXNET3_REG_ALIGN),
>Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â rxq->cmd_ring[ring_idx].next2fill);
From harish.patil@qlogic.com Thu Jul 23 08:51:58 2015
Return-Path: <harish.patil@qlogic.com>
Received: from mx0b-0016ce01.pphosted.com (mx0b-0016ce01.pphosted.com
[67.231.156.153]) by dpdk.org (Postfix) with ESMTP id BBF7658CB
for <dev@dpdk.org>; Thu, 23 Jul 2015 08:51:58 +0200 (CEST)
Received: from pps.filterd (m0085408.ppops.net [127.0.0.1])
by mx0b-0016ce01.pphosted.com (8.15.0.59/8.15.0.59) with SMTP id
t6N6m3Ua015958; Wed, 22 Jul 2015 23:51:57 -0700
Received: from avcashub1.qlogic.com (avcashub3.qlogic.com [198.70.193.117])
by mx0b-0016ce01.pphosted.com with ESMTP id 1vqnsmxvye-1
(version=TLSv1/SSLv3 cipher®S128-SHA bits\x128 verify=NOT);
Wed, 22 Jul 2015 23:51:57 -0700
Received: from AVMB1.qlogic.org ([fe80::b816:e739:5ab3:5221]) by
avcashub3.qlogic.org ([::1]) with mapi id 14.03.0235.001; Wed, 22 Jul 2015
23:51:56 -0700
From: Harish Patil <harish.patil@qlogic.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Thread-Topic: [PATCH v5 0/4] bnx2x: poll mode driver
Thread-Index: AQHQwwnF3xcML92/X0uftJCdXdgo+53oog0A
Date: Thu, 23 Jul 2015 06:51:55 +0000
Message-ID: <D1D5DCEA.A5E3E%harish.patil@qlogic.com>
References: <1437410000-15907-1-git-send-email-stephen@networkplumber.org>
In-Reply-To: <1437410000-15907-1-git-send-email-stephen@networkplumber.org>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.1.4.10]
Content-Type: text/plain; charset="utf-8"
Content-ID: <FEB4F8AE4079584595FD29BD2A27879E@qlogic.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-Proofpoint-Virus-Version: vendor=nai engineW00 definitionsx70
signaturesg0613
X-Proofpoint-Spam-Details: rule=notspam policyÞfault score=0 spamscore=0
suspectscore=0
malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam
adjust=0 reason=mlx scancount=1 engine=8.0.1-1506180000
definitions=main-1507230111
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v5 0/4] bnx2x: poll mode driver
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 23 Jul 2015 06:51:59 -0000
Pg0KPkNoYW5nZXMgaW4gdGhpcyB2ZXJzaW9uICh2NSk6DQo+ICAtIHJlYmFzZSB0byAyLjEuMC1y
YzENCj4gIC0gZml4IHZlcnNpb24gbWFwIGZvciAyLjENCj4gIC0gZml4IGRveHlnZW4gY29tbWVu
dHMNCj4gIC0gYWRkIEJDTTU3NDEwIGlkcw0KPg0KPlN0ZXBoZW4gSGVtbWluZ2VyICg0KToNCj4g
IGVhbDogcHJvdmlkZSBmdW5jdGlvbnMgdG8gYWNjZXNzIFBDSSBjb25maWcNCj4gIGJueDJ4OiBk
cml2ZXIgY29yZQ0KPiAgYm54Mng6IGRyaXZlciBzdXBwb3J0IHJvdXRpbmVzDQo+ICBibngyeDog
ZW5hYmxlIFBNRCBidWlsZA0KPg0KPiBNQUlOVEFJTkVSUyAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICB8ICAgICAzICsNCj4gY29uZmlnL2NvbW1vbl9saW51eGFwcCAgICAgICAg
ICAgICAgICAgICAgICAgICAgfCAgICAxMCArDQo+IGRyaXZlcnMvbmV0L01ha2VmaWxlICAgICAg
ICAgICAgICAgICAgICAgICAgICAgIHwgICAgIDEgKw0KPiBkcml2ZXJzL25ldC9ibngyeC9NYWtl
ZmlsZSAgICAgICAgICAgICAgICAgICAgICB8ICAgIDI4ICsNCj4gZHJpdmVycy9uZXQvYm54Mngv
Ym54MnguYyAgICAgICAgICAgICAgICAgICAgICAgfCAxMTgyMQ0KPisrKysrKysrKysrKysrKysr
KysNCj4gZHJpdmVycy9uZXQvYm54MngvYm54MnguaCAgICAgICAgICAgICAgICAgICAgICAgfCAg
MTk5OCArKysrDQo+IGRyaXZlcnMvbmV0L2JueDJ4L2JueDJ4X2V0aGRldi5jICAgICAgICAgICAg
ICAgIHwgICA1NDIgKw0KPiBkcml2ZXJzL25ldC9ibngyeC9ibngyeF9ldGhkZXYuaCAgICAgICAg
ICAgICAgICB8ICAgIDc5ICsNCj4gZHJpdmVycy9uZXQvYm54MngvYm54MnhfbG9ncy5oICAgICAg
ICAgICAgICAgICAgfCAgICA1MSArDQo+IGRyaXZlcnMvbmV0L2JueDJ4L2JueDJ4X3J4dHguYyAg
ICAgICAgICAgICAgICAgIHwgICA0ODcgKw0KPiBkcml2ZXJzL25ldC9ibngyeC9ibngyeF9yeHR4
LmggICAgICAgICAgICAgICAgICB8ICAgIDg1ICsNCj4gZHJpdmVycy9uZXQvYm54MngvYm54Mnhf
c3RhdHMuYyAgICAgICAgICAgICAgICAgfCAgMTYxOSArKysNCj4gZHJpdmVycy9uZXQvYm54Mngv
Ym54Mnhfc3RhdHMuaCAgICAgICAgICAgICAgICAgfCAgIDYzMiArDQo+IGRyaXZlcnMvbmV0L2Ju
eDJ4L2JueDJ4X3ZmcGYuYyAgICAgICAgICAgICAgICAgIHwgICA1OTcgKw0KPiBkcml2ZXJzL25l
dC9ibngyeC9ibngyeF92ZnBmLmggICAgICAgICAgICAgICAgICB8ICAgMzE1ICsNCj4gZHJpdmVy
cy9uZXQvYm54MngvZGVidWcuYyAgICAgICAgICAgICAgICAgICAgICAgfCAgIDExMyArDQo+IGRy
aXZlcnMvbmV0L2JueDJ4L2Vjb3JlX2Z3X2RlZnMuaCAgICAgICAgICAgICAgIHwgICA0MjIgKw0K
PiBkcml2ZXJzL25ldC9ibngyeC9lY29yZV9oc2kuaCAgICAgICAgICAgICAgICAgICB8ICA2MzQ4
ICsrKysrKysrKysNCj4gZHJpdmVycy9uZXQvYm54MngvZWNvcmVfaW5pdC5oICAgICAgICAgICAg
ICAgICAgfCAgIDg0MSArKw0KPiBkcml2ZXJzL25ldC9ibngyeC9lY29yZV9pbml0X29wcy5oICAg
ICAgICAgICAgICB8ICAgODg2ICsrDQo+IGRyaXZlcnMvbmV0L2JueDJ4L2Vjb3JlX21md19yZXEu
aCAgICAgICAgICAgICAgIHwgICAyMDYgKw0KPiBkcml2ZXJzL25ldC9ibngyeC9lY29yZV9yZWcu
aCAgICAgICAgICAgICAgICAgICB8ICAzNjYzICsrKysrKw0KPiBkcml2ZXJzL25ldC9ibngyeC9l
Y29yZV9zcC5jICAgICAgICAgICAgICAgICAgICB8ICA1NDU1ICsrKysrKysrKw0KPiBkcml2ZXJz
L25ldC9ibngyeC9lY29yZV9zcC5oICAgICAgICAgICAgICAgICAgICB8ICAxNzk1ICsrKw0KPiBk
cml2ZXJzL25ldC9ibngyeC9lbGluay5jICAgICAgICAgICAgICAgICAgICAgICB8IDEzMzc4DQo+
KysrKysrKysrKysrKysrKysrKysrKw0KPiBkcml2ZXJzL25ldC9ibngyeC9lbGluay5oICAgICAg
ICAgICAgICAgICAgICAgICB8ICAgNjA5ICsNCj4gZHJpdmVycy9uZXQvYm54MngvcnRlX3BtZF9i
bngyeF92ZXJzaW9uLm1hcCAgICAgfCAgICAgNCArDQo+IGxpYi9saWJydGVfZWFsL2JzZGFwcC9l
YWwvZWFsX3BjaS5jICAgICAgICAgICAgIHwgICAgODMgKw0KPiBsaWIvbGlicnRlX2VhbC9ic2Rh
cHAvZWFsL3J0ZV9lYWxfdmVyc2lvbi5tYXAgICB8ICAgICAzICstDQo+IGxpYi9saWJydGVfZWFs
L2NvbW1vbi9pbmNsdWRlL3J0ZV9wY2kuaCAgICAgICAgIHwgICAgMzIgKw0KPiBsaWIvbGlicnRl
X2VhbC9jb21tb24vaW5jbHVkZS9ydGVfcGNpX2Rldl9pZHMuaCB8ICAgIDYxICsNCj4gbGliL2xp
YnJ0ZV9lYWwvbGludXhhcHAvZWFsL2VhbF9wY2kuYyAgICAgICAgICAgfCAgICA1MCArDQo+IGxp
Yi9saWJydGVfZWFsL2xpbnV4YXBwL2VhbC9lYWxfcGNpX2luaXQuaCAgICAgIHwgICAgMTEgKw0K
PiBsaWIvbGlicnRlX2VhbC9saW51eGFwcC9lYWwvZWFsX3BjaV91aW8uYyAgICAgICB8ICAgIDE0
ICsNCj4gbGliL2xpYnJ0ZV9lYWwvbGludXhhcHAvZWFsL2VhbF9wY2lfdmZpby5jICAgICAgfCAg
ICAxNiArDQo+IGxpYi9saWJydGVfZWFsL2xpbnV4YXBwL2VhbC9ydGVfZWFsX3ZlcnNpb24ubWFw
IHwgICAgIDMgKy0NCj4gbWsvcnRlLmFwcC5tayAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgfCAgICAgMiArDQo+IDM3IGZpbGVzIGNoYW5nZWQsIDUyMjYxIGluc2VydGlvbnMoKyks
IDIgZGVsZXRpb25zKC0pDQo+IGNyZWF0ZSBtb2RlIDEwMDY0NCBkcml2ZXJzL25ldC9ibngyeC9N
YWtlZmlsZQ0KPiBjcmVhdGUgbW9kZSAxMDA2NDQgZHJpdmVycy9uZXQvYm54MngvYm54MnguYw0K
PiBjcmVhdGUgbW9kZSAxMDA2NDQgZHJpdmVycy9uZXQvYm54MngvYm54MnguaA0KPiBjcmVhdGUg
bW9kZSAxMDA2NDQgZHJpdmVycy9uZXQvYm54MngvYm54MnhfZXRoZGV2LmMNCj4gY3JlYXRlIG1v
ZGUgMTAwNjQ0IGRyaXZlcnMvbmV0L2JueDJ4L2JueDJ4X2V0aGRldi5oDQo+IGNyZWF0ZSBtb2Rl
IDEwMDY0NCBkcml2ZXJzL25ldC9ibngyeC9ibngyeF9sb2dzLmgNCj4gY3JlYXRlIG1vZGUgMTAw
NjQ0IGRyaXZlcnMvbmV0L2JueDJ4L2JueDJ4X3J4dHguYw0KPiBjcmVhdGUgbW9kZSAxMDA2NDQg
ZHJpdmVycy9uZXQvYm54MngvYm54Mnhfcnh0eC5oDQo+IGNyZWF0ZSBtb2RlIDEwMDY0NCBkcml2
ZXJzL25ldC9ibngyeC9ibngyeF9zdGF0cy5jDQo+IGNyZWF0ZSBtb2RlIDEwMDY0NCBkcml2ZXJz
L25ldC9ibngyeC9ibngyeF9zdGF0cy5oDQo+IGNyZWF0ZSBtb2RlIDEwMDY0NCBkcml2ZXJzL25l
dC9ibngyeC9ibngyeF92ZnBmLmMNCj4gY3JlYXRlIG1vZGUgMTAwNjQ0IGRyaXZlcnMvbmV0L2Ju
eDJ4L2JueDJ4X3ZmcGYuaA0KPiBjcmVhdGUgbW9kZSAxMDA2NDQgZHJpdmVycy9uZXQvYm54Mngv
ZGVidWcuYw0KPiBjcmVhdGUgbW9kZSAxMDA2NDQgZHJpdmVycy9uZXQvYm54MngvZWNvcmVfZndf
ZGVmcy5oDQo+IGNyZWF0ZSBtb2RlIDEwMDY0NCBkcml2ZXJzL25ldC9ibngyeC9lY29yZV9oc2ku
aA0KPiBjcmVhdGUgbW9kZSAxMDA2NDQgZHJpdmVycy9uZXQvYm54MngvZWNvcmVfaW5pdC5oDQo+
IGNyZWF0ZSBtb2RlIDEwMDY0NCBkcml2ZXJzL25ldC9ibngyeC9lY29yZV9pbml0X29wcy5oDQo+
IGNyZWF0ZSBtb2RlIDEwMDY0NCBkcml2ZXJzL25ldC9ibngyeC9lY29yZV9tZndfcmVxLmgNCj4g
Y3JlYXRlIG1vZGUgMTAwNjQ0IGRyaXZlcnMvbmV0L2JueDJ4L2Vjb3JlX3JlZy5oDQo+IGNyZWF0
ZSBtb2RlIDEwMDY0NCBkcml2ZXJzL25ldC9ibngyeC9lY29yZV9zcC5jDQo+IGNyZWF0ZSBtb2Rl
IDEwMDY0NCBkcml2ZXJzL25ldC9ibngyeC9lY29yZV9zcC5oDQo+IGNyZWF0ZSBtb2RlIDEwMDY0
NCBkcml2ZXJzL25ldC9ibngyeC9lbGluay5jDQo+IGNyZWF0ZSBtb2RlIDEwMDY0NCBkcml2ZXJz
L25ldC9ibngyeC9lbGluay5oDQo+IGNyZWF0ZSBtb2RlIDEwMDY0NCBkcml2ZXJzL25ldC9ibngy
eC9ydGVfcG1kX2JueDJ4X3ZlcnNpb24ubWFwDQo+DQo+LS0NCj4yLjEuNA0KPg0KPg0KDQpBY2tl
ZC1ieTogSGFyaXNoIFBhdGlsIDxoYXJpc2gucGF0aWxAcWxvZ2ljLmNvbT4NCg0KVGhhbmtzLA0K
SGFyaXNoDQoNCg0KDQoNCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fDQoNClRoaXMg
bWVzc2FnZSBhbmQgYW55IGF0dGFjaGVkIGRvY3VtZW50cyBjb250YWluIGluZm9ybWF0aW9uIGZy
b20gdGhlIHNlbmRpbmcgY29tcGFueSBvciBpdHMgcGFyZW50IGNvbXBhbnkocyksIHN1YnNpZGlh
cmllcywgZGl2aXNpb25zIG9yIGJyYW5jaCBvZmZpY2VzIHRoYXQgbWF5IGJlIGNvbmZpZGVudGlh
bC4gSWYgeW91IGFyZSBub3QgdGhlIGludGVuZGVkIHJlY2lwaWVudCwgeW91IG1heSBub3QgcmVh
ZCwgY29weSwgZGlzdHJpYnV0ZSwgb3IgdXNlIHRoaXMgaW5mb3JtYXRpb24uIElmIHlvdSBoYXZl
IHJlY2VpdmVkIHRoaXMgdHJhbnNtaXNzaW9uIGluIGVycm9yLCBwbGVhc2Ugbm90aWZ5IHRoZSBz
ZW5kZXIgaW1tZWRpYXRlbHkgYnkgcmVwbHkgZS1tYWlsIGFuZCB0aGVuIGRlbGV0ZSB0aGlzIG1l
c3NhZ2UuDQo
^ permalink raw reply [flat|nested] 5+ messages in thread
* 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...
2015-07-23 5:27 ` Stephen Hemminger
2015-07-23 5:44 ` [dpdk-dev] 回覆︰ " MAC Lee
@ 2015-07-23 7:31 ` Vithal S Mohare
2015-07-23 16:45 ` Stephen Hemminger
1 sibling, 1 reply; 5+ messages in thread
From: Vithal S Mohare @ 2015-07-23 7:31 UTC (permalink / raw)
To: Stephen Hemminger, mac_leehk; +Cc: dev
How about the below changes? I have been using below changes and helping to resolve the issue.
===============================================================================
===== dpdk/lib/librte_pmd_vmxnet3/vmxnet3_ring.h#3 edit (text) =====
@@ -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;
};
typedef struct vmxnet3_rx_queue {
struct rte_mempool *mp;
struct vmxnet3_hw *hw;
===== dpdk/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c#5 edit (text) =====
@@ -645,10 +645,32 @@ rcd_done:
break;
}
}
}
+ /* 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' descriptors, packets from
+ * network will not be 'DMA'd to driver. While the only way to refresh 'rxd' back to hw is
+ * though above i.e. when packet is received from hw. So, there is potential dead-lock.
+ *
+ * Now, to break the deadlock, vmxnet3_post_rx_bufs() is triggered below when the poll
+ * goes empty 'rcd'. vmxnet3_post_rx_bufs() is no-op if all the descriptors are allocated
+ * in hw
+ */
+
+ if (rcd->gen != rxq->comp_ring.gen) {
+ ring_idx = (uint8_t)((rcd->rqID == 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++;
+ }
+ }
+
return (nb_rx);
}
===============================================================================
-----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 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...
On Thu, 23 Jul 2015 09:48:55 +0800
mac_leehk@yahoo.com.hk wrote:
> From: marco <marco@ubuntu.(none)>
Thank you for addressing a real bug.
But there are several issues with the patch as submitted:
* the standard way to handle allocation failure in network drivers is to drop 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 deadlock.
* 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 summary
followed by a description. I don't know what mail client you used but everything
is smashed into the Subject.
* all patches require a Signed-off-by with a real name for Developer's Certificate Of Origin
* the style is wrong, indentation is a mess please indent with tabs not spaces.
* 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
> => 100755 drivers/net/vmxnet3/vmxnet3_rxtx.c
>
> diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> 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;
> }
>
> +static inline void
> +vmxnet3_renew_desc(vmxnet3_rx_queue_t *rxq, uint8_t ring_id,struct
> +rte_mbuf *mbuf) {
> + uint32_t val = 0;
> + struct vmxnet3_cmd_ring *ring = &rxq->cmd_ring[ring_id];
> +
> + struct Vmxnet3_RxDesc *rxd;
> + vmxnet3_buf_info_t *buf_info = &ring->buf_info[ring->next2fill];
> +
> + rxd = (struct Vmxnet3_RxDesc *)(ring->base + ring->next2fill);
> +
> + if (ring->rid == 0) {
> + /* Usually: One HEAD type buf per packet
> + * val = (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 = VMXNET3_RXD_BTYPE_HEAD;
> + } else {
> + /* All BODY type buffers for 2nd ring; which won't be used at all by ESXi */
> + val = 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 = mbuf;
> + buf_info->len = (uint16_t)(mbuf->buf_len -
> + RTE_PKTMBUF_HEADROOM);
> + buf_info->bufPA = RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mbuf);
> +
> + /* Load Rx Descriptor with the buffer's GPA */
> + rxd->addr = buf_info->bufPA;
> +
> + /* After this point rxd->addr MUST not be NULL */
> + rxd->btype = val;
> + rxd->len = buf_info->len;
> + /* Flip gen bit at the end to change ownership */
> + rxd->gen = ring->gen;
> +
> + vmxnet3_cmd_ring_adv_next2fill(ring);
> +
> +}
> /*
> * Allocates mbufs and clusters. Post rx descriptors with buffer details
> * 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)
> }
>
> while (rcd->gen == rxq->comp_ring.gen) {
> + struct rte_mbuf *rep;
> if (nb_rx >= nb_pkts)
> break;
> +
> + rep = rte_rxmbuf_alloc(rxq->mp);
> + if (rep == NULL) {
> + rxq->stats.rx_buf_alloc_failure++;
> + break;
> + }
>
> idx = rcd->rxdIdx;
> ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1); @@ -657,7
> +709,7 @@ rcd_done:
> VMXNET3_INC_RING_IDX_ONLY(rxq->cmd_ring[ring_idx].next2comp,
> rxq->cmd_ring[ring_idx].size);
>
> /* 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 * VMXNET3_REG_ALIGN),
> rxq->cmd_ring[ring_idx].next2fill);
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-07-23 16:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-23 1:48 [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(). The RXD will not have "free" mbuf for it but the counter still increment. Finally, no packet can be received. This fix is allocate the mbuf first, if the allocation is failed, then don't receive any packet and the packet will remain in RXD to prevent any packet drop.If the allocation is sucess, the vmxnet3_post_rx_bufs() will call vmxnet3_renew_desc() and RXD will be renew inside mac_leehk
2015-07-23 5:27 ` Stephen Hemminger
2015-07-23 5:44 ` [dpdk-dev] 回覆︰ " MAC Lee
2015-07-23 7:31 ` [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 Vithal S Mohare
2015-07-23 16:45 ` Stephen Hemminger
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).