netdev - Re: [PATCH net-next] net: enetc: fix TX ring interrupt storm

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

Message-ID: <20210409182025.181ab75c@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Fri, 9 Apr 2021 18:20:25 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Claudiu Manoil <claudiu.manoil@...il.com>,
        Vladimir Oltean <olteanv@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        Alex Marginean <alexandru.marginean@....com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Yangbo Lu <yangbo.lu@....com>,
        Vladimir Oltean <vladimir.oltean@....com>
Subject: Re: [PATCH net-next] net: enetc: fix TX ring interrupt storm

On Sat, 10 Apr 2021 00:29:23 +0300 Claudiu Manoil wrote:
> On 09.04.2021 22:27, Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@....com>
> > 
> > The blamed commit introduced a bit in the TX software buffer descriptor
> > structure for determining whether a BD is final or not; we rearm the TX
> > interrupt vector for every frame (hence final BD) transmitted.
> > 
> > But there is a problem with the patch: it replaced a condition whose
> > expression is a bool which was evaluated at the beginning of the "while"
> > loop with a bool expression that is evaluated on the spot: tx_swbd->is_eof.
> > 
> > The problem with the latter expression is that the tx_swbd has already
> > been incremented at that stage, so the tx_swbd->is_eof check is in fact
> > with the _next_ software BD. Which is _not_ final.
> > 
> > The effect is that the CPU is in 100% load with ksoftirqd because it
> > does not acknowledge the TX interrupt, so the handler keeps getting
> > called again and again.
> > 
> > The fix is to restore the code structure, and keep the local bool is_eof
> > variable, just to assign it the tx_swbd->is_eof value instead of
> > !!tx_swbd->skb.
> > 
> > Fixes: d504498d2eb3 ("net: enetc: add a dedicated is_eof bit in the TX software BD")
> > Reported-by: Alex Marginean <alexandru.marginean@....com>
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>  
> 
> Reviewed-by: Claudiu Manoil <claudiu.manoil@....com>

Applied, thanks!