netdev - Re: [net-next v3 1/2] net: sched: use queue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMDZJNXOn_mhmGJ4QsLY_aJEEQ8a1DEMnB8smpq6rZiW+HQfBw@mail.gmail.com>
Date:   Tue, 14 Dec 2021 09:56:23 +0800
From:   Tonghao Zhang <xiangxia.m.yue@...il.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jiri Pirko <jiri@...nulli.us>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Alexander Lobakin <alobakin@...me>,
        Paolo Abeni <pabeni@...hat.com>,
        Talal Ahmad <talalahmad@...gle.com>,
        Kevin Hao <haokexin@...il.com>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Kees Cook <keescook@...omium.org>,
        Kumar Kartikeya Dwivedi <memxor@...il.com>,
        Antoine Tenart <atenart@...nel.org>,
        Wei Wang <weiwan@...gle.com>, Arnd Bergmann <arnd@...db.de>
Subject: Re: [net-next v3 1/2] net: sched: use queue_mapping to pick tx queue

On Tue, Dec 14, 2021 at 6:58 AM Cong Wang <xiyou.wangcong@...il.com> wrote:
>
> On Thu, Dec 9, 2021 at 6:36 PM <xiangxia.m.yue@...il.com> wrote:
> >
> > From: Tonghao Zhang <xiangxia.m.yue@...il.com>
> >
> > This patch fix issue:
> > * If we install tc filters with act_skbedit in clsact hook.
> >   It doesn't work, because netdev_core_pick_tx() overwrites
> >   queue_mapping.
> >
> >   $ tc filter ... action skbedit queue_mapping 1
>
> Interesting, but bpf offers a helper (bpf_set_hash()) to overwrite
> skb->hash which could _indirectly_ be used to change queue_mapping.
> Does it not work?
No
The root cause is that queue_mapping is overwritten by netdev_core_pick_tx.
Tx queues are picked by xps, ndo_select_queue of netdev driver, or skb hash
in netdev_core_pick_tx(). We can't only change the skb-hash. BTW, this patch
fix the issue, more importantly, this is useful (more details, see my
commit message):
* We can use FQ + EDT to implement efficient policies. Tx queues
  are picked by xps, ndo_select_queue of netdev driver, or skb hash
  in netdev_core_pick_tx(). In fact, the netdev driver, and skb
  hash are _not_ under control. xps uses the CPUs map to select Tx
  queues, but we can't figure out which task_struct of pod/containter
  running on this cpu in most case. We can use clsact filters to classify
  one pod/container traffic to one Tx queue. Why ?

  In containter networking environment, there are two kinds of pod/
  containter/net-namespace. One kind (e.g. P1, P2), the high throughput
  is key in these applications. But avoid running out of network resource,
  the outbound traffic of these pods is limited, using or sharing one
  dedicated Tx queues assigned HTB/TBF/FQ Qdisc. Other kind of pods
  (e.g. Pn), the low latency of data access is key. And the traffic is not
  limited. Pods use or share other dedicated Tx queues assigned FIFO Qdisc.
  This choice provides two benefits. First, contention on the HTB/FQ Qdisc
  lock is significantly reduced since fewer CPUs contend for the same queue.
  More importantly, Qdisc contention can be eliminated completely if each
  CPU has its own FIFO Qdisc for the second kind of pods.

  There must be a mechanism in place to support classifying traffic based on
  pods/container to different Tx queues. Note that clsact is outside of Qdisc
  while Qdisc can run a classifier to select a sub-queue under the lock.

  In general recording the decision in the skb seems a little heavy handed.
  This patch introduces a per-CPU variable, suggested by Eric.

>
> BTW, for this skbedit action, I guess it is probably because skbedit was
> introduced much earlier than clsact.
Yes
> Thanks.

-- 
Best regards, Tonghao