netdev - Re: [net-next PATCH 3/3] qdisc: catch misconfig of attaching qdisc to tx_queue

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

Date:   Tue, 8 Nov 2016 08:46:35 +0100
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     Maciej Żenczykowski <zenczykowski@...il.com>
Cc:     Linux NetDev <netdev@...r.kernel.org>, Phil Sutter <phil@....cc>,
        Robert Olsson <robert@...julf.se>,
        Jamal Hadi Salim <jhs@...atatu.com>, brouer@...hat.com
Subject: Re: [net-next PATCH 3/3] qdisc: catch misconfig of attaching qdisc
 to tx_queue_len zero device


On Mon, 7 Nov 2016 22:14:37 -0800 Maciej Żenczykowski <zenczykowski@...il.com> wrote:

> Just FYI:
> 
> I'm tangentially aware of internal Google code that:
> - expects a bonding device running HTB with non-zero txqueuelen
> - wants to remove HTB and get a noqueue interface (the normal default
> for bonding)
> 
> The code currently removes HTB, which gets us to mq, sets txqueuelen
> to 0, adds a pfifo, removes the pfifo, which gets us to noqueue.

This clearly shows that the older userspace interface, of tx_queue_len
having double meaning, was a mess!

> After this patch this would ?possibly? break (adding pfifo, would
> change txqueuelen, so when we remove it we wouldn't end up with
> noqueue).

No, you will still end-up with "noqueue".  It is now the flag
IFF_NO_QUEUE that determine if a device gets "noqueue" when the default
qdisc is attached. The tx_queue_len no longer have any effect on
getting "noqueue".  The IFF_NO_QUEUE system removed this double meaning
of tx_queue_len.


> From what I fuzzily recall, HTB with txquelelen == 0 drops traffic
> hard, while pfifo continues to function, hence the ordering...
> 
> Obviously our code can be fixed, but I'm worried there's a more
> generic backwards compatibility problem here.

It is good you bring it up, but I don't see a backwards compatibility
problem with your usage after the patchset.
 
> (note: this is mostly about 3.11 and 4.3 and might no longer be
> relevant with 4.10... maybe the new kernel's default qdisc selection
> logic doesn't depend on txqueuelen and checks the flag instead???)

If I were you, I would now implement a validation check that reported
the problem if not getting into the expected "noqueue" state.  Then
when you eventually upgrade to a more recent kernel, you would get
alerted of improper state.

Something like:

noqueue=$(ip link show dev $DEV 2> /dev/null | grep -q "noqueue" && echo "noqueue" || echo "bad")
if [[ "$noqueue" != "noqueue" ]]; then
    echo "report-problem";
fi

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer