netdev - Re: [RFC net-next v2 2/2] tg3: Link queues to NAPIs

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: <CALs4sv1Ea1ke2CHOZ0U75JVY84uY=NNyaJrW8wVwcytON2ofog@mail.gmail.com>
Date: Fri, 4 Oct 2024 21:03:58 +0530
From: Pavan Chebbi <pavan.chebbi@...adcom.com>
To: Joe Damato <jdamato@...tly.com>, Pavan Chebbi <pavan.chebbi@...adcom.com>, 
	netdev@...r.kernel.org, Michael Chan <mchan@...adcom.com>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [RFC net-next v2 2/2] tg3: Link queues to NAPIs

> > The local counter variable for the ring ids might work because irqs
> > are requested sequentially.
>
> Yea, my proposal relies on the sequential ordering.
>
> > Thinking out loud, a better way would be to save the tx/rx id inside
> > their struct tg3_napi in the tg3_request_irq() function.
>
> I think that could work, yes. I wasn't sure if you'd be open to such
> a change.
>
> It seems like in that case, though, we'd need to add some state
> somewhere.
>
> It's not super clear to me where the appropriate place for the state
> would be because tg3_request_irq is called in a couple places (like
> tg3_test_interrupt).
>
> Another option would be to modify tg3_enable_msix and modify:
>
>   for (i = 0; i < tp->irq_max; i++)
>           tp->napi[i].irq_vec = msix_ent[i].vector;
Hi Joe, not in favor of this change.
>
> But, all of that is still a bit invasive compared to the running
> rxq_idx txq_idx counters I proposed in my previous message.
>
> I am open to doing whatever you suggest/prefer, though, since it is
> your driver after all :)
>
> > And have a separate new function (I know you did something similar for
> > v1 of irq-napi linking) to link queues and napi.
> > I think it should work, and should help during de-linking also. Let me
> > know what you think.
>
> I think it's possible, it's just disruptive and it's not clear if
> it's worth it? Some other code path might break and it might be fine
> to just rely on the sequential indexing? Not sure.
>
I don't have strong opposition to your proposal of using local counters.
Just that an alternate solution like what I suggested may look less
arbitrary, imo.
So if you want to use the local counters you may go ahead unless
Michael has any other suggestions.

> Let me know what you think; thanks for taking the time to review and
> respond.

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