[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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.

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

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.

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

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]