netdev - Re: [next-queue v2 2/4] i40e: Record number TXes cleaned during 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]

Date:   Thu, 6 Oct 2022 15:35:36 -0700
From:   Jesse Brandeburg <jesse.brandeburg@...el.com>
To:     Joe Damato <jdamato@...tly.com>,
        "Samudrala, Sridhar" <sridhar.samudrala@...el.com>
CC:     Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
        <kuba@...nel.org>, <davem@...emloft.net>,
        <anthony.l.nguyen@...el.com>
Subject: Re: [next-queue v2 2/4] i40e: Record number TXes cleaned during NAPI

On 10/6/2022 10:32 AM, Joe Damato wrote:
> Sorry, but I don't see the value in the second param. NAPI decides what to
> do based on nb_pkts. That's the only parameter that matters for the purpose
> of NAPI going into poll mode or not, right?
> 
> If so: I don't see any reason why a second parameter is necessary.

Sridhar and I talked about this offline. We agree now that you can just 
proceed with the single parameter.

> 
> As I mentioned earlier: if it's just that the name of the parameter isn't
> right (e.g., you want it to be 'tx_processed' instead of 'tx_cleaned') then
> that's an easy fix; I'll just change the name.

I think the name change isn't necessary, since we're not going to extend 
this patch with full XDP events printed (see below)

> 
> It doesn't seem helpful to have xsk_frames as an out parameter for
> i40e_napi_poll tracepoint; that value is not used to determine anything
> about i40e's NAPI.
> 
>> I am not completely clear on the reasoning behind setting clean_complete
>> based on number of packets transmitted in case of XDP.
>>>
>>>> That might reduce the complexity a bit, and will probably still be pretty
>>>> useful for people tuning their non-XDP workloads.
>>
>> This option is fine too.
> 
> I'll give Jesse a chance to weigh in before I proceed with spinning a v3.

I'm ok with the patch you have now, that shows nb_pkts because it's the 
input to the polling decision. We can add the detail about XDP transmits 
cleaned in a later series or patch that is by someone who wants the XDP 
details in the napi poll context.