[libvirt] [PATCH] virnetdevbandwidth: Compute quantum value
I've noticed couple of warning in dmesg while debugging something: [ 9683.973754] HTB: quantum of class 10001 is big. Consider r2q change. [ 9683.976460] HTB: quantum of class 10002 is big. Consider r2q change. I've read the HTB documentation and linux kernel code to find out what's wrong. Basically we need to pass another argument to our tc cmd line because the default computed by HTB does not always work in which case the warning message is printed out. You can read more details here: http://luxik.cdi.cz/~devik/qos/htb/manual/userg.htm#sharing Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevbandwidth.c | 39 +++++++++++++++++++++++++++++++++++++++ tests/virnetdevbandwidthtest.c | 4 ++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index da3a0d3..a74cb1d 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -43,6 +43,36 @@ virNetDevBandwidthFree(virNetDevBandwidthPtr def) VIR_FREE(def); } +static int +virNetDevBandwidthSetOptimalQuantum(virCommandPtr cmd, + const virNetDevBandwidthRate *rate) +{ + const unsigned long long mtu = 1500; + unsigned long long r2q; + char *r2q_str; + + /* When two or more classes compete for unused bandwidth they are each + * given some number of bytes before serving other competing class. This + * number is called quantum. It's advised in HTB docs that the number + * should be equal to MTU. The class quantum is computed from its rate + * divided by global r2q parameter. However, if rate is too small the + * default value will not suffice and thus we must provide our own value. + * */ + + r2q = rate->average * 1024 / 8 / mtu; + if (!r2q) + r2q = 1; + + if (r2q != 10) { + if (virAsprintf(&r2q_str, "%llu", r2q) < 0) + return -1; + virCommandAddArgList(cmd, "quantum", r2q_str, NULL); + VIR_FREE(r2q_str); + } + + return 0; +} + /** * virNetDevBandwidthManipulateFilter: * @ifname: interface to operate on @@ -280,6 +310,8 @@ virNetDevBandwidthSet(const char *ifname, virCommandAddArgList(cmd, "class", "add", "dev", ifname, "parent", "1:", "classid", "1:1", "htb", "rate", average, "ceil", peak ? peak : average, NULL); + if (virNetDevBandwidthSetOptimalQuantum(cmd, bandwidth->in) < 0) + goto cleanup; if (virCommandRun(cmd, NULL) < 0) goto cleanup; } @@ -295,6 +327,9 @@ virNetDevBandwidthSet(const char *ifname, if (burst) virCommandAddArgList(cmd, "burst", burst, NULL); + if (virNetDevBandwidthSetOptimalQuantum(cmd, bandwidth->in) < 0) + goto cleanup; + if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -549,6 +584,8 @@ virNetDevBandwidthPlug(const char *brname, virCommandAddArgList(cmd, "class", "add", "dev", brname, "parent", "1:1", "classid", class_id, "htb", "rate", floor, "ceil", ceil, NULL); + if (virNetDevBandwidthSetOptimalQuantum(cmd, bandwidth->in) < 0) + goto cleanup; if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -672,6 +709,8 @@ virNetDevBandwidthUpdateRate(const char *ifname, virCommandAddArgList(cmd, "class", "change", "dev", ifname, "classid", class_id, "htb", "rate", rate, "ceil", ceil, NULL); + if (virNetDevBandwidthSetOptimalQuantum(cmd, bandwidth->in) < 0) + goto cleanup; if (virCommandRun(cmd, NULL) < 0) goto cleanup; diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index 5a3f02c..b29318e 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -137,7 +137,7 @@ mymain(void) (TC " qdisc del dev eth0 root\n" TC " qdisc del dev eth0 ingress\n" TC " qdisc add dev eth0 root handle 1: htb default 1\n" - TC " class add dev eth0 parent 1: classid 1:1 htb rate 1024kbps\n" + TC " class add dev eth0 parent 1: classid 1:1 htb rate 1024kbps quantum 87\n" TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n" TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw flowid 1\n")); @@ -157,7 +157,7 @@ mymain(void) (TC " qdisc del dev eth0 root\n" TC " qdisc del dev eth0 ingress\n" TC " qdisc add dev eth0 root handle 1: htb default 1\n" - TC " class add dev eth0 parent 1: classid 1:1 htb rate 1kbps ceil 2kbps burst 4kb\n" + TC " class add dev eth0 parent 1: classid 1:1 htb rate 1kbps ceil 2kbps burst 4kb quantum 1\n" TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n" TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw flowid 1\n" TC " qdisc add dev eth0 ingress\n" -- 2.4.10
On Tue, Feb 02, 2016 at 05:20:57PM +0100, Michal Privoznik wrote:
I've noticed couple of warning in dmesg while debugging something:
[ 9683.973754] HTB: quantum of class 10001 is big. Consider r2q change. [ 9683.976460] HTB: quantum of class 10002 is big. Consider r2q change.
I've read the HTB documentation and linux kernel code to find out what's wrong.
Basically we need to pass another argument to our
I would rather mention the argument by name here.
tc cmd line because the default computed by HTB does not always work in which case the warning message is printed out.
You can read more details here:
http://luxik.cdi.cz/~devik/qos/htb/manual/userg.htm#sharing
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevbandwidth.c | 39 +++++++++++++++++++++++++++++++++++++++ tests/virnetdevbandwidthtest.c | 4 ++-- 2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index da3a0d3..a74cb1d 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -43,6 +43,36 @@ virNetDevBandwidthFree(virNetDevBandwidthPtr def) VIR_FREE(def); }
+static int +virNetDevBandwidthSetOptimalQuantum(virCommandPtr cmd,
This does not set the quantum, it just adds it to the command line. Maybe virNetDevBandwidthCmdAddOptimalQuantum?
+ const virNetDevBandwidthRate *rate) +{ + const unsigned long long mtu = 1500; + unsigned long long r2q; + char *r2q_str; + + /* When two or more classes compete for unused bandwidth they are each + * given some number of bytes before serving other competing class. This + * number is called quantum. It's advised in HTB docs that the number + * should be equal to MTU. The class quantum is computed from its rate + * divided by global r2q parameter. However, if rate is too small the + * default value will not suffice and thus we must provide our own value. + * */ + + r2q = rate->average * 1024 / 8 / mtu; + if (!r2q) + r2q = 1; + + if (r2q != 10) {
Is there any harm in adding it to the command line unconditionally?
+ if (virAsprintf(&r2q_str, "%llu", r2q) < 0) + return -1;
Using virCommandAddArgFormat would remove the need for the temporary variable and leave the error checking up to caller's virCommandRun.
+ virCommandAddArgList(cmd, "quantum", r2q_str, NULL); + VIR_FREE(r2q_str); + } + + return 0; +} + /** * virNetDevBandwidthManipulateFilter: * @ifname: interface to operate on
ACK Jan
On 05.02.2016 13:06, Ján Tomko wrote:
On Tue, Feb 02, 2016 at 05:20:57PM +0100, Michal Privoznik wrote:
I've noticed couple of warning in dmesg while debugging something:
[ 9683.973754] HTB: quantum of class 10001 is big. Consider r2q change. [ 9683.976460] HTB: quantum of class 10002 is big. Consider r2q change.
I've read the HTB documentation and linux kernel code to find out what's wrong.
Basically we need to pass another argument to our
I would rather mention the argument by name here.
tc cmd line because the default computed by HTB does not always work in which case the warning message is printed out.
You can read more details here:
http://luxik.cdi.cz/~devik/qos/htb/manual/userg.htm#sharing
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevbandwidth.c | 39 +++++++++++++++++++++++++++++++++++++++ tests/virnetdevbandwidthtest.c | 4 ++-- 2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index da3a0d3..a74cb1d 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -43,6 +43,36 @@ virNetDevBandwidthFree(virNetDevBandwidthPtr def) VIR_FREE(def); }
+static int +virNetDevBandwidthSetOptimalQuantum(virCommandPtr cmd,
This does not set the quantum, it just adds it to the command line. Maybe virNetDevBandwidthCmdAddOptimalQuantum?
+ const virNetDevBandwidthRate *rate) +{ + const unsigned long long mtu = 1500; + unsigned long long r2q; + char *r2q_str; + + /* When two or more classes compete for unused bandwidth they are each + * given some number of bytes before serving other competing class. This + * number is called quantum. It's advised in HTB docs that the number + * should be equal to MTU. The class quantum is computed from its rate + * divided by global r2q parameter. However, if rate is too small the + * default value will not suffice and thus we must provide our own value. + * */ + + r2q = rate->average * 1024 / 8 / mtu; + if (!r2q) + r2q = 1; + + if (r2q != 10) {
Is there any harm in adding it to the command line unconditionally?
+ if (virAsprintf(&r2q_str, "%llu", r2q) < 0) + return -1;
Using virCommandAddArgFormat would remove the need for the temporary variable and leave the error checking up to caller's virCommandRun.
+ virCommandAddArgList(cmd, "quantum", r2q_str, NULL); + VIR_FREE(r2q_str); + } + + return 0; +} + /** * virNetDevBandwidthManipulateFilter: * @ifname: interface to operate on
ACK
Jan
Fixed and pushed. Thanks. Michal
participants (2)
-
Ján Tomko -
Michal Privoznik