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]
Message-ID: <CACKFLi=Eh2ase5qnQ0ioYL+yS6-oSeZreHsaqAn9Zgwkv_d-Bw@mail.gmail.com>
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)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ