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: <CAM_iQpUO3vedg+XOcMb8s6hE=+hdvjPJp9DitjHZE6oNtDVkVQ@mail.gmail.com>
Date:   Tue, 10 Sep 2019 09:56:12 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Holger Hoffstätte <holger@...lied-asynchrony.com>
Cc:     Netdev <netdev@...r.kernel.org>
Subject: Re: Default qdisc not correctly initialized with custom MTU

On Tue, Sep 10, 2019 at 2:14 AM Holger Hoffstätte
<holger@...lied-asynchrony.com> wrote:
>
> 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.

It is _not_ created when sysctl is configured, it is either created via tc
command, or implicitly created by kernel when you bring up eth0.
sysctl only tells kernel what to create by default, but never commits it.

>
> > 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.

Hmm? You did change MTU after adding fq_codel to eth0, right?
So how do you fix this without notification or recreation of fq_codel
in your mind?

I am happy to hear more details.

> Also I don't think that would work for scenarios with multiple links
> using different MTUs.

The fq_codel you created is apparently attached to a netdev,
I don't think this is even a problem. I _guess_ you somehow
believe you create a standalone fq_codel during sysctl setting,
this is just impossible. It must be attached to an interface, no
matter who creates it.

>
> > Is there any reason you can't change that order?
>
> Yes, because that wouldn't solve anything?

Really? You already said it works for you like below, I am confused.


> 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.

The default qdisc is created by kernel when you don't create any.
Again, you can create your own after changing the MTU, this should
solve the problem you see. It is all about ordering.

Thanks.