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

Pavan Chebbi
2024-10-04T15:34:11Z

[-- Attachment #1: Type: text/plain, Size: 1821 bytes --]

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

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]