netdev - Re: [PATCH net-next v3 2/3] bnxt: count packets discarded because of netpoll

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20210826181510.3f71e103@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Thu, 26 Aug 2021 18:15:10 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Michael Chan <michael.chan@...adcom.com>
Cc:     David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>, olteanv@...il.com
Subject: Re: [PATCH net-next v3 2/3] bnxt: count packets discarded because
 of netpoll

On Thu, 26 Aug 2021 14:17:45 -0700 Michael Chan wrote:
> On Thu, Aug 26, 2021 at 12:18 PM Jakub Kicinski <kuba@...nel.org> wrote:
> > On Thu, 26 Aug 2021 11:43:58 -0700 Michael Chan wrote:  
> > > 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.

Oh, [rt]x_total_discard_pkts are also just a sum of current counters?
I missed that. In that case if netpoll discards reset it's not a big
deal, I'll respin the patch tomorrow. Let me also rename from
rx_netpoll_discards to rx_total_netpoll_discards, adding the "total_"
will hopefully signal the similarity of semantics?