ACK: [SRU][F/Oracle][PATCH 3/3] bnxt_en: Fix statistics counters issue during ifdown with older firmware.

[William Breathitt Gray]

On Thu, Aug 20, 2020 at 12:57:48PM -0400, Khalid Elmously wrote:
> From: Michael Chan <michael.chan at broadcom.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1892397
> 
> On older firmware, the hardware statistics are not cleared when the
> driver frees the hardware stats contexts during ifdown.  The driver
> expects these stats to be cleared and saves a copy before freeing
> the stats contexts.  During the next ifup, the driver will likely
> allocate the same hardware stats contexts and this will cause a big
> increase in the counters as the old counters are added back to the
> saved counters.
> 
> We fix it by making an additional firmware call to clear the counters
> before freeing the hw stats contexts when the firmware is the older
> 20.x firmware.
> 
> Fixes: b8875ca356f1 ("bnxt_en: Save ring statistics before reset.")
> Reported-by: Jakub Kicinski <kicinski at fb.com>
> Reviewed-by: Vasundhara Volam <vasundhara-v.volam at broadcom.com>
> Signed-off-by: Michael Chan <michael.chan at broadcom.com>
> Tested-by: Jakub Kicinski <kicinski at fb.com>
> Signed-off-by: David S. Miller <davem at davemloft.net>
> (backported from commit c2dec363feb41544a76c8083aca2378990e17166)
> [ kmously: Minor context adjustments ]
> Signed-off-by: Khalid Elmously <khalid.elmously at canonical.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 93d02e8339c4..9af620b05ba8 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -6306,6 +6306,7 @@ int bnxt_hwrm_set_coal(struct bnxt *bp)
>  static int bnxt_hwrm_stat_ctx_free(struct bnxt *bp)
>  {
>  	int rc = 0, i;
> +	struct hwrm_stat_ctx_clr_stats_input req0 = {0};
>  	struct hwrm_stat_ctx_free_input req = {0};
>  
>  	if (!bp->bnapi)
> @@ -6314,6 +6315,7 @@ static int bnxt_hwrm_stat_ctx_free(struct bnxt *bp)
>  	if (BNXT_CHIP_TYPE_NITRO_A0(bp))
>  		return 0;
>  
> +	bnxt_hwrm_cmd_hdr_init(bp, &req0, HWRM_STAT_CTX_CLR_STATS, -1, -1);
>  	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_STAT_CTX_FREE, -1, -1);
>  
>  	mutex_lock(&bp->hwrm_cmd_lock);
> @@ -6323,6 +6325,11 @@ static int bnxt_hwrm_stat_ctx_free(struct bnxt *bp)
>  
>  		if (cpr->hw_stats_ctx_id != INVALID_STATS_CTX_ID) {
>  			req.stat_ctx_id = cpu_to_le32(cpr->hw_stats_ctx_id);
> +			if (BNXT_FW_MAJ(bp) <= 20) {
> +				req0.stat_ctx_id = req.stat_ctx_id;
> +				_hwrm_send_message(bp, &req0, sizeof(req0),
> +						   HWRM_CMD_TIMEOUT);
> +			}
>  

Upstream version gets rid of this newline, but I don't think it'll be
that bad to keep it (might make the code easier to read with overall).

Acked-by: William Breathitt Gray <william.gray at canonical.com>

>  			rc = _hwrm_send_message(bp, &req, sizeof(req),
>  						HWRM_CMD_TIMEOUT);
> -- 
> 2.17.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20200820/19593702/attachment.sig>