netdev - RE: [PATCH v1 net] lan743x: fix return value for lan743x_tx_napi

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]

Message-ID: <90A7E81AE28BAE4CBDDB3B35F187D264407528DB@CHN-SV-EXMX02.mchp-main.com>
Date:   Wed, 21 Nov 2018 16:44:58 +0000
From:   <Bryan.Whitehead@...rochip.com>
To:     <f.fainelli@...il.com>, <andrew@...n.ch>
CC:     <davem@...emloft.net>, <netdev@...r.kernel.org>,
        <UNGLinuxDriver@...rochip.com>
Subject: RE: [PATCH v1 net] lan743x: fix return value for
 lan743x_tx_napi_poll

> Did you look at the output of "perf top" or something along those lines to
> figure out if your lan743x driver is indeed responsible for that by not being
> scheduler friendly? What is likely happening is that you do not reclaim
> "weight" packets and instead keep looping into NAPI context, which
> prevents the system from making further progress.
> 
I'm having trouble installing "perf" for my kernel. But I have used GPIO's and a scope
To make sure my poll routine is called and returns in a timely manner. I've never seen
A problem with it "not being scheduler friendly"

> Calling napi_complete_done() for the TX path is not necessary AFAICT, what
> you really want to do is call napi_complete() and make sure you:
> 
> - reclaim/free as many TX buffers as possible, without looking at the NAPI
> weight which becomes irrelevant
> - if you have been able to reclaim enough descriptors, wake-up the transmit
> queue
> 
> So ignoring the NAPI weight like you do is correct, but calling
> napi_complete_done() with a 0 argument does not sound correct to me.
> --
> Florian

I see that napi_complete just maps to napi_complete_done with a 0 argument anyway.
So they are currently functionally identical, but I will make a new version that uses
napi_complete as you suggested.

But it seems you would definitely agree that the previous version of lan743x driver
Which called napi_complete_done with 'weight' as the argument is wrong,
And therefor this fix is correct.