From patchwork Mon Oct 26 03:56:16 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ajit Khaparde X-Patchwork-Id: 82144 X-Patchwork-Delegate: ajit.khaparde@broadcom.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 488D8A04B5; Mon, 26 Oct 2020 05:01:47 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 155E85A62; Mon, 26 Oct 2020 04:57:25 +0100 (CET) Received: from mail-pf1-f193.google.com (mail-pf1-f193.google.com [209.85.210.193]) by dpdk.org (Postfix) with ESMTP id E3D344C87 for ; Mon, 26 Oct 2020 04:56:47 +0100 (CET) Received: by mail-pf1-f193.google.com with SMTP id j18so5475793pfa.0 for ; Sun, 25 Oct 2020 20:56:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version; bh=sShdDqBUQQpNqT/OuLkQXWBFWwhgPCrOx/3PpeypyZA=; b=UNRBl3jzTBH0TKMMMr7THuewG0bOj6x3pX2gq2TDUwo9AAcnKph++xSNG8/aBrU9V7 P/fuc9SOlp7BIqr4qgQ+I46JxttZcVeQtW1f5XwHF/oVGNuCnT9CPUWV9OIHqSglALKa GA9nMxfY1sUUNFLdUFi2hNiit0N/974LbGCs8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version; bh=sShdDqBUQQpNqT/OuLkQXWBFWwhgPCrOx/3PpeypyZA=; b=gfYdGHAjwLxY3QPieYVTIuYNgoOsUAQipcGyN6FMBtcZOJ/FSCSbI6S28n5GSN4YY9 Jcl69caboavIXUeNDm+9+4Esa+10erQ0pkMlYagmm7FrbjhnF7qa5cm2Sa+OljuFVdwa AWMYzFv/hIw888m16ruTxdQzKvkbnH8QWmIkmsJO4uWdtaOLz3RV/yXFCvpkM5gv9BxQ n6xePpcTj98pa/93aV2GEhRc5ieFTvfV7fFl0p+LgU840yGlC7jsjLi/zsW3h9V0GWN/ UGGE9Sy7Lta333h/z6ACEyCp2A8Kvn/9YklA7x2jJkntGvGwT3ed9wCOY9QYm1N/VT4S ERsg== X-Gm-Message-State: AOAM533qOoENgjc5z1QqLq4utjAARPNw2AC9+sS1ZFawvEaMI93zPUPj SlJ9XrFoWVTc9nZqyfXDWpXgo5+bcr8Uwwr8TJfUBjTy6dE8+NYjPd7mK9aOGt7SvwOMUpbqQzI 3pD0AtwSbNB8D2QLebQ84gWnEyCG/ThOf413Se0dgFnAnQ0K0jeOEfu+zQMTfUuJ4iQ== X-Google-Smtp-Source: ABdhPJwpgfdY97ihsSM1y+iYgi1GqRIjL6eYXYg0SjLC99dV/+/2nft3KtZ+zitRMeime7dQ00Wk2Q== X-Received: by 2002:a63:fb11:: with SMTP id o17mr13833598pgh.109.1603684605759; Sun, 25 Oct 2020 20:56:45 -0700 (PDT) Received: from localhost.localdomain ([192.19.228.250]) by smtp.gmail.com with ESMTPSA id z185sm10207463pfz.32.2020.10.25.20.56.44 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 25 Oct 2020 20:56:45 -0700 (PDT) From: Ajit Khaparde To: dev@dpdk.org Cc: Rahul Gupta , Somnath Kotur Date: Sun, 25 Oct 2020 20:56:16 -0700 Message-Id: <20201026035616.19264-16-ajit.khaparde@broadcom.com> X-Mailer: git-send-email 2.21.1 (Apple Git-122.3) In-Reply-To: <20201026035616.19264-1-ajit.khaparde@broadcom.com> References: <20201026035616.19264-1-ajit.khaparde@broadcom.com> MIME-Version: 1.0 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: [dpdk-dev] [PATCH v4 15/15] net/bnxt: fix Rx performance by removing spinlock 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: Rahul Gupta The spinlock was trying to protect scenarios where rx_queue stop/start could be initiated dynamically. Assigning bnxt_dummy_recv_pkts and bnxt_dummy_xmit_pkts immediately to avoid concurrent access of mbuf in Rx and cleanup path should help achieve the same result. Fixes: 14255b351537 ("net/bnxt: fix queue start/stop operations") Reviewed-by: Ajit Khaparde Reviewed-by: Somnath Kotur Signed-off-by: Rahul Gupta --- drivers/net/bnxt/bnxt.h | 4 ++++ drivers/net/bnxt/bnxt_cpr.c | 12 ++++++++++++ drivers/net/bnxt/bnxt_cpr.h | 1 + drivers/net/bnxt/bnxt_rxq.c | 4 ---- drivers/net/bnxt/bnxt_rxq.h | 3 --- drivers/net/bnxt/bnxt_rxr.c | 5 +---- drivers/net/bnxt/bnxt_rxr.h | 2 -- drivers/net/bnxt/bnxt_txr.h | 2 -- 8 files changed, 18 insertions(+), 15 deletions(-) diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h index 57178192d2..90ced972c0 100644 --- a/drivers/net/bnxt/bnxt.h +++ b/drivers/net/bnxt/bnxt.h @@ -890,6 +890,10 @@ void bnxt_print_link_info(struct rte_eth_dev *eth_dev); uint16_t bnxt_rss_hash_tbl_size(const struct bnxt *bp); int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete); +uint16_t bnxt_dummy_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, + uint16_t nb_pkts); +uint16_t bnxt_dummy_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, + uint16_t nb_pkts); extern const struct rte_flow_ops bnxt_flow_ops; diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c index 91d1ffe46c..ee96ae81bf 100644 --- a/drivers/net/bnxt/bnxt_cpr.c +++ b/drivers/net/bnxt/bnxt_cpr.c @@ -121,6 +121,12 @@ void bnxt_handle_async_event(struct bnxt *bp, PMD_DRV_LOG(INFO, "Port conn async event\n"); break; case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_RESET_NOTIFY: + /* + * Avoid any rx/tx packet processing during firmware reset + * operation. + */ + bnxt_stop_rxtx(bp); + /* Ignore reset notify async events when stopping the port */ if (!bp->eth_dev->data->dev_started) { bp->flags |= BNXT_FLAG_FATAL_ERROR; @@ -337,3 +343,9 @@ bool bnxt_is_recovery_enabled(struct bnxt *bp) return false; } + +void bnxt_stop_rxtx(struct bnxt *bp) +{ + bp->eth_dev->rx_pkt_burst = &bnxt_dummy_recv_pkts; + bp->eth_dev->tx_pkt_burst = &bnxt_dummy_xmit_pkts; +} diff --git a/drivers/net/bnxt/bnxt_cpr.h b/drivers/net/bnxt/bnxt_cpr.h index cccd6cdbe0..ff9697f4c8 100644 --- a/drivers/net/bnxt/bnxt_cpr.h +++ b/drivers/net/bnxt/bnxt_cpr.h @@ -126,4 +126,5 @@ void bnxt_wait_for_device_shutdown(struct bnxt *bp); bool bnxt_is_recovery_enabled(struct bnxt *bp); bool bnxt_is_master_func(struct bnxt *bp); +void bnxt_stop_rxtx(struct bnxt *bp); #endif diff --git a/drivers/net/bnxt/bnxt_rxq.c b/drivers/net/bnxt/bnxt_rxq.c index 78514143e5..e0ec342162 100644 --- a/drivers/net/bnxt/bnxt_rxq.c +++ b/drivers/net/bnxt/bnxt_rxq.c @@ -210,8 +210,6 @@ void bnxt_rx_queue_release_mbufs(struct bnxt_rx_queue *rxq) if (!rxq || !rxq->rx_ring) return; - rte_spinlock_lock(&rxq->lock); - sw_ring = rxq->rx_ring->rx_buf_ring; if (sw_ring) { for (i = 0; @@ -248,7 +246,6 @@ void bnxt_rx_queue_release_mbufs(struct bnxt_rx_queue *rxq) } } - rte_spinlock_unlock(&rxq->lock); } void bnxt_free_rx_mbufs(struct bnxt *bp) @@ -389,7 +386,6 @@ int bnxt_rx_queue_setup_op(struct rte_eth_dev *eth_dev, rxq->rx_started = true; } eth_dev->data->rx_queue_state[queue_idx] = queue_state; - rte_spinlock_init(&rxq->lock); /* Configure mtu if it is different from what was configured before */ if (!queue_idx) diff --git a/drivers/net/bnxt/bnxt_rxq.h b/drivers/net/bnxt/bnxt_rxq.h index 201bda2269..c72105cf06 100644 --- a/drivers/net/bnxt/bnxt_rxq.h +++ b/drivers/net/bnxt/bnxt_rxq.h @@ -16,9 +16,6 @@ struct bnxt; struct bnxt_rx_ring_info; struct bnxt_cp_ring_info; struct bnxt_rx_queue { - rte_spinlock_t lock; /* Synchronize between rx_queue_stop - * and fast path - */ struct rte_mempool *mb_pool; /* mbuf pool for RX ring */ uint64_t mbuf_initializer; /* val to init mbuf */ uint16_t nb_rx_desc; /* num of RX desc */ diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c index e41833cc43..4a8326e335 100644 --- a/drivers/net/bnxt/bnxt_rxr.c +++ b/drivers/net/bnxt/bnxt_rxr.c @@ -843,8 +843,7 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, return 0; /* If Rx Q was stopped return */ - if (unlikely(!rxq->rx_started || - !rte_spinlock_trylock(&rxq->lock))) + if (unlikely(!rxq->rx_started)) return 0; #if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) @@ -946,8 +945,6 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, } done: - rte_spinlock_unlock(&rxq->lock); - return nb_rx_pkts; } diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h index b874e54a8c..2e2d1242eb 100644 --- a/drivers/net/bnxt/bnxt_rxr.h +++ b/drivers/net/bnxt/bnxt_rxr.h @@ -77,8 +77,6 @@ struct bnxt_rx_ring_info { uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts); -uint16_t bnxt_dummy_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, - uint16_t nb_pkts); void bnxt_free_rx_rings(struct bnxt *bp); int bnxt_init_rx_ring_struct(struct bnxt_rx_queue *rxq, unsigned int socket_id); int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq); diff --git a/drivers/net/bnxt/bnxt_txr.h b/drivers/net/bnxt/bnxt_txr.h index d241227d4c..3dfc8ef9b4 100644 --- a/drivers/net/bnxt/bnxt_txr.h +++ b/drivers/net/bnxt/bnxt_txr.h @@ -49,8 +49,6 @@ int bnxt_init_one_tx_ring(struct bnxt_tx_queue *txq); int bnxt_init_tx_ring_struct(struct bnxt_tx_queue *txq, unsigned int socket_id); uint16_t bnxt_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts); -uint16_t bnxt_dummy_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, - uint16_t nb_pkts); #if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) uint16_t bnxt_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts);