linux-kernel - Re: [PATCH 2/2] tg3: Fix the TX ring stall

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 1 Nov 2023 21:42:10 -0700
From:   Michael Chan <michael.chan@...adcom.com>
To:     alexey.pakhunov@...cex.com
Cc:     linux-kernel@...r.kernel.org, mchan@...adcom.com,
        netdev@...r.kernel.org, prashant@...adcom.com,
        siva.kallam@...adcom.com, vincent.wong2@...cex.com
Subject: Re: [PATCH 2/2] tg3: Fix the TX ring stall

On Wed, Nov 1, 2023 at 9:11 PM <alexey.pakhunov@...cex.com> wrote:
>
> Hi,
>
> > Thanks for the patch.  An alternative fix that may be simpler is to
> > add a goto after calling tg3_tso_bug().  Something like this:
> >
> >         tg3_tso_bug();
> >         goto update_tx_mbox;
> > ...
> >
> > update_tx_mbox:
> >         if (!netdev_xmit_more() || netif_xmit_stopped())
> >                 tw32_tx_mbox();
> > ...
>
> Yes, I considered this approach but in the end it seemed more fragile. All
> future updates to tg3_start_xmit() would need to be careful to make sure
> all return paths go through "update_tx_mbox". This is much more
> straightforward with a separate wrapper function.
>
> The sizes of both patches are roughly the same. The wrapper function
> version:
>
>  drivers/net/ethernet/broadcom/tg3.c | 46 +++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 35 insertions(+), 11 deletions(-)
>
> The goto version touches four different return paths: three tg3_tso_bug()
> calls and the return at the very top of the function:
>
>  drivers/net/ethernet/broadcom/tg3.c | 46 +++++++++++++++++++++++++++++++++-------------
>  1 file changed, 33 insertions(+), 13 deletions(-)
>
> Let me re-test the goto version and resubmit it as v2. Please let me know
> which version of the patch you prefer more.
>

I did not realize the goto version is almost as big.  In that case,
your original version is fine.

You might want to declare the variables in reverse Xmas tree style for
any new code.  This driver is old and most of the existing code does
not follow that style.

Thanks.

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