On Thu, Aug 26, 2021 at 12:18 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 26 Aug 2021 11:43:58 -0700 Michael Chan wrote:
> > On Thu, Aug 26, 2021 at 6:12 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > > @@ -10646,11 +10653,15 @@ static void bnxt_get_ring_stats(struct bnxt *bp,
> > >                 stats->multicast += BNXT_GET_RING_STATS64(sw, rx_mcast_pkts);
> > >
> > >                 stats->tx_dropped += BNXT_GET_RING_STATS64(sw, tx_error_pkts);
> > > +
> > > +               bsw_stats->rx.rx_netpoll_discards +=
> > > +                       cpr->sw_stats.rx.rx_netpoll_discards;
> >
> > Can we just add these rx_netpoll_discards counters directly to
> > stats->rx_dropped?  It looks simpler if we do it that way, right?
>
> To make sure - are you saying that instead of adding
>
>         struct bnxt_sw_stats    sw_stats_prev;
>
> we should accumulate in net_stats_prev->rx_dropped, and have
> the ethtool counter only report the discards since last down/up?
>
> Or to use the atomic counter on the netdev and never report
> in ethtool (since after patch 3 rx_dropped is a mix of reasons)?

OK.  I've reviewed the patch again and you need to keep the previous
netpoll discard counter so that you can report the total current and
previous netpoll discard counter under ethtool -S.

My suggestion would lump the previous netpoll discard counter into the
previous rx_dropped counter and you can only report the current
netpoll discard counter under ethtool -S.  But note that all the ring
related counters we currently report are current counters and do not
include old counters before the last reset.