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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 27 May 2020 11:25:57 -0700
From:   Vinicius Costa Gomes <vinicius.gomes@...el.com>
To:     Vladimir Oltean <olteanv@...il.com>, davem@...emloft.net
Cc:     andrew@...n.ch, f.fainelli@...il.com, vivien.didelot@...il.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: dsa: sja1105: offload the Credit-Based Shaper qdisc

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