Re: [net-next v1 1/2] net: sched: use queue_mapping to pick tx queue

On Tue, Dec 7, 2021 at 11:45 PM Jakub Kicinski [off-list ref] wrote:

On Tue, 7 Dec 2021 11:22:28 +0800 Tonghao Zhang wrote:

quoted

On Tue, Dec 7, 2021 at 10:33 AM Jakub Kicinski [off-list ref] wrote:

quoted

On Tue, 7 Dec 2021 10:10:22 +0800 Tonghao Zhang wrote:

quoted

Yes, we can refactor netdev_core_pick_tx to
1. select queue_index and invoke skb_set_queue_mapping, but don't
return the txq.
2. after egress hook, use skb_get_queue_mapping/netdev_get_tx_queue to get txq.

I'm not sure that's what I meant, I meant the information you need to
store does not need to be stored in the skb, you can pass a pointer to
a stack variable to both egress handling and pick_tx.

Thanks, I got it. I think we store the txq index in skb->queue_mapping
better. because in egress hook,
act_skbedit/act_bpf can change the skb queue_mapping. Then we can
pick_tx depending on queue_mapping.

Actually Eric pointed out in another thread that xmit_more() is now
done via a per-CPU variable, you can try that instead of plumbing a
variable all the way into actions and back out to pick_tx().

Thanks Jakub, Eric
v2 is sent, please review
https://patchwork.kernel.org/project/netdevbpf/list/?series=592341

Please make sure to include the analysis of the performance impact
when the feature is _not_ used in the next version.

Ok, I updated the commit message. Thanks!

quoted

quoted

quoted

I have no idea about mq, I think clsact may make the things more flexible.
and act_bpf can also support to change sk queue_mapping. queue_mapping
was included in __sk_buff.

Qdiscs can run a classifier to select a sub-queue. The advantage of
the classifier run by the Qdisc is that it runs after pick_tx.

Yes, we should consider the qdisc lock too. Qdisc lock may affect
performance and latency when running a classifier in Qdisc
and clsact is outside of qdisc.

-- 
Best regards, Tonghao