From patchwork Wed Jun 24 01:11:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Long Li X-Patchwork-Id: 72046 X-Patchwork-Delegate: ferruh.yigit@amd.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id D41C0A0350; Wed, 24 Jun 2020 03:11:58 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0FDB91D6EE; Wed, 24 Jun 2020 03:11:58 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by dpdk.org (Postfix) with ESMTP id 2C6C41D6ED for ; Wed, 24 Jun 2020 03:11:56 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1004) id 49D1320B7192; Tue, 23 Jun 2020 18:11:55 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 49D1320B7192 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linuxonhyperv.com; s=default; t=1592961115; bh=UWNK/l3o3J0NiO9fJ03giMFOJX93JfRRS6dZWOdzJO4=; h=From:To:Cc:Subject:Date:From; b=kPLz5YaMMex3R6Gr1m80qnnV3i1LHawF+wt3wis7KZrj403PdAQ35JTsPopSJQKsU Zvxt0ZEIEptFrJpcvK0tYGggZaUKqgsego+MbwBiVmH48+ybBWYCZkOcQ1qd0DrJaw FskNUeA1aR6aE5ycsy0yBStwYvgVIVLg6XoBxaw0= From: longli@linuxonhyperv.com To: Stephen Hemminger Cc: dev@dpdk.org, Long Li Date: Tue, 23 Jun 2020 18:11:45 -0700 Message-Id: <1592961106-82820-1-git-send-email-longli@linuxonhyperv.com> X-Mailer: git-send-email 1.8.3.1 Subject: [dpdk-dev] [PATCH 1/2] net/netvsc: fix underflow error when external mbuf are used in the receive path X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Long Li When rte_pktmbuf_attach_extbuf() is used, the driver should not decrease the reference count in its callback function hn_rx_buf_free_cb, because the reference count is already decreased by rte_pktmbuf. Doing it twice may result in underflow and driver may never send an ack packet over vmbus to host. Also declares rxbuf_outstanding as atomic, because this value is shared among all receive queues. Signed-off-by: Long Li Acked-by: Stephen Hemminger --- drivers/net/netvsc/hn_nvs.c | 1 - drivers/net/netvsc/hn_rxtx.c | 30 +++++++++++------------------- drivers/net/netvsc/hn_var.h | 2 +- 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/drivers/net/netvsc/hn_nvs.c b/drivers/net/netvsc/hn_nvs.c index 0fa74cd46..466b1948e 100644 --- a/drivers/net/netvsc/hn_nvs.c +++ b/drivers/net/netvsc/hn_nvs.c @@ -99,7 +99,6 @@ __hn_nvs_execute(struct hn_data *hv, /* Silently drop received packets while waiting for response */ if (hdr->type == NVS_TYPE_RNDIS) { hn_nvs_ack_rxbuf(chan, xactid); - --hv->rxbuf_outstanding; goto retry; } diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c index a4f6d1b6a..aece38e2d 100644 --- a/drivers/net/netvsc/hn_rxtx.c +++ b/drivers/net/netvsc/hn_rxtx.c @@ -519,24 +519,13 @@ hn_rndis_rxinfo(const void *info_data, unsigned int info_dlen, return 0; } -/* - * Ack the consumed RXBUF associated w/ this channel packet, - * so that this RXBUF can be recycled by the hypervisor. - */ -static void hn_rx_buf_release(struct hn_rx_bufinfo *rxb) +static void hn_rx_buf_free_cb(void *buf __rte_unused, void *opaque) { - struct rte_mbuf_ext_shared_info *shinfo = &rxb->shinfo; + struct hn_rx_bufinfo *rxb = opaque; struct hn_data *hv = rxb->hv; - if (rte_mbuf_ext_refcnt_update(shinfo, -1) == 0) { - hn_nvs_ack_rxbuf(rxb->chan, rxb->xactid); - --hv->rxbuf_outstanding; - } -} - -static void hn_rx_buf_free_cb(void *buf __rte_unused, void *opaque) -{ - hn_rx_buf_release(opaque); + rte_atomic32_dec(&hv->rxbuf_outstanding); + hn_nvs_ack_rxbuf(rxb->chan, rxb->xactid); } static struct hn_rx_bufinfo *hn_rx_buf_init(const struct hn_rx_queue *rxq, @@ -576,7 +565,8 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb, * some space available in receive area for later packets. */ if (dlen >= HN_RXCOPY_THRESHOLD && - hv->rxbuf_outstanding < hv->rxbuf_section_cnt / 2) { + (uint32_t)rte_atomic32_read(&hv->rxbuf_outstanding) < + hv->rxbuf_section_cnt / 2) { struct rte_mbuf_ext_shared_info *shinfo; const void *rxbuf; rte_iova_t iova; @@ -590,8 +580,9 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb, iova = rte_mem_virt2iova(rxbuf) + RTE_PTR_DIFF(data, rxbuf); shinfo = &rxb->shinfo; - if (rte_mbuf_ext_refcnt_update(shinfo, 1) == 1) - ++hv->rxbuf_outstanding; + /* shinfo is already set to 1 by the caller */ + if (rte_mbuf_ext_refcnt_update(shinfo, 1) == 2) + rte_atomic32_inc(&hv->rxbuf_outstanding); rte_pktmbuf_attach_extbuf(m, data, iova, dlen + headroom, shinfo); @@ -832,7 +823,8 @@ hn_nvs_handle_rxbuf(struct rte_eth_dev *dev, } /* Send ACK now if external mbuf not used */ - hn_rx_buf_release(rxb); + if (rte_mbuf_ext_refcnt_update(&rxb->shinfo, -1) == 0) + hn_nvs_ack_rxbuf(rxb->chan, rxb->xactid); } /* diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h index 881832d85..7cb7713e9 100644 --- a/drivers/net/netvsc/hn_var.h +++ b/drivers/net/netvsc/hn_var.h @@ -113,7 +113,7 @@ struct hn_data { struct rte_mem_resource *rxbuf_res; /* UIO resource for Rx */ struct hn_rx_bufinfo *rxbuf_info; uint32_t rxbuf_section_cnt; /* # of Rx sections */ - volatile uint32_t rxbuf_outstanding; + rte_atomic32_t rxbuf_outstanding; uint16_t max_queues; /* Max available queues */ uint16_t num_queues; uint64_t rss_offloads; From patchwork Wed Jun 24 01:11:46 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Long Li X-Patchwork-Id: 72047 X-Patchwork-Delegate: ferruh.yigit@amd.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4FFDBA0350; Wed, 24 Jun 2020 03:12:09 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9850A1D705; Wed, 24 Jun 2020 03:12:07 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by dpdk.org (Postfix) with ESMTP id 0CD841D703 for ; Wed, 24 Jun 2020 03:12:06 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1004) id 58ECA20B7192; Tue, 23 Jun 2020 18:12:05 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 58ECA20B7192 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linuxonhyperv.com; s=default; t=1592961125; bh=n73Z0Ad4ZUntvjqaGraG5Y63nQJkCV07Oo/cXut0ako=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=GLl0TKkObWMg7CX7Y4rOlLUSfOC/e1WLladTmxK5E+smrQfotp2lRC4N7aqwG7hVT 2p2sT+A8imJuvSGKgGeGjukrh0MJbVYPKgLhYPyW59MuOTvuSJvrID31auikqUl6BX YLqGMzxQp5SxL6ssLq5FYb679bjWdUyZeN0a4isg= From: longli@linuxonhyperv.com To: Stephen Hemminger Cc: dev@dpdk.org, Long Li Date: Tue, 23 Jun 2020 18:11:46 -0700 Message-Id: <1592961106-82820-2-git-send-email-longli@linuxonhyperv.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1592961106-82820-1-git-send-email-longli@linuxonhyperv.com> References: <1592961106-82820-1-git-send-email-longli@linuxonhyperv.com> Subject: [dpdk-dev] [PATCH 2/2] net/netvsc: detach external buffer on failure X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Long Li When external buffer is used, driver should detach it if it doesn't make it successfully to the queue. Signed-off-by: Long Li Acked-by: Stephen Hemminger --- drivers/net/netvsc/hn_rxtx.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c index aece38e2d..52f5c0191 100644 --- a/drivers/net/netvsc/hn_rxtx.c +++ b/drivers/net/netvsc/hn_rxtx.c @@ -550,6 +550,7 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb, { struct hn_data *hv = rxq->hv; struct rte_mbuf *m; + bool use_extbuf = false; m = rte_pktmbuf_alloc(rxq->mb_pool); if (unlikely(!m)) { @@ -587,6 +588,7 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb, rte_pktmbuf_attach_extbuf(m, data, iova, dlen + headroom, shinfo); m->data_off = headroom; + use_extbuf = true; } else { /* Mbuf's in pool must be large enough to hold small packets */ if (unlikely(rte_pktmbuf_tailroom(m) < dlen)) { @@ -614,6 +616,8 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb, if (!hv->vlan_strip && rte_vlan_insert(&m)) { PMD_DRV_LOG(DEBUG, "vlan insert failed"); ++rxq->stats.errors; + if (use_extbuf) + rte_pktmbuf_detach_extbuf(m); rte_pktmbuf_free(m); return; } @@ -648,6 +652,8 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb, if (unlikely(rte_ring_sp_enqueue(rxq->rx_ring, m) != 0)) { ++rxq->stats.ring_full; PMD_RX_LOG(DEBUG, "rx ring full"); + if (use_extbuf) + rte_pktmbuf_detach_extbuf(m); rte_pktmbuf_free(m); } }