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

[Michal Privoznik]

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.

> 
>>
>> Just like I've added support for setting rx_queue_size (in
>> c56cdf259 and friends), qemu just gained support for setting tx
>> ring size.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>
>> diff to v1:
>> - Resend after 2e7d4916967 which introduced yet another qemu capability and
>>   thus made my patch not apply cleanly.
>>
>>  docs/formatdomain.html.in                                | 16 +++++++++++++++-
>>  docs/schemas/domaincommon.rng                            |  5 +++++
>>  src/conf/domain_conf.c                                   | 16 ++++++++++++++++
>>  src/conf/domain_conf.h                                   |  1 +
>>  src/qemu/qemu_capabilities.c                             |  4 ++++
>>  src/qemu/qemu_capabilities.h                             |  3 +++
>>  src/qemu/qemu_command.c                                  |  8 ++++++++
>>  src/qemu/qemu_domain.c                                   | 16 +++++++++++-----
>>  ...e.args => qemuxml2argv-net-virtio-rxtxqueuesize.args} |  4 ++--
>>  ...ize.xml => qemuxml2argv-net-virtio-rxtxqueuesize.xml} |  2 +-
>>  tests/qemuxml2argvtest.c                                 |  5 +++--
>>  ...e.xml => qemuxml2xmlout-net-virtio-rxtxqueuesize.xml} |  2 +-
>>  tests/qemuxml2xmltest.c                                  |  2 +-
>>  13 files changed, 71 insertions(+), 13 deletions(-)
>>  rename tests/qemuxml2argvdata/{qemuxml2argv-net-virtio-rxqueuesize.args => qemuxml2argv-net-virtio-rxtxqueuesize.args} (85%)
>>  rename tests/qemuxml2argvdata/{qemuxml2argv-net-virtio-rxqueuesize.xml => qemuxml2argv-net-virtio-rxtxqueuesize.xml} (93%)
>>  rename tests/qemuxml2xmloutdata/{qemuxml2xmlout-net-virtio-rxqueuesize.xml => qemuxml2xmlout-net-virtio-rxtxqueuesize.xml} (96%)
>>
>> 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
>> +        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
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.

Michal