netdev - Re: [next-queue PATCH v4 3/4] net/sched: Introduce Credit Based Shaper (CBS) qdisc

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: <CAEYbN3RjUXGMyxo0t88-ASNVEVQdfXkMzBbMtMHAhqWScOO=Cg@mail.gmail.com>
Date:   Thu, 5 Oct 2017 12:09:32 -0600
From:   Levi Pearson <levipearson@...il.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     Vinicius Costa Gomes <vinicius.gomes@...el.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        intel-wired-lan@...ts.osuosl.org,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>, andre.guedes@...el.com,
        Ivan Briano <ivan.briano@...el.com>,
        jesus.sanchez-palencia@...el.com, boon.leong.ong@...el.com,
        richardcochran@...il.com, Henrik Austad <henrik@...tad.us>,
        Rodney Cummings <rodney.cummings@...com>
Subject: Re: [next-queue PATCH v4 3/4] net/sched: Introduce Credit Based
 Shaper (CBS) qdisc

On Wed, Oct 4, 2017 at 12:36 AM, Jiri Pirko <jiri@...nulli.us> wrote:

>>+static struct Qdisc_ops cbs_qdisc_ops __read_mostly = {
>>+      .next           =       NULL,
>>+      .id             =       "cbs",
>>+      .priv_size      =       sizeof(struct cbs_sched_data),
>>+      .enqueue        =       cbs_enqueue,
>>+      .dequeue        =       qdisc_dequeue_head,
>>+      .peek           =       qdisc_peek_dequeued,
>>+      .init           =       cbs_init,
>>+      .reset          =       qdisc_reset_queue,
>>+      .destroy        =       cbs_destroy,
>>+      .change         =       cbs_change,
>>+      .dump           =       cbs_dump,
>>+      .owner          =       THIS_MODULE,
>>+};
>
> I don't see a software implementation for this. Looks like you are
> trying abuse tc subsystem to bypass kernel. Could you please explain
> this? The golden rule is: implement in kernel, then offload.

It would be a shame if this were blocked due to a missing software
implementation. This module is analogous to (and designed to work
with) the mqprio module; it directly configures the 802.1Qav
(Forwarding and Queuing for Time-Sensitive Streams) functionality of
multi-queue NICs with that capability. I'm not sure what makes it seem
like an attempt to "bypass the kernel"; it's actually an attempt to
get an appropriate configuration path *into* the kernel, which has
been missing for some time.

While it would be valuable to have a CBS software-only implementation,
and Vinicius and colleagues have mentioned plans to implement one,
most users will have chosen Qav-compliant NICs and will prefer to use
the hardware capability. In fact they are often *already* using that
capability, but configure it via non-standardized interfaces in
out-of-tree or vendor-tree drivers. I believe it's valuable to have
the "knobs" fit in with the mqprio qdisc and the overall tc subsystem
rather than forcing users through various unrelated configuration
tools, but ultimately the hooks just need to be in the network
subsystem so the drivers can be told how the user wants to set the
registers.

It *might* be reasonable to add the functionality of this to mqprio
instead of a separate module, but this is only one of many possible
802.1Q shapers that could be selected and configured (with more being
defined by IEEE 802.1 working groups for different use cases), and it
seems cleaner to me to have their configuration be through separate
modules than crammed into an already-confusing one, especially since
mqprio has much broader applicability than CBS and it probably doesn't
make sense to burden all mqprio users with the configuration option
overhead.

This meets a specific need in industry (this is widely used in
automotive infotainment devices with broad hardware support across the
SoCs targeted at that industry) that is not well-served by a software
implementation of class-level shaping. As a maintainer of the OpenAvnu
project (sponsored by Avnu, an industry alliance formed around the TSN
standards), I will be integrating support for this as soon as it's
available to our traffic shaping management userspace tools, which
currently have to rely on out-of-tree drivers with custom interfaces
or the HTB shaper which can be configured close to CBS, but with
greatly increased overhead.

Levi