[libvirt] [PATCH v2] Add support for virtio-net.tx_queue_size

[Michal Privoznik]

[Adding MST who wrote qemu patches]

On 07/18/2017 12:21 PM, Peter Krempa wrote:
> On Tue, Jul 18, 2017 at 09:01:31 +0200, Michal Privoznik wrote:
>> On 07/18/2017 08:23 AM, Peter Krempa wrote:
>>> On Mon, Jul 17, 2017 at 15:39:56 +0200, Michal Privoznik wrote:
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1462653
>>>
>>> This bugzilla is not public.
>>
>> Okay, I'll drop it from the commit message.
> 
> Also add proper explanation what the benefits are, since upstream can't
> read the motivation from the bugzilla.
> 
>>>> Just like I've added support for setting rx_queue_size (in
>>>> c56cdf259 and friends), qemu just gained support for setting tx
>>>> ring size.
> 
> [...]
> 
>>>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>>>> index c12efcf78..58662cf48 100644
>>>> --- a/docs/formatdomain.html.in
>>>> +++ b/docs/formatdomain.html.in
>>>
>>> [...]
>>>
>>>> @@ -5201,6 +5201,20 @@ qemu-kvm -net nic,model=? /dev/null
>>>>          <b>In general you should leave this option alone, unless you
>>>>          are very certain you know what you are doing.</b>
>>>>        </dd>
>>>> +      <dt><code>tx_queue_size</code></dt>
>>>> +      <dd>
>>>> +        The optional <code>tx_queue_size</code> attribute controls
>>>> +        the size of virtio ring for each queue as described above.
>>>> +        The default value is hypervisor dependent and may change
>>>> +        across its releases. Moreover, some hypervisors may pose
>>>> +        some restrictions on actual value. For instance, latest
>>>> +        QEMU (as of 2017-07-13) requires value to be a power of two
> 
> Refer to a proper version of qemu when this is supported, not a date.
> 
>>>> +        from [256, 1024] range.
>>>> +        <span class="since">Since 3.6.0 (QEMU and KVM only)</span><br/><br/>
>>>
>>> This is ridiculous. Since we can't figure out how to set this, how are
>>> users supposed to figure this out?
>>
>> Well, you've cut off the line that reads;
>>
>>   <b>In general you should leave this option alone, unless you
>>   are very certain you know what you are doing.</b>
>>
>> So only users that know how virtio works under the hood are expected to
>> also know what rx/tx queue size is and how to set it. But frankly, I
> 
> This statement is ridiculous by itself.

Why? There are more experienced users (for whom libvirt's just a stable
API) and less experienced ones (for whom libvirt's and tools on the top
of it are great for their automatic chose of parameters, e.g. gnome boxes).

> 
>> think users setting this are always gonna go with the highest value
>> avaliable (1024). Such detailed description is a copy of rx_virtio_queue
>> size description which is result of review.
>>
>>>
>>> Is it really needed? How should it be configured? Can't we or qemu pick
>>> a sane value?
>>>
>>
>> No. Some users need bigger virtio rings otherwise they see a packet
>> drop. So this is a fine tuning that heavily depends on the use case.
>> Thus libvirt should not try to come up with some value.
> 
> That's why I think it's wrong.  What's the drawback of setting it to
> maximum? Which workloads will hit it? Why is the default not sufficient?
> 
> And most notably, how do the users figure out that they need it?

I'll leave this for MST to answer.

> 
> In this case, there are no anchor points that users can use to figure
> out if they need a setting like this. In addition putting in a warning
> that a setting should not be touched makes it rather useless.

Well, it can be viewed as counter part to rx_queue_size which we already
have.

> 
> Is there any writeup that we could point users to which would explain
> this feature?
> 

I'm afraid there's nothing else than BZ I've linked and qemu patches.

Michal