netdev - Re: [PATCH net-next] net: dsa: sja1105: offload the Credit-Based Shaper 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] [day] [month] [year] [list]

Date:   Wed, 27 May 2020 22:29:53 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Vinicius Costa Gomes <vinicius.gomes@...el.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net: dsa: sja1105: offload the Credit-Based
 Shaper qdisc

Hi Vinicius,

On Wed, 27 May 2020 at 21:25, Vinicius Costa Gomes
<vinicius.gomes@...el.com> wrote:
>
> Hi,
>
> Vladimir Oltean <olteanv@...il.com> writes:
>
> > From: Vladimir Oltean <vladimir.oltean@....com>
> >
> > SJA1105, being AVB/TSN switches, provide hardware assist for the
> > Credit-Based Shaper as described in the IEEE 8021Q-2018 document.
> >
> > First generation has 10 shapers, freely assignable to any of the 4
> > external ports and 8 traffic classes, and second generation has 16
> > shapers.
> >
> > We also need to provide a dummy implementation of mqprio qdisc offload,
> > since this seems to be necessary for shaping any traffic class other
> > than zero.
> >
> > The Credit-Based Shaper tables are accessed through the dynamic
> > reconfiguration interface, so we have to restore them manually after a
> > switch reset. The tables are backed up by the static config only on
> > P/Q/R/S, and we don't want to add custom code only for that family,
> > since the procedure that is in place now works for both.
> >
> > Tested with the following commands:
> >
> > data_rate_kbps=34000
> > port_transmit_rate_kbps=1000000
> > idleslope=$data_rate_kbps
> > sendslope=$(($idleslope - $port_transmit_rate_kbps))
> > locredit=$((-0x7fffffff))
> > hicredit=$((0x7fffffff))
> > tc qdisc add dev sw1p3 root handle 1: mqprio num_tc 8
>
> This (and implementing the dummy mqprio offload callback) seem a bit
> hackish: I am reading this is more a way to bypass mqprio parameter
> validation (the priority to queue mapping) than anything else.
>
> And I don't think that accepting any parameters without doing any
> validation is really what you want.
>
> Question:
>
> $ tc qdisc replace dev $IFACE parent root handle 100 mqprio \
>       num_tc 3 \
>       map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
>       queues 1@0 1@1 1@2 \
>       hw 0
>
> $ tc qdisc replace dev $IFACE handle 200 parent 100:1 cbs \
>       idleslope 100000 sendslope -900000 hicredit 150 locredit -1362 \
>       offload 1
>
> Why doesn't something like this work for your hardware?
>
>
> Cheers,
> --
> Vinicius

No, it absolutely does, thanks.
I had tried a few combinations of tc mqprio commands with hw 0, and
they all returned this cryptic:

tc qdisc add dev swp2 root handle 100 mqprio num_tc 8 map 0 1 2 3 4 5 6 7 hw 0
RTNETLINK answers: Invalid argument
tc qdisc add dev swp2 root handle 100 mqprio num_tc 8 hw 0
RTNETLINK answers: Invalid argument

which I had absolutely no desire to debug, especially since the same
commands, with hw 1, were accepted.
I just chose to be deliberately wrong, knowing that it's the quickest
way to find out the real solution :)

Cheers,
-Vladimir