2022-05-10 20:37:06

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Simplify queue selection

On 10.05.22 17:58, Alexander Wetzel wrote:
> Let iTXQ drivers also register four queues in netdev and move queue
> assignment to ndo_select_queue(), like it's done for other drivers.
>
> This gets rid of a special case in mac80211 and also increases the
> chance that when we call skb_get_hash() the skb is still hot in the CPU
> buffers.
>
> Signed-off-by: Alexander Wetzel <[email protected]>

This has the disadvantage of requiring a redundant sta lookup in the tx
path for iTXQ drivers. I think the CPU cost of that one is probably
higher than any potential gain from calling skb_get_hash a bit earlier.

- Felix



2022-05-10 21:25:33

by Alexander Wetzel

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Simplify queue selection

On 10.05.22 18:10, Felix Fietkau wrote:
> On 10.05.22 17:58, Alexander Wetzel wrote:
>> Let iTXQ drivers also register four queues in netdev and move queue
>> assignment to ndo_select_queue(), like it's done for other drivers.
>>
>> This gets rid of a special case in mac80211 and also increases the
>> chance that when we call skb_get_hash() the skb is still hot in the CPU
>> buffers.
>>
>> Signed-off-by: Alexander Wetzel <[email protected]>
>
> This has the disadvantage of requiring a redundant sta lookup in the tx
> path for iTXQ drivers. I think the CPU cost of that one is probably
> higher than any potential gain from calling skb_get_hash a bit earlier.

Found that one, yes. But why do we then not drop ndo_select_queue() for
all drivers?

Or maybe just call skb_get_hash() in ndo_select_queue()... But I guess
then it would make more sense to move the ndo_select_queue() into
netdev, so all drivers get the optimization.

Alexander

2022-05-10 23:14:29

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Simplify queue selection

On Tue, 2022-05-10 at 18:10 +0200, Felix Fietkau wrote:
> On 10.05.22 17:58, Alexander Wetzel wrote:
> > Let iTXQ drivers also register four queues in netdev and move queue
> > assignment to ndo_select_queue(), like it's done for other drivers.
> >
> > This gets rid of a special case in mac80211 and also increases the
> > chance that when we call skb_get_hash() the skb is still hot in the CPU
> > buffers.
> >
> > Signed-off-by: Alexander Wetzel <[email protected]>
>
> This has the disadvantage of requiring a redundant sta lookup in the tx
> path for iTXQ drivers. I think the CPU cost of that one is probably
> higher than any potential gain from calling skb_get_hash a bit earlier.
>

However, that's independent - we can still calculate the hash there, and
then bail out, i.e. put it right before the "if (wake_tx_queue) return"
part, no?

OTOH we don't even need the hash in the other cases, do we?

johannes

2022-05-11 06:35:39

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Simplify queue selection

Johannes Berg <[email protected]> writes:

> On Tue, 2022-05-10 at 18:10 +0200, Felix Fietkau wrote:
>> On 10.05.22 17:58, Alexander Wetzel wrote:
>> > Let iTXQ drivers also register four queues in netdev and move queue
>> > assignment to ndo_select_queue(), like it's done for other drivers.
>> >
>> > This gets rid of a special case in mac80211 and also increases the
>> > chance that when we call skb_get_hash() the skb is still hot in the CPU
>> > buffers.
>> >
>> > Signed-off-by: Alexander Wetzel <[email protected]>
>>
>> This has the disadvantage of requiring a redundant sta lookup in the tx
>> path for iTXQ drivers. I think the CPU cost of that one is probably
>> higher than any potential gain from calling skb_get_hash a bit earlier.
>>
>
> However, that's independent - we can still calculate the hash there, and
> then bail out, i.e. put it right before the "if (wake_tx_queue) return"
> part, no?
>
> OTOH we don't even need the hash in the other cases, do we?

Nope, that was my point on the "moving it to shared netdev core"
question :)

-Toke