[libvirt] [PATCH] virnetdevbandwidth: Compute quantum value

[Ján Tomko]

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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160205/ac029795/attachment-0001.sig>