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]

Date: Tue, 11 Jul 2023 18:09:37 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Michael Chan <michael.chan@...adcom.com>
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, 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 ..?

> > The XDP code path can potentially crash in a similar way if we get a
> > bad completion from hardware.  I'm not sure if we should add similar
> > logic to the XDP code path.

Ack, will fix.