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 於 2015年07月23日 (週四) 1:27 PM 寫道﹕ On Thu, 23 Jul 2015 09:48:55 +0800 mac_leehk@yahoo.com.hk wrote: > From: marco 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: Received: from mx0b-0016ce01.pphosted.com (mx0b-0016ce01.pphosted.com [67.231.156.153]) by dpdk.org (Postfix) with ESMTP id BBF7658CB for ; 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 cipherS128-SHA bits8 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 To: Stephen Hemminger 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: 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: Content-Transfer-Encoding: base64 MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=nai engineW00 definitionsx70 signaturesg0613 X-Proofpoint-Spam-Details: rule=notspam policyfault 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" 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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