Re: [PATCH net-next 2/2] bnx2: Save statistics during reset.

[Patrick Rabau <pr2345@gmail.com>]

This patch has already been applied, but seems to have a problem.

> MTU changes, ring size changes, etc cause the chip to be reset and the
> statisctics flushed.  To keep track of the accumulated statistics, we
> add code to save the whole statistics block before reset.  We also
> modify the macros and statistics functions to return the sum of the
> saved and current counters.
>
> Based on original patch by Breno Leitao <leitao@xxxxxxxxxxxxxxxxxx>
>
> Signed-off-by: Michael Chan <mchan@xxxxxxxxxxxx>
> ---
>  drivers/net/bnx2.c |   64 +++++++++++++++++++++++++++++++++++++++++++++------
>  drivers/net/bnx2.h |    1 +
>  2 files changed, 57 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index 47fb508..d83512d 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c

...

> @@ -6542,6 +6544,30 @@ bnx2_close(struct net_device *dev)
>        return 0;
>  }
>
> +static void
> +bnx2_save_stats(struct bnx2 *bp)
> +{
> +       u32 *hw_stats = (u32 *) bp->stats_blk;
> +       u32 *temp_stats = (u32 *) bp->temp_stats_blk;
> +       int i;
> +
> +       /* The 1st 10 counters are 64-bit counters */
> +       for (i = 0; i < 20; i += 2) {
> +               u32 hi;
> +               u64 lo;
> +
> +               hi = *(temp_stats + i) + *(hw_stats + i);
> +               lo = *(temp_stats + i + 1) + *(hw_stats + i + 1);

The rhs of the assignment above will always have type u32,
because temp_stats and hw_stats have type pointer to u32.
Would need to cast before doing the addition, otherwise
declaring lo as u64 is not doing anything.

I think it would also be more readable to use array indexing
notation in various places.  For example:
  hi = temp_stats[i] + hs_stats[i];
  lo = (u64)temp_stats[i + 1] + (u64)hw_stats[i + 1];

> +               if (lo > 0xffffffff)
> +                       hi++;
> +               *(temp_stats + i) = hi;
> +               *(temp_stats + i + 1) = lo & 0xffffffff;
> +       }
> +
> +       for ( ; i < sizeof(struct statistics_block) / 4; i++)
> +               *(temp_stats + i) = *(temp_stats + i) + *(hw_stats + i);
> +}
>

-- PR