2022-05-10 16:38:55

by Felix Fietkau

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


On 10.05.22 18:13, Alexander Wetzel wrote:
> 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.

When not using iTXQ, packets are buffered in netdev qdisc. In order for
that to work, ndo_select_queue needs to be called *before* packets are
put into qdisc (so long before the actual mac80211 xmit handler).
To fix this properly, we'd need to move to iTXQ for all drivers (by
having mac80211 push packets via drv_tx calls after pulling from iTXQ).
This can probably be done without having to modify the drivers.

- Felix




2022-05-10 20:55:14

by Johannes Berg

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

On Tue, 2022-05-10 at 18:21 +0200, Felix Fietkau wrote:
>
> To fix this properly, we'd need to move to iTXQ for all drivers (by
> having mac80211 push packets via drv_tx calls after pulling from iTXQ).
> This can probably be done without having to modify the drivers.
>

I looked at this, and I kind of really want to do that, but it's not
_entirely_ trivial... If somebody's sufficiently motivated I'd
definitely be happy :)

johannes

2022-05-15 14:57:22

by Alexander Wetzel

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

On 10.05.22 18:22, Johannes Berg wrote:
> On Tue, 2022-05-10 at 18:21 +0200, Felix Fietkau wrote:
>>
>> To fix this properly, we'd need to move to iTXQ for all drivers (by
>> having mac80211 push packets via drv_tx calls after pulling from iTXQ).
>> This can probably be done without having to modify the drivers.
>>
>
> I looked at this, and I kind of really want to do that, but it's not
> _entirely_ trivial... If somebody's sufficiently motivated I'd
> definitely be happy :)

No promise I'll see that trough... But I'll start to look into that.
While I don't have much time for coding at the moment this sounds
worthwhile and instructive at the same time.

Also thanks to all for the other feedback's. I'm starting to see the
woods and not only the trees:-)


Alexander