Re: [libvirt] [PATCH] virnetdevbandwidth: Compute quantum value

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(a)redhat.com&gt; > --- > 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