Re: [dpdk-stable] patch 'net/bnxt: clear cached statistics' has been queued to stable release 19.11.10

[Christian Ehrhardt <christian.ehrhardt@canonical.com>]

On Tue, Aug 10, 2021 at 5:43 PM <christian.ehrhardt@canonical.com> wrote:
>
> Hi,
>
> FYI, your patch has been queued to stable release 19.11.10

Hi,
while applying cleanly your patch caused build time failures like:

../drivers/net/bnxt/bnxt_stats.c: In function ‘bnxt_clear_prev_stat’:
../drivers/net/bnxt/bnxt_stats.c:433:11: error: ‘struct bnxt’ has no
member named ‘prev_rx_ring_stats’
  433 |  memset(bp->prev_rx_ring_stats, 0, sizeof(struct
bnxt_ring_stats) * bp->rx_cp_nr_rings);
      |           ^~
../drivers/net/bnxt/bnxt_stats.c:433:43: error: invalid application of
‘sizeof’ to incomplete type ‘struct bnxt_ring_stats’
  433 |  memset(bp->prev_rx_ring_stats, 0, sizeof(struct
bnxt_ring_stats) * bp->rx_cp_nr_rings);
      |                                           ^~~~~~
../drivers/net/bnxt/bnxt_stats.c:434:11: error: ‘struct bnxt’ has no
member named ‘prev_tx_ring_stats’
  434 |  memset(bp->prev_tx_ring_stats, 0, sizeof(struct
bnxt_ring_stats) * bp->tx_cp_nr_rings);
      |           ^~
../drivers/net/bnxt/bnxt_stats.c:434:43: error: invalid application of
‘sizeof’ to incomplete type ‘struct bnxt_ring_stats’
  434 |  memset(bp->prev_tx_ring_stats, 0, sizeof(struct
bnxt_ring_stats) * bp->tx_cp_nr_rings);
      |

Therefore the patch will be de-qeueud from the stable branch that shall become
19.11.10.
Please consider having a look and providing a backport.

A backport should contain a reference to the DPDK main branch commit
in it's commit message in the following fashion:
    [ upstream commit <commit's dpdk main branch SHA-1 checksum> ]

For example:
    https://git.dpdk.org/dpdk-stable/commit/?h=18.11&id=d90e6ae6f936ecdc2fd3811ff9f26aec7f3c06eb

When sending the backported patch, please indicate the target branch in the
subject line, as we have multiple branches, for example:
    [PATCH 19.11] foo/bar: fix baz

With git format-patch, this can be achieved by appending the parameter:
    --subject-prefix='PATCH 19.11'

Send the backported patch to "stable@dpdk.org" but not "dev@dpdk.org".

FYI, branch 19.11 is located at tree:
   https://git.dpdk.org/dpdk-stable

Thanks in advance,
Chrtistian

> Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
> It will be pushed if I get no objections before 08/12/21. So please
> shout if anyone has objections.
>
> Also note that after the patch there's a diff of the upstream commit vs the
> patch applied to the branch. This will indicate if there was any rebasing
> needed to apply to the stable branch. If there were code changes for rebasing
> (ie: not only metadata diffs), please double check that the rebase was
> correctly done.
>
> Queued patches are on a temporary branch at:
> https://github.com/cpaelzer/dpdk-stable-queue
>
> This queued commit can be viewed at:
> https://github.com/cpaelzer/dpdk-stable-queue/commit/a0776bdb0995d658f4c9793b70b56fe7f4118e5f
>
> Thanks.
>
> Christian Ehrhardt <christian.ehrhardt@canonical.com>
>
> ---
> From a0776bdb0995d658f4c9793b70b56fe7f4118e5f Mon Sep 17 00:00:00 2001
> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Date: Tue, 13 Jul 2021 19:04:13 +0530
> Subject: [PATCH] net/bnxt: clear cached statistics
>
> [ upstream commit 84fd852caac19795d0a05af8dabce8f011fe61a4 ]
>
> As part of the workaround put in the commit "219842b9990c",
> driver caches the last read stats values from the hardware.
> But this is not cleared during the clear stats operation. This
> results in showing up stale stats values while reading the stats
> after the clear operation.
>
> Fixes: 219842b9990c ("net/bnxt: workaround spurious zero stats in Thor")
>
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Lance Richardson <lance.richardson@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> ---
>  drivers/net/bnxt/bnxt_stats.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c
> index 692b0967eb..a30e2b465d 100644
> --- a/drivers/net/bnxt/bnxt_stats.c
> +++ b/drivers/net/bnxt/bnxt_stats.c
> @@ -424,6 +424,16 @@ int bnxt_stats_get_op(struct rte_eth_dev *eth_dev,
>         return rc;
>  }
>
> +static void bnxt_clear_prev_stat(struct bnxt *bp)
> +{
> +       /*
> +        * Clear the cached values of stats returned by HW in the previous
> +        * get operation.
> +        */
> +       memset(bp->prev_rx_ring_stats, 0, sizeof(struct bnxt_ring_stats) * bp->rx_cp_nr_rings);
> +       memset(bp->prev_tx_ring_stats, 0, sizeof(struct bnxt_ring_stats) * bp->tx_cp_nr_rings);
> +}
> +
>  int bnxt_stats_reset_op(struct rte_eth_dev *eth_dev)
>  {
>         struct bnxt *bp = eth_dev->data->dev_private;
> @@ -446,6 +456,8 @@ int bnxt_stats_reset_op(struct rte_eth_dev *eth_dev)
>                 rte_atomic64_clear(&rxq->rx_mbuf_alloc_fail);
>         }
>
> +       bnxt_clear_prev_stat(bp);
> +
>         return ret;
>  }
>
> @@ -611,5 +623,7 @@ int bnxt_dev_xstats_reset_op(struct rte_eth_dev *eth_dev)
>                 PMD_DRV_LOG(ERR, "Failed to reset xstats: %s\n",
>                             strerror(-ret));
>
> +       bnxt_clear_prev_stat(bp);
> +
>         return ret;
>  }
> --
> 2.32.0
>
> ---
>   Diff of the applied patch vs upstream commit (please double-check if non-empty:
> ---
> --- -   2021-08-10 15:11:15.728060604 +0200
> +++ 0070-net-bnxt-clear-cached-statistics.patch 2021-08-10 15:11:13.074638504 +0200
> @@ -1 +1 @@
> -From 84fd852caac19795d0a05af8dabce8f011fe61a4 Mon Sep 17 00:00:00 2001
> +From a0776bdb0995d658f4c9793b70b56fe7f4118e5f Mon Sep 17 00:00:00 2001
> @@ -5,0 +6,2 @@
> +[ upstream commit 84fd852caac19795d0a05af8dabce8f011fe61a4 ]
> +
> @@ -13 +14,0 @@
> -Cc: stable@dpdk.org
> @@ -24 +25 @@
> -index c7b23f46a1..991eafc644 100644
> +index 692b0967eb..a30e2b465d 100644
> @@ -27 +28 @@
> -@@ -603,6 +603,16 @@ int bnxt_stats_get_op(struct rte_eth_dev *eth_dev,
> +@@ -424,6 +424,16 @@ int bnxt_stats_get_op(struct rte_eth_dev *eth_dev,
> @@ -44 +45 @@
> -@@ -625,6 +635,8 @@ int bnxt_stats_reset_op(struct rte_eth_dev *eth_dev)
> +@@ -446,6 +456,8 @@ int bnxt_stats_reset_op(struct rte_eth_dev *eth_dev)
> @@ -53 +54 @@
> -@@ -933,6 +945,8 @@ int bnxt_dev_xstats_reset_op(struct rte_eth_dev *eth_dev)
> +@@ -611,5 +623,7 @@ int bnxt_dev_xstats_reset_op(struct rte_eth_dev *eth_dev)
> @@ -61 +61,0 @@
> -



-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd