netdev - Re: [RFC v2 net-next 06/10] net/sched: Introduce the TBS 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:   Tue, 23 Jan 2018 13:45:16 -0800
From:   Jesus Sanchez-Palencia <jesus.sanchez-palencia@...el.com>
To:     Jamal Hadi Salim <jhs@...atatu.com>, netdev@...r.kernel.org
Cc:     xiyou.wangcong@...il.com, jiri@...nulli.us,
        vinicius.gomes@...el.com, richardcochran@...il.com,
        intel-wired-lan@...ts.osuosl.org, anna-maria@...utronix.de,
        henrik@...tad.us, tglx@...utronix.de, john.stultz@...aro.org,
        andre.guedes@...el.com, ivan.briano@...el.com,
        levi.pearson@...man.com
Subject: Re: [RFC v2 net-next 06/10] net/sched: Introduce the TBS Qdisc

Hi,


On 01/18/2018 05:44 AM, Jamal Hadi Salim wrote:
> One more comment:
> Probably try to run a test with a very small delta with
> no offload (probably using something like prio as the root qdisc)
> and dump the stats.
> My gut feeling is your accounting of the backlog in particular is off.


You were right, thanks. It'll be fixed on our next version.

Regards,
Jesus


> 
> cheers,
> jamal
> 
> On 18-01-18 08:35 AM, Jamal Hadi Salim wrote:
>> On 18-01-17 06:06 PM, Jesus Sanchez-Palencia wrote:
>>> From: Vinicius Costa Gomes <vinicius.gomes@...el.com>
>>>
>>> TBS (Time Based Scheduler) uses the information added earlier in this
>>> series (the socket option SO_TXTIME and the new role of
>>> sk_buff->tstamp) to schedule traffic transmission based on absolute
>>> time.
>>>
>>> For some workloads, just bandwidth enforcement is not enough, and
>>> precise control of the transmission of packets is necessary.
>>>
>>> Example:
>>>
>>> $ tc qdisc replace dev enp2s0 parent root handle 100 mqprio num_tc 3 \
>>
>> handle 100:0 ?
>>
>>>             map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 queues 1@0 1@1 2@2 hw 0
>>>
>>> $ tc qdisc add dev enp2s0 parent 100:1 tbs delta 60000 clockid 11 offload 1
>>>
>>>
>>> In this example, the Qdisc will try to enable offloading (offload 1)
>>> the control of the transmission time to the network adapter, the
>>> time stamp in socket are in reference to the clockid '11' (CLOCK_TAI)
>>> and packets leave the Qdisc "delta" (60000) nanoseconds before its
>>> transmission time.
>>>
>>
>>
>>> When offloading is disabled, the network adapter will ignore the
>>> sk_buff time stamp, and so, the transmission time will be only "best
>>> effort" from the Qdisc.
>>>
>>
>> General comments:
>> 1) iproute2: Avoid magic numbers like 1 or 11 please; "offload"
>> (without 1) and "TAI" will be more human friendly.
>>
>> 2) Experience shows that adding padding fields in the control structs
>> implies they will _never ever_ be used. That was not design intent
>> for netlink but over years shit like that has happened.
>> Maybe look at using a 32 bitmap? It is more "future proof".
>> You seem to only have 2-3 flags but it gives you opportunity
>> to add more changes later. If you are 100% sure youll never need
>> it - then maybe just move the tc_tbs_qopt::offload to the end of
>> of the struct.
>>
>> 3)It would be helpful for debugging to increment some stats other
>> than drop counters on enqueu/dequeue obsolete packet drop. Maybe use
>> overlimits for the dequeu drops (in addition)?
>>
>> 4) I may be misreading things - but did you need to reset the
>> watchdog on dequeue? It is already being kicked for every incoming packet.
>>
>> cheers,
>> jamal
>