From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8E391A0C43 for ; Mon, 16 Aug 2021 16:30:27 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 894024115C; Mon, 16 Aug 2021 16:30:27 +0200 (CEST) Received: from relay.smtp-ext.broadcom.com (lpdvsmtp11.broadcom.com [192.19.166.231]) by mails.dpdk.org (Postfix) with ESMTP id 0B2E241157 for ; Mon, 16 Aug 2021 16:30:26 +0200 (CEST) Received: from dhcp-10-123-153-22.dhcp.broadcom.net (bgccx-dev-host-lnx2.bec.broadcom.net [10.123.153.22]) by relay.smtp-ext.broadcom.com (Postfix) with ESMTP id C55622477F for ; Mon, 16 Aug 2021 07:30:24 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 relay.smtp-ext.broadcom.com C55622477F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=broadcom.com; s=dkimrelay; t=1629124225; bh=nJ8KTH/te6uOdmrB4aYY5mTxIZ5S5LQtwJ0qXu5WGQU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=LOjcBBMFP+SAIB/KHRE34AuFpXjaNlTTb2amXe8ZMfD41X2+Qrvgz8bJwHIXujUD+ Zr1yrnL36SfnCcY/ud86PlP4kATVlPE+5czZpfcz2EJc4CMry4jXYPC9oTngOJ+4WQ 9ZGATbUhdTAfMbdwrPJi40+fZCOcPnmtLu+GPsDM= From: Kalesh A P To: stable@dpdk.org Date: Mon, 16 Aug 2021 20:20:58 +0530 Message-Id: <20210816145059.31065-6-kalesh-anakkur.purayil@broadcom.com> X-Mailer: git-send-email 2.10.1 In-Reply-To: <20210816145059.31065-1-kalesh-anakkur.purayil@broadcom.com> References: <20210816145059.31065-1-kalesh-anakkur.purayil@broadcom.com> Subject: [dpdk-stable] [PATCH 19.11 5/6] net/bnxt: workaround spurious zero stats in Thor X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" From: Somnath Kotur [ upstream commit 219842b9990c2a3a426c14c7911a44d2cb9b6fdf ] There is a HW bug that can result in certain stats being reported as zero. Workaround this by ignoring stats with a value of zero based on the previously stored snapshot of the same stat. This bug mainly manifests in the output of func_qstats as FW aggregrates each ring's stat value to give the per function stat and if one of them is zero, the per function stat value ends up being lower than the previous snapshot which shows up as a zero PPS value in testpmd. Eliminate invocation of func_qstats and aggregate the per-ring stat values in the driver itself to derive the func_qstats output post accounting for the spurious zero stat value. Bugzilla ID: 641 Fixes: f8168ca0e690 ("net/bnxt: support thor controller") Signed-off-by: Somnath Kotur Reviewed-by: Lance Richardson Reviewed-by: Kalesh AP Acked-by: Ajit Khaparde --- drivers/net/bnxt/bnxt.h | 46 ++++++++++++++++++ drivers/net/bnxt/bnxt_ethdev.c | 39 +++++++++++++++ drivers/net/bnxt/bnxt_hwrm.c | 105 ++++++++++++++++++++++++++++++++++------- drivers/net/bnxt/bnxt_hwrm.h | 5 +- drivers/net/bnxt/bnxt_stats.c | 63 ++++++++++++++++++++++--- 5 files changed, 233 insertions(+), 25 deletions(-) diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h index 60173e4..ceff0a7 100644 --- a/drivers/net/bnxt/bnxt.h +++ b/drivers/net/bnxt/bnxt.h @@ -492,6 +492,50 @@ struct bnxt_error_recovery_info { #define BNXT_FW_STATUS_SHUTDOWN 0x100000 #define BNXT_HWRM_SHORT_REQ_LEN sizeof(struct hwrm_short_input) + +struct bnxt_ring_stats { + /* Number of transmitted unicast packets */ + uint64_t tx_ucast_pkts; + /* Number of transmitted multicast packets */ + uint64_t tx_mcast_pkts; + /* Number of transmitted broadcast packets */ + uint64_t tx_bcast_pkts; + /* Number of packets discarded in transmit path */ + uint64_t tx_discard_pkts; + /* Number of packets in transmit path with error */ + uint64_t tx_error_pkts; + /* Number of transmitted bytes for unicast traffic */ + uint64_t tx_ucast_bytes; + /* Number of transmitted bytes for multicast traffic */ + uint64_t tx_mcast_bytes; + /* Number of transmitted bytes for broadcast traffic */ + uint64_t tx_bcast_bytes; + /* Number of received unicast packets */ + uint64_t rx_ucast_pkts; + /* Number of received multicast packets */ + uint64_t rx_mcast_pkts; + /* Number of received broadcast packets */ + uint64_t rx_bcast_pkts; + /* Number of packets discarded in receive path */ + uint64_t rx_discard_pkts; + /* Number of packets in receive path with errors */ + uint64_t rx_error_pkts; + /* Number of received bytes for unicast traffic */ + uint64_t rx_ucast_bytes; + /* Number of received bytes for multicast traffic */ + uint64_t rx_mcast_bytes; + /* Number of received bytes for broadcast traffic */ + uint64_t rx_bcast_bytes; + /* Number of aggregated unicast packets */ + uint64_t rx_agg_pkts; + /* Number of aggregated unicast bytes */ + uint64_t rx_agg_bytes; + /* Number of aggregation events */ + uint64_t rx_agg_events; + /* Number of aborted aggregations */ + uint64_t rx_agg_aborts; +}; + struct bnxt { void *bar0; @@ -661,6 +705,8 @@ struct bnxt { /* Struct to hold adapter error recovery related info */ struct bnxt_error_recovery_info *recovery_info; + struct bnxt_ring_stats *prev_rx_ring_stats; + struct bnxt_ring_stats *prev_tx_ring_stats; }; static diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 4d428b6..06b043d 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -334,6 +334,39 @@ static int bnxt_setup_one_vnic(struct bnxt *bp, uint16_t vnic_id) return rc; } +static void bnxt_free_prev_ring_stats(struct bnxt *bp) +{ + rte_free(bp->prev_rx_ring_stats); + rte_free(bp->prev_tx_ring_stats); + + bp->prev_rx_ring_stats = NULL; + bp->prev_tx_ring_stats = NULL; +} + +static int bnxt_alloc_prev_ring_stats(struct bnxt *bp) +{ + bp->prev_rx_ring_stats = rte_zmalloc("bnxt_prev_rx_ring_stats", + sizeof(struct bnxt_ring_stats) * + bp->rx_cp_nr_rings, + 0); + if (bp->prev_rx_ring_stats == NULL) + return -ENOMEM; + + bp->prev_tx_ring_stats = rte_zmalloc("bnxt_prev_tx_ring_stats", + sizeof(struct bnxt_ring_stats) * + bp->tx_cp_nr_rings, + 0); + if (bp->prev_tx_ring_stats == NULL) + goto error; + + return 0; + +error: + bnxt_free_prev_ring_stats(bp); + return -ENOMEM; +} + + static int bnxt_init_chip(struct bnxt *bp) { struct rte_eth_link new; @@ -880,6 +913,10 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) if (rc) goto error; + rc = bnxt_alloc_prev_ring_stats(bp); + if (rc) + goto error; + eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev); eth_dev->data->dev_started = 1; @@ -905,6 +942,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) bnxt_free_tx_mbufs(bp); bnxt_free_rx_mbufs(bp); bnxt_hwrm_if_change(bp, false); + bnxt_free_prev_ring_stats(bp); eth_dev->data->dev_started = 0; return rc; } @@ -1055,6 +1093,7 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev) bnxt_int_handler(eth_dev); bnxt_shutdown_nic(bp); bnxt_hwrm_if_change(bp, false); + bnxt_free_prev_ring_stats(bp); bp->rx_cosq_cnt = 0; } diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c index 724fa25..22e92a9 100644 --- a/drivers/net/bnxt/bnxt_hwrm.c +++ b/drivers/net/bnxt/bnxt_hwrm.c @@ -3690,8 +3690,20 @@ int bnxt_hwrm_exec_fwd_resp(struct bnxt *bp, uint16_t target_id, return rc; } -int bnxt_hwrm_ctx_qstats(struct bnxt *bp, uint32_t cid, int idx, - struct rte_eth_stats *stats, uint8_t rx) +static void bnxt_update_prev_stat(uint64_t *cntr, uint64_t *prev_cntr) +{ + /* One of the HW stat values that make up this counter was zero as + * returned by HW in this iteration, so use the previous + * iteration's counter value + */ + if (*prev_cntr && *cntr == 0) + *cntr = *prev_cntr; + else + *prev_cntr = *cntr; +} + +int bnxt_hwrm_ring_stats(struct bnxt *bp, uint32_t cid, int idx, + struct bnxt_ring_stats *ring_stats, bool rx) { int rc = 0; struct hwrm_stat_ctx_query_input req = {.req_type = 0}; @@ -3706,21 +3718,82 @@ int bnxt_hwrm_ctx_qstats(struct bnxt *bp, uint32_t cid, int idx, HWRM_CHECK_RESULT(); if (rx) { - stats->q_ipackets[idx] = rte_le_to_cpu_64(resp->rx_ucast_pkts); - stats->q_ipackets[idx] += rte_le_to_cpu_64(resp->rx_mcast_pkts); - stats->q_ipackets[idx] += rte_le_to_cpu_64(resp->rx_bcast_pkts); - stats->q_ibytes[idx] = rte_le_to_cpu_64(resp->rx_ucast_bytes); - stats->q_ibytes[idx] += rte_le_to_cpu_64(resp->rx_mcast_bytes); - stats->q_ibytes[idx] += rte_le_to_cpu_64(resp->rx_bcast_bytes); - stats->q_errors[idx] = rte_le_to_cpu_64(resp->rx_err_pkts); - stats->q_errors[idx] += rte_le_to_cpu_64(resp->rx_drop_pkts); + struct bnxt_ring_stats *prev_stats = &bp->prev_rx_ring_stats[idx]; + + ring_stats->rx_ucast_pkts = rte_le_to_cpu_64(resp->rx_ucast_pkts); + bnxt_update_prev_stat(&ring_stats->rx_ucast_pkts, + &prev_stats->rx_ucast_pkts); + + ring_stats->rx_mcast_pkts = rte_le_to_cpu_64(resp->rx_mcast_pkts); + bnxt_update_prev_stat(&ring_stats->rx_mcast_pkts, + &prev_stats->rx_mcast_pkts); + + ring_stats->rx_bcast_pkts = rte_le_to_cpu_64(resp->rx_bcast_pkts); + bnxt_update_prev_stat(&ring_stats->rx_bcast_pkts, + &prev_stats->rx_bcast_pkts); + + ring_stats->rx_ucast_bytes = rte_le_to_cpu_64(resp->rx_ucast_bytes); + bnxt_update_prev_stat(&ring_stats->rx_ucast_bytes, + &prev_stats->rx_ucast_bytes); + + ring_stats->rx_mcast_bytes = rte_le_to_cpu_64(resp->rx_mcast_bytes); + bnxt_update_prev_stat(&ring_stats->rx_mcast_bytes, + &prev_stats->rx_mcast_bytes); + + ring_stats->rx_bcast_bytes = rte_le_to_cpu_64(resp->rx_bcast_bytes); + bnxt_update_prev_stat(&ring_stats->rx_bcast_bytes, + &prev_stats->rx_bcast_bytes); + + ring_stats->rx_error_pkts = rte_le_to_cpu_64(resp->rx_err_pkts); + bnxt_update_prev_stat(&ring_stats->rx_error_pkts, + &prev_stats->rx_error_pkts); + + ring_stats->rx_discard_pkts = rte_le_to_cpu_64(resp->rx_drop_pkts); + bnxt_update_prev_stat(&ring_stats->rx_discard_pkts, + &prev_stats->rx_discard_pkts); + + ring_stats->rx_agg_pkts = rte_le_to_cpu_64(resp->rx_agg_pkts); + bnxt_update_prev_stat(&ring_stats->rx_agg_pkts, + &prev_stats->rx_agg_pkts); + + ring_stats->rx_agg_bytes = rte_le_to_cpu_64(resp->rx_agg_bytes); + bnxt_update_prev_stat(&ring_stats->rx_agg_bytes, + &prev_stats->rx_agg_bytes); + + ring_stats->rx_agg_events = rte_le_to_cpu_64(resp->rx_agg_events); + bnxt_update_prev_stat(&ring_stats->rx_agg_events, + &prev_stats->rx_agg_events); + + ring_stats->rx_agg_aborts = rte_le_to_cpu_64(resp->rx_agg_aborts); + bnxt_update_prev_stat(&ring_stats->rx_agg_aborts, + &prev_stats->rx_agg_aborts); } else { - stats->q_opackets[idx] = rte_le_to_cpu_64(resp->tx_ucast_pkts); - stats->q_opackets[idx] += rte_le_to_cpu_64(resp->tx_mcast_pkts); - stats->q_opackets[idx] += rte_le_to_cpu_64(resp->tx_bcast_pkts); - stats->q_obytes[idx] = rte_le_to_cpu_64(resp->tx_ucast_bytes); - stats->q_obytes[idx] += rte_le_to_cpu_64(resp->tx_mcast_bytes); - stats->q_obytes[idx] += rte_le_to_cpu_64(resp->tx_bcast_bytes); + struct bnxt_ring_stats *prev_stats = &bp->prev_tx_ring_stats[idx]; + + ring_stats->tx_ucast_pkts = rte_le_to_cpu_64(resp->tx_ucast_pkts); + bnxt_update_prev_stat(&ring_stats->tx_ucast_pkts, + &prev_stats->tx_ucast_pkts); + + ring_stats->tx_mcast_pkts = rte_le_to_cpu_64(resp->tx_mcast_pkts); + bnxt_update_prev_stat(&ring_stats->tx_mcast_pkts, + &prev_stats->tx_mcast_pkts); + + ring_stats->tx_bcast_pkts = rte_le_to_cpu_64(resp->tx_bcast_pkts); + bnxt_update_prev_stat(&ring_stats->tx_bcast_pkts, + &prev_stats->tx_bcast_pkts); + + ring_stats->tx_ucast_bytes = rte_le_to_cpu_64(resp->tx_ucast_bytes); + bnxt_update_prev_stat(&ring_stats->tx_ucast_bytes, + &prev_stats->tx_ucast_bytes); + + ring_stats->tx_mcast_bytes = rte_le_to_cpu_64(resp->tx_mcast_bytes); + bnxt_update_prev_stat(&ring_stats->tx_mcast_bytes, + &prev_stats->tx_mcast_bytes); + + ring_stats->tx_bcast_bytes = rte_le_to_cpu_64(resp->tx_bcast_bytes); + bnxt_update_prev_stat(&ring_stats->tx_bcast_bytes, + &prev_stats->tx_bcast_bytes); + } HWRM_UNLOCK(); diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h index 8ee9a7b..2fb0d97 100644 --- a/drivers/net/bnxt/bnxt_hwrm.h +++ b/drivers/net/bnxt/bnxt_hwrm.h @@ -113,9 +113,6 @@ int bnxt_hwrm_ring_grp_alloc(struct bnxt *bp, unsigned int idx); int bnxt_hwrm_ring_grp_free(struct bnxt *bp, unsigned int idx); int bnxt_hwrm_stat_clear(struct bnxt *bp, struct bnxt_cp_ring_info *cpr); -int bnxt_hwrm_ctx_qstats(struct bnxt *bp, uint32_t cid, int idx, - struct rte_eth_stats *stats, uint8_t rx); - int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t timeout); int bnxt_hwrm_vnic_alloc(struct bnxt *bp, struct bnxt_vnic_info *vnic); @@ -230,4 +227,6 @@ int bnxt_clear_one_vnic_filter(struct bnxt *bp, int bnxt_hwrm_poll_ver_get(struct bnxt *bp); int bnxt_hwrm_port_phy_qcaps(struct bnxt *bp); int bnxt_hwrm_rx_ring_reset(struct bnxt *bp, int queue_index); +int bnxt_hwrm_ring_stats(struct bnxt *bp, uint32_t cid, int idx, + struct bnxt_ring_stats *stats, bool rx); #endif diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c index 692b096..2b8b927 100644 --- a/drivers/net/bnxt/bnxt_stats.c +++ b/drivers/net/bnxt/bnxt_stats.c @@ -377,8 +377,48 @@ void bnxt_free_stats(struct bnxt *bp) } } +static void bnxt_fill_rte_eth_stats(struct rte_eth_stats *stats, + struct bnxt_ring_stats *ring_stats, + unsigned int i, bool rx) +{ + if (rx) { + stats->q_ipackets[i] = ring_stats->rx_ucast_pkts; + stats->q_ipackets[i] += ring_stats->rx_mcast_pkts; + stats->q_ipackets[i] += ring_stats->rx_bcast_pkts; + + stats->ipackets += stats->q_ipackets[i]; + + stats->q_ibytes[i] = ring_stats->rx_ucast_bytes; + stats->q_ibytes[i] += ring_stats->rx_mcast_bytes; + stats->q_ibytes[i] += ring_stats->rx_bcast_bytes; + + stats->ibytes += stats->q_ibytes[i]; + + stats->q_errors[i] = ring_stats->rx_discard_pkts; + stats->q_errors[i] += ring_stats->rx_error_pkts; + + stats->imissed += ring_stats->rx_discard_pkts; + stats->ierrors += ring_stats->rx_error_pkts; + stats->ierrors += ring_stats->rx_discard_pkts; + } else { + stats->q_opackets[i] = ring_stats->tx_ucast_pkts; + stats->q_opackets[i] += ring_stats->tx_mcast_pkts; + stats->q_opackets[i] += ring_stats->tx_bcast_pkts; + + stats->opackets += stats->q_opackets[i]; + + stats->q_obytes[i] = ring_stats->tx_ucast_bytes; + stats->q_obytes[i] += ring_stats->tx_mcast_bytes; + stats->q_obytes[i] += ring_stats->tx_bcast_bytes; + + stats->obytes += stats->q_obytes[i]; + + stats->oerrors += ring_stats->tx_discard_pkts; + } +} + int bnxt_stats_get_op(struct rte_eth_dev *eth_dev, - struct rte_eth_stats *bnxt_stats) + struct rte_eth_stats *bnxt_stats) { int rc = 0; unsigned int i; @@ -398,11 +438,17 @@ int bnxt_stats_get_op(struct rte_eth_dev *eth_dev, for (i = 0; i < num_q_stats; i++) { struct bnxt_rx_queue *rxq = bp->rx_queues[i]; struct bnxt_cp_ring_info *cpr = rxq->cp_ring; + struct bnxt_ring_stats ring_stats = {0}; - rc = bnxt_hwrm_ctx_qstats(bp, cpr->hw_stats_ctx_id, i, - bnxt_stats, 1); + if (!rxq->rx_started) + continue; + + rc = bnxt_hwrm_ring_stats(bp, cpr->hw_stats_ctx_id, i, + &ring_stats, true); if (unlikely(rc)) return rc; + + bnxt_fill_rte_eth_stats(bnxt_stats, &ring_stats, i, true); bnxt_stats->rx_nombuf += rte_atomic64_read(&rxq->rx_mbuf_alloc_fail); } @@ -413,14 +459,19 @@ int bnxt_stats_get_op(struct rte_eth_dev *eth_dev, for (i = 0; i < num_q_stats; i++) { struct bnxt_tx_queue *txq = bp->tx_queues[i]; struct bnxt_cp_ring_info *cpr = txq->cp_ring; + struct bnxt_ring_stats ring_stats = {0}; - rc = bnxt_hwrm_ctx_qstats(bp, cpr->hw_stats_ctx_id, i, - bnxt_stats, 0); + if (!txq->tx_started) + continue; + + rc = bnxt_hwrm_ring_stats(bp, cpr->hw_stats_ctx_id, i, + &ring_stats, false); if (unlikely(rc)) return rc; + + bnxt_fill_rte_eth_stats(bnxt_stats, &ring_stats, i, false); } - rc = bnxt_hwrm_func_qstats(bp, 0xffff, bnxt_stats); return rc; } -- 2.10.1