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

[Michael Chan <michael.chan@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]

On Mon, Jul 10, 2023 at 1:56 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Invalid Tx completions should never happen (tm) but when they do
> they crash the host, because driver blindly trusts that there is
> a valid skb pointer on the ring.
>
> The completions I've seen appear to be some form of FW / HW
> miscalculation or staleness, they have typical (small) values
> (<100), but they are most often higher than number of queued
> descriptors. They usually happen after boot.
>
> Instead of crashing print a warning and schedule a reset.

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.

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.

Thanks.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]