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

Message-ID: <ZwAaJSKlofFuS1_8@LQ3V64L9R2>
Date: Fri, 4 Oct 2024 12:39:01 -0400
From: Joe Damato <jdamato@...tly.com>
To: Pavan Chebbi <pavan.chebbi@...adcom.com>
Cc: 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

On Fri, Oct 04, 2024 at 09:03:58PM +0530, Pavan Chebbi wrote:

[...]

> > > 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.

OK

[...]

> > 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.

I don't see where the state would be added for tracking the current
rxq_idx and txq_idx, though. And I don't necessarily agree that the
counters are arbitrary?

Unless tg3 is currently rx and tx index somewhere, then just
assuming they are linear seems fine ?

> So if you want to use the local counters you may go ahead unless
> Michael has any other suggestions.

I'll send another RFC and you can see what it looks like before
deciding.