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

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

On Mon,  6 Dec 2021 16:05:11 +0800 xiangxia.m.yue@gmail.com wrote:

quoted

  +----+      +----+      +----+
  | P1 |      | P2 |      | Pn |
  +----+      +----+      +----+
    |           |           |
    +-----------+-----------+
                |
                | clsact/skbedit
                |    MQ
                v
    +-----------+-----------+
    | q0        | q1        | qn
    v           v           v
   HTB         HTB   ...   FIFO

Hi Jakub, thanks for your comments

The usual suggestion these days is to try to use FQ + EDT to
implement efficient policies. You don't need dedicated qdiscs,
just modulate transmission time appropriately on egress of the
container.

FQ+EDT is good solution. But this patch should be used on another scenario.
1. the containers  which outbound traffic is not limited, want to use
the fifo qdisc.
If this traffic share the FQ/HTB Qdisc, the qdisc lock will affect the
performance and latency.
2. we can support user to select tx queue, range from A to B. skb hash
or cgroup classid is good to do load balance.
patch 2/2: https://patchwork.kernel.org/project/netdevbpf/patch/20211206080512.36610-3-xiangxia.m.yue@gmail.com/

In general recording the decision in the skb seems a little heavy
handed. We just need to carry the information from the egress hook
to the queue selection a few lines below. Or in fact maybe egress

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.

hook shouldn't be used for this in the first place, and we need
a more appropriate root qdisc than simple mq?

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.

Not sure. What I am sure of is that you need to fix these warnings:

Ok

include/linux/skbuff.h:937: warning: Function parameter or member 'tc_skip_txqueue' not described in 'sk_buff'

ERROR: spaces required around that '=' (ctx:VxW)
#103: FILE: net/sched/act_skbedit.c:42:
+       queue_mapping= (queue_mapping & 0xff) + hash % mapping_mod;
                     ^

;)

-- 
Best regards, Tonghao