[libvirt] [PATCH] virnetdevbandwidth: Compute quantum value

[Michal Privoznik]

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 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
> 


Fixed and pushed. Thanks.

Michal