netdev - Re: [PATCH net-next 0/5] net_sched: Adjust qdisc_change() for command "#tc qdisc change/replace ..."

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: <20140307060818.fa549a2981b601bcec517227@gmail.com>
Date:	Fri, 7 Mar 2014 06:08:18 +0900
From:	Hiroaki SHIMODA <shimoda.hiroaki@...il.com>
To:	yangyingliang@...wei.com
Cc:	davem@...emloft.net, netdev@...r.kernel.org,
	stephen@...workplumber.org, eric.dumazet@...il.com
Subject: Re: [PATCH net-next 0/5] net_sched: Adjust qdisc_change() for
 command "#tc qdisc change/replace ..."

On Thu, 6 Mar 2014 21:08:36 +0800
Yang Yingliang <yangyingliang@...wei.com> wrote:

> Current commands "#tc qdisc replace..." and "#tc qdisc change..."
> are not doing what they're supposed to do.
> 
> E.g.
> 
> With "#tc qdisc replace ...", it won't clear old option if not specified in
> qdisc of netem.
> 
>    # tc qdisc add dev eth4 handle 1: root netem rate 10mbit
>    # tc qdisc show
>      qdisc netem 1: dev eth4 root refcnt 2 limit 1000 rate 10Mbit
> 
>    # tc qdisc replace dev eth4 handle 1: root netem latency 10ms
>    # tc qdisc show
>      qdisc netem 1: dev eth4 root refcnt 2 limit 1000 delay 10.0ms rate 10Mbit
> The old option "rate" is still there.

It looks like you are trying to replace existing qdisc 1: with
the same qdisc 1:. So the effect is the same as change.
If you want to do replace the existing qdisc, you should specify
other handle or different qdisc.

> With "#tc qdisc change ... ", it will clear old options if not specified in
> qdisc of tbf.
> 
>   # tc qdisc add dev eth4 handle 1: root tbf rate 10mbit burst 10kb latency 50ms mtu 64kb peakrate 20mbit
>   # tc qdisc show
>     qdisc tbf 1: dev eth4 root refcnt 2 rate 10Mbit burst 10Kb peakrate 20Mbit minburst 64Kb lat 50.0ms
>   # tc qdisc change dev eth4 handle 1: root tbf rate 20mbit burst 10kb latency 50ms
>   # tc qdisc show
>     qdisc tbf 1: dev eth4 root refcnt 2 rate 20Mbit burst 10Kb lat 50.0ms
> The old peakrate and minburst are cleared.

You are assumed to implicitly specify "peakrate 0" at change.
It seems that you are arguing the tc command, not kernel code.
There would exist userland commmand whose rate and peakrate
command options are always mandatory.
How they support its command options is up to userland I think.

> This patchset adds a flag parameter in qdisc_change and a replace func in struct Qdisc_ops.
> If the flag has NLM_F_REPLACE, we call the replace function, or call the change function in
> qdisc_change(). And, add tbf_replace() and netem_replace() needed by tbf and netem.
> 
> Yang Yingliang (5):
>   net_sched: add flag parameter in qdisc_change
>   net_sched: add replace func in struct Qdisc_ops
>   sch_tbf: change name "tbf_change" to "tbf_replace"
>   sch_tbf: add tbf_change for #tc qdisc change ...
>   sch_netem: add netem_replace for #tc qdisc replace ...
> 
>  include/net/sch_generic.h |   1 +
>  net/sched/sch_api.c       |  16 ++++--
>  net/sched/sch_netem.c     | 127 ++++++++++++++++++++++++++++++++++++++++++++++
>  net/sched/sch_tbf.c       | 120 ++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 258 insertions(+), 6 deletions(-)
> 
> -- 
> 1.8.0
> 
> 
> --
> 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
--
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