netdev - Re: [PATCH net-next 3/3] eth: bnxt: handle invalid Tx completions more gracefully

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACKFLi=5U_vpBL9tF6wv9WR_ZvuQzQnW+ETKAwUV_eD6xXEgOQ@mail.gmail.com>
Date: Tue, 11 Jul 2023 21:11:02 -0700
From: Michael Chan <michael.chan@...adcom.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com, 
	pabeni@...hat.com
Subject: Re: [PATCH net-next 3/3] eth: bnxt: handle invalid Tx completions
 more gracefully

On Tue, Jul 11, 2023 at 6:09 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Tue, 11 Jul 2023 17:01:08 -0700 Michael Chan wrote:
> > > It generally looks good to me.  I have a few comments below.
> > >
> > > The logic is very similar to the bnapi->in_reset logic to reset due to
> > > RX errors.  We have a counter for the number of times we do the RX
> > > reset so I think it might be good to add a similar TX reset counter.
> >
> > Never mind about the counter.  Since we are doing a complete reset,
> > the cpr structure will be freed anyway and the counter won't persist.
> >
> > Later when we add support for per TX ring reset, we can add the
> > counter at that time.
>
> Oh, if all the cpr stats get lost during reset or re-config, that's
> quite unfortunate. We should get that fixed without waiting for per
> ring resets of any sort, just in case that takes long.
>
> Can we stash the old sum somewhere and report in ethtool as "non-ring"
> or "old" or ..?
>

These are all the software stats that will be reset during a complete reset:

struct bnxt_rx_sw_stats {
        u64                     rx_l4_csum_errors;
        u64                     rx_resets;
        u64                     rx_buf_errors;
        u64                     rx_oom_discards;
        u64                     rx_netpoll_discards;
};

struct bnxt_cmn_sw_stats {
        u64                     missed_irqs;
};

These are per ring counters.  During complete reset, we free the ring
and allocate a new ring.  So re-applying these counters to the new
ring doesn't make sense because the new ring is not necessarily the
same as the old one.  So I think we'll need to have a total for each
of these and we'll save the snapshot of the total before reset and
restore the snapshot after reset.  Does that make sense?

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4209 bytes)