netdev - Re: Default qdisc not correctly initialized with custom MTU

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: <dbc359d3-5cac-9b2e-6520-df4a25964bd3@applied-asynchrony.com>
Date:   Tue, 10 Sep 2019 11:14:19 +0200
From:   Holger Hoffstätte <holger@...lied-asynchrony.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Netdev <netdev@...r.kernel.org>
Subject: Re: Default qdisc not correctly initialized with custom MTU

On 9/10/19 12:52 AM, Cong Wang wrote:
> On Mon, Sep 9, 2019 at 5:44 AM Holger Hoffstätte
> <holger@...lied-asynchrony.com> wrote:
>> I can't help but feel this is a slight bug in terms of initialization order,
>> and that the default qdisc should only be created when it's first being
>> used/attached to a link, not when the sysctls are configured.
> 
> Yeah, this is because the fq_codel qdisc is initialized once and
> doesn't get any notification when the netdev's MTU get changed.

My point was that it shouldn't be created or initialized at all when
the sysctl is configured, only the name should be validated/stored and
queried when needed. If any interface is brought up before that point,
no value (yet) would just mean "trod along with the defaults" to whoever
is doing the work.

> We can "fix" this by adding a NETDEV_CHANGEMTU notifier to
> qdisc's, but I don't know if it is really worth the effort.

This is essentially the opposite of what I had in mind. The problem is
that the entity was created, not that it needs to be notified.
Also I don't think that would work for scenarios with multiple links
using different MTUs.

> Is there any reason you can't change that order?

Yes, because that wouldn't solve anything?
Like i said I can just kick the root qdisc to update itself in
a post interface-setup script, and that works fine. Since I need
that script anyway for setting several other parameters for
the device it's no big deal - just another workaround.

A brief look at the initialization in sch_mq/sch_generic unfortunately
didn't really help clear things up for me, hence I guess my real
question is whether a qdisc *must* be created early for some reason
(assuming sysctls come before link setup), or whether this is something
that could be delayed and done on-demand.

thanks,
Holger