netdev - Re: [PATCH] netdev: handle setting transmit queue length on active device.

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: <20090318111605.06ed6548@nehalam>
Date:	Wed, 18 Mar 2009 11:16:05 -0700
From:	Stephen Hemminger <shemminger@...tta.com>
To:	Patrick McHardy <kaber@...sh.net>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] netdev: handle setting transmit queue length on active
 device.

On Wed, 18 Mar 2009 19:08:07 +0100
Patrick McHardy <kaber@...sh.net> wrote:

> Stephen Hemminger wrote:
> > On Wed, 18 Mar 2009 18:15:38 +0100
> > Patrick McHardy <kaber@...sh.net> wrote:
> > 
> >> Stephen Hemminger wrote:
> >>> Some virtual devices like VLAN's or bridges by default have not transmit queue.
> >>> In order to do queuing disciplines on these devices a queue must be added
> >>> by setting transmit queue length. The kernel allows doing this at any time, but
> >>> setting the queue length value is not enough. Setting the queue length does
> >>> not create the transmit queue (going from 0 to non-zero) or destroy it
> >>> (setting to zero); the queue is only created (or destroyed) when device
> >>> comes up (or goes down).
> >>>
> >>> This patch handles this be doing the necessary activations.
> >>>
> >> That seems quite heavy-weight in case of hierarchical qdiscs, it will
> >> walk and reset the entire tree and looses all state, including packets
> >> as a side effect. Its also a bit inconsistent since it only affects
> >> the default pfifos at the root, not the inner default qdiscs if I'm
> >> not mistaken.
> >>
> >> A less intrusive way would be to simply use dev->tx_queue_len directly
> >> in pfifo when no limit is configured. That would require to make a
> >> runtime decision for each packet though. Two more alternatives:
> >>
> >> - add a special pfifo variant that always uses tx_queue_len. Duplication
> >>    comes down to pfifo_enqueue (a few lines) and a new struct Qdisc_ops
> >>
> >> - if you really only care about the root, reconfigure it explicitly
> >>    using fifo_set_limit()
> >>
> > 
> > The problem is when setting limit to 0 to force noop and setting it
> > to non-zero to force pfifo_fast back again.
> 
> 
> I'd suggest to simply attach a pfifo qdisc with an explicitly
> configured limit to activate queueing on virtual devices.

The existing code is broken if user does:

# ip li set dev eth0.200 txq 200
# tc qdisc set dev eth0.200 root sfq
# tc qdisc del dev eth0.200 root
# ip li set dev eth0.200 txq 0

The vlan is now dead because it is impossible to go back
to the no queueing (noop) qdisc.  Also, when doing performance tests
it is useful to be able to turn off transmit queue entirely.

Other alternative would be to reject changes to txq when device is
up, but that probably would break existing users. And would be a pain
in the ass when trying to configure PPP and tunnel endpoints.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html