From patchwork Mon Nov 14 10:40:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Puiu X-Patchwork-Id: 17011 Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 9A719374D; Mon, 14 Nov 2016 11:40:33 +0100 (CET) Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by dpdk.org (Postfix) with ESMTP id 31307374C for ; Mon, 14 Nov 2016 11:40:32 +0100 (CET) Received: by mail-wm0-f65.google.com with SMTP id m203so14025799wma.3 for ; Mon, 14 Nov 2016 02:40:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=/iiFM0a0G83sW3QdWLSshWIi8CdPWngbknCsWQ6W09k=; b=GEHpZSKvRgE+3ScXwi8VFcuZFKwVM5S3DnjCQVInrvaSrc6Mzl7oWoOJH8ydu4vFUP S+x3/Zgk0W18qsbx47jEuSefYhODhaj6wg61aV76WhRBiQpWykBO5GxvRGtxE4QDDGl3 v391Gwx4PbwjWX5jjlntKchodvmlkJV0GRKYofXQZF3ZP4+PikcP1nwkOIMPLXQmXhj6 Ghu6KVYYUD7g7BzAfip7dbpYzWKLNZb0utk7PPOZtgZRixESQ7pv14o0e78fSRMJ25tw eazFCWYzPzWE8sUc6OeUslz3CF8nMl5Y5jT/AnFVvKbauxFcDaIf2lSvWlS4lSKjciaa TN3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=/iiFM0a0G83sW3QdWLSshWIi8CdPWngbknCsWQ6W09k=; b=OTlo2FBP7IAUkKSvi07TGZjkrPQ+pEGr8A71YsRQaZ5DkzxSkYpHFb9B0vcXg4HkGs 4+YcRjuxSUlSZ9C2s2lWLISINOxsX7lCdARDmLcp27bN5TtVzpekXFJNFAIA3Zo3dtXj G9s5oPlrm9O8SfOdxPv3PB1OBASDrSEqlnGcNUadgz3HMbkUryqC0qGMnPMEB2h/rxib 0mMZCohM2h0p2819pPvj+rYsj/58MD0a7Ean7h8MHVqis7hgsEkcA6BPC2+KQNhXMh7m Fsqb3H4vQN1aAJJAg5CDRyqe9Zo9RZQ4GCrozaUZJdoikX4wFdEkk67qu3fk0XYX3m92 49Cw== X-Gm-Message-State: ABUngvc5onqpIr7l9C1Z+ra3PI2lhNserKt4Ti9tova76TBzoWsxxsyGwB+l5kDzSFFiGA== X-Received: by 10.28.66.194 with SMTP id k63mr10852536wmi.140.1479120031815; Mon, 14 Nov 2016 02:40:31 -0800 (PST) Received: from spuiu-vm2.anuesystems.local ([109.100.41.154]) by smtp.gmail.com with ESMTPSA id wg8sm23101311wjb.42.2016.11.14.02.40.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 14 Nov 2016 02:40:31 -0800 (PST) From: Stefan Puiu To: dev@dpdk.org Cc: mac_leehk@yahoo.com.hk, yongwang@vmware.com, Stefan Puiu Date: Mon, 14 Nov 2016 12:40:22 +0200 Message-Id: <1479120022-48481-1-git-send-email-stefan.puiu@gmail.com> X-Mailer: git-send-email 1.9.1 Subject: [dpdk-dev] [PATCH] vmxnet3: fix Rx deadlock 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Our use case is that we have an app that needs to keep mbufs around for a while. We've seen cases when calling vmxnet3_post_rx_bufs() from vmxet3_recv_pkts(), it might not succeed to add any mbufs to any RX descriptors (where it returns -err). Since there are no mbufs that the virtual hardware can use, and since nobody calls vmxnet3_post_rx_bufs() after that, no packets will be received after this. I call this a deadlock for lack of a better term - the virtual HW waits for free mbufs, while the app waits for the hardware to notify it for data. Note that after this, the app can't recover. This fix is a rework of this patch by Marco Lee: http://dpdk.org/dev/patchwork/patch/6575/. I had to forward port it, address review comments and also reverted the allocation failure handing to the first version of the patch (http://dpdk.org/ml/archives/dev/2015-July/022079.html), since that's the only approach that seems to work, and seems to be what other drivers are doing (I checked ixgbe and em). Reusing the mbuf that's getting passed to the application doesn't seem to make sense, and it was causing weird issues in our app. Also, reusing rxm without checking if it's NULL could cause the code to crash. --- drivers/net/vmxnet3/vmxnet3_rxtx.c | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c index b109168..c9d2488 100644 --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c @@ -518,6 +518,32 @@ 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 = + (struct Vmxnet3_RxDesc *)(ring->base + ring->next2fill); + vmxnet3_buf_info_t *buf_info = &ring->buf_info[ring->next2fill]; + + if (ring_id == 0) + val = VMXNET3_RXD_BTYPE_HEAD; + else + val = VMXNET3_RXD_BTYPE_BODY; + + 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); + + rxd->addr = buf_info->bufPA; + rxd->btype = val; + rxd->len = buf_info->len; + 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. @@ -657,9 +683,17 @@ 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 *newm; if (nb_rx >= nb_pkts) break; + newm = rte_mbuf_raw_alloc(rxq->mp); + if (unlikely(newm == NULL)) { + PMD_RX_LOG(ERR, "Error allocating mbuf"); + rxq->stats.rx_buf_alloc_failure++; + break; + } + idx = rcd->rxdIdx; ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1); rxd = (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx; @@ -759,8 +793,8 @@ 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); + /* It's time to renew descriptors */ + vmxnet3_renew_desc(rxq, ring_idx, newm); 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);