[dpdk-stable] patch 'net/bnxt: fix stats errors handling' has been queued to LTS release 17.11.10

[Kalesh Anakkur Purayil]

Hi Luca,

NAK for this patch as the rebasing was not correctly done. You can ignore
this patch for now from 17.11 stable inclusion.

Regards,
Kalesh

On Thu, Dec 19, 2019 at 8:07 PM <luca.boccassi at gmail.com> wrote:

> Hi,
>
> FYI, your patch has been queued to LTS release 17.11.10
>
> Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
> It will be pushed if I get no objections before 12/21/19. 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.
>
> Thanks.
>
> Luca Boccassi
>
> ---
> From ed1b2592aee44b786ac897651e9e5fc7ed350e9c Mon Sep 17 00:00:00 2001
> From: Kalesh AP <kalesh-anakkur.purayil at broadcom.com>
> Date: Wed, 9 Oct 2019 18:41:42 -0700
> Subject: [PATCH] net/bnxt: fix stats errors handling
>
> [ upstream commit 9f55e6ac7500e828d42e95100d5cb061e912be5c ]
>
> This patch fixes few checks and few return values while getting
> and clearing device statistics.
>
> 1. Fixed to return standard error code.
> 2. Clubbed few error checks
> 3. Removed an unnecessary return check
>
> Fixes: bfb9c2260be2 ("net/bnxt: support xstats get/reset")
> Fixes: 88920136688c ("net/bnxt: support xstats get by id")
>
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil at broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur at broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde at broadcom.com>
> ---
>  drivers/net/bnxt/bnxt_stats.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c
> index f8bb4ed9e5..6497926162 100644
> --- a/drivers/net/bnxt/bnxt_stats.c
> +++ b/drivers/net/bnxt/bnxt_stats.c
> @@ -360,16 +360,17 @@ int bnxt_dev_xstats_get_names_op(__rte_unused struct
> rte_eth_dev *eth_dev,
>  void bnxt_dev_xstats_reset_op(struct rte_eth_dev *eth_dev)
>  {
>         struct bnxt *bp = (struct bnxt *)eth_dev->data->dev_private;
> +       int ret;
>
> -       if (bp->flags & BNXT_FLAG_PORT_STATS && !BNXT_NPAR_PF(bp))
> -               bnxt_hwrm_port_clr_stats(bp);
> -
> -       if (BNXT_VF(bp))
> -               RTE_LOG(ERR, PMD, "Operation not supported on a VF
> device\n");
> -       if (BNXT_NPAR_PF(bp))
> -               RTE_LOG(ERR, PMD, "Operation not supported on a MF
> device\n");
> -       if (!(bp->flags & BNXT_FLAG_PORT_STATS))
> +       if (BNXT_VF(bp) || !BNXT_NPAR_PF(bp) ||
> +           !(bp->flags & BNXT_FLAG_PORT_STATS)) {
>                 RTE_LOG(ERR, PMD, "Operation not supported\n");
> +       }
> +
> +       ret = bnxt_hwrm_port_clr_stats(bp);
> +       if (ret != 0)
> +               RTE_LOG(ERR, PMD, "Failed to reset xstats: %s\n",
> +                           strerror(-ret));
>  }
>
>  int bnxt_dev_xstats_get_by_id_op(struct rte_eth_dev *dev, const uint64_t
> *ids,
> @@ -389,7 +390,7 @@ int bnxt_dev_xstats_get_by_id_op(struct rte_eth_dev
> *dev, const uint64_t *ids,
>         for (i = 0; i < limit; i++) {
>                 if (ids[i] >= stat_cnt) {
>                         RTE_LOG(ERR, PMD, "id value isn't valid");
> -                       return -1;
> +                       return -EINVAL;
>                 }
>                 values[i] = values_copy[ids[i]];
>         }
> @@ -415,7 +416,7 @@ int bnxt_dev_xstats_get_names_by_id_op(struct
> rte_eth_dev *dev,
>         for (i = 0; i < limit; i++) {
>                 if (ids[i] >= stat_cnt) {
>                         RTE_LOG(ERR, PMD, "id value isn't valid");
> -                       return -1;
> +                       return -EINVAL;
>                 }
>                 strcpy(xstats_names[i].name,
>                                 xstats_names_copy[ids[i]].name);
> --
> 2.20.1
>
> ---
>   Diff of the applied patch vs upstream commit (please double-check if
> non-empty:
> ---
> --- -   2019-12-19 14:32:28.664004493 +0000
> +++ 0059-net-bnxt-fix-stats-errors-handling.patch       2019-12-19
> 14:32:26.109297904 +0000
> @@ -1,8 +1,10 @@
> -From 9f55e6ac7500e828d42e95100d5cb061e912be5c Mon Sep 17 00:00:00 2001
> +From ed1b2592aee44b786ac897651e9e5fc7ed350e9c Mon Sep 17 00:00:00 2001
>  From: Kalesh AP <kalesh-anakkur.purayil at broadcom.com>
>  Date: Wed, 9 Oct 2019 18:41:42 -0700
>  Subject: [PATCH] net/bnxt: fix stats errors handling
>
> +[ upstream commit 9f55e6ac7500e828d42e95100d5cb061e912be5c ]
> +
>  This patch fixes few checks and few return values while getting
>  and clearing device statistics.
>
> @@ -12,90 +14,57 @@
>
>  Fixes: bfb9c2260be2 ("net/bnxt: support xstats get/reset")
>  Fixes: 88920136688c ("net/bnxt: support xstats get by id")
> -Cc: stable at dpdk.org
>
>  Signed-off-by: Kalesh AP <kalesh-anakkur.purayil at broadcom.com>
>  Reviewed-by: Somnath Kotur <somnath.kotur at broadcom.com>
>  Reviewed-by: Ajit Khaparde <ajit.khaparde at broadcom.com>
>  ---
> - drivers/net/bnxt/bnxt_stats.c | 36 +++++++++++------------------------
> - 1 file changed, 11 insertions(+), 25 deletions(-)
> + drivers/net/bnxt/bnxt_stats.c | 21 +++++++++++----------
> + 1 file changed, 11 insertions(+), 10 deletions(-)
>
>  diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c
> -index 21012e1fee..f486a5634b 100644
> +index f8bb4ed9e5..6497926162 100644
>  --- a/drivers/net/bnxt/bnxt_stats.c
>  +++ b/drivers/net/bnxt/bnxt_stats.c
> -@@ -360,7 +360,7 @@ int bnxt_stats_get_op(struct rte_eth_dev *eth_dev,
> -       memset(bnxt_stats, 0, sizeof(*bnxt_stats));
> -       if (!(bp->flags & BNXT_FLAG_INIT_DONE)) {
> -               PMD_DRV_LOG(ERR, "Device Initialization not complete!\n");
> --              return -1;
> -+              return -EIO;
> -       }
> -
> -       num_q_stats = RTE_MIN(bp->rx_cp_nr_rings,
> -@@ -390,9 +390,8 @@ int bnxt_stats_get_op(struct rte_eth_dev *eth_dev,
> -               if (unlikely(rc))
> -                       return rc;
> -       }
> -+
> -       rc = bnxt_hwrm_func_qstats(bp, 0xffff, bnxt_stats);
> --      if (unlikely(rc))
> --              return rc;
> -       return rc;
> - }
> +@@ -360,16 +360,17 @@ int bnxt_dev_xstats_get_names_op(__rte_unused
> struct rte_eth_dev *eth_dev,
> + void bnxt_dev_xstats_reset_op(struct rte_eth_dev *eth_dev)
> + {
> +       struct bnxt *bp = (struct bnxt *)eth_dev->data->dev_private;
> ++      int ret;
>
> -@@ -573,30 +572,17 @@ int bnxt_dev_xstats_reset_op(struct rte_eth_dev
> *eth_dev)
> -       if (ret)
> -               return ret;
> -
> --      if (bp->flags & BNXT_FLAG_PORT_STATS && BNXT_SINGLE_PF(bp)) {
> --              ret = bnxt_hwrm_port_clr_stats(bp);
> --              if (ret != 0) {
> --                      PMD_DRV_LOG(ERR, "Operation failed: %s\n",
> --                                  strerror(-ret));
> --                      return ret;
> --              }
> --      }
> --
> --      ret = 0;
> +-      if (bp->flags & BNXT_FLAG_PORT_STATS && !BNXT_NPAR_PF(bp))
> +-              bnxt_hwrm_port_clr_stats(bp);
>  -
> --      if (BNXT_VF(bp)) {
> --              PMD_DRV_LOG(ERR, "Operation not supported on a VF
> device\n");
> --              ret = -ENOTSUP;
> --      }
> --      if (!BNXT_SINGLE_PF(bp)) {
> --              PMD_DRV_LOG(ERR, "Operation not supported on a MF
> device\n");
> --              ret = -ENOTSUP;
> --      }
> --      if (!(bp->flags & BNXT_FLAG_PORT_STATS)) {
> -+      if (BNXT_VF(bp) || !BNXT_SINGLE_PF(bp) ||
> +-      if (BNXT_VF(bp))
> +-              RTE_LOG(ERR, PMD, "Operation not supported on a VF
> device\n");
> +-      if (BNXT_NPAR_PF(bp))
> +-              RTE_LOG(ERR, PMD, "Operation not supported on a MF
> device\n");
> +-      if (!(bp->flags & BNXT_FLAG_PORT_STATS))
> ++      if (BNXT_VF(bp) || !BNXT_NPAR_PF(bp) ||
>  +          !(bp->flags & BNXT_FLAG_PORT_STATS)) {
> -               PMD_DRV_LOG(ERR, "Operation not supported\n");
> -               ret = -ENOTSUP;
> -       }
> -
> +               RTE_LOG(ERR, PMD, "Operation not supported\n");
> ++      }
> ++
>  +      ret = bnxt_hwrm_port_clr_stats(bp);
>  +      if (ret != 0)
> -+              PMD_DRV_LOG(ERR, "Failed to reset xstats: %s\n",
> ++              RTE_LOG(ERR, PMD, "Failed to reset xstats: %s\n",
>  +                          strerror(-ret));
> -+
> -       return ret;
>   }
>
> -@@ -625,7 +611,7 @@ int bnxt_dev_xstats_get_by_id_op(struct rte_eth_dev
> *dev, const uint64_t *ids,
> + int bnxt_dev_xstats_get_by_id_op(struct rte_eth_dev *dev, const uint64_t
> *ids,
> +@@ -389,7 +390,7 @@ int bnxt_dev_xstats_get_by_id_op(struct rte_eth_dev
> *dev, const uint64_t *ids,
>         for (i = 0; i < limit; i++) {
>                 if (ids[i] >= stat_cnt) {
> -                       PMD_DRV_LOG(ERR, "id value isn't valid");
> +                       RTE_LOG(ERR, PMD, "id value isn't valid");
>  -                      return -1;
>  +                      return -EINVAL;
>                 }
>                 values[i] = values_copy[ids[i]];
>         }
> -@@ -659,7 +645,7 @@ int bnxt_dev_xstats_get_names_by_id_op(struct
> rte_eth_dev *dev,
> +@@ -415,7 +416,7 @@ int bnxt_dev_xstats_get_names_by_id_op(struct
> rte_eth_dev *dev,
>         for (i = 0; i < limit; i++) {
>                 if (ids[i] >= stat_cnt) {
> -                       PMD_DRV_LOG(ERR, "id value isn't valid");
> +                       RTE_LOG(ERR, PMD, "id value isn't valid");
>  -                      return -1;
>  +                      return -EINVAL;
>                 }
>


-- 
Regards,
Kalesh A P