Re: [Qemu-devel] [PATCH v3 1/1] virtio-rng: hardware random number generator device

[Anthony Liguori <anthony@codemonkey.ws>]

On 06/22/2012 01:50 PM, Amit Shah wrote:
> On (Fri) 22 Jun 2012 [08:44:52], Anthony Liguori wrote:
>> On 06/22/2012 08:34 AM, Daniel P. Berrange wrote:
>>>
>>> Oh, that's a good point.
>
> But easily fixed.

Of course, except that now we have to maintain compatibility so some hideous 
hack goes in.

This is what fundamentally makes using events a bad approach.  There are more 
problems lurking.  This is the fundamental complexity of having two 
non-synchronized communication channels when you're trying to synchronize some 
sort of activity.

BTW, despite what danpb mentioned, you are rate limiting entropy requests in 
this patch series....

>>>> All of these problems are naturally solved using a protocol over a CharDriverState.
>>>
>>> Can we at least agree on merging a patch which just includes the
>>> raw chardev backend support for virtio-rng ? ie drop the QMP
>>> event for now, so we can make some step forward.
>>
>> All we need to do to support EGD is instead of doing:
>>
>> +    QObject *data;
>> +
>> +    data = qobject_from_jsonf("{ 'bytes': %" PRId64 " }",
>> +                              size);
>> +    monitor_protocol_event(QEVENT_ENTROPY_NEEDED, data);
>> +    qobject_decref(data);
>>
>> Do:
>>
>>      while (size>  0) {
>>          uint8_t partial_size = MIN(255, size);
>>          uint8_t msg[2] = { 0x02, partial_size };
>>
>>          qemu_chr_write(s->chr, msg, sizeof(msg));
>>
>>          size -= partial_size;
>>      }
>>
>> And that's it.  It's an extremely simple protocol to support.  It
>> would actually reduce the total size of the patch.
>
> So I now get what your objection is, and that is because of an
> assumption you're making which is incorrect.
>
> An OS asking for 5038 bytes of entropy is doing just that: asking for
> those bytes.  That doesn't mean a hardware device can provide it with
> that much entropy.  So, the hardware device here will just provide
> whatever is available, and the OS has to be happy with what it got.
> If it got 200 bytes from the device, the OS is then free to demand
> even 7638 bytes more, but it may not get anything for quite a while.
>
> (This is exactly how things work with /dev/random and /dev/urandom
> too, btw.  And /dev/urandom was invented for apps which can't wait for
> all the data they're asking to come to them.)

As it turns out, the EGD protocol also has a mechanism to ask for ask much 
entropy as is available at the current moment.  It also has a mechanism to query 
the amount of available entropy.

And no, you're assertion that we don't care about how much entropy the guest is 
requesting is wrong.  If we lose entropy requests, then we never know if the 
guest is in a state where it's expecting entropy and we're not sending it.

Consider:

- Guest requests entropy (X bytes)
- libvirt sees request
- libvirt sends X bytes to guest
- Guest requests entropy (Y bytes), QEMU filters due to rate limiting

The guest will never request entropy again and libvirt will never send entropy 
again.  The device is effectively dead-locked.

And none of this is a problem using the EGD protocol like I illustrated above.

> All this is expected.  Keep in mind that the hardware-generated
> entropy is read from /dev/hwrng, which again is a /dev/random-like
> interface, and /dev/hwrng entropy can be fed into /dev/random by using
> rngd.  All that is standard stuff.
>
> So: the QMP event here is just a note to libvirt that the guest is
> asking for data, and as an additional hint, it also mentions how much
> data the guest wants right now.  No party is in any kind of contract
> to provide exactly what's asked for.

But you are not using it as a hint!

There is no way for in the virtio-rng protocol for libvirt to just send data to 
the guest out of the kindness of it's heart.  virtio is fundamentally a 
request-response protocol.  Guest requests MUST be processed by something that 
generates a response.

This should be plumbed as a request-response protocol (i.e. EGD).

>>> As mentioned in the previous thread, I see no issue with
>>> later implementing an alternate protocol on the chardev
>>> backend eg as we have raw vs telnet mode for serial ports,
>>> we ought to be able to have a choice of raw vs egd mode for
>>> virtio-rng backends
>>
>> I don't really understand how raw mode works other than reading as
>> much from /dev/urandom as possible and filling the socket buffer
>> buffer with it.
>
> That's all that's needed for the simplest (while being as effective as
> any other) implementation to work.
>
>> I think the only two modes that make sense are EGD over a socket and
>> direct open of /dev/urandom.
>
> Directly wiring /dev/urandom via chardev is more flexible, and doesn't
> involve anything else.  No chardev gimmicks as well.
>
>> But I think the EGD mode is the more important of the two.
>
> Why?  There's nothing standard about it.

Except it's been around for over a decade and is widely supported.

Regards,

Anthony Liguori

>
> 		Amit