netdev - Re: [PATCH net-next] net_sched: restore "mpu xxx" handling

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: <03cc89aa-1837-dacc-29d7-fcf6a5e45284@bracey.fi>
Date:   Wed, 12 Jan 2022 08:36:37 +0200
From:   Kevin Bracey <kevin@...cey.fi>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     <netdev@...r.kernel.org>, Eric Dumazet <edumazet@...gle.com>,
        Jiri Pirko <jiri@...nulli.us>, Vimalkumar <j.vimal@...il.com>
Subject: Re: [PATCH net-next] net_sched: restore "mpu xxx" handling

On 12/01/2022 07:06, Jakub Kicinski wrote:
> On Fri, 7 Jan 2022 22:22:50 +0200 Kevin Bracey wrote:
>> commit 56b765b79e9a ("htb: improved accuracy at high rates") broke
>> "overhead X", "linklayer atm" and "mpu X" attributes.
>>
>> "overhead X" and "linklayer atm" have already been fixed. This restores
>> the "mpu X" handling, as might be used by DOCSIS or Ethernet shaping:
>>
>>      tc class add ... htb rate X overhead 4 mpu 64
>>
>> The code being fixed is used by htb, tbf and act_police. Cake has its
>> own mpu handling. qdisc_calculate_pkt_len still uses the size table
>> containing values adjusted for mpu by user space.
>>
>> Fixes: 56b765b79e9a ("htb: improved accuracy at high rates")
> Are you sure this worked and got broken? I can't seem to grep out any
> uses of mpu in this code. commit 175f9c1bba9b ("net_sched: Add size
> table for qdiscs") adds it as part of the struct but I can't find a
> single use of it.

Indeed, There has never been any kernel handling of tc_ratespec::mpu - 
the kernel merely stored the value.

But the system functionality worked because the length-to-time that tc 
passed to TCA_HTB_RTAB was based on the mpu value.

Since 56b765b79e9a ("htb: improved accuracy at high rates"), the table 
has been ignored, and the kernel has done its own immediate calculations.

So this would be the first time the kernel itself has acted on the 
tc_ratespec::mpu value. But it echoes the changes made in 01cb71d2d47b 
("net_sched: restore "overhead xxx" handling") and 8a8e3d84b171 
("net_sched: restore "linklayer atm" handling")

The overhead had been similarly passed to the kernel but not originally 
acted on. Linklayer had to be added to tc_ratespec.

I noticed this because I'd been messing with the htbs on a 2.6.26-based 
router, and when I migrated to a 4.1-based one it was obvious the mpu 
values set in tc weren't sticking. Inspection showed that not only was 
the kernel not storing them, it wasn't any longer using the table based 
on them, so they were ineffective.

Kevin