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@xxxxxxxxxx> >> --- >> 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list